On 06/01/2025 17:49, Mike Snitzer wrote:
+++ b/drivers/md/dm-table.c
@@ -1593,6 +1593,7 @@ int dm_calculate_queue_limits(struct dm_table *t,
        struct queue_limits ti_limits;
        unsigned int zone_sectors = 0;
        bool zoned = false;
+       bool atomic_writes = true;
dm_set_stacking_limits(limits); @@ -1602,8 +1603,12 @@ int dm_calculate_queue_limits(struct dm_table *t, if (!dm_target_passes_integrity(ti->type))
                        t->integrity_supported = false;
+               if (!dm_target_supports_atomic_writes(ti->type))
+                       atomic_writes = false;
        }
+ if (atomic_writes)
+               limits->features |= BLK_FEAT_ATOMIC_WRITES_STACKED;
        for (unsigned int i = 0; i < t->num_targets; i++) {
                struct dm_target *ti = dm_table_get_target(t, i);
@@ -1616,6 +1621,13 @@ int dm_calculate_queue_limits(struct dm_table *t,
                        goto combine_limits;
                }
+ /*
+                * dm_set_device_limits() -> blk_stack_limits() considers
+                * ti_limits as 'top', so set BLK_FEAT_ATOMIC_WRITES_STACKED
+                * here also.
+                */
+               if (atomic_writes)
+                       ti_limits.features |= BLK_FEAT_ATOMIC_WRITES_STACKED;
                /*
                 * Combine queue limits of all the devices this target uses.
                 */
You're referring to this code that immediately follows this ^ comment
which stacks up the limits of a target's potential to have N component
data devices:

                 ti->type->iterate_devices(ti, dm_set_device_limits,
                                           &ti_limits);

Your comment and redundant feature flag setting is feels wrong.  I'd
expect code comparable to what is done for zoned, e.g.:

                 if (!zoned && (ti_limits.features & BLK_FEAT_ZONED)) {
                         /*
                          * After stacking all limits, validate all devices
                          * in table support this zoned model and zone sectors.
                          */
                         zoned = (ti_limits.features & BLK_FEAT_ZONED);
                         zone_sectors = ti_limits.chunk_sectors;
                 }

Meaning, for zoned devices, a side-effect of the
ti->type->iterate_devices() call (and N blk_stack_limits calls) is
ti_limits.features having BLK_FEAT_ZONED enabled.  Why wouldn't the same
side-effect happen for BLK_FEAT_ATOMIC_WRITES_STACKED (speaks to
blk_stack_limits being different/wrong for atomic writes support)?

ok, do I admit that my code did not feel quite right, so I will check the zoned code as a reference.


Just feels not quite right... but I could be wrong, please see if
there is any "there" there

Will do.

Cheers,
John


Reply via email to