Reinoud Zandijk writes:
> 
> Index: kern/vfs_bio.c
> ===================================================================
> RCS file: /cvsroot/src/sys/kern/vfs_bio.c,v
> retrieving revision 1.148
> diff -u -r1.148 vfs_bio.c
> --- kern/vfs_bio.c	24 Dec 2005 19:12:23 -0000	1.148
> +++ kern/vfs_bio.c	3 Jan 2006 16:05:47 -0000
> @@ -904,6 +904,16 @@
>  		wakeup(bp);
>  	}
>  
[snip]
>  
>  /*
> + * Get a new managed buffer. Memory assigned to it is NOT owned by the buffe
> r
> + * cache but by the caller, typically a filingsystem.
> + */
> +struct buf *
> +getmanagedbuf(int blkno, caddr_t base, int size)
> +{
> +	int s;
> +	struct buf *bp;
> +
> +	KASSERT(base != NULL);
> +	KASSERT(size > 0);
> +
> +	s = splbio();
> +	simple_lock(&bqueue_slock);
> +
> +	/* please don't destroy valueable data */
> +	bp = pool_get(&bufpool, PR_WAITOK);
> +
> +	/* Initialise buffer */
> +	memset(bp, 0, sizeof(struct buf));
> +	BUF_INIT(bp);
> +	bp->b_flags |= B_MANAGED;
> +
> +	bp->b_blkno = blkno;
> +	bp->b_data = base;
> +	bp->b_bufsize = bp->b_bcount = bp->b_resid = size;
> +
> +	simple_unlock(&bqueue_slock);
> +	splx(s);

is locking/unlocking bqueue_slock really needed here?  Isn't that 
just for mucking with bufqueues[] and friends?  Also, are you allowed 
to PR_WAITOK w/ bqueue_slock locked??  (I don't think so...)  If nothing 
else, can the unlock and splx() be moved up to right after the 
pool_get() ?  (i.e. only block things for as long as really necessary? )  

(sys/kern_physio.c:getphysbuf() seems to indicate that the 
simple_lock/simple_unlock for pool_get(&bufpool,_) isn't needed.)

> +
> +	return bp;
> +}
> +
> +/*
> + * Get a new buffer nested into the specified buffer at the given offset and
> + * length. NO read/write actions ought to be caried out on the master buffer
> + * anymore only on the nested buffers as they effectively split up the maste
> r
> + * buffer's action.
> + *
> + * Bug alert: make sure all nested buffers cover the complete mbp->resid
> + * space.  If space is to be skipped, mbp->resid needs to be accounted for o
> r
> + * the biodone on mbp won't be called!
> + */
> +struct buf *
> +nestbuf(struct buf *mbp, int blkno, caddr_t base, int size)
> +{
> +	int s;
> +	struct buf *bp;
> +
> +	KASSERT(base != NULL);
> +	KASSERT(size > 0);
> +	KASSERT((mpb==NULL) || (mpb && mbp->b_count < offset+size));
> +
> +	s = splbio();
> +	simple_lock(&bqueue_slock);

Again, is this needed? 

> +	/* please don't destroy valueable data */
> +	bp = pool_get(&bufpool, PR_WAITOK);

Again, I don't think you can PR_WAITOK w/ a lock held.  A LOCKDEBUG 
kernel would have a fit :) 

> +	/*
> +	 * Adjust relevant information from the master buffer. set nested info
> +	 * and clear callback info and softdep info.
> +	 */
> +	memcpy(bp, mbp, sizeof(struct buf));
> +	bp->b_flags &= ~B_CALL;
> +	bp->b_flags |= B_MANAGED | B_NESTED;
> +	bp->b_masterbuf = mbp;
> +
> +	bp->b_blkno = blkno;
> +	bp->b_data = base;
> +	bp->b_bufsize = bp->b_bcount = bp->b_resid = size;
> +
> +	/* avoid confusion for softdep? */
> +	LIST_INIT(&bp->b_dep);

Missing a:

	simple_unlock(&bqueue_slock);
	splx(s);

here?  (or, preferably, right after the pool_get() if that would be 
sufficient.

> +	return bp;
> +}

Later...

Greg Oster