Re: [PATCH 2/6] scsi: hisi_sas: Use sas_task_find_rq()

[John Garry]

On 29/09/2022 03:11, Damien Le Moal wrote:
> On 9/28/22 21:27, John Garry wrote:
> > Use sas_task_find_rq() to lookup the request per task for its driver tag.
> >
> > Signed-off-by: John Garry <john.garry@xxxxxxxxxx>
>
> Looks good, modulo the question below.
>
> Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>

Cheers

> > ---
> >   drivers/scsi/hisi_sas/hisi_sas_main.c | 26 ++++++++------------------
> >   1 file changed, 8 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
> > index 4c37ae9eb6b6..1011dffed51f 100644
> > --- a/drivers/scsi/hisi_sas/hisi_sas_main.c
> > +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
> > @@ -177,13 +177,13 @@ static void hisi_sas_slot_index_set(struct hisi_hba *hisi_hba, int slot_idx)
> >   }
> >   
> >   static int hisi_sas_slot_index_alloc(struct hisi_hba *hisi_hba,
> > -				     struct scsi_cmnd *scsi_cmnd)
> > +				     struct request *rq)
> >   {
> >   	int index;
> >   	void *bitmap = hisi_hba->slot_index_tags;
> >   
> > -	if (scsi_cmnd)
> > -		return scsi_cmd_to_rq(scsi_cmnd)->tag;
> > +	if (rq)
> > +		return rq->tag;
> >   
> >   	spin_lock(&hisi_hba->lock);
> >   	index = find_next_zero_bit(bitmap, hisi_hba->slot_index_count,
> > @@ -461,11 +461,11 @@ static int hisi_sas_queue_command(struct sas_task *task, gfp_t gfp_flags)
> >   	struct asd_sas_port *sas_port = device->port;
> >   	struct hisi_sas_device *sas_dev = device->lldd_dev;
> >   	bool internal_abort = sas_is_internal_abort(task);
> > -	struct scsi_cmnd *scmd = NULL;
> >   	struct hisi_sas_dq *dq = NULL;
> >   	struct hisi_sas_port *port;
> >   	struct hisi_hba *hisi_hba;
> >   	struct hisi_sas_slot *slot;
> > +	struct request *rq = NULL;
>
> Do you really need the NULL initialization here ?

Yes, as rq is only set in one case of the switch statement and checked 
outside the switch statement.

Thanks,
John