Re: vgone() calling VOP_CLOSE() -> blocked threads?

Hello Bruce,

* Bruce Evans <brde@xxxxxxxxxxxxxxx> wrote:
On Wed, 19 Mar 2008, Ed Schouten wrote:

I just changed my TTY code to perform some garbage collecting on TTY's.
It now only performs a device cleanup when si_threadcount == 1 and
TF_OPENED is unset. Unfortunately, I'm checking for these conditions in
all the cdev ops, which is quite expensive.

It does the trick, but if someone has a better idea, I'm willing to
implement it.

When does si_threadcount go to 0 -- can it be 1 due to something other
than a cdev op holding a reference?

It would probably reach 0, but because the thread calling the close
routine is also taken into account, it will be 1.

If revoke() is the only problem, and if non-cdev ops can hold a
reference, then it might work to acquire a reference at the time of
the revoke. Hold this reference in some process (could even be in
userland), and consider releasing it some time later (and later again
if the synchronization hasn't completed). While this reference is
held, si_refcount cannot go to 0, so it is only necessary to check
si_threadcount == 1 when considering releasing this reference.

Because I'm not really sure about si_threadcount's locking and don't
want to rely on undocumented tricks too much, I just introduced a
t_threadcnt, which is adjusted when entering/leaving one of the cdev
ops. This makes it a lot easier to lock as well.

It should be waterproof w.r.t. referencing the TTY, but won't protect it
against any open/close races yet.

New opens on the device probably need to be blocked while the state is
unsynchronized -- otherwise too many states are possible.

Fortunately, the close() path doesn't block yet, but I was thinking
about adding flags, which should be set when entering tricky parts of
the TTY layer, such as line discipline changing and device opening and
closing.

For now, the current solution should suffice. I'm more worried about
getting uart(4) to work right now. ;-)

--
Ed Schouten <ed@xxxxxxxx>
WWW: http://g-rave.nl/

Attachment: pgpuSqYlK0FcN.pgp
Description: PGP signature

Relevant Pages

  • Re: devctl (alike?) for devfs
    ... The cdev structure is freed only after the last reference to cdev is ... Typical holder of the reference is the devfs vnode. ... in dev_unlock_and_freeI send notifications and call dev_rel on the devices. ... I slightly don't like a fact that parent-child destroy notifications are sent in reverse order because of my simplistic LIST usage. ...
    (freebsd-hackers)
  • Re: [PATCH] Take 2: cdev_unmap()
    ... > that means nobody objects:) ... > reference to every cdev in cdev_map. ... send the line "unsubscribe linux-kernel" in ...
    (Linux-Kernel)
  • Re: [PATCH RESEND] char_dev: add cdev->release() and convert cdev_alloc() to use it
    ... Well, as cdev can be referenced from userspace, ->release is required ... As CUSE can create and destroy devices regardless of module reference ... sure the backing structure doesn't go away till cdev is released. ...
    (Linux-Kernel)
  • [PATCH] Driver Core update for 2.6.3-rc1
    ... the current cdev implementation keeps an uncounted ... reference to every cdev in cdev_map. ... int unregister_chrdev ... +static void cdev_unmap ...
    (Linux-Kernel)
  • Re: vgone() calling VOP_CLOSE() -> blocked threads?
    ... than a cdev op holding a reference? ... the revoke. ... and consider releasing it some time later (and later again ...
    (freebsd-arch)