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

[Yanling Song <songyl@ramaxel.com>]

On Sat, 9 Oct 2021 20:52:13 -0700
Bart Van Assche <bvanassche@acm.org> wrote:

> 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.

get_unaligned_be*() is an inline which has the same function as our
current code and there is no difference on performance.  
But current code is better when supporting old kernels since it is
implemented on SCSI spec and there is no dependency on
get_unaligned_be*(), which means the code can work on any version
of kernel. 
So I prefer to keep current implementation. What's your opinion?
 
> 
> Thanks,
> 
> Bart.