Re: [PATCH] fs-writeback: drop wb->list_lock during blk_finish_plug()

[Josef Bacik <jbacik@fb.com>]

On 09/11/2015 04:37 PM, Linus Torvalds wrote:
> On Fri, Sep 11, 2015 at 1:02 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> How about we instead:
>>
>>   (a) revert that commit d353d7587 as broken (because it clearly is)
>>
>>   (b) add a big honking comment about the fact that we hold 'list_lock'
>> in writeback_sb_inodes()
>>
>>   (c) move the plugging up to wb_writeback() and writeback_inodes_wb()
>> _outside_ the spinlock.
>
> Ok, I've done (a) and (b) in my tree. And attached is the totally
> untested patch for (c). It looks ObviouslyCorrect(tm), but since this
> is a performance issue, I'm not going to commit it without some more
> ACK's from people.
>
> I obviously think this is a *much* better approach than dropping and
> retaking the lock, but there might be something silly I'm missing.
>
> For example, maybe we want to unplug and replug around the
> "inode_sleep_on_writeback()" in wb_writeback()? So while the revert
> was a no-brainer, this one I really want people to think about.

So we talked about this when we were trying to figure out a solution. 
The problem with this approach is now we have a plug that covers 
multiple super blocks (__writeback_inodes_wb loops through the sb's 
starts writeback), which is likely to give us crappier performance than 
no plug at all.  Thanks,

Josef