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

[Yanling Song <songyl@ramaxel.com>]

On Mon, 11 Oct 2021 12:40:05 -0700
Bart Van Assche <bvanassche@acm.org> wrote:

> On 10/11/21 01:34, Yanling Song wrote:
> > 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?  
> 
> Hi Yanling,
> 
> On all architectures I'm familiar with get_unaligned_be*() is faster 
> than fetching individual bytes and combining these with shift 
> operations. As an example, x86 CPU's support unaligned memory
> accesses and for these CPU's the Linux kernel translates
> get_unaligned_be*() into a single (potentially unaligned) memory
> access.
> 
> Kernel drivers that are submitted for upstream inclusion should use
> the upstream kernel API. Whether or not get_unaligned_be*() is
> available in older kernels does not matter - since it is available in
> the upstream kernel and since it makes code faster and easier to
> read, using that function is strongly recommended. Additionally, it
> can happen that after a driver has been accepted upstream that
> someone writes a Coccinelle script to change all open-coded
> get_unaligned_be*() calls into get_unaligned_be*() calls.
> Compatibility with older kernels would not be accepted as a valid
> argument against such a patch.
> 
> I think that get_unaligned_be*() functions are supported since kernel 
> version 2.6.26, released in 2008, 13 years ago. Does this address
> your concern about supporting older kernel versions?

Ok. Will used get_unaligned_be*() functions in the next verison.
> 
> Regarding supporting older kernel versions, a common approach is to 
> develop against the latest upstream kernel. To support older kernels, 
> include a header file called e.g. backport.h and in that header file 
> implement the latest kernel API in terms of older kernel functions.
> An example:
> 
> #if LINUX_VERSION_CODE < KERNEL_VERSION(4, 14, 0) &&	\
> 	!defined(CONFIG_SUSE_KERNEL)
> static inline void bio_set_dev(struct bio *bio,
>                                 struct block_device *bdev)
> {
> 	bio->bi_bdev = bdev;
> }
> #endif

Thanks. I'll use this way when supporting older kernel.

> 
> Bart.