From mboxrd@z Thu Jan 1 00:00:00 1970 From: Curt Wohlgemuth Subject: Re: [PATCH 3/5] mm: Implement IO-less balance_dirty_pages() Date: Wed, 16 Mar 2011 14:26:54 -0700 Message-ID: References: <1299623475-5512-1-git-send-email-jack@suse.cz> <1299623475-5512-4-git-send-email-jack@suse.cz> <20110310000731.GE10346@redhat.com> <20110314204821.GC4998@quack.suse.cz> <20110315152310.GD24984@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, Wu Fengguang , Peter Zijlstra , Andrew Morton , Christoph Hellwig , Dave Chinner To: Vivek Goyal , jack@suse.cz Return-path: In-Reply-To: <20110315152310.GD24984@redhat.com> Sender: owner-linux-mm@kvack.org List-Id: linux-fsdevel.vger.kernel.org Hi Jan: On Tue, Mar 15, 2011 at 8:23 AM, Vivek Goyal wrote: > On Mon, Mar 14, 2011 at 09:48:21PM +0100, Jan Kara wrote: >> On Wed 09-03-11 19:07:31, Vivek Goyal wrote: >> > > +static void balance_dirty_pages(struct address_space *mapping, >> > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 unsigned long writ= e_chunk) >> > > +{ >> > > + struct backing_dev_info *bdi =3D mapping->backing_dev_info; >> > > + struct balance_waiter bw; >> > > + struct dirty_limit_state st; >> > > + int dirty_exceeded =3D check_dirty_limits(bdi, &st); >> > > + >> > > + if (dirty_exceeded < DIRTY_MAY_EXCEED_LIMIT || >> > > + =A0 =A0 (dirty_exceeded =3D=3D DIRTY_MAY_EXCEED_LIMIT && >> > > + =A0 =A0 =A0!bdi_task_limit_exceeded(&st, current))) { >> > > + =A0 =A0 =A0 =A0 if (bdi->dirty_exceeded && >> > > + =A0 =A0 =A0 =A0 =A0 =A0 dirty_exceeded < DIRTY_MAY_EXCEED_LIMIT) >> > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bdi->dirty_exceeded =3D 0; >> > > =A0 =A0 =A0 =A0 =A0 /* >> > > - =A0 =A0 =A0 =A0 =A0* Increase the delay for each loop, up to our p= revious >> > > - =A0 =A0 =A0 =A0 =A0* default of taking a 100ms nap. >> > > + =A0 =A0 =A0 =A0 =A0* In laptop mode, we wait until hitting the hig= her threshold >> > > + =A0 =A0 =A0 =A0 =A0* before starting background writeout, and then= write out all >> > > + =A0 =A0 =A0 =A0 =A0* the way down to the lower threshold. =A0So sl= ow writers cause >> > > + =A0 =A0 =A0 =A0 =A0* minimal disk activity. >> > > + =A0 =A0 =A0 =A0 =A0* >> > > + =A0 =A0 =A0 =A0 =A0* In normal mode, we start background writeout = at the lower >> > > + =A0 =A0 =A0 =A0 =A0* background_thresh, to keep the amount of dirt= y memory low. >> > > =A0 =A0 =A0 =A0 =A0 =A0*/ >> > > - =A0 =A0 =A0 =A0 pause <<=3D 1; >> > > - =A0 =A0 =A0 =A0 if (pause > HZ / 10) >> > > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pause =3D HZ / 10; >> > > + =A0 =A0 =A0 =A0 if (!laptop_mode && dirty_exceeded =3D=3D DIRTY_EX= CEED_BACKGROUND) >> > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bdi_start_background_writeback(bdi= ); >> > > + =A0 =A0 =A0 =A0 return; >> > > =A0 } >> > > >> > > - /* Clear dirty_exceeded flag only when no task can exceed the limi= t */ >> > > - if (!min_dirty_exceeded && bdi->dirty_exceeded) >> > > - =A0 =A0 =A0 =A0 bdi->dirty_exceeded =3D 0; >> > > + if (!bdi->dirty_exceeded) >> > > + =A0 =A0 =A0 =A0 bdi->dirty_exceeded =3D 1; >> > >> > Will it make sense to move out bdi_task_limit_exceeded() check in a >> > separate if condition statement as follows. May be this is little >> > easier to read. >> > >> > =A0 =A0 if (dirty_exceeded < DIRTY_MAY_EXCEED_LIMIT) { >> > =A0 =A0 =A0 =A0 =A0 =A0 if (bdi->dirty_exceeded) >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bdi->dirty_exceeded =3D 0; >> > >> > =A0 =A0 =A0 =A0 =A0 =A0 if (!laptop_mode && dirty_exceeded =3D=3D DIRT= Y_EXCEED_BACKGROUND) >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bdi_start_background_writeback= (bdi); >> > >> > =A0 =A0 =A0 =A0 =A0 =A0 return; >> > =A0 =A0 } >> > >> > =A0 =A0 if (dirty_exceeded =3D=3D DIRTY_MAY_EXCEED_LIMIT && >> > =A0 =A0 =A0 =A0 !bdi_task_limit_exceeded(&st, current)) >> > =A0 =A0 =A0 =A0 =A0 =A0 return; >> =A0 But then we have to start background writeback here as well. Which i= s >> actually a bug in the original patch as well! So clearly your way is mor= e >> readable :) I'll change it. Thanks. > > I was thinking about that starting of bdi writeback here. But I was > assuming that if we are here then we most likely have visited above > loop of < DIRTY_MAY_EXCEED_LIMIT and started background writeback. Maybe I'm missing something, but at the point in balance_dirty_pages() where we kick the flusher thread , before we put the current task to sleep, how do you know that background writeback is taking place? Are you simply assuming that in previous calls to balance_dirty_pages(), that background writeback has been started, and is still taking place at the time we need to do throttling? Thanks, Curt > > Thanks > Vivek > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. =A0For more info on Linux MM, > see: http://www.linux-mm.org/ . > Fight unfair telecom internet charges in Canada: sign http://stopthemeter= .ca/ > Don't email: email@kvack.org > -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: email@kvack.org