The nvme spec(1.4a, figure 248) says:
"A value smaller than 9 (i.e., 512 bytes) is not supported."
Signed-off-by: Li Feng <[email protected]>
---
drivers/nvme/host/core.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f320273fc672..1f02e6e49a05 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2161,6 +2161,12 @@ static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_id_ns *id)
blk_mq_freeze_queue(ns->disk->queue);
ns->lba_shift = id->lbaf[lbaf].ds;
+ if (ns->lba_shift < 9) {
+ pr_warn("%s: bad lba_shift(%d)\n", ns->disk->disk_name, ns->lba_shift);
+ ret = -1;
+ goto out_unfreeze;
+ }
+
nvme_set_queue_limits(ns->ctrl, ns->queue);
if (ns->head->ids.csi == NVME_CSI_ZNS) {
--
2.29.2
On 13/01/2021 17:07, Li Feng wrote:
> The nvme spec(1.4a, figure 248) says:
> "A value smaller than 9 (i.e., 512 bytes) is not supported."
>
> Signed-off-by: Li Feng <[email protected]>
> ---
> drivers/nvme/host/core.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index f320273fc672..1f02e6e49a05 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2161,6 +2161,12 @@ static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_id_ns *id)
>
> blk_mq_freeze_queue(ns->disk->queue);
> ns->lba_shift = id->lbaf[lbaf].ds;
> + if (ns->lba_shift < 9) {
> + pr_warn("%s: bad lba_shift(%d)\n", ns->disk->disk_name, ns->lba_shift);
> + ret = -1;
> + goto out_unfreeze;
> + }
> +
> nvme_set_queue_limits(ns->ctrl, ns->queue);
>
> if (ns->head->ids.csi == NVME_CSI_ZNS) {
>
But this only catches a physical block size < 512 for NVMe, not any other block device.
Please fix it for the general case in blk_queue_physical_block_size().
Thanks,
Johannes
>> The nvme spec(1.4a, figure 248) says:
>> "A value smaller than 9 (i.e., 512 bytes) is not supported."
>>
>> Signed-off-by: Li Feng <[email protected]>
>> ---
>> drivers/nvme/host/core.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index f320273fc672..1f02e6e49a05 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -2161,6 +2161,12 @@ static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_id_ns *id)
>>
>> blk_mq_freeze_queue(ns->disk->queue);
>> ns->lba_shift = id->lbaf[lbaf].ds;
>> + if (ns->lba_shift < 9) {
>> + pr_warn("%s: bad lba_shift(%d)\n", ns->disk->disk_name, ns->lba_shift);
>> + ret = -1;
Meaningful errno would be useful. Also better to use dev_warn
>> + goto out_unfreeze;
>> + }
>> +
>> nvme_set_queue_limits(ns->ctrl, ns->queue);
>>
>> if (ns->head->ids.csi == NVME_CSI_ZNS) {
>>
>
>
> But this only catches a physical block size < 512 for NVMe, not any other block device.
>
> Please fix it for the general case in blk_queue_physical_block_size().
We actually call that later and would probably be better to check here..
Sagi Grimberg <[email protected]> 于2021年1月14日周四 上午6:13写道：
>
>
> >> The nvme spec(1.4a, figure 248) says:
> >> "A value smaller than 9 (i.e., 512 bytes) is not supported."
> >>
> >> Signed-off-by: Li Feng <[email protected]>
> >> ---
> >> drivers/nvme/host/core.c | 6 ++++++
> >> 1 file changed, 6 insertions(+)
> >>
> >> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> >> index f320273fc672..1f02e6e49a05 100644
> >> --- a/drivers/nvme/host/core.c
> >> +++ b/drivers/nvme/host/core.c
> >> @@ -2161,6 +2161,12 @@ static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_id_ns *id)
> >>
> >> blk_mq_freeze_queue(ns->disk->queue);
> >> ns->lba_shift = id->lbaf[lbaf].ds;
> >> + if (ns->lba_shift < 9) {
> >> + pr_warn("%s: bad lba_shift(%d)\n", ns->disk->disk_name, ns->lba_shift);
> >> + ret = -1;
>
> Meaningful errno would be useful. Also better to use dev_warn
>
> >> + goto out_unfreeze;
> >> + }
> >> +
> >> nvme_set_queue_limits(ns->ctrl, ns->queue);
> >>
> >> if (ns->head->ids.csi == NVME_CSI_ZNS) {
> >>
> >
> >
> > But this only catches a physical block size < 512 for NVMe, not any other block device.
> >
> > Please fix it for the general case in blk_queue_physical_block_size().
>
> We actually call that later and would probably be better to check here..
Thanks for your comments.
The prototype is:
void blk_queue_logical_block_size(struct request_queue *, unsigned int);
So change it to:
bool blk_queue_logical_block_size(struct request_queue *, unsigned int);
And check all callers of this function, right?
On Wed, Jan 13, 2021 at 02:12:59PM -0800, Sagi Grimberg wrote:
>> But this only catches a physical block size < 512 for NVMe, not any other block device.
>>
>> Please fix it for the general case in blk_queue_physical_block_size().
>
> We actually call that later and would probably be better to check here..
We had a series for that a short while ago that got lost.
>>> But this only catches a physical block size < 512 for NVMe, not any other block device.
>>>
>>> Please fix it for the general case in blk_queue_physical_block_size().
>>
>> We actually call that later and would probably be better to check here..
>
> We had a series for that a short while ago that got lost.
What was it called?
Christoph Hellwig <[email protected]> 于2021年1月15日周五 上午1:43写道：
>
> On Wed, Jan 13, 2021 at 02:12:59PM -0800, Sagi Grimberg wrote:
> >> But this only catches a physical block size < 512 for NVMe, not any other block device.
> >>
> >> Please fix it for the general case in blk_queue_physical_block_size().
> >
> > We actually call that later and would probably be better to check here..
>
> We had a series for that a short while ago that got lost.
Where is the series? Or could you advise on how to fix this issue is acceptable?
This issue will generate an oops, check it here:
https://lkml.org/lkml/2021/1/12/1064
Thanks,
Feng Li