linux-kernel - Re: [PATCH 05/18] fs: inode split IO and LRU lists

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20101008073808.GR4681@dastard>
Date:	Fri, 8 Oct 2010 18:38:08 +1100
From:	Dave Chinner <david@...morbit.com>
To:	Christoph Hellwig <hch@....de>
Cc:	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 05/18] fs: inode split IO and LRU lists

On Fri, Oct 08, 2010 at 03:14:17AM -0400, Christoph Hellwig wrote:
> On Fri, Oct 08, 2010 at 04:21:19PM +1100, Dave Chinner wrote:
> > From: Nick Piggin <npiggin@...e.de>
> > 
> > The use of the same inode list structure (inode->i_list) for two
> > different list constructs with different lifecycles and purposes
> > makes it impossible to separate the locking of the different
> > operations. Therefore, to enable the separation of the locking of
> > the writeback and reclaim lists, split the inode->i_list into two
> > separate lists dedicated to their specific tracking functions.
> 
> > @@ -410,7 +410,11 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
> >  			redirty_tail(inode);
> >  		} else {
> >  			/* The inode is clean */
> > -			list_move(&inode->i_list, &inode_unused);
> > +			list_del_init(&inode->i_io);
> > +			if (list_empty(&inode->i_lru)) {
> > +				list_add(&inode->i_lru, &inode_unused);
> > +				percpu_counter_inc(&nr_inodes_unused);
> > +			}
> 
> This looks like it belongs into the earlier patch.

I'm not sure it can be moved to an earlier patch. Until the LRU is
separated, we cannot tell what list the inode is on when we get
here. Yes, it means that the nr_inodes_unused counter is probably
broken for a couple of patches in this series. I'll look at it a bit
more, but I don't think it's a huge deal....

> Also instead of
> making nr_inodes_unused non-static a helper to manipulate it might
> be a better idea.

That happens later in the series as more code gets converted to be
identical.

Cheers,

Dave.
-- 
Dave Chinner
david@...morbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/