Re: [2.6.22.y] {01**/17} - do_anonymous_page-race - series for stablekernel

[Hugh Dickins]

On Sat, 2 Feb 2008, Linus Torvalds wrote:
> On Sat, 2 Feb 2008, Oliver Pinter (Pint?r Oliv?r) wrote:
> >
> > NOT IN MAINLINE
> > 
> > Linus it's go or drop it?
> 
> I have no idea, because you've used some horrible and stupid attachment 
> format that I can't even read. Patches should be inline so that people can 
> see them and comment on them.

Here it is, below, from a more accessible earlier posting by Oliver.
I'd say that this is clearly not suitable to be rushed into stable
ahead of mainline (though it wouldn't do worse than tiny slowdown).

(a) It's from three years ago, what's changed around meanwhile?
(b) It's a mod to bio_endio(), I don't see how it can relate to
    the title's unexplained "do_anonymous_page race" (anonymous?).
(c) If this adds an smp_wmb, where's the corresponding smp_rmb?
(d) Comment doesn't even make sense: a lock_unlock or lock_unlock?

I suspect it relates to the smp_wmb's 2.6.9 added to clear_user_highpage
and copy_user_highpage around the same time (the ones which Nick wishes
to move to SetPageUptodate); maybe that was mainline's preferred answer
to the same problem.

Andrea, if this is still needed, I think it'd be best if you send a
proper description of the problem and argument for your solution to
Andrew cc Nick and Jens, after that it could be considered for stable.

Thanks,
Hugh (who was intrigued by the "do_anonymous_page race" title)

commit 8327e0f191341cc32fb89bf4d7bee7c2524ae4e0
Author: Andrea Arcangeli <andrea@xxxxxxx>
Date:   Mon Jan 28 20:52:29 2008 +0100

    Subject: Race condition in userspace testcase
    References: 46948, LTC11574
    
    
    Additional Comment #103 From Andrea Arcangeli 2004-10-15 19:41
    the last patch I attached is the safest I believe.
    
    I'm not sure if a lock_unlock or lock_unlock is always guaranteed to happen
    after the I/O, and this makes sure no race can happen anymore.

diff --git a/fs/bio.c b/fs/bio.c
index 093345f..984d589 100644
--- a/fs/bio.c
+++ b/fs/bio.c
@@ -1028,6 +1028,15 @@ void bio_endio(struct bio *bio, unsigned int bytes_done, int error)
 	bio->bi_size -= bytes_done;
 	bio->bi_sector += (bytes_done >> 9);
 
+	if (bio_data_dir(bio) == READ)
+		/*
+		 * If the current cpu has written to the page by hand
+		 * without dma, we must enforce ordering to be sure
+		 * this written data will be visible before we expose
+		 * the page contents to other cpus (for example with
+		 * a set_pte).
+		 */
+		smp_wmb();
 	if (bio->bi_end_io)
 		bio->bi_end_io(bio, bytes_done, error);
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/