Re: [PATCH v7] mm: cma: support sysfs

[John Hubbard <jhubbard@nvidia.com>]

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