From: Linus Torvalds <torvalds@linux-foundation.org>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
linux-unionfs@vger.kernel.org
Subject: Re: [GIT PULL] overlayfs update for 4.15
Date: Fri, 17 Nov 2017 13:49:01 -0800 [thread overview]
Message-ID: <CA+55aFxPPXo1UD_4kuSamJYx1FbcpVFx3ANzfxJpFKVhd_gROQ@mail.gmail.com> (raw)
In-Reply-To: <20171117151317.GA5348@veci.piliscsaba.szeredi.hu>
On Fri, Nov 17, 2017 at 7:13 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> Created a path_put_init() helper that clears out the pointers after putting the
> ref. I think this could be useful elsewhere, so added it to <linux/path.h>.
Slight eww.
The problem with your helper is that we've seen gcc generate really
horrible code for things like that.
So when you do
*path = (struct path) { };
we've seen gcc first create an local empty "struct path" on stack, and
then memcpy() it over the target. Which is _technically_ what that
code does, of course, but it's also excessively stupid.
So I suspect that would be better off as just
memset(path, 0, sizeof(*path));
which then matches the code that you actually would expect gcc to generate.
I hope that "struct path" is small enough that gcc doesn't mess up,
and that odd code generation is probably specific to some gcc versions
anyway, but we've definitely seen this.
NOTE! The above pattern of assignment is very different from the
initialization pattern. Gcc generally does well on structure
initializers:
struct xyz a = { .. };
generally generates reasonable code in ways that
struct xyz a;
..
a = (struct xyz) { ...};
sometimes doesn't. I suspect it's mainly a "initializers are common,
unnamed temporary local structures are not" thing.
Linus
prev parent reply other threads:[~2017-11-17 21:49 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-17 15:13 [GIT PULL] overlayfs update for 4.15 Miklos Szeredi
2017-11-17 21:49 ` Linus Torvalds [this message]
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=CA+55aFxPPXo1UD_4kuSamJYx1FbcpVFx3ANzfxJpFKVhd_gROQ@mail.gmail.com \
--to=torvalds@linux-foundation.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-unionfs@vger.kernel.org \
--cc=miklos@szeredi.hu \
/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;
as well as URLs for NNTP newsgroup(s).