From mboxrd@z Thu Jan 1 00:00:00 1970 From: Josef Bacik Subject: Re: [PATCH] fs-writeback: drop wb->list_lock during blk_finish_plug() Date: Fri, 11 Sep 2015 16:40:11 -0400 Message-ID: <55F33C2B.1010508@fb.com> References: <20150911193747.GA4150@ret.masoncoding.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit To: Linus Torvalds , Chris Mason , LKML , linux-fsdevel , Dave Chinner , Neil Brown , Jan Kara , Christoph Hellwig Return-path: Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:48023 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754032AbbIKUkX (ORCPT ); Fri, 11 Sep 2015 16:40:23 -0400 In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 09/11/2015 04:37 PM, Linus Torvalds wrote: > On Fri, Sep 11, 2015 at 1:02 PM, Linus Torvalds > 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