From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wu Fengguang Subject: Re: [PATCH 1/3 v2] mm: Stop background writeback if there is other work queued for the thread Date: Mon, 9 Aug 2010 06:55:57 +0800 Message-ID: <20100808225557.GA8013@localhost> References: <1281034399-13055-1-git-send-email-jack@suse.cz> <1281034399-13055-2-git-send-email-jack@suse.cz> <20100805164535.f28d8807.akpm@linux-foundation.org> <20100808041230.GF26402@dastard> <20100808072949.GA5332@localhost> <4C5E8FEE.4080209@fusionio.com> <20100808135917.GA3340@quack.suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Jens Axboe , Dave Chinner , Andrew Morton , "linux-fsdevel@vger.kernel.org" , "hch@infradead.org" To: Jan Kara Return-path: Received: from mga03.intel.com ([143.182.124.21]:6964 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754712Ab0HHW4B (ORCPT ); Sun, 8 Aug 2010 18:56:01 -0400 Content-Disposition: inline In-Reply-To: <20100808135917.GA3340@quack.suse.cz> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Sun, Aug 08, 2010 at 09:59:18PM +0800, Jan Kara wrote: > On Sun 08-08-10 07:07:26, Jens Axboe wrote: > > On 08/08/2010 03:29 AM, Wu Fengguang wrote: > > > On Sun, Aug 08, 2010 at 12:12:30PM +0800, Dave Chinner wrote: > > >> On Thu, Aug 05, 2010 at 04:45:35PM -0700, Andrew Morton wrote: > > >>> On Thu, 5 Aug 2010 20:53:17 +0200 > > >>> Jan Kara wrote: > > >>>> --- > > >>>> fs/fs-writeback.c | 8 ++++++++ > > >>>> 1 files changed, 8 insertions(+), 0 deletions(-) > > >>>> > > >>>> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c > > >>>> index d5be169..542471e 100644 > > >>>> --- a/fs/fs-writeback.c > > >>>> +++ b/fs/fs-writeback.c > > >>>> @@ -633,6 +633,14 @@ static long wb_writeback(struct bdi_writeback *wb, > > >>>> break; > > >>>> > > >>>> /* > > >>>> + * Background writeout and kupdate-style writeback are > > >>>> + * easily livelockable. Stop them if there is other work > > >>>> + * to do so that e.g. sync can proceed. > > >>>> + */ > > >>>> + if ((work->for_background || work->for_kupdate) && > > >>>> + !list_empty(&wb->bdi->work_list)) > > >>>> + break; > > >>>> + /* > > >>> > > >>> So what happens if an application sits in a loop doing write&fsync to a > > >>> file? The vm's call for help gets ignored and your data doesn't get > > >>> written back for three days?? > > >> > > >> To avoid the possibility of any such occurrence, perhaps requeuing > > >> the work rather than cancelling it would be better? i.e. stop, put > > >> it behind whatever work just came in and so when the new work > > >> completes, we restart the background/expiry based writeback? > > > > > > That would be better. Ie. add a flag BDI_background_writeback to > > > indicate the background work should be started after finishing with > > > the queued works. bdi_start_background_writeback() can be modified to > > > simply set the bit and wake up the flusher thread. > > > > I think that is better, we ideally want background writeout to > > interleave smoothly with any other write activity. But why not just > > mark the 'background writeout was interrupted' bit here when breaking, > > and have the thread check-clear that when finishing some other piece > > of work (or when going idle)? > OK, although what would probably work as well would be to extend > wb_check_old_data_flush() to also start background writeback (not only > kupdate) if needed. Which probably makes sense regardless of any > interruption of background writeback we might do... Maybe let's talk about > this in some of the writeback sessions. Good idea. Then bdi_start_background_writeback() can simply wake it up. I'd vote for this way before hitting more requirements possibility in future. Thanks, Fengguang