linux-kernel - Re: [PATCH v7] mm: cma: support sysfs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 24 Mar 2021 15:23:09 -0700
From:   John Hubbard <jhubbard@...dia.com>
To:     Dmitry Osipenko <digetx@...il.com>,
        Minchan Kim <minchan@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>
CC:     linux-mm <linux-mm@...ck.org>, LKML <linux-kernel@...r.kernel.org>,
        <gregkh@...uxfoundation.org>, <surenb@...gle.com>,
        <joaodias@...gle.com>, <willy@...radead.org>,
        Colin Ian King <colin.king@...onical.com>
Subject: Re: [PATCH v7] mm: cma: support sysfs

On 3/24/21 3:11 PM, Dmitry Osipenko wrote:
> 25.03.2021 01:01, John Hubbard пишет:
>> On 3/24/21 2:31 PM, Dmitry Osipenko wrote:
>>> ...
>>>> +#include <linux/kobject.h>
>>>> +
>>>> +struct cma_kobject {
>>>> +    struct cma *cma;
>>>> +    struct kobject kobj;
>>>
>>> If you'll place the kobj as the first member of the struct, then
>>> container_of will be a no-op.
>>>
>>
>> However, *this does not matter*. Let's not get carried away. If
>> container_of() ends up as a compile-time addition of +8, instead
>> of +0, there is not going to be a visible effect in the world.
>> Or do you have some perf data to the contrary?
>>
>> Sometimes these kinds of things matter. But other times, they are
>> just pointless to fret about, and this is once such case.
> 
> Performance is out of question here, my main point is about maintaining

In that case, there is even less reason to harass people about the order
of members of a struct.

> a good coding style. Otherwise there is no point in not embedding kobj
> into cma struct as well, IMO.


We really don't need to worry about the order of members in a struct,
from a "coding style" point of view. It is a solid step too far.

Sorry if that sounds a little too direct. But this review has tended to
go quite too far into nitpicks that are normally left as-is, and I've
merely picked one that is particularly questionable. I realize that other
coding communities have their own standards. Here, I'm explaining what
I have observed about linux-mm and linux-kernel, which needs to be respected.

thanks,
-- 
John Hubbard
NVIDIA