Re: [PATCH] spraid: initial commit of Ramaxel spraid driver

[Bart Van Assche <bvanassche@acm.org>]

On 10/9/21 06:32, Yanling Song wrote:
> op_is_write() does not meet our requirement: Both read and write
> commands have to be checked, not just write command.

Right, I should have proposed to compare the operation type with 
REQ_OP_READ / REQ_OP_WRITE. However, that approach only works for SCSI 
commands that come from the block layer and not for SG_IO 
(REQ_OP_DRV_IN/OUT).

>> Please remove all of the above code and use blk_rq_pos(),
>> blk_rq_sectors() and rq->cmd_flags & REQ_FUA instead.
> 
> I did not quite get your point. The above is commonly used in many
> similar use cases. For example: megasas_build_ldio() in
> megaraid_sas_base.c.
> What's the benefit to switch to another way: use
> blk_rq_pos(),blk_rq_sectors()?

I expect that using blk_rq_pos() and blk_rq_sectors() will result in 
faster code. However, these functions only work for SCSI commands that 
come from the block layer and not for SG_IO. If you want a common code 
path for SG_IO and block layer requests, please take a look at how 
get_unaligned_be*() is used in drivers/scsi/scsi_trace.c.

Thanks,

Bart.