Re: [PATCH v2] scsi: scsi_transport_sas: fix error handling in sas_rphy_add()

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

On 18/11/2022 03:11, Yang Yingliang wrote:
>>>> );
>> There is a slight problem with doing this in that if
>> transport_device_add() ever fails it's likely because memory pressure
>> caused the allocation of the internal_container to fail. What that
>> means is that the visible sysfs attributes don't get added, but
>> otherwise the rphy is fully functional as far as the driver sees it, so
>> this condition doesn't have to be a fatal error which kills the device.
>>
>> There are two ways of handling this:
>>
>>     1. The above to move the condition from an ignored to a fatal error.
>>        It's so rare that we almost never see it in practice and if it
>>        ever happened, the machine is so low on memory that something
>>        else is bound to fail an allocation and kill the device anyway,
>>        so treating it as non-fatal likely serves no purpose.
>>     2. Simply to make the assumption that transport_remove_device() is
>>        idempotent true by adding a flag in the internal_class to signify
>>        removal is required. This would preserve current behaviour and
>>        have the bonus that it only requires a single patch, not one
>>        patch per transport class object that has this problem.
>>
>> I'd probably prefer 2. since it's way less work, but others might have
>> different opinions.
> Current some callers ignore the return value of transport_add_device(), 
> if it fails,
> it will cause null-ptr-deref in transport_remove_device().
> 
> James suggested that add some check in transport_remove_device(), so all 
> can
> be fix in one patch.
> 
> Do you have any suggestion for this ?

Personally I prefer 1. However did you develop a prototype patch for how 
2. would look? And how many changes are still required for 1.?

Thanks,
John