* [PATCH] nvme: reject the ns when the block size is smaller than a sector
@ 2021-01-13 16:06 Li Feng
2021-01-13 16:18 ` Johannes Thumshirn
0 siblings, 1 reply; 7+ messages in thread
From: Li Feng @ 2021-01-13 16:06 UTC (permalink / raw)
To: martin.petersen, Johannes.Thumshirn, Keith Busch, Jens Axboe,
Christoph Hellwig, Sagi Grimberg, open list:NVM EXPRESS DRIVER,
open list
Cc: lifeng1519, Li Feng
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 <fengli@smartx.com>
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] nvme: reject the ns when the block size is smaller than a sector
2021-01-13 16:06 [PATCH] nvme: reject the ns when the block size is smaller than a sector Li Feng
@ 2021-01-13 16:18 ` Johannes Thumshirn
2021-01-13 22:12 ` Sagi Grimberg
0 siblings, 1 reply; 7+ messages in thread
From: Johannes Thumshirn @ 2021-01-13 16:18 UTC (permalink / raw)
To: Li Feng, martin.petersen, Keith Busch, Jens Axboe,
Christoph Hellwig, Sagi Grimberg, open list:NVM EXPRESS DRIVER,
open list
Cc: lifeng1519
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 <fengli@smartx.com>
> ---
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nvme: reject the ns when the block size is smaller than a sector
2021-01-13 16:18 ` Johannes Thumshirn
@ 2021-01-13 22:12 ` Sagi Grimberg
2021-01-14 5:17 ` Feng Li
2021-01-14 17:43 ` Christoph Hellwig
0 siblings, 2 replies; 7+ messages in thread
From: Sagi Grimberg @ 2021-01-13 22:12 UTC (permalink / raw)
To: Johannes Thumshirn, Li Feng, martin.petersen, Keith Busch,
Jens Axboe, Christoph Hellwig, open list:NVM EXPRESS DRIVER,
open list
Cc: lifeng1519
>> 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 <fengli@smartx.com>
>> ---
>> 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..
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nvme: reject the ns when the block size is smaller than a sector
2021-01-13 22:12 ` Sagi Grimberg
@ 2021-01-14 5:17 ` Feng Li
2021-01-14 17:43 ` Christoph Hellwig
1 sibling, 0 replies; 7+ messages in thread
From: Feng Li @ 2021-01-14 5:17 UTC (permalink / raw)
To: Sagi Grimberg
Cc: Johannes Thumshirn, Li Feng, martin.petersen, Keith Busch,
Jens Axboe, Christoph Hellwig, open list:NVM EXPRESS DRIVER,
open list
Sagi Grimberg <sagi@grimberg.me> 于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 <fengli@smartx.com>
> >> ---
> >> 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?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nvme: reject the ns when the block size is smaller than a sector
2021-01-13 22:12 ` Sagi Grimberg
2021-01-14 5:17 ` Feng Li
@ 2021-01-14 17:43 ` Christoph Hellwig
2021-01-14 21:13 ` Sagi Grimberg
2021-01-15 3:30 ` Li Feng
1 sibling, 2 replies; 7+ messages in thread
From: Christoph Hellwig @ 2021-01-14 17:43 UTC (permalink / raw)
To: Sagi Grimberg
Cc: Johannes Thumshirn, Li Feng, martin.petersen, Keith Busch,
Jens Axboe, Christoph Hellwig, open list:NVM EXPRESS DRIVER,
open list, lifeng1519
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nvme: reject the ns when the block size is smaller than a sector
2021-01-14 17:43 ` Christoph Hellwig
@ 2021-01-14 21:13 ` Sagi Grimberg
2021-01-15 3:30 ` Li Feng
1 sibling, 0 replies; 7+ messages in thread
From: Sagi Grimberg @ 2021-01-14 21:13 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Johannes Thumshirn, Li Feng, martin.petersen, Keith Busch,
Jens Axboe, open list:NVM EXPRESS DRIVER, open list, lifeng1519
>>> 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?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nvme: reject the ns when the block size is smaller than a sector
2021-01-14 17:43 ` Christoph Hellwig
2021-01-14 21:13 ` Sagi Grimberg
@ 2021-01-15 3:30 ` Li Feng
1 sibling, 0 replies; 7+ messages in thread
From: Li Feng @ 2021-01-15 3:30 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Sagi Grimberg, Johannes Thumshirn, martin.petersen, Keith Busch,
Jens Axboe, open list:NVM EXPRESS DRIVER, open list, lifeng1519
Christoph Hellwig <hch@lst.de> 于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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-01-15 3:31 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-13 16:06 [PATCH] nvme: reject the ns when the block size is smaller than a sector Li Feng
2021-01-13 16:18 ` Johannes Thumshirn
2021-01-13 22:12 ` Sagi Grimberg
2021-01-14 5:17 ` Feng Li
2021-01-14 17:43 ` Christoph Hellwig
2021-01-14 21:13 ` Sagi Grimberg
2021-01-15 3:30 ` Li Feng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).