| Message ID | 1538699190-4857-1-git-send-email-zjwu@marvell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | lightnvm: consider max hw sectors supported for max_write_pgs | expand |
> On 5 Oct 2018, at 02.26, Zhoujie Wu <zjwu@marvell.com> wrote: > > When do GC, the number of read/write sectors are determined > by max_write_pgs(see gc_rq preparation in pblk_gc_line_prepare_ws). > > Due to max_write_pgs doesn't consider max hw sectors > supported by nvme controller(128K), which leads to GC > tries to read 64 * 4K in one command, and see below error > caused by pblk_bio_map_addr in function pblk_submit_read_gc. > > [ 2923.005376] pblk: could not add page to bio > [ 2923.005377] pblk: could not allocate GC bio (18446744073709551604) > > Signed-off-by: Zhoujie Wu <zjwu@marvell.com> > --- > drivers/lightnvm/pblk-init.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c > index e357388..2e51875 100644 > --- a/drivers/lightnvm/pblk-init.c > +++ b/drivers/lightnvm/pblk-init.c > @@ -393,6 +393,7 @@ static int pblk_core_init(struct pblk *pblk) > struct nvm_tgt_dev *dev = pblk->dev; > struct nvm_geo *geo = &dev->geo; > int ret, max_write_ppas; > + struct request_queue *bqueue = dev->q; > Detail: Can you move this under struct nvm_tgt_dev *dev = pblk->dev;? So that we maintain ordering? > atomic64_set(&pblk->user_wa, 0); > atomic64_set(&pblk->pad_wa, 0); > @@ -407,6 +408,9 @@ static int pblk_core_init(struct pblk *pblk) > pblk->min_write_pgs = geo->ws_opt; > max_write_ppas = pblk->min_write_pgs * geo->all_luns; > pblk->max_write_pgs = min_t(int, max_write_ppas, NVM_MAX_VLBA); > + /* consider the max hw sector as well */ No need for this comment. > + pblk->max_write_pgs = min_t(int, pblk->max_write_pgs, > + queue_max_hw_sectors(bqueue) / (geo->csecs >> 9)); > pblk_set_sec_per_write(pblk, pblk->min_write_pgs); > > pblk->pad_dist = kcalloc(pblk->min_write_pgs - 1, sizeof(atomic64_t), > -- > 1.9.1 Besides the comment above, it looks good to me. Reviewed-by: Javier González <javier@javigon.com>
On 10/05/2018 01:05 AM, Javier González wrote: > External Email > > ---------------------------------------------------------------------- >> On 5 Oct 2018, at 02.26, Zhoujie Wu <zjwu@marvell.com> wrote: >> >> When do GC, the number of read/write sectors are determined >> by max_write_pgs(see gc_rq preparation in pblk_gc_line_prepare_ws). >> >> Due to max_write_pgs doesn't consider max hw sectors >> supported by nvme controller(128K), which leads to GC >> tries to read 64 * 4K in one command, and see below error >> caused by pblk_bio_map_addr in function pblk_submit_read_gc. >> >> [ 2923.005376] pblk: could not add page to bio >> [ 2923.005377] pblk: could not allocate GC bio (18446744073709551604) >> >> Signed-off-by: Zhoujie Wu <zjwu@marvell.com> >> --- >> drivers/lightnvm/pblk-init.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c >> index e357388..2e51875 100644 >> --- a/drivers/lightnvm/pblk-init.c >> +++ b/drivers/lightnvm/pblk-init.c >> @@ -393,6 +393,7 @@ static int pblk_core_init(struct pblk *pblk) >> struct nvm_tgt_dev *dev = pblk->dev; >> struct nvm_geo *geo = &dev->geo; >> int ret, max_write_ppas; >> + struct request_queue *bqueue = dev->q; >> > Detail: Can you move this under struct nvm_tgt_dev *dev = pblk->dev;? So > that we maintain ordering? Good suggestion. > >> atomic64_set(&pblk->user_wa, 0); >> atomic64_set(&pblk->pad_wa, 0); >> @@ -407,6 +408,9 @@ static int pblk_core_init(struct pblk *pblk) >> pblk->min_write_pgs = geo->ws_opt; >> max_write_ppas = pblk->min_write_pgs * geo->all_luns; >> pblk->max_write_pgs = min_t(int, max_write_ppas, NVM_MAX_VLBA); >> + /* consider the max hw sector as well */ > No need for this comment. ok, will remove it. >> + pblk->max_write_pgs = min_t(int, pblk->max_write_pgs, >> + queue_max_hw_sectors(bqueue) / (geo->csecs >> 9)); >> pblk_set_sec_per_write(pblk, pblk->min_write_pgs); >> >> pblk->pad_dist = kcalloc(pblk->min_write_pgs - 1, sizeof(atomic64_t), >> -- >> 1.9.1 > Besides the comment above, it looks good to me. Will send out v2 soon. Thanks so much. > > Reviewed-by: Javier González <javier@javigon.com> >
diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c index e357388..2e51875 100644 --- a/drivers/lightnvm/pblk-init.c +++ b/drivers/lightnvm/pblk-init.c @@ -393,6 +393,7 @@ static int pblk_core_init(struct pblk *pblk) struct nvm_tgt_dev *dev = pblk->dev; struct nvm_geo *geo = &dev->geo; int ret, max_write_ppas; + struct request_queue *bqueue = dev->q; atomic64_set(&pblk->user_wa, 0); atomic64_set(&pblk->pad_wa, 0); @@ -407,6 +408,9 @@ static int pblk_core_init(struct pblk *pblk) pblk->min_write_pgs = geo->ws_opt; max_write_ppas = pblk->min_write_pgs * geo->all_luns; pblk->max_write_pgs = min_t(int, max_write_ppas, NVM_MAX_VLBA); + /* consider the max hw sector as well */ + pblk->max_write_pgs = min_t(int, pblk->max_write_pgs, + queue_max_hw_sectors(bqueue) / (geo->csecs >> 9)); pblk_set_sec_per_write(pblk, pblk->min_write_pgs); pblk->pad_dist = kcalloc(pblk->min_write_pgs - 1, sizeof(atomic64_t),
When do GC, the number of read/write sectors are determined by max_write_pgs(see gc_rq preparation in pblk_gc_line_prepare_ws). Due to max_write_pgs doesn't consider max hw sectors supported by nvme controller(128K), which leads to GC tries to read 64 * 4K in one command, and see below error caused by pblk_bio_map_addr in function pblk_submit_read_gc. [ 2923.005376] pblk: could not add page to bio [ 2923.005377] pblk: could not allocate GC bio (18446744073709551604) Signed-off-by: Zhoujie Wu <zjwu@marvell.com> --- drivers/lightnvm/pblk-init.c | 4 ++++ 1 file changed, 4 insertions(+)