Re: [PATCH RFC 2/2] nvme: enable multiple atomicity mode

[Daniel Gomez <da.gomez@kernel.org>]

On 2026-11-03T11:38:56+02:00, Arthur Simchaev <arthur.simchaev@sandisk.com> wrote:
> On Tue, Sep 29, 2026 at 01:17:12PM +0200, Daniel Gomez wrote:
> [...]
> > @@ -993,6 +993,9 @@ static bool nvme_valid_atomic_write(struct request *req)
> >  	struct request_queue *q = req->q;
> >  	u32 boundary_bytes = queue_atomic_write_boundary_bytes(q);
> >  
> > +	if (q->limits.features & BLK_FEAT_ATOMIC_WRITE_MULTI)
> > +		return true;
> > +
>
> With the flag set this skips all checks. Since each REQ_ATOMIC bio carries
> exactly one RWF_ATOMIC write (blkdev and iomap reject atomic writes that need
> more than one bio), could it instead check every bio of the merged request with
> __rq_for_each_bio(), using the existing size and boundary checks? That would
> keep the safety net described in the comment above the function, at the cost of
> a few comparisons per bio.

I don't think this is really necessary in NVMe. But I agree with the
suggestion to honor the request and potentially other drivers that may
be more strict.

>
> [...]
> > +static bool nvme_mam_compliant(struct nvme_id_ns *id)
> > +{
> > +	if (id->nabspf != id->nawupf)
> > +		return false;
> > +	if (id->nabsn && id->nabsn != id->nabspf)
> > +		return false;
> > +	if (id->nawun && id->nawun != id->nawupf)
> > +		return false;
> > +	return true;
> > +}
>
> This deliberately accepts NABSN == 0 and NAWUN == 0. I assume this is because
> Linux only uses the power-fail values (NAWUPF/NABSPF) for atomic writes? If so,

I'll work on the clarification.

> a comment would help. Still, NVM Command Set 1.1 Figure 9 requires NABSN to
> equal NAWUN (or AWUN when NAWUN is not reported) and NABSPF in MAM. NABSN == 0
> means no boundary for normal operation while the device claims MAM, and NAWUN ==
> 0 would need NABSN == AWUN, which the driver does not track today. Would it be
> safer to treat both as inconsistent, as your QEMU series does?

I'd need to double check this.

>
> [...]
> > +		if (id->nsfeat & NVME_NS_FEAT_MAM) {
> > +			if (nvme_mam_compliant(id))
> > +				lim->features |= BLK_FEAT_ATOMIC_WRITE_MULTI;
> > +			else
> > +				dev_warn_once(ns->ctrl->device,
> > +					"Inconsistent MAM parameters, ignoring\n");
> > +		}
>
> Minor: dev_warn_once() uses one static flag per call site, so only the first
> affected controller is reported. Maybe dev_warn() (this runs on namespace scan,
> not in the I/O path), and print the four values to make the report actionable?

Do we need the 4 values when users can check them out with nvme-cli?