* [PATCH] lightnvm: consider max hw sectors supported for max_write_pgs
@ 2018-10-05 0:26 Zhoujie Wu
2018-10-05 8:05 ` Javier González
0 siblings, 1 reply; 3+ messages in thread
From: Zhoujie Wu @ 2018-10-05 0:26 UTC (permalink / raw)
To: mb, linux-block; +Cc: hongd, Zhoujie Wu
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;
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),
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] lightnvm: consider max hw sectors supported for max_write_pgs
2018-10-05 0:26 [PATCH] lightnvm: consider max hw sectors supported for max_write_pgs Zhoujie Wu
@ 2018-10-05 8:05 ` Javier González
2018-10-05 17:34 ` [EXT] " Zhoujie Wu
0 siblings, 1 reply; 3+ messages in thread
From: Javier González @ 2018-10-05 8:05 UTC (permalink / raw)
To: Zhoujie Wu; +Cc: Matias Bjørling, linux-block, hongd
[-- Attachment #1: Type: text/plain, Size: 1980 bytes --]
> 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>
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [EXT] Re: [PATCH] lightnvm: consider max hw sectors supported for max_write_pgs
2018-10-05 8:05 ` Javier González
@ 2018-10-05 17:34 ` Zhoujie Wu
0 siblings, 0 replies; 3+ messages in thread
From: Zhoujie Wu @ 2018-10-05 17:34 UTC (permalink / raw)
To: Javier González; +Cc: Matias Bjørling, linux-block, hongd
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>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-10-06 0:34 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-05 0:26 [PATCH] lightnvm: consider max hw sectors supported for max_write_pgs Zhoujie Wu
2018-10-05 8:05 ` Javier González
2018-10-05 17:34 ` [EXT] " Zhoujie Wu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.