From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Chinner Subject: Re: [PATCH] fs-writeback: drop wb->list_lock during blk_finish_plug() Date: Thu, 17 Sep 2015 12:14:53 +1000 Message-ID: <20150917021453.GO3902@dastard> References: <20150912230027.GE4150@ret.masoncoding.com> <20150913231258.GS26895@dastard> <20150916151621.GA8624@ret.masoncoding.com> <20150916195806.GD29530@quack.suse.cz> <20150916200012.GB8624@ret.masoncoding.com> <20150916220704.GM3902@dastard> <20150917003738.GN3902@dastard> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Chris Mason , Jan Kara , Josef Bacik , LKML , linux-fsdevel , Neil Brown , Christoph Hellwig , Tejun Heo To: Linus Torvalds Return-path: Received: from ipmail06.adl2.internode.on.net ([150.101.137.129]:31550 "EHLO ipmail06.adl2.internode.on.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752596AbbIQCPD (ORCPT ); Wed, 16 Sep 2015 22:15:03 -0400 Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Wed, Sep 16, 2015 at 06:12:29PM -0700, Linus Torvalds wrote: > On Wed, Sep 16, 2015 at 5:37 PM, Dave Chinner wrote: > > > > TL;DR: Results look really bad - not only is the plugging > > problematic, baseline writeback performance has regressed > > significantly. > > Dave, if you're testing my current -git, the other performance issue > might still be the spinlock thing. I have the fix as the first commit in my local tree - it'll remain there until I get a conflict after an update. :) > The plugging IO pauses are interesting, though. Plugging really > *shouldn't* cause that kind of pauses, _regardless_ of what level it > happens on, so I wonder if the patch ends up just exposing some really > basic problem that just normally goes hidden. Right, that's what I suspect - it didn't happen on older kernels, but we've just completely reworked the writeback code for the control group awareness since I last looked really closely at this... > Can you match up the IO wait times with just *where* it is > waiting? Is it waiting for that inode I_SYNC thing in > inode_sleep_on_writeback()? I'll do some more investigation. > But it does sound like we should just revert the whole plugging for > now, if only because "it has odd effects". Yup - we can add it again next merge window once we get to the bottom of whatever is going on and have had time to test the new code properly. Cheers, Dave. -- Dave Chinner david@fromorbit.com