Re: [PATCH 1/2] Avoid bio_endio recursion

[Mikulas Patocka]

On Wed, 2 Jul 2008, Jens Axboe wrote:

> On Wed, Jul 02 2008, Mikulas Patocka wrote:
> > > Right, that wont work of course. Completions are typically done through
> > > a softirq, so it is not currently done with hard interrupts disabled.
> >
> > I thought, from hardirq - that's what IDE is doing. And they are called
> > with interrupts disabled (maybe unless you specify unmaskirq, which is not
> > default). What block driver does completions with softirq? ... and why?
>
> The key word is 'typically', the old IDE driver really isn't used very
> much. The SCSI layer and eg cciss uses the block layer softirq
> completions, so that is what 99% of the uses will be.

I use the old IDE driver, I don't see a reason why driver should create 
SCSI requests and lower layer translate them to ATA commands.

Just to look, this is the first version of the patch without disabling 
interrupts. If you are concerned about disabling interrupts, you can go 
this way.

Mikulas

Index: linux-2.6.26-rc5-devel/fs/bio.c
===================================================================
--- linux-2.6.26-rc5-devel.orig/fs/bio.c	2008-06-18 16:05:45.000000000 +0200
+++ linux-2.6.26-rc5-devel/fs/bio.c	2008-06-18 23:27:28.000000000 +0200
@@ -1168,6 +1168,25 @@
  **/
 void bio_endio(struct bio *bio, int error)
 {
+	static DEFINE_PER_CPU(local_t, bio_end_queue) = LOCAL_INIT(0);
+	local_t *bio_end_queue_ptr;
+	unsigned long l;
+
+	bio->bi_next = NULL;
+
+	bio_end_queue_ptr = &get_cpu_var(bio_end_queue);
+	if (local_read(bio_end_queue_ptr)) {
+insert_race:
+		bio->bi_next = (void *)local_read(bio_end_queue_ptr);
+		if (unlikely(local_cmpxchg(bio_end_queue_ptr, (unsigned long)bio->bi_next, (unsigned long)bio) != (unsigned long)bio->bi_next))
+			goto insert_race;
+		put_cpu_var(bio_end_queue);
+		return;
+	}
+
+	local_set(bio_end_queue_ptr, 1);
+
+process_bio:
 	if (error)
 		clear_bit(BIO_UPTODATE, &bio->bi_flags);
 	else if (!test_bit(BIO_UPTODATE, &bio->bi_flags))
@@ -1175,6 +1194,19 @@

 	if (bio->bi_end_io)
 		bio->bi_end_io(bio, error);
+
+remove_race:
+	l = local_read(bio_end_queue_ptr);
+	if (l != 1) {
+		bio = (void *)l;
+		if (unlikely(local_cmpxchg(bio_end_queue_ptr, (unsigned long)bio, (unsigned long)bio->bi_next) != (unsigned long)bio))
+			goto remove_race;
+		goto process_bio;
+	}
+	if (unlikely(local_cmpxchg(bio_end_queue_ptr, 1, 0) != 1))
+		goto remove_race;
+
+	put_cpu_var(bio_end_queue);
 }

 void bio_pair_release(struct bio_pair *bp)
--
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/