| Message ID | 20220216093020.175351-3-kch@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | null_blk: cleanup alloc_cmd() | expand |
On 2/16/22 18:30, Chaitanya Kulkarni wrote: > Follow the pattern what we have in bio_alloc() to set the structure s/what/that > members in the structure allocation function in alloc_cmd() and pass > bio to initialize newly allocated cmd->bio member. > > Follow the pattern in copy_to_nullb() to use result of one function call > (null_cache_active()) to be used as a parameter to another function call > (null_insert_page()), use result of alloc_cmd() as a first parameter to > the null_handle_cmd() in null_submit_bio() function. This allow us to > remove the local variable cmd on stack in null_submit_bio() that is in > fast path. > > Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com> > --- > drivers/block/null_blk/main.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c > index d78fc3edb22e..e19340f686a8 100644 > --- a/drivers/block/null_blk/main.c > +++ b/drivers/block/null_blk/main.c > @@ -719,7 +719,7 @@ static struct nullb_cmd *__alloc_cmd(struct nullb_queue *nq) > return NULL; > } > > -static struct nullb_cmd *alloc_cmd(struct nullb_queue *nq) > +static struct nullb_cmd *alloc_cmd(struct nullb_queue *nq, struct bio *bio) > { > struct nullb_cmd *cmd; > DEFINE_WAIT(wait); > @@ -730,8 +730,10 @@ static struct nullb_cmd *alloc_cmd(struct nullb_queue *nq) > * __alloc_cmd() and a fast path call to prepare_to_wait(). > */ > cmd = __alloc_cmd(nq); > - if (cmd) > + if (cmd) { > + cmd->bio = bio; > return cmd; > + } > prepare_to_wait(&nq->wait, &wait, TASK_UNINTERRUPTIBLE); > io_schedule(); > finish_wait(&nq->wait, &wait); > @@ -1473,12 +1475,8 @@ static void null_submit_bio(struct bio *bio) > sector_t nr_sectors = bio_sectors(bio); > struct nullb *nullb = bio->bi_bdev->bd_disk->private_data; > struct nullb_queue *nq = nullb_to_queue(nullb); > - struct nullb_cmd *cmd; > - > - cmd = alloc_cmd(nq); > - cmd->bio = bio; > > - null_handle_cmd(cmd, sector, nr_sectors, bio_op(bio)); > + null_handle_cmd(alloc_cmd(nq, bio), sector, nr_sectors, bio_op(bio)); > } > > static bool should_timeout_request(struct request *rq) Since patch 1 rewrite this function already, you could squash this patch into it, replacing the can_wait argument with the bio argument.
>> static bool should_timeout_request(struct request *rq) > > Since patch 1 rewrite this function already, you could squash this patch > into it, replacing the can_wait argument with the bio argument. > > if it is easier to review sure... Thanks a lot for your review comments, sent out V2 please have a look. -ck
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c index d78fc3edb22e..e19340f686a8 100644 --- a/drivers/block/null_blk/main.c +++ b/drivers/block/null_blk/main.c @@ -719,7 +719,7 @@ static struct nullb_cmd *__alloc_cmd(struct nullb_queue *nq) return NULL; } -static struct nullb_cmd *alloc_cmd(struct nullb_queue *nq) +static struct nullb_cmd *alloc_cmd(struct nullb_queue *nq, struct bio *bio) { struct nullb_cmd *cmd; DEFINE_WAIT(wait); @@ -730,8 +730,10 @@ static struct nullb_cmd *alloc_cmd(struct nullb_queue *nq) * __alloc_cmd() and a fast path call to prepare_to_wait(). */ cmd = __alloc_cmd(nq); - if (cmd) + if (cmd) { + cmd->bio = bio; return cmd; + } prepare_to_wait(&nq->wait, &wait, TASK_UNINTERRUPTIBLE); io_schedule(); finish_wait(&nq->wait, &wait); @@ -1473,12 +1475,8 @@ static void null_submit_bio(struct bio *bio) sector_t nr_sectors = bio_sectors(bio); struct nullb *nullb = bio->bi_bdev->bd_disk->private_data; struct nullb_queue *nq = nullb_to_queue(nullb); - struct nullb_cmd *cmd; - - cmd = alloc_cmd(nq); - cmd->bio = bio; - null_handle_cmd(cmd, sector, nr_sectors, bio_op(bio)); + null_handle_cmd(alloc_cmd(nq, bio), sector, nr_sectors, bio_op(bio)); } static bool should_timeout_request(struct request *rq)
Follow the pattern what we have in bio_alloc() to set the structure members in the structure allocation function in alloc_cmd() and pass bio to initialize newly allocated cmd->bio member. Follow the pattern in copy_to_nullb() to use result of one function call (null_cache_active()) to be used as a parameter to another function call (null_insert_page()), use result of alloc_cmd() as a first parameter to the null_handle_cmd() in null_submit_bio() function. This allow us to remove the local variable cmd on stack in null_submit_bio() that is in fast path. Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com> --- drivers/block/null_blk/main.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)