cvs commit: src/sys/dev/io iodev.c

Bruce Evans brde at optusnet.com.au
Sat Aug 9 04:35:57 UTC 2008


On Fri, 8 Aug 2008, John Baldwin wrote:

> On Friday 08 August 2008 09:43:56 am Ed Schouten wrote:
>> ed          2008-08-08 13:43:56 UTC
>>
>>   Apart from this change, I think some fishy things may happen when using
>>   /dev/io in multithreaded applications. I haven't tested, but looking at
>>   the code, the flag doesn't get cleared when close() is called from
>>   another thread, but this may not be this important.

Of course it isn't.  The flag only gets cleared on last close.  Threads
probably involve mainly file descriptors, and for file descriptors close()
doesn't even reach vn_close() until the fd reference count drops to 0.
Then for files, vn_close() normally doesn't reach device close until the
file reference count drops to 0.

> It should be setting D_TRACKCLOSE though so that close() reliably clears the
> flag even in single-threaded processes.  You can still get odd behavior if

You mean "should _not_ be setting D_TRACKCLOSE", so that close() works
normally.

close() can never reliably clear the flag, since even vn_close() is not
reached after open()/dup()/close() or open()/fork()/close()...

> you explicitly open it twice in an app and then close one of the two fd's.
> You will no longer have IO permission even though you still have one fd open.
> However, if you do that I think you deserve what you asked for. :)

You asked for normal last-close behaviour and deserve that not being broken
by setting D_TRACKCLOSE.

D_TRACKCLOSE allows different handling of non-last closes, but this is
rarely wanted, and in theory it allows better determination of last
closes, since vfs doesn't count last closes right.  However, it is
difficult to count last closes right, and most uses of D_TRACKCLOSE handle
last closes are worse than vfs:
- ast: uses D_TRACKCLOSE to trash (write a filemark) and/or rewind tapes
   on non-last closes.  After the trash and rewind, astclose() uses
   count_dev() to avoid doing a full hardware close if the device is still
   open.  This involves using count_dev(), but count_dev() is just an
   interface to the vfs count, so it miscounts in the same way as vfs.  In
   particular, it doesn't count incompleted opens.  I don't know if ast's
   locking is sufficient to prevent close() being called while a new open
   is in progress.  For most devices, it isn't.
- drm: it uses its own count of opens and closes, and it doesn't use
   count_dev().  The own count has some chance of working, but needs
   delicate locking of device open and device close to prevent them
   running into each other, and some defense against the vfs bugs which
   at least used to result in missing and/or extra closes.
- smb: like drm, except it limits its own count to 0 or 1 to give half-
   baked exclusive access (fork() and dup(), etc, still give non-exclusive
   access).  D_TRACKCLOSE thus has no effect except for its interaction with
   other bugs.
- geom: unlike most or all of the others, this may actually need
   D_TRACKCLOSE, to implement multiple logical drivers per physical device.
   I don't understand its details.
- apm: like drm
- bpf: like smb

Bruce


More information about the cvs-all mailing list