From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:20163 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755214AbcDNPlN (ORCPT ); Thu, 14 Apr 2016 11:41:13 -0400 Subject: Re: [PATCH 0/3] writeback: minor tweaks To: Jan Kara References: <1460486633-26099-1-git-send-email-axboe@fb.com> <20160413000857.GB10643@dastard> <570DA5B9.3000507@fb.com> <20160413100754.GI15098@quack2.suse.cz> CC: Dave Chinner , , From: Jens Axboe Message-ID: <570FBA0F.80305@fb.com> Date: Thu, 14 Apr 2016 09:41:03 -0600 MIME-Version: 1.0 In-Reply-To: <20160413100754.GI15098@quack2.suse.cz> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 04/13/2016 04:07 AM, Jan Kara wrote: > On Tue 12-04-16 19:49:45, Jens Axboe wrote: >> On 04/12/2016 06:08 PM, Dave Chinner wrote: >>> On Tue, Apr 12, 2016 at 12:43:50PM -0600, Jens Axboe wrote: >>>> Hi, >>>> >>>> Three top level patches out of the writeback throttling patchset, that >>>> I think should make it into mainline. No functional changes in the >>>> first two patches, and the last patch just bumps reclaim/sync >>>> writeback to use WRITE_SYNC as a hint to the block layer. >>> >>> Whatever happened to adding a new flag to indicate that write >>> requests can be throttled, rather than overloading WRITE_SYNC with >>> yet another (conflicting) meaning? >> >> WRITE_SYNC means someone will be waiting for it. This just extends it to >> cover more instances where that is the case (reclaim, for_sync). > > Well, the question really is how soon waiting for the write is warranting > WRITE_SYNC? Someone waits (or will likely soon wait) for background > writeback if we are close to dirty limit after all. Do we want WRITE_SYNC > set also in that case? In your patch 3, you change the logic to issue > WRITE_SYNC writes when for_reclaim or for_sync are set and these are IMHO > questionable as well - for_reclaim just means that there are dirty pages > near the end of LRU and thus MM blindly issues writeback requests and hopes > things will improve. for_sync means we are doing the first pass of > writeback for sync(2). In either of these cases I'm not sure prioritizing > writes is a clear win for the system overall, although in both cases it > makes some sense. Yeah, I'm going to revert course on that a little bit. I'm refactoring to leave the SYNC part as it is, and adding an explicit WRITE_BACKGROUND. That allows me to throttle that explicitly fairly thoroughly, and throttle normal writes a bit less (and O_SYNC/O_DIRECT/etc not at all). -- Jens Axboe