From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de ([195.135.220.15]:38968 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965432AbcDMKH5 (ORCPT ); Wed, 13 Apr 2016 06:07:57 -0400 Date: Wed, 13 Apr 2016 12:07:54 +0200 From: Jan Kara To: Jens Axboe Cc: Dave Chinner , linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org, jack@suse.cz Subject: Re: [PATCH 0/3] writeback: minor tweaks Message-ID: <20160413100754.GI15098@quack2.suse.cz> References: <1460486633-26099-1-git-send-email-axboe@fb.com> <20160413000857.GB10643@dastard> <570DA5B9.3000507@fb.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <570DA5B9.3000507@fb.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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. Honza -- Jan Kara SUSE Labs, CR