From mboxrd@z Thu Jan 1 00:00:00 1970 From: Richard Kennedy Subject: Re: + mm-balance_dirty_pages-reduce-calls-to-global_page_state-to-reduce-c ache-references.patch added to -mm tree Date: Wed, 02 Sep 2009 14:53:31 +0100 Message-ID: <1251899612.2284.29.camel@castor> References: <200908212250.n7LMox3g029154@imap1.linux-foundation.org> <20090822025150.GB7798@localhost> <1250964701.7538.101.camel@twins> <1251880300.7547.91.camel@twins> <20090902095739.GA31516@localhost> <1251888324.7547.147.camel@twins> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Cc: Wu Fengguang , "akpm@linux-foundation.org" , "mm-commits@vger.kernel.org" , "chris.mason@oracle.com" , "jens.axboe@oracle.com" , "mbligh@mbligh.org" , "miklos@szeredi.hu" , "linux-fsdevel@vger.kernel.org" To: Peter Zijlstra Return-path: Received: from lon1-post-1.mail.demon.net ([195.173.77.148]:40527 "EHLO lon1-post-1.mail.demon.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751778AbZIBNxd (ORCPT ); Wed, 2 Sep 2009 09:53:33 -0400 In-Reply-To: <1251888324.7547.147.camel@twins> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Wed, 2009-09-02 at 12:45 +0200, Peter Zijlstra wrote: > On Wed, 2009-09-02 at 17:57 +0800, Wu Fengguang wrote: > > On Wed, Sep 02, 2009 at 04:31:40PM +0800, Peter Zijlstra wrote: > > > On Sat, 2009-08-22 at 20:11 +0200, Peter Zijlstra wrote: > > > > > > + /* always throttle if over threshold */ > > > > > > + if (nr_reclaimable + nr_writeback < dirty_thresh) { > > > > > > > > > > That 'if' is a big behavior change. It effectively blocks every one > > > > > and canceled Peter's proportional throttling work: the less a process > > > > > dirtied, the less it should be throttled. > > > > > > > > Hmm, I think you're right, I had not considered that, thanks for > > > > catching that. > > > > > > So in retrospect I think I might have been wrong here. > > > > > > The per task thing causes the bdi limit to be lower than the bdi limit > > > based on writeback speed alone. That is, the more a task dirties, the > > > lower the bdi limit is as seen for that task. > > > > Right. If I understand it right, there will be a safety margin of about > > (1/8) * dirty_limit for 1 heavy dirtier case, and that gap scales down > > when there are more concurrent heavy dirtiers. > > Right, with say 4 heavy writers the gap will be 1/4-th of 1/8-th, which > is 1/32-nd. > > With the side node that I think 1/8 is too much on large memory systems, > and I have posted a sqrt patch numerous times, but I don't think we've > ever found out if that helps or not... > > > In principle, the ceiling will be a bit higher for a light dirtier to > > make it easy to pass in the presence of more heavy dirtiers. > > Right. > > > > So if we get a task that generates tons of dirty pages (dd) then it > > > won't ever actually hit the full dirty limit, even if its the only task > > > on the system, and this outer if() will always be true. > > > > Right, we have the safety margin :) > > > > > Only when we actually saturate the full dirty limit will we fall through > > > and throttle, but that is ok -- we want to enforce the full limit. > > > > > > In short, a very aggressive dirtier will have a bdi limit lower than the > > > total limit (at all times) leaving a little room at the top for the > > > occasional dirtier to make quick progress. > > > > > > Wu, does that cover the scenario you had in mind? > > > > Yes thanks! Please correct me if wrong: > > - the lower-ceiling-for-heavier-dirtier algorithm in task_dirty_limit() > > is elegant enough to prevent heavy dirtier to block light ones > > ack > > > - the test (nr_reclaimable + nr_writeback < dirty_thresh) is not > > relevant in normal, but can be kept for safety in the form of > > > > if (bdi_nr_reclaimable + bdi_nr_writeback < bdi_thresh && > > nr_reclaimable + nr_writeback < dirty_thresh) > > break; > > ack > > > - clip_bdi_dirty_limit() could be removed: we have been secured by the > > above test > > ack. I've noticed that there's a difference in the handling of the dirty_exceeded flag, because this change no longer clips the bdi_thresh then the flag may get cleared more quickly here :- if (bdi_nr_reclaimable + bdi_nr_writeback < bdi_thresh && bdi->dirty_exceeded) bdi->dirty_exceeded = 0; So it then could call balance_dirty_pages a lot less often. I've got an updated version of this patch that moves the clip_bdi logic up into balance_dirty_pages that should be closer to the existing behavior & tests so far look good. I can post it for comments if you're interested ? regards Richard