[PATCH 11/11] block: move integrity information into queue_limits

[Hannes Reinecke]

(Trimming reply for better readability)

On 6/7/24 08:29, Hannes Reinecke wrote:
> On 6/7/24 07:59, Christoph Hellwig wrote:
[ .. ]
>> diff --git a/block/blk-settings.c b/block/blk-settings.c
>> index 996f247fc98e80..f11c8676eb4c67 100644
>> --- a/block/blk-settings.c
>> +++ b/block/blk-settings.c
>> @@ -6,7 +6,7 @@
>>   #include <linux/module.h>
>>   #include <linux/init.h>
>>   #include <linux/bio.h>
>> -#include <linux/blkdev.h>
>> +#include <linux/blk-integrity.h>
>>   #include <linux/pagemap.h>
>>   #include <linux/backing-dev-defs.h>
>>   #include <linux/gcd.h>
>> @@ -97,6 +97,36 @@ static int blk_validate_zoned_limits(struct 
>> queue_limits *lim)
>>       return 0;
>>   }
>> +static int blk_validate_integrity_limits(struct queue_limits *lim)
>> +{
>> +    struct blk_integrity *bi = &lim->integrity;
>> +
>> +    if (!bi->tuple_size) {
>> +        if (bi->csum_type != BLK_INTEGRITY_CSUM_NONE ||
>> +            bi->tag_size || ((bi->flags & BLK_INTEGRITY_REF_TAG))) {
>> +            pr_warn("invalid PI settings.\n");
>> +            return -EINVAL;
>> +        }
>> +        return 0;
>> +    }
>> +
>> +    if (!IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY)) {
>> +        pr_warn("integrity support disabled.\n");
>> +        return -EINVAL;
>> +    }
>> +
> Why is that an error?
> Surely 'validate' should not return an error if BLK_DEV_INTEGRITY is 
> disabled and no limits are set?
> 
>> +    if (bi->csum_type == BLK_INTEGRITY_CSUM_NONE &&
>> +        (bi->flags & BLK_INTEGRITY_REF_TAG)) {
>> +        pr_warn("ref tag not support without checksum.\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    if (!bi->interval_exp)
>> +        bi->interval_exp = ilog2(lim->logical_block_size);
>> +
>> +    return 0;
>> +}
>> +
>>   /*
>>    * Check that the limits in lim are valid, initialize defaults for 
>> unset
>>    * values, and cap values based on others where needed.
>> @@ -105,6 +135,7 @@ static int blk_validate_limits(struct queue_limits 
>> *lim)
>>   {
>>       unsigned int max_hw_sectors;
>>       unsigned int logical_block_sectors;
>> +    int err;
>>       /*
>>        * Unless otherwise specified, default to 512 byte logical 
>> blocks and a
>> @@ -230,6 +261,9 @@ static int blk_validate_limits(struct queue_limits 
>> *lim)
>>           lim->misaligned = 0;
>>       }
>> +    err = blk_validate_integrity_limits(lim);
>> +    if (err)
>> +        return err;
> Wouldn't we always fail to validate the limits if BLK_DEV_INTEGRITY is 
> disabled, given the check above?
> 
Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare at suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich