[ofa-general] OFED, the backported <linux/scatterlist.h> header and sg_init_table()

[Bart Van Assche]

On Sun, May 3, 2009 at 3:04 PM, Jack Morgenstein
<jackm at dev.mellanox.co.il> wrote:
> On Saturday 02 May 2009 14:46, Bart Van Assche wrote:
>> Hello,
>>
>> Yesterday I installed OFED-1.4.1-rc4 on a CentOS 5.3 system and started
>> looking at the backported kernel headers. I found the following in the
>> header file
>> /usr/src/ofa_kernel-1.4.1/kernel_addons/backport/2.6.18-EL5.3/include/linux/scatterlist.h:
>>
>> #define sg_init_table(a, b)
>>
>> Or: sg_init_table() is defined to do nothing. I was expecting the following
>> however:
>>
>> #define sg_init_table(sgl, nents) memset(sgl, 0, sizeof(*sgl) * nents);
>>
>> The sg_init_table() function is implemented in e.g. 2.6.29 as follows:
>>
>> void sg_init_table(struct scatterlist *sgl, unsigned int nents)
>> {
>>         memset(sgl, 0, sizeof(*sgl) * nents);
>> #ifdef CONFIG_DEBUG_SG
>>         {
>>                 unsigned int i;
>>                 for (i = 0; i < nents; i++)
>>                         sgl[i].sg_magic = SG_MAGIC;
>>         }
>> #endif
>>         sg_mark_end(&sgl[nents - 1]);
>> }
>>
>> Does anyone know why sg_init_table() is defined such that it does nothing in
>> the backported OFED headers ?
>
> I checked this more carefully.
> Use of sg_init_table was introduced in 2.6.24 by Jens Axboe, in commit
> 45711f1af6eff1a6d010703b4862e0d2b9afd056. (see chunks for core/umem.c)
>
> Before this, no initialization was done on the sg page_list, and we had no
> problems.  When doing the backport, then, I simply made this a NOP.
> I'm not convinced that sg_init_table needs to be implemented in kernels earlier
> than 2.6.24, since this call is not replacing anything (e.g., a kzalloc), and
> the page list was not previously zeroed out before usage.
>
> What do you think?

My opinion is that it is really dangerous and confusing to have one
version of the sg_init_table() macro that performs initialization and
another version that does not. As an example, the OFED source file
net/sunrpc/xdr.c invokes sg_init_table(). When this code is compiled
against e.g. a 2.6.27 kernel, invoking sg_init_table() will
initialize the sg-list properly because in this case the
sg_init_table() included with the 2.6.27 kernel is used. When this
code is compiled against e.g. an RHEL 5.3 kernel, invoking the
sg_init_table() macro will have no effect because the sg_init_table()
macro from OFED's backported header files is used. Is this effect
really desired ?

Bart.