Re: [PATCH 3/4] scsi: pm8001: Use libsas internal abort support

[Damien Le Moal]

On 3/4/22 18:41, John Garry wrote:
>>>   /**
>>>     * pm8001_queue_command - register for upper layer used, all IO commands sent
>>>     * to HBA are from this interface.
>>> @@ -379,16 +428,15 @@ int pm8001_queue_command(struct sas_task *task, gfp_t gfp_flags)
>>>   	enum sas_protocol task_proto = task->task_proto;
>>>   	struct domain_device *dev = task->dev;
>>>   	struct pm8001_device *pm8001_dev = dev->lldd_dev;
>>> +	bool internal_abort = sas_is_internal_abort(task);
>>>   	struct pm8001_hba_info *pm8001_ha;
>>>   	struct pm8001_port *port = NULL;
>>>   	struct pm8001_ccb_info *ccb;
>>> -	struct sas_tmf_task *tmf = task->tmf;
>>> -	int is_tmf = !!task->tmf;
>>>   	unsigned long flags;
>>>   	u32 n_elem = 0;
>>>   	int rc = 0;
>>>   
>>> -	if (!dev->port) {
>>> +	if (!internal_abort && !dev->port) {
>>>   		ts->resp = SAS_TASK_UNDELIVERED;
>>>   		ts->stat = SAS_PHY_DOWN;
>>>   		if (dev->dev_type != SAS_SATA_DEV)
>>> @@ -410,7 +458,8 @@ int pm8001_queue_command(struct sas_task *task, gfp_t gfp_flags)
>>>   	pm8001_dev = dev->lldd_dev;
>>>   	port = &pm8001_ha->port[sas_find_local_port_id(dev)];
>>>   
>>> -	if (DEV_IS_GONE(pm8001_dev) || !port->port_attached) {
>>> +	if (!internal_abort && (DEV_IS_GONE(pm8001_dev) ||
>>> +				!port->port_attached)) {
>> Wrapping the line after "&&" would make this a lot cleaner and easier to read.
> 
> Agreed, I can do it.
> 
> But if you can see any neater ways to skip these checks for internal 
> abort in the common queue command path then I would be glad to hear it.

Would need to check the locking context, but if there is no race
possible with the context setting the device as gone, libata should
already be aware of it and not issuing the command in the first place.
So these checks should not be needed at all.

Will try to have a look when I have some time.

There are several things that needs better integration with libata
anyway, like the "manual" read log on error. We should try to address
these for 5.19.


-- 
Damien Le Moal
Western Digital Research