Thanks for sharing comments! I did such changes in v2:
- Remove Fixes tag(Keith Busch: ds has never been validated)
- Change -EIO to -ENODEV (Keith Busch)
- Add missing "queue_limits_cancel_update()" in error path (Keith Busch)
- Add warning for ds == 0 (LBA format not available) (Maurizio Lombardi)

---
On Mon, Apr 20, 2026 at 02:51:00PM -0700, Keith Busch wrote:
> I think ENODEV is more appropriate errno.

Fixed in v2.

> you're missing the corresponding queue_limits_cancel_update() for this
> error case.

Added in both error paths in v2.

> This fixes tag is wrong.

Removed.

On Mon, Apr 20, 2026 at 02:22:04PM +0200, Daniel Wagner wrote:
> This fixes tag is wrong. Probably a5b1cd61820e is the better choice here.

Keith pointed out that ds has never been validated so no commit
introduced a regression — removed the Fixes tag entirely.

On Mon, Apr 20, 2026 at 02:22:04PM +0200, Maurizio Lombardi wrote:
> ds == 0 has a special meaning: 'LBA format is not currently available.'

Thanks,added dev_warn_once() for ds==0 in v2.

Chao Shi (1):
  nvme: core: reject invalid LBA data size from Identify Namespace

 drivers/nvme/host/core.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

-- 
2.43.0

On Mon, Apr 20, 2026 at 07:11:13PM -0400, Chao Shi wrote:
> On Mon, Apr 20, 2026 at 02:51:00PM -0700, Keith Busch wrote:
> 
> Fixed in v2.
> 
> > you're missing the corresponding queue_limits_cancel_update() for this
> > error case.
> 
> Added in both error paths in v2.

Not quite what I suggested. Instead, if you move up the validation
checks, you don't have any cleanup in the error case to do.

And don't bother splitting the error cases. The message about the "0"
LBADS is in reference to the the broadcast identification. It's never
valid for a controller to return an unusable flbas index value for an
attached namespace.

Hi Keith,

Thanks for the follow-up — I misread your v1 comment.

> Not quite what I suggested. Instead, if you move up the validation
> checks, you don't have any cleanup in the error case to do.

Understood now.  In v3 the validation is hoisted above
queue_limits_start_update() and blk_mq_freeze_queue(), so the
error path is a plain `goto out` with no cancel_update / unfreeze
needed. (Please let me know if I things on the right track)

> And don't bother splitting the error cases.  The message about the
> "0" LBADS is in reference to the broadcast identification.  It's
> never valid for a controller to return an unusable flbas index
> value for an attached namespace.

Collapsed into a single check in v3.

Sending v3 shortly.

Thanks,
Chao

On Mon, Apr 20, 2026 at 8:25 PM Keith Busch <kbusch@kernel.org> wrote:
>
> On Mon, Apr 20, 2026 at 07:11:13PM -0400, Chao Shi wrote:
> > On Mon, Apr 20, 2026 at 02:51:00PM -0700, Keith Busch wrote:
> >
> > Fixed in v2.
> >
> > > you're missing the corresponding queue_limits_cancel_update() for this
> > > error case.
> >
> > Added in both error paths in v2.
>
> Not quite what I suggested. Instead, if you move up the validation
> checks, you don't have any cleanup in the error case to do.
>
> And don't bother splitting the error cases. The message about the "0"
> LBADS is in reference to the the broadcast identification. It's never
> valid for a controller to return an unusable flbas index value for an
> attached namespace.