* [PATCH] write-back: fix break condition
@ 2009-02-07 3:33 Federico Cuello
2009-02-09 23:21 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Federico Cuello @ 2009-02-07 3:33 UTC (permalink / raw)
To: linux-kernel; +Cc: Federico Cuello
Commit 673353723e7a6550625fb719059c5f31cfaecd18 fixed nr_to_write
counter, but didn't set the break condition properly.
If nr_to_write == 0 after being decremented it will loop one more time
before setting done = 1 and breaking the loop.
---
mm/page-writeback.c | 27 ++++++++++++++-------------
1 files changed, 14 insertions(+), 13 deletions(-)
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)) {
--
1.6.1.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] write-back: fix break condition
2009-02-07 3:33 [PATCH] write-back: fix break condition Federico Cuello
@ 2009-02-09 23:21 ` Andrew Morton
2009-02-10 0:34 ` Nick Piggin
2009-02-10 7:46 ` Artem Bityutskiy
0 siblings, 2 replies; 6+ messages in thread
From: Andrew Morton @ 2009-02-09 23:21 UTC (permalink / raw)
To: Federico Cuello; +Cc: linux-kernel, fedux, Artem Bityutskiy, Nick Piggin
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?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] write-back: fix break condition
2009-02-09 23:21 ` Andrew Morton
@ 2009-02-10 0:34 ` Nick Piggin
2009-02-10 6:22 ` Damien Wyart
2009-02-10 7:46 ` Artem Bityutskiy
1 sibling, 1 reply; 6+ messages in thread
From: Nick Piggin @ 2009-02-10 0:34 UTC (permalink / raw)
To: Andrew Morton; +Cc: Federico Cuello, linux-kernel, Artem Bityutskiy
On Mon, Feb 09, 2009 at 03:21:40PM -0800, Andrew Morton wrote:
>
> 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?
Yes, this looks OK by me.
Acked-by: Nick Piggin <npiggin@suse.de>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] write-back: fix break condition
2009-02-10 0:34 ` Nick Piggin
@ 2009-02-10 6:22 ` Damien Wyart
2009-02-10 7:47 ` Artem Bityutskiy
0 siblings, 1 reply; 6+ messages in thread
From: Damien Wyart @ 2009-02-10 6:22 UTC (permalink / raw)
To: Nick Piggin
Cc: Andrew Morton, Federico Cuello, linux-kernel, Artem Bityutskiy
* Nick Piggin <npiggin@suse.de> [2009-02-10 01:34]:
> On Mon, Feb 09, 2009 at 03:21:40PM -0800, Andrew Morton wrote:
> > 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?
> Yes, this looks OK by me.
> Acked-by: Nick Piggin <npiggin@suse.de>
Shouldn't this go to stable 2.6.28, too ? They have not been in Cc for
now. I think the problem was at first detected in 2.6.28.X, but it is
not yet sure if it is solved or if this interacts with some ext4
problems.
--
Damien Wyart
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] write-back: fix break condition
2009-02-10 6:22 ` Damien Wyart
@ 2009-02-10 7:47 ` Artem Bityutskiy
0 siblings, 0 replies; 6+ messages in thread
From: Artem Bityutskiy @ 2009-02-10 7:47 UTC (permalink / raw)
To: Damien Wyart
Cc: Nick Piggin, Andrew Morton, Federico Cuello,
linux-kernel@vger.kernel.org
Damien Wyart wrote:
>>> Artem, Nick, please check?
>
>> Yes, this looks OK by me.
>
>> Acked-by: Nick Piggin <npiggin@suse.de>
>
> Shouldn't this go to stable 2.6.28, too ? They have not been in Cc for
> now. I think the problem was at first detected in 2.6.28.X, but it is
> not yet sure if it is solved or if this interacts with some ext4
> problems.
It seems like this and my previous patch should go together to 2.6.28.X.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] write-back: fix break condition
2009-02-09 23:21 ` Andrew Morton
2009-02-10 0:34 ` Nick Piggin
@ 2009-02-10 7:46 ` Artem Bityutskiy
1 sibling, 0 replies; 6+ messages in thread
From: Artem Bityutskiy @ 2009-02-10 7:46 UTC (permalink / raw)
To: Andrew Morton; +Cc: Federico Cuello, linux-kernel@vger.kernel.org, Nick Piggin
Andrew Morton wrote:
>>
>> - 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?
I have not tested it, but it looks OK to me.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-02-10 7:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-02-07 3:33 [PATCH] write-back: fix break condition Federico Cuello
2009-02-09 23:21 ` Andrew Morton
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
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.