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

[John Hubbard]

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