Re: [PATCH v2 1/2] block: ensure correct integrity capability propagation in stacked devices

[Anuj Gupta <anuj20.g@xxxxxxxxxxx>]

On Mon, Mar 03, 2025 at 03:12:36PM +0100, Christoph Hellwig wrote:
> On Wed, Feb 26, 2025 at 04:50:34PM +0530, Anuj Gupta wrote:
> > diff --git a/block/blk-settings.c b/block/blk-settings.c
> > index c44dadc35e1e..8bd0d0f1479c 100644
> > --- a/block/blk-settings.c
> > +++ b/block/blk-settings.c
> > @@ -861,7 +861,8 @@ bool queue_limits_stack_integrity(struct queue_limits *t,
> >  
> >  	if (!ti->tuple_size) {
> >  		/* inherit the settings from the first underlying device */
> > -		if (!(ti->flags & BLK_INTEGRITY_STACKED)) {
> > +		if (!(ti->flags & BLK_INTEGRITY_STACKED) &&
> > +		    (bi->flags & BLK_INTEGRITY_DEVICE_CAPABLE)) {
> >  			ti->flags = BLK_INTEGRITY_DEVICE_CAPABLE |
> >  				(bi->flags & BLK_INTEGRITY_REF_TAG);
> >  			ti->csum_type = bi->csum_type;
> 
> As mentioned last round this still does the wrong thing if the first
> device(s) is/are not PI-capable but the next one(s) is/are.  Please
> look into the pseudocode I posted in reply to the previous iteration
> on how to fix it.

Christoph,
Right, based on your comment modified this patch.
Does this look ok?

diff --git a/block/blk-settings.c b/block/blk-settings.c
index c44dadc35e1e..d0469a812734 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -859,36 +859,28 @@ bool queue_limits_stack_integrity(struct queue_limits *t,
 	if (!IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY))
 		return true;
 
-	if (!ti->tuple_size) {
-		/* inherit the settings from the first underlying device */
-		if (!(ti->flags & BLK_INTEGRITY_STACKED)) {
-			ti->flags = BLK_INTEGRITY_DEVICE_CAPABLE |
-				(bi->flags & BLK_INTEGRITY_REF_TAG);
-			ti->csum_type = bi->csum_type;
-			ti->tuple_size = bi->tuple_size;
-			ti->pi_offset = bi->pi_offset;
-			ti->interval_exp = bi->interval_exp;
-			ti->tag_size = bi->tag_size;
-			goto done;
-		}
-		if (!bi->tuple_size)
-			goto done;
+	if (ti->flags & BLK_INTEGRITY_STACKED) {
+		if (ti->tuple_size != bi->tuple_size)
+			goto incompatible;
+		if (ti->interval_exp != bi->interval_exp)
+			goto incompatible;
+		if (ti->tag_size != bi->tag_size)
+			goto incompatible;
+		if (ti->csum_type != bi->csum_type)
+			goto incompatible;
+		if ((ti->flags & BLK_INTEGRITY_REF_TAG) !=
+		    (bi->flags & BLK_INTEGRITY_REF_TAG))
+			goto incompatible;
+	} else {
+		ti->flags = BLK_INTEGRITY_STACKED;
+		ti->flags |= (bi->flags & BLK_INTEGRITY_DEVICE_CAPABLE) |
+			     (bi->flags & BLK_INTEGRITY_REF_TAG);
+		ti->csum_type = bi->csum_type;
+		ti->tuple_size = bi->tuple_size;
+		ti->pi_offset = bi->pi_offset;
+		ti->interval_exp = bi->interval_exp;
+		ti->tag_size = bi->tag_size;
 	}
-
-	if (ti->tuple_size != bi->tuple_size)
-		goto incompatible;
-	if (ti->interval_exp != bi->interval_exp)
-		goto incompatible;
-	if (ti->tag_size != bi->tag_size)
-		goto incompatible;
-	if (ti->csum_type != bi->csum_type)
-		goto incompatible;
-	if ((ti->flags & BLK_INTEGRITY_REF_TAG) !=
-	    (bi->flags & BLK_INTEGRITY_REF_TAG))
-		goto incompatible;
-
-done:
-	ti->flags |= BLK_INTEGRITY_STACKED;
 	return true;
 
 incompatible:
-- 
2.25.1