Re: [PATCH v2] add bidi support for block pc requests

[Boaz Harrosh <bharrosh@panasas.com>]

Boaz Harrosh wrote:
> FUJITA Tomonori wrote:
>>> FUJITA Tomonori wrote:
>> One thing that I found is:
>>
>> +#define scsi_resid(cmd) ((cmd)->sg_table->resid)
>>
>>
>> This doesn't work for some drivers (at least ipr) since they set
>> cmd->resid even with commands without data transfer.
>>
> 
> James, Tomo.
> 
> the last accessor:
> +#define scsi_resid(cmd) ((cmd)->resid)
> 
> used as an l-value in drivers does not serve our purpose, as seen by the test
> implementation of scsi_sg_table. Now clearly this needs an accessor and it is a
> bidi parameter (need 2 of them).
> 
> I would suggest doing a set_ inline accessor and using that in drivers:
> +static inline void scsi_set_resid(struct scsi_cmnd *cmd, resid)
> +{
> +	cmd->resid = resid;
> +}
> 
> I would even suggest to make all accessors inlines and not macros just to make sure
> they are not used as l-value and modified. Though I have not seen such use in
> Tomo's patchset.
> 
> example:
> +static inline unsigned scsi_bufflen(struct scsi_cmnd *cmd)
> +{
> +	return cmd->request_bufflen;
> +}
> 
> Boaz
> -

I forgot to say. Tomo, I have started to convert to the above, based on your
cleanup git-tree. Do you need that I send a patch?
(One thing I still don't have a good solution for. When having a git-tree like that
with a long patchset. How to fix/update individual patches?)

Boaz