linux-kernel - Re: [PATCH RFC 2/6] scsi: scsi_transport_sas: Allocate end device target id in the rphy alloc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <55ee46e7-a07d-d1ef-72fd-1f99b2a04684@huawei.com>
Date:   Wed, 21 Sep 2022 10:21:57 +0100
From:   John Garry <john.garry@...wei.com>
To:     Damien Le Moal <damien.lemoal@...nsource.wdc.com>,
        <jejb@...ux.ibm.com>, <martin.petersen@...cle.com>, <hare@...e.de>,
        <hch@....de>
CC:     <linux-ide@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-scsi@...r.kernel.org>, <brking@...ibm.com>
Subject: Re: [PATCH RFC 2/6] scsi: scsi_transport_sas: Allocate end device
 target id in the rphy alloc

On 20/09/2022 23:02, Damien Le Moal wrote:
>>
>>       return &rdev->rphy;
>>   }
>>   EXPORT_SYMBOL(sas_end_device_alloc);
>> @@ -1500,6 +1505,16 @@ struct sas_rphy *sas_expander_alloc(struct 
>> sas_port *parent,
>>   }
>>   EXPORT_SYMBOL(sas_expander_alloc);
>> +static bool sas_rphy_end_device_valid_tproto(struct sas_rphy *rphy)
>> +{
>> +    struct sas_identify *identify = &rphy->identify;
>> +
>> +    if (identify->target_port_protocols &
>> +        (SAS_PROTOCOL_SSP | SAS_PROTOCOL_STP | SAS_PROTOCOL_SATA))
>> +        return true;
>> +    return false;
> 
> You could just do:
> 
> return identify->target_port_protocols &
>      (SAS_PROTOCOL_SSP | SAS_PROTOCOL_STP | SAS_PROTOCOL_SATA))

OK

> 
>> +}
>> +
>>   /**
>>    * sas_rphy_add  -  add a SAS remote PHY to the device hierarchy
>>    * @rphy:    The remote PHY to be added
>> @@ -1529,16 +1544,10 @@ int sas_rphy_add(struct sas_rphy *rphy)
>>       mutex_lock(&sas_host->lock);
>>       list_add_tail(&rphy->list, &sas_host->rphy_list);
>> -    if (identify->device_type == SAS_END_DEVICE &&
>> -        (identify->target_port_protocols &
>> -         (SAS_PROTOCOL_SSP | SAS_PROTOCOL_STP | SAS_PROTOCOL_SATA)))
>> -        rphy->scsi_target_id = sas_host->next_target_id++;
>> -    else if (identify->device_type == SAS_END_DEVICE)
>> -        rphy->scsi_target_id = -1;
>>       mutex_unlock(&sas_host->lock);
>>       if (identify->device_type == SAS_END_DEVICE &&
> 
> You could move this check inside sas_rphy_end_device_valid_tproto(),
> no ?
> 

Yeah, I suppose I could. I might require a function rename with that.

>> -        rphy->scsi_target_id != -1) {
>> +        sas_rphy_end_device_valid_tproto(rphy)) {
>>           int lun;
>>           if (identify->target_port_protocols & SAS_PROTOCOL_SSP)
>> @@ -1667,7 +1676,7 @@ static int sas_user_scan(struct Scsi_Host 
>> *shost, uint channel,
>>       mutex_lock(&sas_host->lock);
>>       list_for_each_entry(rphy, &sas_host->rphy_list, list) {
>>           if (rphy->identify.device_type != SAS_END_DEVICE ||
>> -            rphy->scsi_target_id == -1)
>> +            !sas_rphy_end_device_valid_tproto(rphy))
>>               continue; 

Thanks,
John