[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160222121222.GF6357@twins.programming.kicks-ass.net>
Date: Mon, 22 Feb 2016 13:12:22 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: Jan Kara <jack@...e.cz>
Cc: Dave Chinner <david@...morbit.com>,
Waiman Long <Waiman.Long@....com>,
Alexander Viro <viro@...iv.linux.org.uk>,
Jan Kara <jack@...e.com>,
Jeff Layton <jlayton@...chiereds.net>,
"J. Bruce Fields" <bfields@...ldses.org>,
Tejun Heo <tj@...nel.org>,
Christoph Lameter <cl@...ux-foundation.org>,
linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
Ingo Molnar <mingo@...hat.com>,
Andi Kleen <andi@...stfloor.org>,
Dave Chinner <dchinner@...hat.com>,
Scott J Norton <scott.norton@...com>,
Douglas Hatch <doug.hatch@...com>
Subject: Re: [PATCH v2 3/3] vfs: Use per-cpu list for superblock's inode list
On Mon, Feb 22, 2016 at 12:54:35PM +0100, Jan Kara wrote:
> > Also, I think fsnotify_unmount_inodes() (as per mainline) is missing a
> > final iput(need_iput) at the very end, but I could be mistaken, that
> > code hurts my brain.
>
> I think the code is actually correct since need_iput contains "inode
> further in the list than the current inode". Thus we will always go though
> another iteration of the loop which will drop the reference. And inode
> cannot change state to I_FREEING or I_WILL_FREE because we hold inode
> reference. But it is subtle as hell so I agree that code needs rewrite.
So while talking to dchinner, he doubted fsnotify will actually remove
inodes from the sb-list, but wasn't sure and too tired to check now.
(I got lost in the fsnotify code real quick and gave up, for I was
mostly trying to make a point that we don't need the CPP magic and can
do with 'readable' code).
If it doesn't, it doesn't need to do this extra special magic dance and
can use the 'normal' iterator pattern used in all the other functions,
greatly reducing complexity.
Powered by blists - more mailing lists