From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754159AbbIKUkZ (ORCPT ); Fri, 11 Sep 2015 16:40:25 -0400 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 Subject: Re: [PATCH] fs-writeback: drop wb->list_lock during blk_finish_plug() To: Linus Torvalds , Chris Mason , LKML , linux-fsdevel , Dave Chinner , Neil Brown , Jan Kara , Christoph Hellwig References: <20150911193747.GA4150@ret.masoncoding.com> From: Josef Bacik Message-ID: <55F33C2B.1010508@fb.com> Date: Fri, 11 Sep 2015 16:40:11 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [192.168.52.123] X-Proofpoint-Spam-Reason: safe X-FB-Internal: Safe X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:5.14.151,1.0.33,0.0.0000 definitions=2015-09-11_08:2015-09-11,2015-09-11,1970-01-01 signatures=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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