Re: [PATCH v2 3/3] vfs: Use per-cpu list for superblock's inode list

[Waiman Long <waiman.long@hpe.com>]

On 02/22/2016 08:04 AM, Jan Kara wrote:
> On Mon 22-02-16 13:12:22, Peter Zijlstra wrote:
>> 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.
> Yeah, that would be nice. But fsnotify code needs to iterate over all
> inodes, drop sb_list_lock and do some fsnotify magic with the inode which
> is not substantial for our discussion. Now that fsnotify magic may actually
> drop all the remaining inode references so once we drop our reference
> pinning the inode, it can just disappear. We don't want to restart the scan
> for each inode we have to process so that is the reason why we play ugly
> tricks with pinning the next inode in the list.
>
> But I agree it should be possible to just use list_for_each_entry() instead
> of list_for_each_entry_safe() and keep current inode pinned till the next
> iteration to make it stick in the sb->s_inodes list. That would make the
> iteration more standard. Lightly tested patch attached.
>
> 								Honza

Your patch looks good to me. I would like to put your patch into my 
per-cpu list patchset if you don't mind.

Cheers,
Longman