lkml.org 
[lkml]   [2014]   [Dec]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] VERIFY_OCTAL_PERMISSIONS: Move to <linux/sysfs.h> where it belongs
Hi George,

On 15 Dec 2014 19:14:53 -0500 "George Spelvin" <linux@horizon.com> wrote:
>
> Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > Please do *not* mix changes up like this. Split this out into a
> > separate patch, please (1 logical change per patch).
>
> Um... I thought I was doing that. More particularly, the task of
> untangling header file dependencies eseemed sufficiently cohesive
> that it could be considered one logical change.

Well, given the subject of the commit, I expected a simple change that
just did the move (and any immediately associated include changes).
You then said "Some other extraneous header files pruned while I was at
it" and that part I would expect to be in a separate patch.

> It was one round of thinking and analysis about what declarations the
> affected files depend on.

Which is separate from what VERIFY_OCTAL_PERMISSIONS() requires.

> Although syntactically possible, given the small size of the change (I
> deleted a total of 5 #includes, 2 from moduleparam.h and 3 from sysfs.h),
> it didn't seem worth breaking it up further.
>
> > And testing only
> > on x86_64 is not "sure" when talking about header file pruning (but at
> > least you did the "all" configs).
>
> Well, the first round was reading and understanding the headers; the
> compile was just to make sure.

Understood, it was more a "actually changing architectures was more
likely to show breakage than building lots of stuff". I guess I see
more breakage from pruning includes than most people since I build for
multiple architectures more than most.

> The files I was messing with (moduleparam.h and sysfs.h) don't have a
> lot of architecture-specificness within them. The main danger is that
> they're used in a zillion places and some caller might depend on the
> over-inclusion.

The problem is not the direct includes and direct architecture
depenedencies, but the fact that lost of stuff ends up include asm/
include files at some point in the chain and that affects the set of
files implicitly included. X86 seems to implicitly include more than
some other architectures.

> If my haste made me judge wrong, I apologize. Was I very wrong, or
> just a bit over the line?

Probably just a bit over the line. The advantage of the split would be
that when it hits Andrew's tree and then breaks linux-next (for me), I
can pick on a smaller patch to get rid of/correct.

--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2014-12-16 02:01    [W:0.059 / U:0.544 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site