Re: [PATCH] lightnvm: pblk: fix read_bitmap for 32bit archs

[Matias BjÃrling]

On 06/28/2018 12:18 AM, Igor Konopko wrote:
> On 27.06.2018 12:42, Matias BjÃrling wrote:
> > If using pblk on a 32bit architecture, and there is a need to
> > perform a partial read, the partial read bitmap will only have
> > allocated 32 entries, where as 64 are needed.
> >
> > Make sure that the read_bitmap is initialized to 64bits on 32bit
> > architectures as well.
> >
> > Signed-off-by: Matias BjÃrling <mb@xxxxxxxxxxx>
> > ---
> > Â drivers/lightnvm/pblk-read.c | 14 +++++++-------
> > Â 1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/lightnvm/pblk-read.c b/drivers/lightnvm/pblk-read.c
> > index 6e93c489ce57..671635275d56 100644
> > --- a/drivers/lightnvm/pblk-read.c
> > +++ b/drivers/lightnvm/pblk-read.c
> > @@ -401,7 +401,7 @@ int pblk_submit_read(struct pblk *pblk, struct bio 
> > *bio)
> > ÂÂÂÂÂ struct pblk_g_ctx *r_ctx;
> > ÂÂÂÂÂ struct nvm_rq *rqd;
> > ÂÂÂÂÂ unsigned int bio_init_idx;
> > -ÂÂÂ unsigned long read_bitmap; /* Max 64 ppas per request */
> > +ÂÂÂ DECLARE_BITMAP(read_bitmap, 64); /* Max 64 ppas per request */
>
> Probably it would be nicer to use NVM_MAX_VLBA define instead of 
> explicit 64.
>
> > ÂÂÂÂÂ int ret = NVM_IO_ERR;
> > ÂÂÂÂÂ /* logic error: lba out-of-bounds. Ignore read request */
> > @@ -413,7 +413,7 @@ int pblk_submit_read(struct pblk *pblk, struct bio 
> > *bio)
> > ÂÂÂÂÂ generic_start_io_acct(q, READ, bio_sectors(bio), 
> > &pblk->disk->part0);
> > -ÂÂÂ bitmap_zero(&read_bitmap, nr_secs);
> > +ÂÂÂ bitmap_zero(read_bitmap, nr_secs);
> > ÂÂÂÂÂ rqd = pblk_alloc_rqd(pblk, PBLK_READ);
> > @@ -444,19 +444,19 @@ int pblk_submit_read(struct pblk *pblk, struct 
> > bio *bio)
> > ÂÂÂÂÂÂÂÂÂ rqd->ppa_list = rqd->meta_list + pblk_dma_meta_size;
> > ÂÂÂÂÂÂÂÂÂ rqd->dma_ppa_list = rqd->dma_meta_list + pblk_dma_meta_size;
> > -ÂÂÂÂÂÂÂ pblk_read_ppalist_rq(pblk, rqd, bio, blba, &read_bitmap);
> > +ÂÂÂÂÂÂÂ pblk_read_ppalist_rq(pblk, rqd, bio, blba, read_bitmap 
> > >ÂÂÂÂÂÂ } else {
> > -ÂÂÂÂÂÂÂ pblk_read_rq(pblk, rqd, bio, blba, &read_bitmap);
> > +ÂÂÂÂÂÂÂ pblk_read_rq(pblk, rqd, bio, blba, read_bitmap);
> > ÂÂÂÂÂ }
> > -ÂÂÂ if (bitmap_full(&read_bitmap, nr_secs)) {
> > +ÂÂÂ if (bitmap_full(read_bitmap, nr_secs)) {
> > ÂÂÂÂÂÂÂÂÂ atomic_inc(&pblk->inflight_io);
> > ÂÂÂÂÂÂÂÂÂ __pblk_end_io_read(pblk, rqd, false);
> > ÂÂÂÂÂÂÂÂÂ return NVM_IO_DONE;
> > ÂÂÂÂÂ }
> > ÂÂÂÂÂ /* All sectors are to be read from the device */
> > -ÂÂÂ if (bitmap_empty(&read_bitmap, rqd->nr_ppas)) {
> > +ÂÂÂ if (bitmap_empty(read_bitmap, rqd->nr_ppas)) {
> > ÂÂÂÂÂÂÂÂÂ struct bio *int_bio = NULL;
> > ÂÂÂÂÂÂÂÂÂ /* Clone read bio to deal with read errors internally */
> > @@ -480,7 +480,7 @@ int pblk_submit_read(struct pblk *pblk, struct bio 
> > *bio)
> > ÂÂÂÂÂ /* 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.
> > ÂÂÂÂÂÂ */
> > -ÂÂÂ return pblk_partial_read(pblk, rqd, bio, bio_init_idx, 
> > &read_bitmap);
> > +ÂÂÂ return pblk_partial_read(pblk, rqd, bio, bio_init_idx, read_bitmap);
> > Â fail_rqd_free:
> > ÂÂÂÂÂ pblk_free_rqd(pblk, rqd, PBLK_READ);
>
> Otherwise looks good.
>
> Reviewed-by: Igor Konopko <igor.j.konopko@xxxxxxxxx>

Thanks. I've applied it for 4.19 with the fix and also, now that 
NVM_MAX_VBLK is used, removed the max 64 ppas comment.