svn commit: r212661 - head/sys/dev/aac

[Bruce Evans]

On Wed, 15 Sep 2010, Kostik Belousov wrote:

> On Wed, Sep 15, 2010 at 04:57:15PM +0200, Attilio Rao wrote:
>> 2010/9/15 Kostik Belousov <kostikbel at gmail.com>:
>>> On Wed, Sep 15, 2010 at 02:24:21PM +0000, Attilio Rao wrote:
>>>> ...
>>>> Log:
>>>> š Fix bogus busying mechanism from cdevsw callbacks:
>>>> š - D_TRACKCLOSE may be used there as d_close() are expected to match up
>>>> š š d_open() calls

Some mail program made this hard to read by converting ASCII to hard \x9a's.

D_TRACKCLOSE may be useful for something, but it isn't for determining
busyness.  Rather the reverse -- it can be used for getting the driver
called in cases where there is a thread busy (but sleeping) in the
driver.  It can also repeat devfs/vfs's broken device busyness counting.
I know of a couple of useful uses for it in FreeBSD, but these are
unimplemented, and all of the uses that I looked at are nonsense or
broken.  One nonsense and broken case is in ast-tape.c.  This driver
used to enforce exclusive access using vfs counting.  I think the vfs
counting works with exclusive access.  But with exclusive access all
closes are last-ones so D_TRACKCLOSE has no effect.  Now, this driver
doesn't enforce exclusive access, so D_TRACKCLOSE just gives bugs like
writing filemarks and rewinding the rewindable device for non-last closes,
if anyone actually uses non-exclusive access.

>>> VFS is not very good at properly calling VOP_CLOSE(). As example, premature
>>> vnode reclaim due to devfs unmount would cause VOP_CLOSE() to be called
>>> only once despite the number of opens being š> 1.

This seems to invalidate all cases where D_TRACKCLOSE is (ab)used for
reference counting.  One such case is geom_dev.c:g_dev_close().  This
does nothing commital except to decrement some of the r/w/e access
counters.  But if it is not called once for every device open, it
cannot do that.  Also, there is a problem initializing its flags arg
for forced VOP_CLOSE() from general context, and no way to its flags
arg to pass the necessary combined r/w/e adjustments if these adjustments
sum to less than -1 for any of r/w/e.  For normal open/close pairs,
the close flags arg must equal the open flags arg, and a forced close
from unrelated context will find it either difficult to find the open
flags arg if there was only 1 open, or usually impossible to combine
2 open flags args if there was > 1 open.  Otherwise, g_dev_close is
the only useful use of D_TRACKCLOSE that I know of.

>> Yes.
>> That makes implementing a similar semantic in drivers very difficult
>> and not very well fixable, in particular within the d_* callbacks.

Vfs doesn't properly determine busyness (due to complications in itself
and at the driver level), so individualy drivers have no chance of
determining it (since to do so they would have to understand not only
therir own complications, but also vfs complications which vfs itself
doesn't completely understand).

>> I'm seriously wondering if we might just make a shortcut just for
>> supporting such a feature (busying the device on real devfs entry
>> activity) as several of them may be needing and may be probabilly
>> needing to be 100%.
>> Luckilly, it seems that such paths are not experienced very frequently.
>
> I am not sure what do you mean by "busying the device on real devfs entry
> activity". Is the operation made by dev_refthread() and friends enough ?
>
> Device cannot be destroyed by destroy_dev() until all threads leave the
> cdevsw methods.

Note that there may be other threads in a cdevsw method even when
d_close() is called for the last-close.  I think no other threads can
be in a cdevsw read, write or ioctl method when last-close is called
(since they must hold a dup'ed fd open, so any close cannot be the
last one).  Also, no other thread can be in an open method, due to
bugs in devfs/vfs last-close counting (the count is incremented before
devfs_open() is called; so cdevsw close is never called if there is
a thread in an open method and D_TRACKCLOSE is not set).  But any number
of threads can be in the close method when cdevsw close is called, even
if D_TRACKCLOSE is not set, due to a lesser bug in devfs/vfs last-close
counting (the count is decremented before devfs_close() is called, so
cdevsw close may be called for last-close before a previous call completes).

Bruce