> On 22 Mar 2019, at 22.48, Igor Konopko <[email protected]> wrote: > > This patch is made in order to prepare read path for new approach to > partial read handling (which is needed for multi-page bvec handling) > The most important change is to move the handling of completed and > failed bio from the pblk_make_rq() to particular read and write > functions. This is needed, since after partial read path changes, > sometimes completed/failed bio will be different from original one, so > we cannot do this any longer in pblk_make_rq(). Other changes are > small read path refactor in order to reduce the size of another patch > with partial read changes. Generally the goal of this patch is not to > change the functionality, but just to prepare the code for the > following changes. > > Signed-off-by: Igor Konopko <[email protected]> > --- > drivers/lightnvm/pblk-cache.c | 7 ++-- > drivers/lightnvm/pblk-init.c | 14 ++------ > drivers/lightnvm/pblk-read.c | 83 ++++++++++++++++++++----------------------- > drivers/lightnvm/pblk.h | 4 +-- > 4 files changed, 47 insertions(+), 61 deletions(-) > > diff --git a/drivers/lightnvm/pblk-cache.c b/drivers/lightnvm/pblk-cache.c > index c9fa26f..d2a3683 100644 > --- a/drivers/lightnvm/pblk-cache.c > +++ b/drivers/lightnvm/pblk-cache.c > @@ -18,7 +18,7 @@ > > #include "pblk.h" > > -int pblk_write_to_cache(struct pblk *pblk, struct bio *bio, unsigned long > flags) > +void pblk_write_to_cache(struct pblk *pblk, struct bio *bio, unsigned long > flags) > { > struct request_queue *q = pblk->dev->q; > struct pblk_w_ctx w_ctx; > @@ -43,6 +43,7 @@ int pblk_write_to_cache(struct pblk *pblk, struct bio *bio, > unsigned long flags) > goto retry; > case NVM_IO_ERR: > pblk_pipeline_stop(pblk); > + bio_io_error(bio); > goto out; > } > > @@ -79,7 +80,9 @@ int pblk_write_to_cache(struct pblk *pblk, struct bio *bio, > unsigned long flags) > out: > generic_end_io_acct(q, REQ_OP_WRITE, &pblk->disk->part0, start_time); > pblk_write_should_kick(pblk); > - return ret; > + > + if (ret == NVM_IO_DONE) > + bio_endio(bio); > } > > /* > diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c > index e2e1193..4211cd1 100644 > --- a/drivers/lightnvm/pblk-init.c > +++ b/drivers/lightnvm/pblk-init.c > @@ -50,7 +50,6 @@ struct bio_set pblk_bio_set; > static blk_qc_t pblk_make_rq(struct request_queue *q, struct bio *bio) > { > struct pblk *pblk = q->queuedata; > - int ret; > > if (bio_op(bio) == REQ_OP_DISCARD) { > pblk_discard(pblk, bio); > @@ -65,7 +64,7 @@ static blk_qc_t pblk_make_rq(struct request_queue *q, > struct bio *bio) > */ > if (bio_data_dir(bio) == READ) { > blk_queue_split(q, &bio); > - ret = pblk_submit_read(pblk, bio); > + pblk_submit_read(pblk, bio); > } else { > /* Prevent deadlock in the case of a modest LUN configuration > * and large user I/Os. Unless stalled, the rate limiter > @@ -74,16 +73,7 @@ static blk_qc_t pblk_make_rq(struct request_queue *q, > struct bio *bio) > if (pblk_get_secs(bio) > pblk_rl_max_io(&pblk->rl)) > blk_queue_split(q, &bio); > > - ret = pblk_write_to_cache(pblk, bio, PBLK_IOTYPE_USER); > - } > - > - switch (ret) { > - case NVM_IO_ERR: > - bio_io_error(bio); > - break; > - case NVM_IO_DONE: > - bio_endio(bio); > - break; > + pblk_write_to_cache(pblk, bio, PBLK_IOTYPE_USER); > } > > return BLK_QC_T_NONE; > diff --git a/drivers/lightnvm/pblk-read.c b/drivers/lightnvm/pblk-read.c > index 6569746..597fe6d 100644 > --- a/drivers/lightnvm/pblk-read.c > +++ b/drivers/lightnvm/pblk-read.c > @@ -179,7 +179,8 @@ static void pblk_end_user_read(struct bio *bio, int error) > { > if (error && error != NVM_RSP_WARN_HIGHECC) > bio_io_error(bio); > - bio_endio(bio); > + else > + bio_endio(bio); > } > > static void __pblk_end_io_read(struct pblk *pblk, struct nvm_rq *rqd, > @@ -383,7 +384,6 @@ static int pblk_partial_read_bio(struct pblk *pblk, > struct nvm_rq *rqd, > > /* Free allocated pages in new bio */ > pblk_bio_free_pages(pblk, rqd->bio, 0, rqd->bio->bi_vcnt); > - __pblk_end_io_read(pblk, rqd, false); > return NVM_IO_ERR; > } > > @@ -428,7 +428,7 @@ static void pblk_read_rq(struct pblk *pblk, struct nvm_rq > *rqd, struct bio *bio, > } > } > > -int pblk_submit_read(struct pblk *pblk, struct bio *bio) > +void pblk_submit_read(struct pblk *pblk, struct bio *bio) > { > struct nvm_tgt_dev *dev = pblk->dev; > struct request_queue *q = dev->q; > @@ -436,9 +436,9 @@ int pblk_submit_read(struct pblk *pblk, struct bio *bio) > unsigned int nr_secs = pblk_get_secs(bio); > struct pblk_g_ctx *r_ctx; > struct nvm_rq *rqd; > + struct bio *int_bio; > unsigned int bio_init_idx; > DECLARE_BITMAP(read_bitmap, NVM_MAX_VLBA); > - int ret = NVM_IO_ERR; > > generic_start_io_acct(q, REQ_OP_READ, bio_sectors(bio), > &pblk->disk->part0); > @@ -449,74 +449,67 @@ int pblk_submit_read(struct pblk *pblk, struct bio *bio) > > rqd->opcode = NVM_OP_PREAD; > rqd->nr_ppas = nr_secs; > - rqd->bio = NULL; /* cloned bio if needed */ > rqd->private = pblk; > rqd->end_io = pblk_end_io_read; > > r_ctx = nvm_rq_to_pdu(rqd); > r_ctx->start_time = jiffies; > r_ctx->lba = blba; > - r_ctx->private = bio; /* original bio */ > > /* Save the index for this bio's start. This is needed in case > * we need to fill a partial read. > */ > bio_init_idx = pblk_get_bi_idx(bio); > > - if (pblk_alloc_rqd_meta(pblk, rqd)) > - goto fail_rqd_free; > + if (pblk_alloc_rqd_meta(pblk, rqd)) { > + bio_io_error(bio); > + pblk_free_rqd(pblk, rqd, PBLK_READ); > + return; > + } > + > + /* Clone read bio to deal internally with: > + * -read errors when reading from drive > + * -bio_advance() calls during l2p lookup and cache reads > + */ > + int_bio = bio_clone_fast(bio, GFP_KERNEL, &pblk_bio_set); > > if (nr_secs > 1) > pblk_read_ppalist_rq(pblk, rqd, bio, blba, read_bitmap); > else > pblk_read_rq(pblk, rqd, bio, blba, read_bitmap); > > + r_ctx->private = bio; /* original bio */ > + rqd->bio = int_bio; /* internal bio */ > + > if (bitmap_full(read_bitmap, nr_secs)) { > + pblk_end_user_read(bio, 0); > atomic_inc(&pblk->inflight_io); > __pblk_end_io_read(pblk, rqd, false); > - return NVM_IO_DONE; > + return; > } > > - /* All sectors are to be read from the device */ > - if (bitmap_empty(read_bitmap, rqd->nr_ppas)) { > - struct bio *int_bio = NULL; > - > - /* Clone read bio to deal with read errors internally */ > - int_bio = bio_clone_fast(bio, GFP_KERNEL, &pblk_bio_set); > - if (!int_bio) { > - pblk_err(pblk, "could not clone read bio\n"); > - goto fail_end_io; > - } > - > - rqd->bio = int_bio; > - > - if (pblk_submit_io(pblk, rqd)) { > + if (!bitmap_empty(read_bitmap, rqd->nr_ppas)) { > + /* The read bio request could be partially filled by the write > + * buffer, but there are some holes that need to be read from > + * the drive. > + */ > + bio_put(int_bio); > + rqd->bio = NULL; > + if (pblk_partial_read_bio(pblk, rqd, bio_init_idx, read_bitmap, > + nr_secs)) { > pblk_err(pblk, "read IO submission failed\n"); > - ret = NVM_IO_ERR; > - goto fail_end_io; > + bio_io_error(bio); > + __pblk_end_io_read(pblk, rqd, false); > } > - > - return NVM_IO_OK; > + return; > } > > - /* The read bio request could be partially filled by the write buffer, > - * but there are some holes that need to be read from the drive. > - */ > - ret = pblk_partial_read_bio(pblk, rqd, bio_init_idx, read_bitmap, > - nr_secs); > - if (ret) > - goto fail_meta_free; > - > - return NVM_IO_OK; > - > -fail_meta_free: > - nvm_dev_dma_free(dev->parent, rqd->meta_list, rqd->dma_meta_list); > -fail_rqd_free: > - pblk_free_rqd(pblk, rqd, PBLK_READ); > - return ret; > -fail_end_io: > - __pblk_end_io_read(pblk, rqd, false); > - return ret; > + /* All sectors are to be read from the device */ > + if (pblk_submit_io(pblk, rqd)) { > + pblk_err(pblk, "read IO submission failed\n"); > + bio_io_error(bio); > + __pblk_end_io_read(pblk, rqd, false); > + } > } > > static int read_ppalist_rq_gc(struct pblk *pblk, struct nvm_rq *rqd, > diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h > index 381f074..a3c925d 100644 > --- a/drivers/lightnvm/pblk.h > +++ b/drivers/lightnvm/pblk.h > @@ -868,7 +868,7 @@ void pblk_get_packed_meta(struct pblk *pblk, struct > nvm_rq *rqd); > /* > * pblk user I/O write path > */ > -int pblk_write_to_cache(struct pblk *pblk, struct bio *bio, > +void pblk_write_to_cache(struct pblk *pblk, struct bio *bio, > unsigned long flags); > int pblk_write_gc_to_cache(struct pblk *pblk, struct pblk_gc_rq *gc_rq); > > @@ -894,7 +894,7 @@ void pblk_write_kick(struct pblk *pblk); > * pblk read path > */ > extern struct bio_set pblk_bio_set; > -int pblk_submit_read(struct pblk *pblk, struct bio *bio); > +void pblk_submit_read(struct pblk *pblk, struct bio *bio); > int pblk_submit_read_gc(struct pblk *pblk, struct pblk_gc_rq *gc_rq); > /* > * pblk recovery > -- > 2.9.5
The patch looks sane. Reviewed-by: Javier González <[email protected]>
signature.asc
Description: Message signed with OpenPGP
