Re: [patch] use cheaper elv_queue_empty when unplug a device

2005-04-08 Thread Nick Piggin
Jens Axboe wrote: On Fri, Apr 08 2005, Nick Piggin wrote: I guess this isn't a problem, as io contexts should be allocated comparatively rarely. It would be possible to move it out of the lock though if we really want to. Lets just keep it inside the lock, for the fast case it should just be an i

Re: [patch] use cheaper elv_queue_empty when unplug a device

2005-04-08 Thread Jens Axboe
On Fri, Apr 08 2005, Nick Piggin wrote: > Jens Axboe wrote: > >On Wed, Mar 30 2005, Nick Piggin wrote: > > >>So Kenneth if you could look into this one as well, to see if > >>it is worthwhile, that would be great. > > > > > >For that to work, you have to change the get_io_context() allocation to >

Re: [patch] use cheaper elv_queue_empty when unplug a device

2005-04-08 Thread Nick Piggin
Jens Axboe wrote: On Wed, Mar 30 2005, Nick Piggin wrote: So Kenneth if you could look into this one as well, to see if it is worthwhile, that would be great. For that to work, you have to change the get_io_context() allocation to be GFP_ATOMIC. Yes of course, thanks for picking that up. I guess

Re: [patch] use cheaper elv_queue_empty when unplug a device

2005-04-08 Thread Jens Axboe
On Wed, Mar 30 2005, Nick Piggin wrote: > Nick Piggin wrote: > >Jens Axboe wrote: > > > >>Looks good, I've been toying with something very similar for a long time > >>myself. > >> > > > >Here is another thing I just noticed that should further reduce the > >locking by at least 1, sometimes 2 lock/u

Re: [patch] use cheaper elv_queue_empty when unplug a device

2005-03-30 Thread Jens Axboe
On Wed, Mar 30 2005, Nick Piggin wrote: > Nick Piggin wrote: > >Jens Axboe wrote: > > > >>Looks good, I've been toying with something very similar for a long time > >>myself. > >> > > > >Here is another thing I just noticed that should further reduce the > >locking by at least 1, sometimes 2 lock/u

Re: [patch] use cheaper elv_queue_empty when unplug a device

2005-03-29 Thread Nick Piggin
Nick Piggin wrote: Jens Axboe wrote: Looks good, I've been toying with something very similar for a long time myself. Here is another thing I just noticed that should further reduce the locking by at least 1, sometimes 2 lock/unlock pairs per request. At the cost of uglifying the code somewhat. Alt

Re: [patch] use cheaper elv_queue_empty when unplug a device

2005-03-29 Thread Nick Piggin
Jens Axboe wrote: On Tue, Mar 29 2005, Nick Piggin wrote: @@ -2577,19 +2577,18 @@ static int __make_request(request_queue_ spin_lock_prefetch(q->queue_lock); barrier = bio_barrier(bio); - if (barrier && (q->ordered == QUEUE_ORDERED_NONE)) { + if (unlikely(barrier) && (q-

RE: [patch] use cheaper elv_queue_empty when unplug a device

2005-03-29 Thread Chen, Kenneth W
Nick Piggin wrote on Tuesday, March 29, 2005 1:20 AM > Speaking of which, I've had a few ideas lying around for possible > performance improvement in the block code. > > I haven't used a big disk array (or tried any simulation), but I'll > attach the patch if you're looking into that area. > > lin

Re: [patch] use cheaper elv_queue_empty when unplug a device

2005-03-29 Thread Jens Axboe
On Tue, Mar 29 2005, Nick Piggin wrote: > @@ -2577,19 +2577,18 @@ static int __make_request(request_queue_ > spin_lock_prefetch(q->queue_lock); > > barrier = bio_barrier(bio); > - if (barrier && (q->ordered == QUEUE_ORDERED_NONE)) { > + if (unlikely(barrier) && (q->ordered ==

Re: [patch] use cheaper elv_queue_empty when unplug a device

2005-03-29 Thread Nick Piggin
Arjan van de Ven wrote: On Tue, 2005-03-29 at 19:19 +1000, Nick Piggin wrote: - removes the relock/retry merge mechanism in __make_request if we aren't able to get the GFP_ATOMIC allocation. Just fall through and assume the chances of getting a merge will be small (is this a valid assumption?

Re: [patch] use cheaper elv_queue_empty when unplug a device

2005-03-29 Thread Jens Axboe
On Tue, Mar 29 2005, Arjan van de Ven wrote: > On Tue, 2005-03-29 at 19:19 +1000, Nick Piggin wrote: > > - removes the relock/retry merge mechanism in __make_request if we > >aren't able to get the GFP_ATOMIC allocation. Just fall through > >and assume the chances of getting a merge will be

Re: [patch] use cheaper elv_queue_empty when unplug a device

2005-03-29 Thread Arjan van de Ven
On Tue, 2005-03-29 at 19:19 +1000, Nick Piggin wrote: > - removes the relock/retry merge mechanism in __make_request if we >aren't able to get the GFP_ATOMIC allocation. Just fall through >and assume the chances of getting a merge will be small (is this >a valid assumption? Should measu

Re: [patch] use cheaper elv_queue_empty when unplug a device

2005-03-29 Thread Nick Piggin
Jens Axboe wrote: Looks good, I've been toying with something very similar for a long time myself. Here is another thing I just noticed that should further reduce the locking by at least 1, sometimes 2 lock/unlock pairs per request. At the cost of uglifying the code somewhat. Although it is pretty

Re: [patch] use cheaper elv_queue_empty when unplug a device

2005-03-29 Thread Nick Piggin
Jens Axboe wrote: Looks good, I've been toying with something very similar for a long time myself. The unplug change is a no-brainer. Yep - I may have even stolen it from you (or someone) from a patch which had been forgotten. I can't remember for sure, but it is trivial enough that anyone could co

Re: [patch] use cheaper elv_queue_empty when unplug a device

2005-03-29 Thread Jens Axboe
On Tue, Mar 29 2005, Nick Piggin wrote: > Jens Axboe wrote: > >On Mon, Mar 28 2005, Chen, Kenneth W wrote: > > > >>This patch was posted last year and if I remember correctly, Jens said > >>he is OK with the patch. In function __generic_unplug_deivce(), kernel > >>can use a cheaper function elv_qu

Re: [patch] use cheaper elv_queue_empty when unplug a device

2005-03-29 Thread Nick Piggin
Jens Axboe wrote: On Mon, Mar 28 2005, Chen, Kenneth W wrote: This patch was posted last year and if I remember correctly, Jens said he is OK with the patch. In function __generic_unplug_deivce(), kernel can use a cheaper function elv_queue_empty() instead of more expensive elv_next_request to fin

Re: [patch] use cheaper elv_queue_empty when unplug a device

2005-03-29 Thread Nick Piggin
Nick Piggin wrote: I haven't used a big disk array (or tried any simulation), but I'll attach the patch if you're looking into that area. Oh, and this one removes a memory barrier. I think we (Jens and I) agreed this is valid. Whether or not you'll notice a difference is another story ;) This mem

Re: [patch] use cheaper elv_queue_empty when unplug a device

2005-03-29 Thread Jens Axboe
On Mon, Mar 28 2005, Chen, Kenneth W wrote: > This patch was posted last year and if I remember correctly, Jens said > he is OK with the patch. In function __generic_unplug_deivce(), kernel > can use a cheaper function elv_queue_empty() instead of more expensive > elv_next_request to find whether