Date: Fri, 17 Sep 2010 01:32:42 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Kostik Belousov <kostikbel@gmail.com> Cc: Attilio Rao <attilio@FreeBSD.org>, svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r212661 - head/sys/dev/aac Message-ID: <20100916220620.W763@delplex.bde.org> In-Reply-To: <20100915150412.GM2465@deviant.kiev.zoral.com.ua> References: <201009151424.o8FEOLZE039185@svn.freebsd.org> <20100915145209.GK2465@deviant.kiev.zoral.com.ua> <AANLkTikMxAZDa9KGvY-aPfCJ=gQL0gNpF5Marsbw-Wuv@mail.gmail.com> <20100915150412.GM2465@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --0-1831366618-1284651162=:763 Content-Type: TEXT/PLAIN; charset=X-UNKNOWN; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE 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@gmail.com>: >>> On Wed, Sep 15, 2010 at 02:24:21PM +0000, Attilio Rao wrote: >>>> ... >>>> Log: >>>> =9A Fix bogus busying mechanism from cdevsw callbacks: >>>> =9A - D_TRACKCLOSE may be used there as d_close() are expected to matc= h up >>>> =9A =9A d_open() calls Some mail program made this hard to read by converting ASCII to hard \x9a's= =2E 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, prema= ture >>> vnode reclaim due to devfs unmount would cause VOP_CLOSE() to be called >>> only once despite the number of opens being =9A> 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)= =2E Bruce --0-1831366618-1284651162=:763--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100916220620.W763>