[GIT PULL linux-next] Add Compiler Attributes tree

[GIT PULL linux-next] Add Compiler Attributes tree

[Miguel Ojeda]

Hi Stephen,

The Compiler Attributes series has been stable for 10+ days. To
increase testing before 4.20, I would to request it being picked up
for -next.

The changes w.r.t. v5 in the LKML:

  - Rebased on top of next-20180928, which required removing
aligned_largest, which was removed by 9503cd9cbaba
("include/linux/compiler*.h: add version detection to
asm_volatile_goto").
  - Added latest Reviewed-by's and Tested-by's.

Thanks!

Cheers,
Miguel

The following changes since commit 4794a36bf08dfa89fe636e5080db9d8350e255dd:

  Add linux-next specific files for 20180928 (2018-09-28 15:26:51 +1000)

are available in the Git repository at:

  https://github.com/ojeda/linux.git compiler-attributes

for you to fetch changes up to dbce062c0b519db1cdad8d87ab46851f0be6bdea:

  Compiler Attributes: ext4: remove local __nonstring definition
(2018-10-02 15:11:26 +0200)

----------------------------------------------------------------
Miguel Ojeda (15):
      Compiler Attributes: remove unused attributes
      Compiler Attributes: always use the extra-underscores syntax
      Compiler Attributes: remove unneeded tests
      Compiler Attributes: homogenize __must_be_array
      Compiler Attributes: remove unneeded sparse (__CHECKER__) tests
      Compiler Attributes: add missing SPDX ID in compiler_types.h
      Compiler Attributes: use feature checks instead of version checks
      Compiler Attributes: KENTRY used twice the "used" attribute
      Compiler Attributes: remove uses of __attribute__ from compiler.h
      Compiler Attributes: add Doc/process/programming-language.rst
      Compiler Attributes: add MAINTAINERS entry
      Compiler Attributes: add support for __nonstring (gcc >= 8)
      Compiler Attributes: enable -Wstringop-truncation on W=1 (gcc >= 8)
      Compiler Attributes: auxdisplay: panel: use __nonstring
      Compiler Attributes: ext4: remove local __nonstring definition

 Documentation/process/index.rst                |   1 +
 Documentation/process/programming-language.rst |  45 +++++
 MAINTAINERS                                    |   5 +
 drivers/auxdisplay/panel.c                     |   7 +-
 fs/ext4/ext4.h                                 |   9 -
 include/linux/compiler-clang.h                 |   5 -
 include/linux/compiler-gcc.h                   |  70 +------
 include/linux/compiler-intel.h                 |   9 -
 include/linux/compiler.h                       |  19 +-
 include/linux/compiler_attributes.h            | 257 +++++++++++++++++++++++++
 include/linux/compiler_types.h                 | 100 ++--------
 scripts/Makefile.extrawarn                     |   1 +
 12 files changed, 340 insertions(+), 188 deletions(-)
 create mode 100644 Documentation/process/programming-language.rst
 create mode 100644 include/linux/compiler_attributes.h

Re: [GIT PULL linux-next] Add Compiler Attributes tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 990 bytes --]

Hi Miguel,

On Tue, 2 Oct 2018 15:47:12 +0200 Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
>
> The Compiler Attributes series has been stable for 10+ days. To
> increase testing before 4.20, I would to request it being picked up
> for -next.
> 
> The changes w.r.t. v5 in the LKML:
> 
>   - Rebased on top of next-20180928, which required removing

Unfortunately, trees/branches included in linux-next must be based on
something stable (usually Linus' tree, but it could be another
tree/branch that is included in linux-next that does not rebase).
Linux-next itself rebases every day, so snything based on it would drag
in a previous version of all the other trees :-(

> aligned_largest, which was removed by 9503cd9cbaba
> ("include/linux/compiler*.h: add version detection to
> asm_volatile_goto").

That commit is from Andrew's patch series which also rebases (usually
at least every week), so you cannot depend on it.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [GIT PULL linux-next] Add Compiler Attributes tree

[Nick Desaulniers]

On Tue, Oct 2, 2018 at 2:11 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi Miguel,
>
> On Tue, 2 Oct 2018 15:47:12 +0200 Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
> >
> > The Compiler Attributes series has been stable for 10+ days. To
> > increase testing before 4.20, I would to request it being picked up
> > for -next.
> >
> > The changes w.r.t. v5 in the LKML:
> >
> >   - Rebased on top of next-20180928, which required removing
>
> Unfortunately, trees/branches included in linux-next must be based on
> something stable (usually Linus' tree, but it could be another
> tree/branch that is included in linux-next that does not rebase).
> Linux-next itself rebases every day, so snything based on it would drag
> in a previous version of all the other trees :-(

I think of this like a branch that's force pushed to.  Can't base
other branches or trees off of it cause it's always moving/force
rewriting history.

>
> > aligned_largest, which was removed by 9503cd9cbaba
> > ("include/linux/compiler*.h: add version detection to
> > asm_volatile_goto").
>
> That commit is from Andrew's patch series which also rebases (usually
> at least every week), so you cannot depend on it.

Miguel, you should be able to drop that patch from your set then,
since Andrew's -mm tree flows into this -next tree as well, IIUC.
We'll take up that patch from there.

-- 
Thanks,
~Nick Desaulniers

Re: [GIT PULL linux-next] Add Compiler Attributes tree

[Miguel Ojeda]

Hi Stephen,

On Tue, Oct 2, 2018 at 11:11 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi Miguel,
>
> On Tue, 2 Oct 2018 15:47:12 +0200 Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
> >
> > The Compiler Attributes series has been stable for 10+ days. To
> > increase testing before 4.20, I would to request it being picked up
> > for -next.
> >
> > The changes w.r.t. v5 in the LKML:
> >
> >   - Rebased on top of next-20180928, which required removing
>
> Unfortunately, trees/branches included in linux-next must be based on
> something stable (usually Linus' tree, but it could be another
> tree/branch that is included in linux-next that does not rebase).
> Linux-next itself rebases every day, so snything based on it would drag
> in a previous version of all the other trees :-(

I assumed you could apply changes as a diff/patches/cherry-pick, not
as a merge, for those that went on top of others (so that at the new
merge window, conflicts were already solved). Otherwise, why are
next-* tags/branches provided anyway?

>
> > aligned_largest, which was removed by 9503cd9cbaba
> > ("include/linux/compiler*.h: add version detection to
> > asm_volatile_goto").
>
> That commit is from Andrew's patch series which also rebases (usually
> at least every week), so you cannot depend on it.

Then who is solving the conflict?

Thanks!

Cheers,
Miguel

Re: [GIT PULL linux-next] Add Compiler Attributes tree

[Miguel Ojeda]

Hi Nick,

On Tue, Oct 2, 2018 at 11:16 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Tue, Oct 2, 2018 at 2:11 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > Hi Miguel,
> >
> > On Tue, 2 Oct 2018 15:47:12 +0200 Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
> > >
> > > The Compiler Attributes series has been stable for 10+ days. To
> > > increase testing before 4.20, I would to request it being picked up
> > > for -next.
> > >
> > > The changes w.r.t. v5 in the LKML:
> > >
> > >   - Rebased on top of next-20180928, which required removing
> >
> > Unfortunately, trees/branches included in linux-next must be based on
> > something stable (usually Linus' tree, but it could be another
> > tree/branch that is included in linux-next that does not rebase).
> > Linux-next itself rebases every day, so snything based on it would drag
> > in a previous version of all the other trees :-(
>
> I think of this like a branch that's force pushed to.  Can't base
> other branches or trees off of it cause it's always moving/force
> rewriting history.

As I have read, -next is supposed to be a vision of what the merge
window will look like after merging everything, i.e. ideally -rc1. For
that to work for files out-of-tree (like these ones, which are not
maintained by a single tree), changes should be allowed to be stacked
on each other; otherwise, we cannot handle conflicts :-(

>
> >
> > > aligned_largest, which was removed by 9503cd9cbaba
> > > ("include/linux/compiler*.h: add version detection to
> > > asm_volatile_goto").
> >
> > That commit is from Andrew's patch series which also rebases (usually
> > at least every week), so you cannot depend on it.
>
> Miguel, you should be able to drop that patch from your set then,
> since Andrew's -mm tree flows into this -next tree as well, IIUC.
> We'll take up that patch from there.

Not sure what you mean by "drop that patch". There is no patch to
drop, there is a conflict with a change already in -next.

Cheers,
Miguel

Re: [GIT PULL linux-next] Add Compiler Attributes tree

[Dominique Martinet]

Miguel Ojeda wrote on Wed, Oct 03, 2018:
> As I have read, -next is supposed to be a vision of what the merge
> window will look like after merging everything, i.e. ideally -rc1. For
> that to work for files out-of-tree (like these ones, which are not
> maintained by a single tree), changes should be allowed to be stacked
> on each other; otherwise, we cannot handle conflicts :-(

The rule is the same as with a regular mainline pull; I don't have the
reference at hand but in some recent-ish pull request Linus said he
prefers the stable version with the conflict, and optionally you can
provide a second branch with the conflict resolved for reference, but
the pull request should be based on something stable even if it has
conflicts

If there is a conflict Stefen will resolve it like Linus/Greg would, and
the resolved bit will be carried over everyday so it's not much more
work -- exactly like a regular pull request for inclusion in the main
tree :)


(Found another mail arguing for stable base[1], but can't find the mail I
was thinking of)
 [1] http://lkml.kernel.org/r/CA+55aFw+dwofadgvzrM-UCMSih+f1choCwW+xFFM3aPjoRQX_g@mail.gmail.com

-- 
Dominique Martinet

Re: [GIT PULL linux-next] Add Compiler Attributes tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1488 bytes --]

Hi Miguel,

On Wed, 3 Oct 2018 00:36:52 +0200 Dominique Martinet <asmadeus@codewreck.org> wrote:
>
> Miguel Ojeda wrote on Wed, Oct 03, 2018:
> > As I have read, -next is supposed to be a vision of what the merge
> > window will look like after merging everything, i.e. ideally -rc1. For
> > that to work for files out-of-tree (like these ones, which are not
> > maintained by a single tree), changes should be allowed to be stacked
> > on each other; otherwise, we cannot handle conflicts :-(  
> 
> The rule is the same as with a regular mainline pull; I don't have the
> reference at hand but in some recent-ish pull request Linus said he
> prefers the stable version with the conflict, and optionally you can
> provide a second branch with the conflict resolved for reference, but
> the pull request should be based on something stable even if it has
> conflicts
> 
> If there is a conflict Stefen will resolve it like Linus/Greg would, and
> the resolved bit will be carried over everyday so it's not much more
> work -- exactly like a regular pull request for inclusion in the main
> tree :)

Exactly what Dominique said.  I will fix up the conflict (unless it is
a very complex conflict, in which case the author(s) should help) and
the Linus (or Greg) will do the same.  If you do depend on a patch in
Andrew's series, what happens if that patch does not get sent to Linus
during the merge window or Linus rejects it?

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [GIT PULL linux-next] Add Compiler Attributes tree

[Theodore Y. Ts'o]

On Wed, Oct 03, 2018 at 12:12:10AM +0200, Miguel Ojeda wrote:
> As I have read, -next is supposed to be a vision of what the merge
> window will look like after merging everything, i.e. ideally -rc1. For
> that to work for files out-of-tree (like these ones, which are not
> maintained by a single tree), changes should be allowed to be stacked
> on each other; otherwise, we cannot handle conflicts :-(

In general, best practice is to base tree on an -rcX commit.  I
usually will use something like -rc4 which is after most of the major
changes have gone in.  This tends to reduce conflicts for most git
trees.

There are times when a commit in one tree needs to depend on a commit
in another tree.  What to do depends on the circumstances.

One solution is to base one subsystem's git tree on another
subsystem's git tree --- *if* that git tree is one that doesn't get
rebase.  You'll need to talk to the subsystem maintainer to find out
whether or not that's the case.  But that's actually not a great
solution, because what can happen is if the tree A is based on tree B,
and there is something in tree B which Linus objects to, tree B won't
get pulled.  And since tree A depends on tree B, Linus will refuse to
pull tree A as well.  We recently had a case where a subsystem pull
got delayed by a full development cycle because of this.

So another solution is to simply evade the problem.  If the reason why
tree A needs to depend on tree B is that tree B is using some
interface which has changed, if it's a minor change, then Stephen will
fix it up when he pulls the changes; just as Linus will.

If the issue is that there is some common infrastructure which two git
tree needs, what will often happen is that just those patches which
provide the new infastructure will get put on a branch based on -rcX
on one of the git trees.  And then the subsystems will base their work
on that sub-branche.  For example, suppose the file system developers
have collectively decided that there should be a new fallocate(2)
flag, FALLOC_FL_DONT_RANDOMLY_LOSE (ala RFC 748).  The work to define
and enable that feature in the VFS layer might be placed on the
randomly-lose git branch on xfs.git.  And then the xfs and ext4
development branches will both be based on the randomly-lose branch.

Yet another solution is to arrange the code changes to avoid needing
commits that might conflict.  For example, in fs/ext4/ext4.h, I very
deliberately did this.

/* Until this gets included into linux/compiler-gcc.h */
#ifndef __nonstring
#if defined(GCC_VERSION) && (GCC_VERSION >= 80000)
#define __nonstring __attribute__((nonstring))
#else
#define __nonstring
#endif
#endif

You included a cleanup patch to remove it in your git tree, but it
wasn't actually necessary.  If there was a merge conflict, it would be
simple enough to just drop your cleanup patch, since I had carefully
used #ifndef __nonstring... #endif.  So the idea was that if someone
defined __nonstring somewhere else, it wouldn't break the build with a
duplicate #define since it was protected with an #ifndef.

I didn't mind that you included a cleanup patch; but I set things up
so that it would not be necessary, since often the best way to solve a
merge conflict is by avoiding the need for the change (in some other
git tree) in the first place.  :-)

Cheers,

					- Ted

Re: [GIT PULL linux-next] Add Compiler Attributes tree

[Miguel Ojeda]

HI Dominique,

On Wed, Oct 3, 2018 at 12:37 AM Dominique Martinet
<asmadeus@codewreck.org> wrote:
>
> Miguel Ojeda wrote on Wed, Oct 03, 2018:
> > As I have read, -next is supposed to be a vision of what the merge
> > window will look like after merging everything, i.e. ideally -rc1. For
> > that to work for files out-of-tree (like these ones, which are not
> > maintained by a single tree), changes should be allowed to be stacked
> > on each other; otherwise, we cannot handle conflicts :-(
>
> The rule is the same as with a regular mainline pull; I don't have the
> reference at hand but in some recent-ish pull request Linus said he

That is actually the first problem: there is no
reference/documentation at hand. :-P

Cheers,
Miguel

Re: [GIT PULL linux-next] Add Compiler Attributes tree

[Steven Rostedt]

On Wed, 3 Oct 2018 14:14:28 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> HI Dominique,
> 
> On Wed, Oct 3, 2018 at 12:37 AM Dominique Martinet
> <asmadeus@codewreck.org> wrote:
> >
> > Miguel Ojeda wrote on Wed, Oct 03, 2018:  
> > > As I have read, -next is supposed to be a vision of what the merge
> > > window will look like after merging everything, i.e. ideally -rc1. For
> > > that to work for files out-of-tree (like these ones, which are not
> > > maintained by a single tree), changes should be allowed to be stacked
> > > on each other; otherwise, we cannot handle conflicts :-(  
> >
> > The rule is the same as with a regular mainline pull; I don't have the
> > reference at hand but in some recent-ish pull request Linus said he  
> 
> That is actually the first problem: there is no
> reference/documentation at hand. :-P
>

Yes, and one of the topics for Maintainers Summit is to create one. One
is actually in the works led by Dan Williams.

-- Steve

Re: [GIT PULL linux-next] Add Compiler Attributes tree

[Miguel Ojeda]

Hi Stephen,

On Wed, Oct 3, 2018 at 1:00 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi Miguel,
>
> On Wed, 3 Oct 2018 00:36:52 +0200 Dominique Martinet <asmadeus@codewreck.org> wrote:
> >
> > Miguel Ojeda wrote on Wed, Oct 03, 2018:
> > > As I have read, -next is supposed to be a vision of what the merge
> > > window will look like after merging everything, i.e. ideally -rc1. For
> > > that to work for files out-of-tree (like these ones, which are not
> > > maintained by a single tree), changes should be allowed to be stacked
> > > on each other; otherwise, we cannot handle conflicts :-(
> >
> > The rule is the same as with a regular mainline pull; I don't have the
> > reference at hand but in some recent-ish pull request Linus said he
> > prefers the stable version with the conflict, and optionally you can
> > provide a second branch with the conflict resolved for reference, but
> > the pull request should be based on something stable even if it has
> > conflicts
> >
> > If there is a conflict Stefen will resolve it like Linus/Greg would, and
> > the resolved bit will be carried over everyday so it's not much more
> > work -- exactly like a regular pull request for inclusion in the main
> > tree :)
>
> Exactly what Dominique said.  I will fix up the conflict (unless it is
> a very complex conflict, in which case the author(s) should help) and
> the Linus (or Greg) will do the same.  If you do depend on a patch in
> Andrew's series, what happens if that patch does not get sent to Linus
> during the merge window or Linus rejects it?

This doesn't depend on anything. Not sure what is all the fuss about
-- people got confused into thinking we had to drop a patch for some
reason. As explained in the first email, I simply rebased v5 (which is
based on top of rcX) to resolve the conflict myself (i.e. it does
*not* depend on changes in -next). If you are the one solving
conflicts yourself (which is what I asked in my second email), there
is no problem to begin with; I will simply send v6 to you and we are
done.

When I sent the first email, I assumed that changes in -next were
supposed to be clean -- my mistake, but please document somewhere how
-next works! Specially that you are rerere'ing conflicts and
re-resolving them every day.

Then the discussion shifted to what to do with changes that actually
depend on other changes.

Cheers,
Miguel

Re: [GIT PULL linux-next] Add Compiler Attributes tree

[Steven Rostedt]

On Wed, 3 Oct 2018 14:34:28 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> When I sent the first email, I assumed that changes in -next were
> supposed to be clean -- my mistake, but please document somewhere how
> -next works! Specially that you are rerere'ing conflicts and
> re-resolving them every day.

Yes, a document in the Documentation directory on how to work with
linux-next may be a good idea. Something we can point people to when
they start using linux-next. I know there was something in the initial
email, but I've lost that.

-- Steve

Re: [GIT PULL linux-next] Add Compiler Attributes tree

[Theodore Y. Ts'o]

On Wed, Oct 03, 2018 at 01:54:05PM +0200, Miguel Ojeda wrote:
> 
> Exactly. And for this case, I simply assumed Stephen wanted a clean
> series to apply on top of the latest next-* tag (same way we base
> stuff on top of -rc*s). Note that this is *not* really a "tree"
> collecting changes/development, it is a patch series, i.e. what is
> important is only the range-diff.
> 
> What has surprised me is that -next does not allow for range-diffs
> (patches/commits/...) to be inside -next, maybe applied after all the
> normal merges of trees. I simply assumed it did, given what I could
> find about -next (which does not seem to be documented properly, by
> the way).

Part of the reason why I wrote my long message was because the -next
tree usage has been mostly by convention.  Better documentation would
be a good thing; I think the main reason why we don't have it is that
historically, the people who submit direct pull requests to Linus
already know how things work.  Obviously that's not a great
assumption, but in fact there are a large number of things about what
is needed to be a subsystem maintainer which needs to be documented,
and the linux-next tree is just one part of it.  (And perhaps not even
the biggest part.)

Historically there was one source of changes into the linux-next tree
which was done as quilt-style patch series, and that was Andrew
Morton's mmotm tree.  Part of the problem is that it's a lot more work
to consume a patch series, and so these days while Andrew still uses a
patch series, he exports a git tree[1] which is what I believe Stephen
and Linus use.  (For that matter, I use a patch series[2] as well, but
the public interface is the ext4.git tree on git.kernel.org.)

[1] https://www.ozlabs.org/~akpm/mmotm/mmotm-readme.txt
[2] https://ext4.wiki.kernel.org/index.php/Ext4_patchsets

> While this is a simple conflict, I don't really agree with release
> maintainers having to fix conflicts on the fly (even if it is a single
> trivial line), but that is an orthogonal discussion...

Sure; there are some srtong reasons for it, but we can save that for
the Maintainer's Handbook discussion.  I will say that this is
something where attempts to avoid Linus needing to fix conflicts on
the fly, such as a rebase right before a PULL request, violates
Linus's rules which has in the past resulted in him expressing a
"strong" correction e-mail to Maintainers since it was assumed that
they really should have known better.  Linus has said he doesn't want
to yell at Maintainers, so we can support him by getting the
Maintainer's Handbook out there.  :-)

> Alright, I am not sure how to answer this without sounding
> "aggressive" and maybe I misunderstood something you tried to point
> out, so I apologize in advance. There are several points here:
> 
>   * The merge conflict isn't related to this (but let's assume it was,
> since you pointed this out as an example -- I guess; although I am not
> sure why).

I thought I was clear that I used it as an example, but my apologies
if that didn't come across.

>     - Changes should include everything related to them (as far as
> possible, i.e. extreme examples aside). Adding __nonstring and not
> removing the ext4 part would simply be leaving undone work (which has
> to be done sooner or later). To be honest, it could have even been
> done in the same commit and I would say it is logically OK (even if
> not great).
> ...
>
> You seem to argue that it is better to avoid merge conflicts rather
> than doing a proper series. Well, I think we should really try to
> avoid conflicts pushing down the """quality""" of
> patches/commits/series.

And I think this is where the disagreement lies, which is why I
bothered to send my long missive in the first place.  From my point of
view, and I suspect many maintainers share this, avoiding merge
conflicts is way important than making sure everything is "done" in a
single patch series.  There have been plenty of changes where cleanup
is done latter, and/or where preparatory patches are done in one
release, and the big change happens in the next release, 3 months
later.

That's because some of the alternatives, such as basing your git tree
on an unstable head branch, such as linux-next, or a last minute
rebase which means that the actual patches that which gets the pull
rebase hasn't been tested or soaked in linux-next, are far worse
because it can lead to lower quality git trees getting merged during
the merge window.

There is a tension here between maximizing the "quality" of a patch
series, and maximizing the "quality" of the git trees that feed into
the merge window.  Or perhaps the better way of saying this is
minimizing the risk of bad changes getting integreted during the merge
window.  Some technical debt which gets cleaned up in the next release
is in my view a very low price to pay in order to minimize risk.

Part of this is because I've had to waste time debugging changes made
to the ext4 sources which came in via someone else's git tree that
were "obviously correct" or "trivial changes" --- but they weren't.
Most of the time they get caught as part of linux-next testing, but
not always.  So sometimes the interests of one subsystem maintainer
can end up conflicting with the interests of the patch series author,
or the interests of another subsystem maintainer.  And that's why we
have some of these "cultural understandings", many of which we clearly
need to document.

(And yes, in this case I didn't object to the cleanup being in your
patch series; normally I don't care, unless it actually break things.
I was just trying to make the point from my perspective, nothing
*required* that the change be in your git tree, and if it *was*
causing the problem, I had gone out of my way to make sure to make it
easy for you to drop the change; and from my perspective I was doing
you --- and me :-) --- a favor when I added the outer #ifndef, which I
had done with full consideration, specifically for this reason.  Even
if the definition was different, my definition *had* been fully tested
with over a 27+ VM hours of regression testing, and if it turned out
that they were different, cleaning that up three months from now in
the next release is just *fine* as far as I'm concerned.)

Cheers,

						- Ted

Re: [GIT PULL linux-next] Add Compiler Attributes tree

[Theodore Y. Ts'o]

On Wed, Oct 03, 2018 at 11:41:08PM +0200, Miguel Ojeda wrote:
> 
> I forgot to mention that if the definitions were different, it could
> have caused a problem, because your definition wouldn't apply, so your
> 27+ hours of testing wouldn't have mattered :-P Without the #ifndef,
> we would have at least got a redefinition warning about it.

In this case, yes.  Again, I emphasize that I was using the ext4.h
cleanup as an *example*.  The point I was trying to make was that your
change did *not* do a full set of deep ext4 regression tests because
the your change didn't go through the ext4.git tree.  I have seen
cases where "simple mechanical changes" were anything but, and while
the kernel *compiled* it resulted in the file system not working.
(And in the worst case, it resulted actual data loss or file system
corruption.)

My test scripts are public, and in fact I've gone out of my way to
make them easy for other people to run them, with documentation[1][2],
slide sets[3], etc.  But the vast majority of people who submit
patches to ext4.git don't bother --- and as far as I know *no one* who
sends ext4 changes via other git trees *ever* bothered.  (And, for one
recent patchset where the ext4 developer spent a lot of time using
kvm-xfstests before submission, accepting that set of changes was easy
and was a joy.  I'm a big believe in tests.)

[1] https://github.com/tytso/xfstests-bld/blob/master/Documentation/gce-xfstests.md
[2] https://github.com/tytso/xfstests-bld/blob/master/Documentation/kvm-quickstart.md
[3] https://thunk.org/gce-xfstests

As far as what you want to do, there are solutions, but they require a
radically different way of developing.  For example, inside Google,
for the non-public sources (e.g., excluding Android, Chrome OS, etc.)
there is a single source tree, with thousands of projects.  They use
common set of C++ utility libraries/classes, and there is a
standardized build system (bazel[4]), and a standardized way of
writing tests.  More importantly, there is a fully automated
continuous testing system which will automatically trigger and run the
appropriate tests --- at the moment when the patch is submitted into
the Google's custom code review system --- and the results of the test
are automatically submitted as comments in the code review system, so
the automated testing is integrated into the code review.  For changes
that affected all or substantially all projects, it's possible to run
tests across the entire source tree using automated, centralized
resources, and even then it can take a significantlly non-trivial (as
in days) of calendar time running on many systems in parallel.

[4] https://bazel.build/

So if you are willing to completely standardize your testing
infrastructure and devote utterly lavish amounts of automated testing
which is deeply integrated into a standardized, non-email code review
system, the challenge you have identified *has* been solved.  But
trust me when I say that it is a very non-trivial thing to do, and it
requires a lot of very strict code development practices that are
imposed on all Google C++ developers working in the common tree (which
is probably 90%+ of the SWE's inside Google).  I'm not at all
convinced that it could be adopted or imposed on the kernel
development process.  In fact, I'm quite confident it could not be.

I actually think the Linux Kernel's approach of carefully segregating
how code and headers to avoid merge conflicts (and worse, semantic
conflicts that may git merge and build succesfully, but then blow up
in subtle ways leading to user data loss) is actually a pretty good
way of doing things.  It works 99.99% of the the commits.  True, it
wasn't optimal for the changes you were trying to make; but your
experience is the exception, not the rule.

The risk here is that it's very easy to engineer changes in processes
for corner cases, and which make things much worse (either taking more
time, or more effort, or being less reliable) for the common case.

Cheers,

					- Ted