linux-kernel - Re: [PATCH 1/2] Avoid bio

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, 3 Jul 2008 17:08:10 -0400 (EDT)
From:	Mikulas Patocka <mpatocka@...hat.com>
To:	Jens Axboe <jens.axboe@...cle.com>
cc:	linux-kernel@...r.kernel.org, Neil Brown <neilb@...e.de>
Subject: Re: [PATCH 1/2] Avoid bio_endio recursion

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/