From: Tejun Heo <tj@kernel.org>
To: Nick Desaulniers <nick.desaulniers@gmail.com>
Cc: Li Zefan <lizefan@huawei.com>,
Johannes Weiner <hannes@cmpxchg.org>,
cgroups@vger.kernel.org,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Matthias Kaehlcke <mka@chromium.org>,
Michael Davidson <md@google.com>,
Greg Hackmann <ghackmann@google.com>,
android-llvm@google.com
Subject: Re: [PATCH] cgroup: reorder flexible array members of struct cgroup_root
Date: Sat, 21 Oct 2017 08:32:53 -0700 [thread overview]
Message-ID: <20171021153253.GG1302522@devbig577.frc2.facebook.com> (raw)
In-Reply-To: <CAH7mPvjksNSDo4o=z51YkH4yX+gDnGXU-g59gTkOUS-pEva8sA@mail.gmail.com>
Hello, Nick.
On Fri, Oct 20, 2017 at 12:15:55AM -0700, Nick Desaulniers wrote:
> > This is silly tho. We know the the root group embedded there won't
> > have any ancestor_ids.
>
> Sure, but struct cgroup_root is still declared as having a struct
> cgroup not declared as the final member.
Why is that a problem tho? We know that it doesn't have any flexible
array member so there's no storage allocated to it.
> > Also, in general, nothing prevents us from
> > doing something like the following.
> >
> > struct outer_struct {
> > blah blah;
> > struct inner_struct_with_flexible_array_member inner;
> > unsigned long storage_for_flexible_array[NR_ENTRIES];
> > blah blah;
> > };
>
> At that point, then why have the struct with flexible array member in
> the first place?
Because there are different ways to use the struct?
> >> or specific location of the member cgrp within struct cgroup_root AFAICT.
> > I think we should just silence the bogus warning.
>
> Is the order of the members actually important? Otherwise it seems
> that we're taking advantage of a GNU C extension for no real reason,
> which is what I'm trying to avoid. Please reconsider.
Here, not necessarily but I don't want to move it for a bogus reason.
Why would we disallow embedding structs with flexible members in the
middle when it can be done and is useful? If we want to discuss
whether we want to avoid such usages in the kernel (but why?), sure,
let's have that discussion but we can't decide that on "clang warns on
it by default".
Thanks.
--
tejun
next prev parent reply other threads:[~2017-10-21 15:32 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-17 6:33 [PATCH] cgroup: reorder flexible array members of struct cgroup_root Nick Desaulniers
2017-10-17 6:40 ` Nick Desaulniers
2017-10-17 6:45 ` Nick Desaulniers
2017-10-18 13:30 ` Tejun Heo
2017-10-20 7:15 ` Nick Desaulniers
2017-10-21 15:32 ` Tejun Heo [this message]
2017-10-21 15:59 ` Aleksa Sarai
2017-10-21 16:03 ` Tejun Heo
2017-10-21 19:08 ` Nick Desaulniers
2017-10-25 21:54 ` Matthias Kaehlcke
2017-10-26 14:32 ` Tejun Heo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171021153253.GG1302522@devbig577.frc2.facebook.com \
--to=tj@kernel.org \
--cc=android-llvm@google.com \
--cc=cgroups@vger.kernel.org \
--cc=ghackmann@google.com \
--cc=hannes@cmpxchg.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lizefan@huawei.com \
--cc=md@google.com \
--cc=mka@chromium.org \
--cc=nick.desaulniers@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox