Re: [PATCH] scsi: libsas: Fix declaration of ncq priority attributes

[John Garry <john.g.garry@oracle.com>]

On 27/03/2024 02:13, Damien Le Moal wrote:

- old address

> On 3/27/24 11:01, Damien Le Moal wrote:
>> Commit b4d3ddd2df7 ("scsi: libsas: Define NCQ Priority sysfs attributes
>> for SATA devices") introduced support for ATA NCQ priority control for
>> ATA devices managed by libsas. This commit introduces the

nit: /s/This commit introduces the/That commit introduced the/

>> ncq_prio_supported and ncq_prio_enable sysfs device attributes to
>> discover and control the use of this features, similarly to libata.
>> However, libata publicly declares these device attributes and export
>> them for use in ATA low level drivers. This leads to a compilation error
>> when libsas and libata are built-in due to the double definition:
>>
>> ld: drivers/ata/libata-sata.o:/home/Linux/scsi/drivers/ata/libata-sata.c:900:
>> multiple definition of `dev_attr_ncq_prio_supported';
>> drivers/scsi/libsas/sas_ata.o:/home/Linux/scsi/drivers/scsi/libsas/sas_ata.c:984:
>> first defined here
>> ld: drivers/ata/libata-sata.o:/home/Linux/scsi/drivers/ata/libata-sata.c:1026:
>> multiple definition of `dev_attr_ncq_prio_enable';
>> drivers/scsi/libsas/sas_ata.o:/home/Linux/scsi/drivers/scsi/libsas/sas_ata.c:1022:
>> first defined here
>>
>> Resolve this problem by directly declaring the libsas attributes instead
>> of using the DEVICE_ATTR() macro. And for good measure, the device
>> attribute variables are also renamed.
>>
>> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> Fixes: b4d3ddd2df7 ("scsi: libsas: Define NCQ Priority sysfs attributes for SATA devices")
>> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>

Reviewed-by: John Garry <john.g.garry@oracle.com>

> 
> Forgot to add John to the distribution list...
> 
>> ---
>>   drivers/scsi/libsas/sas_ata.c | 12 +++++++-----
>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
>> index b57c041a5544..4c69fc63c119 100644
>> --- a/drivers/scsi/libsas/sas_ata.c
>> +++ b/drivers/scsi/libsas/sas_ata.c
>> @@ -981,7 +981,8 @@ static ssize_t sas_ncq_prio_supported_show(struct device *device,
>>   	return sysfs_emit(buf, "%d\n", supported);
>>   }
>>   
>> -DEVICE_ATTR(ncq_prio_supported, S_IRUGO, sas_ncq_prio_supported_show, NULL);
>> +static struct device_attribute dev_attr_sas_ncq_prio_supported =
>> +	__ATTR(ncq_prio_supported, S_IRUGO, sas_ncq_prio_supported_show, NULL);
>>   
>>   static ssize_t sas_ncq_prio_enable_show(struct device *device,
>>   					struct device_attribute *attr,
>> @@ -1019,12 +1020,13 @@ static ssize_t sas_ncq_prio_enable_store(struct device *device,
>>   	return len;
>>   }
>>   
>> -DEVICE_ATTR(ncq_prio_enable, S_IRUGO | S_IWUSR,
>> -	    sas_ncq_prio_enable_show, sas_ncq_prio_enable_store);
>> +static struct device_attribute dev_attr_sas_ncq_prio_enable =
>> +	__ATTR(ncq_prio_enable, S_IRUGO | S_IWUSR,
>> +	       sas_ncq_prio_enable_show, sas_ncq_prio_enable_store);
>>   
>>   static struct attribute *sas_ata_sdev_attrs[] = {
>> -	&dev_attr_ncq_prio_supported.attr,
>> -	&dev_attr_ncq_prio_enable.attr,
>> +	&dev_attr_sas_ncq_prio_supported.attr,
>> +	&dev_attr_sas_ncq_prio_enable.attr,
>>   	NULL
>>   };
>>   
>