xfs
[Top] [All Lists]

Re: [PATCH 1/8] xfs: Pull EFI/EFD handling out from under the AIL lock

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 1/8] xfs: Pull EFI/EFD handling out from under the AIL lock
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 3 Dec 2010 16:24:45 +1100
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20101202113849.GA21365@xxxxxxxxxxxxx>
References: <1290993152-20999-1-git-send-email-david@xxxxxxxxxxxxx> <1290993152-20999-2-git-send-email-david@xxxxxxxxxxxxx> <20101130201734.GA16079@xxxxxxxxxxxxx> <20101202012841.GL16922@dastard> <20101202113849.GA21365@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
On Thu, Dec 02, 2010 at 06:38:49AM -0500, Christoph Hellwig wrote:
> On Thu, Dec 02, 2010 at 12:28:41PM +1100, Dave Chinner wrote:
> > >  - there is a behaviour change about the xfs_trans_del_item call
> > >    in xfs_efi_item_unpin - before it was protected by the
> > >    XFS_EFI_CANCELED which was never set, and now it's not.
> > 
> > XFS_EFI_CANCELED has not been set in the code base since
> > xfs_efi_cancel() was removed back in 2006 by commit
> > 065d312e15902976d256ddaf396a7950ec0350a8 ("[XFS] Remove unused
> > iop_abort log item operation), and even then xfs_efi_cancel() was
> > never called. I haven't tracked it back further than that (beyond
> > git history), but handling of efis in cancelled transactions has
> > been broken for a long time.
> > 
> > Basically, when we get an IOP_UNPIN(lip, 1); call from
> > xfs_trans_uncommit() (i.e. remove == 1), if we don't free the log
> > item descriptor we leak it. IOWs, the new behaviour introduced in
> > this patch is actually the correct behaviour.
> 
> Maybe fix this issue first in a separate patch, instead of hiding it
> in a bigger one.

Ok, I'll split it out.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>