Re: [PATCH v1 2/2] nvme: stop using queue_limits_stack_bdev for namespace heads

On Mon, Jul 20, 2026 at 04:46:49PM +0200, Christoph Hellwig wrote:
> On Mon, Jul 20, 2026 at 10:22:05AM +0800, Yao Sang wrote:
> > NVMe namespace heads are not generic stacked block devices.
> 
> Not many things actually are, which leads to every recurring problems
> with the stacking scheme..
> 
> > Execution features and request limits continue to accumulate
> > conservatively as paths are refreshed.
> 
> What is this supposed to mean?

Sorry, that wording was unclear. This means that the common fields continue
to use the existing block-layer merge rules as paths are refreshed

> > Keep the helper local to NVMe so its field ownership remains explicit and
> 
> What is "field ownership" supposed to mean?

What I meant was grouping fields by how they are updated: Identify-derived,
head-owned, feature, and path fields.

> > does not impose NVMe namespace-head policy on mapped devices.
> 
> I think you need to clearly state in what way we'll want to diverge
> here.

The key difference is that NVMe paths address the same LBA space, so they do
not need component offsets, mapped alignment, or generic topology merging.

> > +static void nvme_apply_ns_head_identify_limits(struct queue_limits *lim,
> > +					       const struct queue_limits *ns_lim)
> 
> Two tab indents, please.

Yes, I'll fix the indentation.

> >  {
> > -	t->max_open_zones = min_not_zero(t->max_open_zones, b->max_open_zones);
> > -	t->max_active_zones =
> > -		min_not_zero(t->max_active_zones, b->max_active_zones);
> > +	/*
> > +	 * Identify-derived namespace/head attributes come from the refreshed
> > +	 * NVMe namespace limits, not from generic mapped-device stacking.
> > +	 */
> > +	lim->features &= ~(BLK_FEAT_ZONED | BLK_FEAT_ATOMIC_WRITES);
> > +	lim->features |= ns_lim->features & BLK_FEAT_ZONED;
> > +	lim->logical_block_size = ns_lim->logical_block_size;
> > +	lim->physical_block_size = ns_lim->physical_block_size;
> 
> A lot of this still duplicates the core code and creates a nightmare
> when we add new fields.
> 
> Can you try to refactor the core code into lower level building
> blocks and reuse as much as possible while also making the policy
> differences clear?

The initial NVMe-local change was meant to limit block-layer changes and
preserve the established blk_stack_limits() behavior for existing callers
while the common field semantics were unclear.

I agree that the common rules should not be duplicated. I understand "the
core code" to mean the block-layer stacking code in blk_stack_limits(). Based
on that, I would first add a behavior-preserving block-layer refactoring and
factor out the fields that have identical composition rules for generic
stacking and same-LBA paths. In pseudo-code, the helper and its call sites are:

/*
 * Combine constraints required by every underlying queue that may execute
 * an I/O, independent of mapping, topology, operation, user or resource
 * policy.
 */
void blk_stack_io_limits(struct queue_limits *t,
			 const struct queue_limits *b)
{
	/* clear unsupported NOWAIT/POLL/PCI_P2PDMA bits */

	/*
	 * Apply min_not_zero(t->field, b->field) to:
	 * max_hw_sectors, max_dev_sectors,
	 * seg_boundary_mask, virt_boundary_mask,
	 * max_segments, max_integrity_segments, max_segment_size.
	 */
	 
	/* apply max(t->field, b->field) to dma_alignment */
}

blk_stack_limits(t, b, start)
{
	/* existing inherited-feature handling */
	blk_stack_io_limits(t, b);
	/* all remaining existing code stays in its current order */
}

nvme_apply_ns_head_limits(lim, ns_lim)
{
	/* existing Identify/head and inherited-feature handling */
	blk_stack_io_limits(lim, ns_lim);
	/* all remaining NVMe-local code stays in its current order */
}

Existing DM, MD/RAID, DRBD and Btrfs callers keep using blk_stack_limits()
without a behavior change, while NVMe calls the lower-level helper directly.

The key distinction is the composition relationship: NVMe native paths
address the same LBA space, while generic stacking may remap LBAs and compose
topology. This version therefore factors out only the mapping-independent
rules shared by both cases into blk_stack_io_limits(), and leaves the remaining
handling in its existing paths. I would split further only when another common
composition rule is identified.

Does this match the block-layer boundary you had in mind, or should this
series factor out additional composition rules from blk_stack_limits()?

Thanks,
Yao