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 <[email protected]>
---
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 */
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);
--
2.11.0
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 <[email protected]>
> ---
> 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 <[email protected]>
> On 27 Jun 2018, at 21.42, Matias Bjørling <[email protected]> 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 <[email protected]>
> ---
> 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 */
When we removed max_hx_sect we added a constant (NVM_MAX_VLBA). Use this
instead? Now I see Igor already commented on this...
> 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);
> --
> 2.11.0
Thanks for fixing it.
Reviewed-by: Javier González <[email protected]>
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 <[email protected]>
>> ---
>> 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 <[email protected]>
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.