All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Federico Cuello <fedux@lugmen.org.ar>
Cc: linux-kernel@vger.kernel.org, fedux@lugmen.org.ar,
	Artem Bityutskiy <Artem.Bityutskiy@nokia.com>,
	Nick Piggin <npiggin@suse.de>
Subject: Re: [PATCH] write-back: fix break condition
Date: Mon, 9 Feb 2009 15:21:40 -0800	[thread overview]
Message-ID: <20090209152140.aa3f50aa.akpm@linux-foundation.org> (raw)
In-Reply-To: <1233977610-8919-1-git-send-email-fedux@lugmen.org.ar>


Thanks, but please do cc the people who were involved with a patch when
you find a problem with it!

On Sat,  7 Feb 2009 01:33:30 -0200
Federico Cuello <fedux@lugmen.org.ar> wrote:

> Commit 673353723e7a6550625fb719059c5f31cfaecd18 fixed nr_to_write
> counter, but didn't set the break condition properly.

It's actually commit dcf6a79dda5cc2a2bec183e50d829030c0972aaa
("write-back: fix nr_to_write counter").

> If nr_to_write == 0 after being decremented it will loop one more time
> before setting done = 1 and breaking the loop.

We prefer that patches include the author's Signed-off-by:, as per
Documentation/SubmittingPatches, please.

> 
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index bb5fa2b..9e2ae50 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -981,20 +981,21 @@ continue_unlock:
>  				}
>   			}
>  
> -			if (nr_to_write > 0)
> +			if (nr_to_write > 0) {
>  				nr_to_write--;
> -			else if (wbc->sync_mode == WB_SYNC_NONE) {
> -				/*
> -				 * We stop writing back only if we are not
> -				 * doing integrity sync. In case of integrity
> -				 * sync we have to keep going because someone
> -				 * may be concurrently dirtying pages, and we
> -				 * might have synced a lot of newly appeared
> -				 * dirty pages, but have not synced all of the
> -				 * old dirty pages.
> -				 */
> -				done = 1;
> -				break;
> +				if (nr_to_write == 0 && wbc->sync_mode == WB_SYNC_NONE) {
> +					/*
> +					 * We stop writing back only if we are not
> +					 * doing integrity sync. In case of integrity
> +					 * sync we have to keep going because someone
> +					 * may be concurrently dirtying pages, and we
> +					 * might have synced a lot of newly appeared
> +					 * dirty pages, but have not synced all of the
> +					 * old dirty pages.
> +					 */
> +					done = 1;
> +					break;
> +				}
>  			}
>  
>  			if (wbc->nonblocking && bdi_write_congested(bdi)) {

Artem, Nick, please check?

  reply	other threads:[~2009-02-09 23:22 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-07  3:33 [PATCH] write-back: fix break condition Federico Cuello
2009-02-09 23:21 ` Andrew Morton [this message]
2009-02-10  0:34   ` Nick Piggin
2009-02-10  6:22     ` Damien Wyart
2009-02-10  7:47       ` Artem Bityutskiy
2009-02-10  7:46   ` Artem Bityutskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090209152140.aa3f50aa.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=Artem.Bityutskiy@nokia.com \
    --cc=fedux@lugmen.org.ar \
    --cc=linux-kernel@vger.kernel.org \
    --cc=npiggin@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.