| Message ID | 20220216093020.175351-2-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: > Only caller of alloc_cmd() is null_submit_bio() unconditionally sets > second parameter to true & that is statically hard-coded in null_blk. Nit: s/&/and (let's write in proper English :)) > There is no point in having statically hardcoded function parameter. > > Remove the unnecessary parameter can_wait and adjust the code so it > can retain existing behavior of waiting when we don't get valid > nullb_cmd from __alloc_cmd() in alloc_cmd(). > > The restructured code avoids multiple return statements, multiple > calls to __alloc_cmd() and resulting a fast path call to > prepare_to_wait() due to removal of first alloc_cmd() call. > > Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com> > --- > drivers/block/null_blk/main.c | 21 +++++++++------------ > 1 file changed, 9 insertions(+), 12 deletions(-) > > diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c > index 13004beb48ca..d78fc3edb22e 100644 > --- a/drivers/block/null_blk/main.c > +++ b/drivers/block/null_blk/main.c > @@ -719,26 +719,23 @@ static struct nullb_cmd *__alloc_cmd(struct nullb_queue *nq) > return NULL; > } > > -static struct nullb_cmd *alloc_cmd(struct nullb_queue *nq, int can_wait) > +static struct nullb_cmd *alloc_cmd(struct nullb_queue *nq) > { > struct nullb_cmd *cmd; > DEFINE_WAIT(wait); > > - cmd = __alloc_cmd(nq); > - if (cmd || !can_wait) > - return cmd; > - > do { > - prepare_to_wait(&nq->wait, &wait, TASK_UNINTERRUPTIBLE); > + /* > + * This avoids multiple return statements, multiple calls to > + * __alloc_cmd() and a fast path call to prepare_to_wait(). > + */ > cmd = __alloc_cmd(nq); > if (cmd) > - break; > - > + return cmd; > + prepare_to_wait(&nq->wait, &wait, TASK_UNINTERRUPTIBLE); > io_schedule(); > + finish_wait(&nq->wait, &wait); > } while (1); > - > - finish_wait(&nq->wait, &wait); > - return cmd; > } Personally, I would simplify further like this: while (!(cmd = __alloc_cmd(nq)) { prepare_to_wait(&nq->wait, &wait, TASK_UNINTERRUPTIBLE); io_schedule(); finish_wait(&nq->wait, &wait); } return cmd; But that is a matter of taste I guess. > > static void end_cmd(struct nullb_cmd *cmd) > @@ -1478,7 +1475,7 @@ static void null_submit_bio(struct bio *bio) > struct nullb_queue *nq = nullb_to_queue(nullb); > struct nullb_cmd *cmd; > > - cmd = alloc_cmd(nq, 1); > + cmd = alloc_cmd(nq); > cmd->bio = bio; > > null_handle_cmd(cmd, sector, nr_sectors, bio_op(bio));
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c index 13004beb48ca..d78fc3edb22e 100644 --- a/drivers/block/null_blk/main.c +++ b/drivers/block/null_blk/main.c @@ -719,26 +719,23 @@ static struct nullb_cmd *__alloc_cmd(struct nullb_queue *nq) return NULL; } -static struct nullb_cmd *alloc_cmd(struct nullb_queue *nq, int can_wait) +static struct nullb_cmd *alloc_cmd(struct nullb_queue *nq) { struct nullb_cmd *cmd; DEFINE_WAIT(wait); - cmd = __alloc_cmd(nq); - if (cmd || !can_wait) - return cmd; - do { - prepare_to_wait(&nq->wait, &wait, TASK_UNINTERRUPTIBLE); + /* + * This avoids multiple return statements, multiple calls to + * __alloc_cmd() and a fast path call to prepare_to_wait(). + */ cmd = __alloc_cmd(nq); if (cmd) - break; - + return cmd; + prepare_to_wait(&nq->wait, &wait, TASK_UNINTERRUPTIBLE); io_schedule(); + finish_wait(&nq->wait, &wait); } while (1); - - finish_wait(&nq->wait, &wait); - return cmd; } static void end_cmd(struct nullb_cmd *cmd) @@ -1478,7 +1475,7 @@ static void null_submit_bio(struct bio *bio) struct nullb_queue *nq = nullb_to_queue(nullb); struct nullb_cmd *cmd; - cmd = alloc_cmd(nq, 1); + cmd = alloc_cmd(nq); cmd->bio = bio; null_handle_cmd(cmd, sector, nr_sectors, bio_op(bio));
Only caller of alloc_cmd() is null_submit_bio() unconditionally sets second parameter to true & that is statically hard-coded in null_blk. There is no point in having statically hardcoded function parameter. Remove the unnecessary parameter can_wait and adjust the code so it can retain existing behavior of waiting when we don't get valid nullb_cmd from __alloc_cmd() in alloc_cmd(). The restructured code avoids multiple return statements, multiple calls to __alloc_cmd() and resulting a fast path call to prepare_to_wait() due to removal of first alloc_cmd() call. Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com> --- drivers/block/null_blk/main.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-)