Re: kern/38923: ATAIOCCOMMAND ioctl does not work

[Michael van Elst <mlelstv%serpens.de@localhost>]

The following reply was made to PR kern/38923; it has been noted by GNATS.

From: Michael van Elst <mlelstv%serpens.de@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: kern/38923: ATAIOCCOMMAND ioctl does not work
Date: Tue, 17 Jun 2008 13:59:38 +0200

 The ioctl hanging comes from physio waiting for the buffer
 to become "un-busy". Obviously this will never become true
 when the buffer isn't used yet.
 
 physio() is called with a buffer here:
 
 dev/qbus/qd.c
 dev/gpib/mt.c
 arch/hp300/dev/mt.c
 dev/ata/wd.c
 dev/scsipi/scsipi_ioctl.c
 
 dev/qbus/qd.c looks broken..
 
 dev/gpib/mt.c and
 arch/hp300/dev/mt.c don't use buf_init() but poke the structure.
 They use BC_BUSY to reserve a single command buffer that is used
 synchronously by the read/write functions and asynchronously by
 the strategy functions.
 
 dev/ata/wd.c and
 dev/scsipi/scsipi_ioctl.c malloc buffers and use buf_init().
 Reserving a buffer doesn't make sense, as each request uses
 its private buffer.
 
 When physio() isn't called with a buffer, as in most device drivers,
 then it uses getphysbuf() -> getiobuf() to allocate a private one
 from the biopl pool. getiobuf uses buf_init() which now sets BC_BUSY
 and getphysbuf() sets BC_BUSY again.
 
 
 buf_init() is called in getnewbuf() to return a fresh buffer
 from the bufpl pool. If allocating a fresh buffer is not wanted
 then a buffer is fetched from the free list and marked BC_BUSY.
 
 buf_init() is called in
 uvm/uvm_swap.c
 dev/ccd.c
 dev/ata/ld_ataraid.c
 dev/vnd.c
 only to get the cflags overwritten with flags from another buffer.
 
 buf_init() is called in
 dev/fss.c
 to get most data overwritten, maybe the cflags were forgotten.
 I'm not sure wether a busy buffer breaks fss.
 
 buf_init() is called in
 arch/xen/xen/xbdback.c
 arch/xen/xen/xbdback_xenbus.c
 after allocating a buffer from a pool, when that buffer is
 used, it gets overwritten, in particular cflags is set to 0.
 
 buf_init() is called from
 fs/udf/udf_strat_rmw.c
 and then b_cflags is set to BC_BUSY /* needed? */
 However, that buffer isn't used, but split into nested buffers.
 nestiobuf_setup() does set BC_BUSY on the nested buffer.
 
 
 Apparently buf_init() isn't supposed to declare a buffer busy
 and doing so only saves to mark the buffer busy in getnewbuf().
 E.g.:
 
 
 Index: kern/vfs_bio.c
 ===================================================================
 RCS file: /cvsroot/src/sys/kern/vfs_bio.c,v
 retrieving revision 1.203
 diff -u -r1.203 vfs_bio.c
 --- kern/vfs_bio.c     16 Jun 2008 09:47:55 -0000      1.203
 +++ kern/vfs_bio.c     17 Jun 2008 11:57:08 -0000
 @@ -1301,6 +1301,8 @@
                if (bp != NULL) {
                        memset((char *)bp, 0, sizeof(*bp));
                        buf_init(bp);
 +                      /* Buffer is busy now. */
 +                      SET(bp->b_cflags, BC_BUSY);
                        mutex_enter(&bufcache_lock);
  #if defined(DIAGNOSTIC)
                        bp->b_freelistindex = -1;
 @@ -2017,7 +2019,7 @@
        bp->b_dev = NODEV;
        bp->b_error = 0;
        bp->b_flags = 0;
 -      bp->b_cflags = BC_BUSY;
 +      bp->b_cflags = 0;
        bp->b_oflags = 0;
        bp->b_objlock = &buffer_lock;
        bp->b_iodone = NULL;
 
 
 -- 
                                 Michael van Elst
 Internet: mlelstv%serpens.de@localhost
                                 "A potential Snark may lurk in every tree."