On Wed, May 29, 2013 at 02:51:58PM +0400, Glauber Costa wrote: > On 05/29/2013 02:15 PM, Dave Chinner wrote: > > On Tue, May 28, 2013 at 09:26:37PM +0530, Glauber Costa wrote: > >> On 05/25/2013 05:57 AM, Dave Chinner wrote: > >>> On Fri, May 24, 2013 at 03:59:10PM +0530, Glauber Costa wrote: > >>>> From: Dave Chinner <dchinner@xxxxxxxxxx> > >>>> > >>>> Convert the buftarg LRU to use the new generic LRU list and take > >>>> advantage of the functionality it supplies to make the buffer cache > >>>> shrinker node aware. > >>>> > >>>> * v7: Add NUMA aware flag > >>> > >>> I know what is wrong with this patch that causes the unmount hang - > >>> it's the handling of the _XBF_LRU_DISPOSE flag no longer being > >>> modified atomically with the LRU lock. Hence there is a race where > >>> we can either lose the _XBF_LRU_DISPOSE or not see it and hence we > >>> can end up with code not detecting what list the buffer is on > >>> correctly. > >>> > >>> I haven't had a chance to work out a fix for it yet. If this ends up > >>> likely to hold up the patch set, Glauber, then feel free to drop it > >>> from the series and I'll push a fixed version through the XFS tree > >>> in due course.... > >>> > >>> Cheers, > >>> > >>> Dave. > >>> > >> Please let me know what you think about the following two (very coarse) > >> patches. My idea is to expose more of the raw structures so XFS can do > >> the locking itself when needed. > > > > No, I'd prefer not to do that. There's a big difference between a > > callback that passes a pointer to an internal lock that protects the > > list that the item being passed is on and exposing the guts of the > > per node list and lock implementation to everyone.... > > > > As it is, the XFS buffer LRU reclaimation is modelled on the > > inode/dentry cache lru reclaimation where the "on dispose list" flag > > is managed by a lock in the inode/dentry and wraps around the > > outside of the LRU locks. The simplest fix to XFS is to add a > > spinlock to the buffer to handle this in the same way as inodes and > > dentries. I think I might be able to do it in a way that avoids > > the spin lock but I just haven't had time to look at it that closely. > > > > Cheers, > > > Ok. In the interest of having the series merged - we seem to be running > out of problems - I see two courses of action: > > 1) Don't convert this to LRU at all, just convert to the new count/ scan > interface, > > 2) Use a temporary spinlock, and you fix that later. > > I would actually prefer 2). Reason is that this patch actually do both, > meaning I would have to rewrite the patch to do this scan / count loop > without the new list_lru aid. Besides being more error-prone, it is of > course a lot more work. > > Which one you prefer? 3) I'll fix it tomorrow and send you the patch. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
