linux-kernel - Re: sync-Regression in 2.6.28.2?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 5 Feb 2009 11:19:04 +0100
From:	Ralf Hildebrandt <Ralf.Hildebrandt@...rite.de>
To:	Federico Cuello <fedux@...men.org.ar>
Cc:	Nick Piggin <nickpiggin@...oo.com.au>,
	Artem Bityutskiy <Artem.Bityutskiy@...ia.com>,
	linux-kernel@...r.kernel.org
Subject: Re: sync-Regression in 2.6.28.2?

* Federico Cuello <fedux@...men.org.ar>:
> Nick Piggin wrote:
> > [...]
> > Thanks, could you reply-to-all when replying to retain ccs please?
> >
> > Common theme is ext4, which uses no_nrwrite_index_update, and I introduced
> > a bug in there which could possibly cause ext4 to go into a loop...
> >
> > Would it be possible if you can test the following patch?
> >   
> 
> I'll test it as soon as I get home.
> 
> Meanwhile, I think the new patch may be slightly wrong. If I understand
> correctly PageWriteback(page) is called before nr_to_write is tested for
> being > 0 and then decremented if true, but "done" is  not set to 1
> until the next iteration. So another call to PageWriteback(page) while
> take place and then "done" will be set to true (if wbc->sync_mode ==
> WB_SYNC_NONE).
> 
> If nr_to_write == 1 at the beginning of the loop then two pages will be
> written.
> 
> I think the test condition should something like:
> 
> if (--nr_to_write <= 0 && wbc->sync_mode == WB_SYNC_NONE) {
>     done = 1;
>     break;
> }
> 
> .
> .
> 
> > Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@...ia.com>
> > Acked-by: Nick Piggin <npiggin@...e.de>
> > Cc: Andrew Morton <akpm@...ux-foundation.org>
> > ---
> >  mm/page-writeback.c |   21 +++++++++++++++------
> >  1 files changed, 15 insertions(+), 6 deletions(-)
> >
> > diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> > index b493db7..dc32dae 100644
> > --- a/mm/page-writeback.c
> > +++ b/mm/page-writeback.c
> > @@ -1051,13 +1051,22 @@ continue_unlock:
> >  				}
> >   			}
> >  
> > -			if (wbc->sync_mode == WB_SYNC_NONE) {
> > -				wbc->nr_to_write--;
> > -				if (wbc->nr_to_write <= 0) {
> > -					done = 1;
> > -					break;
> > -				}
> > +			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 (wbc->nonblocking && bdi_write_congested(bdi)) {
> >  				wbc->encountered_congestion = 1;
> >  				done = 1;
> >   
> 
While I didn't test the patch, I can confirm that the problem is also
in 2.6.28.3

-- 
Ralf Hildebrandt                                Ralf.Hildebrandt@...rite.de
Charite - Universitätsmedizin Berlin            Tel.  +49 (0)30-450 570-155
Geschäftsbereich IT | Abt. Netzwerk             Fax.  +49 (0)30-450 570-962
Hindenburgdamm 30 | 12200 Berlin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/