On Sat, Jan 22, 2011 at 01:17:24AM +0000, Jens Axboe wrote:
[..]
> mm/page-writeback.c | 2 +-
> mm/readahead.c | 12 ---
> mm/shmem.c | 1 -
> mm/swap_state.c | 5 +-
> mm/swapfile.c | 37 --------
> mm/vmscan.c | 2 +-
> 118 files changed, 153 insertions(+), 1248 deletions(-)
block/blk-throttle.c also uses blk_unplug(). We need to get rid of that
also.
[..]
> @@ -632,8 +630,6 @@ void elv_insert(struct request_queue *q, struct request *rq, int where)
> * don't force unplug of the queue for that case.
> * Clear unplug_it and fall through.
> */
Above comments now seem to be redundant.
> - unplug_it = 0;
> -
> case ELEVATOR_INSERT_FRONT:
> rq->cmd_flags |= REQ_SOFTBARRIER;
> list_add(&rq->queuelist, &q->queue_head);
[..]
> /*
> diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
> index b9e1e15..5ef136c 100644
> --- a/drivers/md/dm-raid.c
> +++ b/drivers/md/dm-raid.c
> @@ -394,7 +394,7 @@ static void raid_unplug(struct dm_target_callbacks *cb)
> {
> struct raid_set *rs = container_of(cb, struct raid_set, callbacks);
>
> - md_raid5_unplug_device(rs->md.private);
> + md_raid5_kick_device(rs->md.private);
With all the unplug logic gone, I think we can get rid of blk_sync_queue()
call from md. It looks like md was syncing the queue just to make sure
that unplug_fn is not called again. Now all that logic is gone so it
should be redundant.
Also we can probably get rid of some queue_lock taking instances in
md code. NeilBrown recently put following patch in, which is taking
queue lock only around plug functions. Now queue plugging gone,
I guess it should not be required.
commit da9cf5050a2e3dbc3cf26a8d908482eb4485ed49
Author: NeilBrown <[email protected]>
Date: Mon Feb 21 18:25:57 2011 +1100
md: avoid spinlock problem in blk_throtl_exit
Thanks
Vivek
On 2011-03-04 05:00, Vivek Goyal wrote:
> On Sat, Jan 22, 2011 at 01:17:24AM +0000, Jens Axboe wrote:
>
> [..]
>> mm/page-writeback.c | 2 +-
>> mm/readahead.c | 12 ---
>> mm/shmem.c | 1 -
>> mm/swap_state.c | 5 +-
>> mm/swapfile.c | 37 --------
>> mm/vmscan.c | 2 +-
>> 118 files changed, 153 insertions(+), 1248 deletions(-)
>
> block/blk-throttle.c also uses blk_unplug(). We need to get rid of that
> also.
Done.
> [..]
>> @@ -632,8 +630,6 @@ void elv_insert(struct request_queue *q, struct request *rq, int where)
>> * don't force unplug of the queue for that case.
>> * Clear unplug_it and fall through.
>> */
>
> Above comments now seem to be redundant.
Killed.
>> diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
>> index b9e1e15..5ef136c 100644
>> --- a/drivers/md/dm-raid.c
>> +++ b/drivers/md/dm-raid.c
>> @@ -394,7 +394,7 @@ static void raid_unplug(struct dm_target_callbacks *cb)
>> {
>> struct raid_set *rs = container_of(cb, struct raid_set, callbacks);
>>
>> - md_raid5_unplug_device(rs->md.private);
>> + md_raid5_kick_device(rs->md.private);
>
> With all the unplug logic gone, I think we can get rid of blk_sync_queue()
> call from md. It looks like md was syncing the queue just to make sure
> that unplug_fn is not called again. Now all that logic is gone so it
> should be redundant.
>
> Also we can probably get rid of some queue_lock taking instances in
> md code. NeilBrown recently put following patch in, which is taking
> queue lock only around plug functions. Now queue plugging gone,
> I guess it should not be required.
Agree on both accounts. I'll leave that out for this version, though.
--
Jens Axboe
On Tue, Mar 08, 2011 at 01:24:01PM +0100, Jens Axboe wrote:
> On 2011-03-04 05:00, Vivek Goyal wrote:
> > On Sat, Jan 22, 2011 at 01:17:24AM +0000, Jens Axboe wrote:
> >
> > [..]
> >> mm/page-writeback.c | 2 +-
> >> mm/readahead.c | 12 ---
> >> mm/shmem.c | 1 -
> >> mm/swap_state.c | 5 +-
> >> mm/swapfile.c | 37 --------
> >> mm/vmscan.c | 2 +-
> >> 118 files changed, 153 insertions(+), 1248 deletions(-)
> >
> > block/blk-throttle.c also uses blk_unplug(). We need to get rid of that
> > also.
>
> Done.
Thanks Jens. Looking at the usage of blk_plug, i think it makes sense to
make use of it in throttle dispatch also. Here is the patch.
blk-throttle: Use blk_plug in throttle dispatch
Use plug in throttle dispatch also as we are dispatching a bunch of
bios in throttle context and some of them might merge.
Signed-off-by: Vivek Goyal <[email protected]>
---
block/blk-throttle.c | 3 +++
1 file changed, 3 insertions(+)
Index: linux-2.6-block/block/blk-throttle.c
===================================================================
--- linux-2.6-block.orig/block/blk-throttle.c 2011-03-08 16:43:14.000000000 -0500
+++ linux-2.6-block/block/blk-throttle.c 2011-03-08 16:59:44.359620804 -0500
@@ -770,6 +770,7 @@ static int throtl_dispatch(struct reques
unsigned int nr_disp = 0;
struct bio_list bio_list_on_stack;
struct bio *bio;
+ struct blk_plug plug;
spin_lock_irq(q->queue_lock);
@@ -798,8 +799,10 @@ out:
* immediate dispatch
*/
if (nr_disp) {
+ blk_start_plug(&plug);
while((bio = bio_list_pop(&bio_list_on_stack)))
generic_make_request(bio);
+ blk_finish_plug(&plug);
}
return nr_disp;
}
On 2011-03-08 23:10, Vivek Goyal wrote:
> On Tue, Mar 08, 2011 at 01:24:01PM +0100, Jens Axboe wrote:
>> On 2011-03-04 05:00, Vivek Goyal wrote:
>>> On Sat, Jan 22, 2011 at 01:17:24AM +0000, Jens Axboe wrote:
>>>
>>> [..]
>>>> mm/page-writeback.c | 2 +-
>>>> mm/readahead.c | 12 ---
>>>> mm/shmem.c | 1 -
>>>> mm/swap_state.c | 5 +-
>>>> mm/swapfile.c | 37 --------
>>>> mm/vmscan.c | 2 +-
>>>> 118 files changed, 153 insertions(+), 1248 deletions(-)
>>>
>>> block/blk-throttle.c also uses blk_unplug(). We need to get rid of that
>>> also.
>>
>> Done.
>
> Thanks Jens. Looking at the usage of blk_plug, i think it makes sense to
> make use of it in throttle dispatch also. Here is the patch.
Yep, it definitely does. Thanks, I'll apply this.
--
Jens Axboe