Hi Pedro, sorry for the late reply!

On Mon, Jun 13, 2005 at 03:26:54PM -0300, Pedro Martelletto wrote:
> Hi,
> 
> Currently, in brelse(), when disassociating a buffer from a vnode,
> the B_DELWRI flag is first removed, then reassignbuf() is called
> followed by brelvp().
> 
> reassignbuf() first removes the buffer from the vnode's clean or dirty
> list, then checks for the B_DELWRI flag, which is absent, and inserts
> the buffer in the vnode's clean list. It also checks if the vnode is on
> the sync list with no dirty buffers, in which case it is removed.
> 
> When brelvp() is called, the buffer is removed from the vnode's clean
> list, and the 'on sync list with no dirty buffers' check is performed
> again.
> 
> I don't think the call to reassignbuf() is necessary, and since it's in
> code protected by IPL_BIO, it might be slowing things down a bit.

-vfs_bio.c-

void
brelse(struct buf *bp)
{
	...
        if ((bp->b_bufsize <= 0) || ISSET(bp->b_flags, B_INVAL)) {
                /*
                 * If it's invalid or empty, dissociate it from its vnode
                 * and put on the head of the appropriate queue.
                 */
                if (LIST_FIRST(&bp->b_dep) != NULL && bioops.io_deallocate)
                        (*bioops.io_deallocate)(bp);
                CLR(bp->b_flags, B_DONE|B_DELWRI);
                if (bp->b_vp) {
                        reassignbuf(bp, bp->b_vp);
                        brelvp(bp);
                }
		...
	}
	...
}

-vfs_subr.c-

void
reassignbuf(struct buf *bp, struct vnode *newvp)
{
	...
	if ((bp->b_flags & B_DELWRI) == 0) {
                listheadp = &newvp->v_cleanblkhd;
                if (TAILQ_EMPTY(&newvp->v_uobj.memq) &&
                    (newvp->v_flag & VONWORKLST) &&
                    LIST_FIRST(&newvp->v_dirtyblkhd) == NULL) {
                        newvp->v_flag &= ~(VWRITEMAPDIRTY|VONWORKLST);
                        LIST_REMOVE(newvp, v_synclist);
                }
		...
	}
	...
	bufinsvn(bp, listheadp);
}

void
brelvp(struct buf *bp)
{
	...
	vp = bp->b_vp;
	...
        if (TAILQ_EMPTY(&vp->v_uobj.memq) && (vp->v_flag & VONWORKLST) &&
            LIST_FIRST(&vp->v_dirtyblkhd) == NULL) {
                vp->v_flag &= ~(VWRITEMAPDIRTY|VONWORKLST);
                LIST_REMOVE(vp, v_synclist);
        }
	...
}


So, the vnode is never on the clean list. Also, bufinsvn() is just inserting
the buffer in the same list again, if I understand it correctly.

I didn't tested your change, but it makes sense to me.

		-- Rui Paulo