Re: [PATCH v3 28/31] scsi: pm8001: Introduce ccb alloc/free helpers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/15/22 20:07, John Garry wrote:
> On 14/02/2022 02:17, Damien Le Moal wrote:
>> Introduce the pm8001_ccb_alloc() and pm8001_ccb_free() helpers to
>> replace the typical code patterns:
>>
>> 	res = pm8001_tag_alloc(pm8001_ha, &ccb_tag);
>> 	if (res)
>> 		...
>> 	ccb = &pm8001_ha->ccb_info[ccb_tag];
>> 	ccb->device = pm8001_ha_dev;
>> 	ccb->ccb_tag = ccb_tag;
>> 	ccb->task = task;
>> 	ccb->n_elem = 0;A
> 
> nit: stray 'A' character
> 
>>
>> and
>>
>> 	ccb->task = NULL;
>> 	ccb->ccb_tag = PM8001_INVALID_TAG;
>> 	pm8001_tag_free(pm8001_ha, tag);
> 
> Isn't it possible to do a "memset struct members", such that we order 
> the members specifically that we can memset(0) a known region? The 
> advantage is that adding/removing a member does not require much code to 
> be touched and possibly missed.
> 
>>
>> With the simpler function calls:
>>
>> 	ccb = pm8001_ccb_alloc(pm8001_ha, pm8001_ha_dev, task);
>> 	if (!ccb)
>> 		...
>>
>> and
>>
>> 	pm8001_ccb_free(pm8001_ha, ccb);
>>
>> The pm8001_ccb_alloc() helper ensures that all fields of the ccb info
>> structure for the newly allocated tag are all initialized, except the
>> buf_prd field. The pm8001_ccb_free() helper clears the initialized
>> fields and the ccb tag to ensure that iteration over the adapter
>> ccb_info array detects ccbs that are in use.
> 
> 
>>
>> All call site of the pm8001_tag_alloc() function that use a ccb info
>> associated with an allocated tag are converted to use the new helpers.
>>
>> Signed-off-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
>> ---
> 
> The comments are not a show stopper, so:
> Reviewed-by: John Garry <john.garry@xxxxxxxxxx>

Thanks. I left the patch as-is and did not try the memset() thing. Let's
do that later.

> 


-- 
Damien Le Moal
Western Digital Research



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux