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

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

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?

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

Bart.