Re: incoming

Re: incoming

[Kees Cook]

On Fri, Sep 10, 2021 at 10:11:53AM -0700, Kees Cook wrote:
> On Thu, Sep 09, 2021 at 08:09:48PM -0700, Andrew Morton wrote:
> > 
> > More post linux-next material.
> > 
> > 9 patches, based on f154c806676ad7153c6e161f30c53a44855329d6.
> > 
> > Subsystems affected by this patch series:
> > 
> >   mm/slab-generic
> >   rapidio
> >   mm/debug
> > 
> > Subsystem: mm/slab-generic
> > 
> >     "Matthew Wilcox (Oracle)" <willy@infradead.org>:
> >       mm: move kvmalloc-related functions to slab.h
> > 
> > Subsystem: rapidio
> > 
> >     Kees Cook <keescook@chromium.org>:
> >       rapidio: avoid bogus __alloc_size warning
> > 
> > Subsystem: mm/debug
> > 
> >     Kees Cook <keescook@chromium.org>:
> >     Patch series "Add __alloc_size() for better bounds checking", v2:
> >       Compiler Attributes: add __alloc_size() for better bounds checking
> >       checkpatch: add __alloc_size() to known $Attribute
> >       slab: clean up function declarations
> >       slab: add __alloc_size attributes for better bounds checking
> >       mm/page_alloc: add __alloc_size attributes for better bounds checking
> >       percpu: add __alloc_size attributes for better bounds checking
> >       mm/vmalloc: add __alloc_size attributes for better bounds checking
> 
> Hi,
> 
> FYI, in overnight build testing I found yet another corner case in
> GCC's handling of the __alloc_size attribute. It's the gift that keeps
> on giving. The fix is here:
> 
> https://lore.kernel.org/lkml/20210910165851.3296624-1-keescook@chromium.org/

I'm so glad it's Friday. Here's the v2 fix... *sigh*

https://lore.kernel.org/lkml/20210910201132.3809437-1-keescook@chromium.org/

-Kees

> 
> > 
> >  Makefile                                 |   15 +++
> >  drivers/of/kexec.c                       |    1 
> >  drivers/rapidio/devices/rio_mport_cdev.c |    9 +-
> >  include/linux/compiler_attributes.h      |    6 +
> >  include/linux/gfp.h                      |    2 
> >  include/linux/mm.h                       |   34 --------
> >  include/linux/percpu.h                   |    3 
> >  include/linux/slab.h                     |  122 ++++++++++++++++++++++---------
> >  include/linux/vmalloc.h                  |   11 ++
> >  scripts/checkpatch.pl                    |    3 
> >  10 files changed, 132 insertions(+), 74 deletions(-)
> > 
> 
> -- 
> Kees Cook

-- 
Kees Cook

Re: [patch 9/9] mm/vmalloc: add __alloc_size attributes for better bounds checking

[Kees Cook]

On Fri, Sep 10, 2021 at 10:23:48AM -0700, Linus Torvalds wrote:
> On Thu, Sep 9, 2021 at 8:10 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> >
> > +__alloc_size(1)
> >  extern void *vmalloc(unsigned long size);
> [...]
> 
> All of these are added in the wrong place - inconsistent with the very
> compiler documentation the patches add.
> 
> The function attributes are generally added _after_ the function,
> although admittedly we've been quite confused here before.
> 
> But the very compiler documentation you point to in the patch that
> adds these macros gives that as the examples both for gcc and clang:
> 
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alloc_005fsize-function-attribute
> + * clang: https://clang.llvm.org/docs/AttributeReference.html#alloc-size
> 
> and honestly I think that is the preferred format because this is
> about the *function*, not about the return type.
> 
> Do both placements work? Yes.

I'm cleaning this up now, and have discovered that the reason for the
before-function placement is consistency with static inlines. If I do this:

static __always_inline void * kmalloc(size_t size, gfp_t flags) __alloc_size(1)
{
	...
}

GCC is very angry:

./include/linux/slab.h:519:1: error: attributes should be specified before the declarator in a function definition
  519 | static __always_inline void *kmalloc_large(size_t size, gfp_t flags) __alloc_size(1)
      | ^~~~~~

It's happy if I treat it as a "return type attribute" in the ordering,
though:

static __always_inline void * __alloc_size(1) kmalloc(size_t size, gfp_t flags)

I'll do that unless you have a preference for somewhere else...

-Kees

-- 
Kees Cook

Re: [patch 9/9] mm/vmalloc: add __alloc_size attributes for better bounds checking

[Joe Perches]

On Tue, 2021-09-21 at 16:37 -0700, Kees Cook wrote:
> On Fri, Sep 10, 2021 at 10:23:48AM -0700, Linus Torvalds wrote:
> > On Thu, Sep 9, 2021 at 8:10 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> > > 
> > > +__alloc_size(1)
> > >  extern void *vmalloc(unsigned long size);
> > [...]
> > 
> > All of these are added in the wrong place - inconsistent with the very
> > compiler documentation the patches add.
> > 
> > The function attributes are generally added _after_ the function,
> > although admittedly we've been quite confused here before.
> > 
> > But the very compiler documentation you point to in the patch that
> > adds these macros gives that as the examples both for gcc and clang:
> > 
> > + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alloc_005fsize-function-attribute
> > + * clang: https://clang.llvm.org/docs/AttributeReference.html#alloc-size
> > 
> > and honestly I think that is the preferred format because this is
> > about the *function*, not about the return type.
> > 
> > Do both placements work? Yes.
> 
> I'm cleaning this up now, and have discovered that the reason for the
> before-function placement is consistency with static inlines. If I do this:
> 
> static __always_inline void * kmalloc(size_t size, gfp_t flags) __alloc_size(1)
> {
> 	...
> }
> 
> GCC is very angry:
> 
> ./include/linux/slab.h:519:1: error: attributes should be specified before the declarator in a function definition
>   519 | static __always_inline void *kmalloc_large(size_t size, gfp_t flags) __alloc_size(1)
>       | ^~~~~~
> 
> It's happy if I treat it as a "return type attribute" in the ordering,
> though:
> 
> static __always_inline void * __alloc_size(1) kmalloc(size_t size, gfp_t flags)
> 
> I'll do that unless you have a preference for somewhere else...

_please_ put it before the return type on a separate line.

[__attributes]
[static inline const] <return type> function(<args...>)

function prototype element ordering

[Kees Cook]

On Tue, Sep 21, 2021 at 04:45:44PM -0700, Joe Perches wrote:
> On Tue, 2021-09-21 at 16:37 -0700, Kees Cook wrote:
> > On Fri, Sep 10, 2021 at 10:23:48AM -0700, Linus Torvalds wrote:
> > > On Thu, Sep 9, 2021 at 8:10 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> > > > 
> > > > +__alloc_size(1)
> > > >  extern void *vmalloc(unsigned long size);
> > > [...]
> > > 
> > > All of these are added in the wrong place - inconsistent with the very
> > > compiler documentation the patches add.
> > > 
> > > The function attributes are generally added _after_ the function,
> > > although admittedly we've been quite confused here before.
> > > 
> > > But the very compiler documentation you point to in the patch that
> > > adds these macros gives that as the examples both for gcc and clang:
> > > 
> > > + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alloc_005fsize-function-attribute
> > > + * clang: https://clang.llvm.org/docs/AttributeReference.html#alloc-size
> > > 
> > > and honestly I think that is the preferred format because this is
> > > about the *function*, not about the return type.
> > > 
> > > Do both placements work? Yes.
> > 
> > I'm cleaning this up now, and have discovered that the reason for the
> > before-function placement is consistency with static inlines. If I do this:
> > 
> > static __always_inline void * kmalloc(size_t size, gfp_t flags) __alloc_size(1)
> > {
> > 	...
> > }
> > 
> > GCC is very angry:
> > 
> > ./include/linux/slab.h:519:1: error: attributes should be specified before the declarator in a function definition
> >   519 | static __always_inline void *kmalloc_large(size_t size, gfp_t flags) __alloc_size(1)
> >       | ^~~~~~
> > 
> > It's happy if I treat it as a "return type attribute" in the ordering,
> > though:
> > 
> > static __always_inline void * __alloc_size(1) kmalloc(size_t size, gfp_t flags)
> > 
> > I'll do that unless you have a preference for somewhere else...
> 
> _please_ put it before the return type on a separate line.
> 
> [__attributes]
> [static inline const] <return type> function(<args...>)

Somehow Linus wasn't in CC. :P

Linus, what do you want here? I keep getting conflicting (or
uncompilable) advice. I'm also trying to prepare a patch for
Documentation/process/coding-style.rst ...

Looking through what was written before[1] and through examples in the
source tree, I find the following categories:

1- storage class: static extern inline __always_inline
2- storage class attributes/hints/???: __init __cold
3- return type: void *
4- return type attributes: __must_check __noreturn __assume_aligned(n)
5- function attributes: __attribute_const__ __malloc
6- function argument attributes: __printf(n, m) __alloc_size(n)

Everyone seems to basically agree on:

[storage class] [return type] [return type attributes] [name]([arg1type] [arg1name], ...)

There is a lot of disagreement over where 5 and 6 should fit in above. And
there is a lot of confusion over 4 (mixed between before and after the
function name) and 2 (see below).

What's currently blocking me is that 6 cannot go after the function
(for definitions) because it angers GCC (see quoted bit above), but 5
can (e.g. __attribute_const__).

Another inconsistency seems to be 2 (mainly section markings like
__init). Sometimes it's after the storage class and sometimes after the
return type, but it certainly feels more like a storage class than a
return type attribute:

$ git grep 'static __init int' | wc -l
349
$ git grep 'static int __init' | wc -l
8402

But it's clearly positioned like a return type attribute in most of the
tree. What's correct?

Regardless, given the constraints above, it seems like what Linus may
want is (on "one line", though it will get wrapped in pathological cases
like kmem_cache_alloc_node_trace):

[storage class] [storage class attributes] [return type] [return type attributes] [function argument attributes] [name]([arg1type] [arg1name], ...) [function attributes]

Joe appears to want (on two lines):

[storage class attributes] [function attributes] [function argument attributes]
[storage class] [return type] [return type attributes] [name]([arg1type] [arg1name], ...)

I would just like to have an arrangement that won't get NAKed by
someone. ;) And I'm willing to document it. :)

-Kees

[1] https://lore.kernel.org/mm-commits/CAHk-=wiOCLRny5aifWNhr621kYrJwhfURsa0vFPeUEm8mF0ufg@mail.gmail.com/

-- 
Kees Cook

Re: function prototype element ordering

[Joe Perches]

On Tue, 2021-09-21 at 19:25 -0700, Kees Cook wrote:
> On Tue, Sep 21, 2021 at 04:45:44PM -0700, Joe Perches wrote:
> > On Tue, 2021-09-21 at 16:37 -0700, Kees Cook wrote:
> > > On Fri, Sep 10, 2021 at 10:23:48AM -0700, Linus Torvalds wrote:
> > > > On Thu, Sep 9, 2021 at 8:10 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> > > > > 
> > > > > +__alloc_size(1)
> > > > >  extern void *vmalloc(unsigned long size);
> > > > [...]
> > > > 
> > > > All of these are added in the wrong place - inconsistent with the very
> > > > compiler documentation the patches add.
> > > > 
> > > > The function attributes are generally added _after_ the function,
> > > > although admittedly we've been quite confused here before.
> > > > 
> > > > But the very compiler documentation you point to in the patch that
> > > > adds these macros gives that as the examples both for gcc and clang:
> > > > 
> > > > + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alloc_005fsize-function-attribute
> > > > + * clang: https://clang.llvm.org/docs/AttributeReference.html#alloc-size
> > > > 
> > > > and honestly I think that is the preferred format because this is
> > > > about the *function*, not about the return type.
> > > > 
> > > > Do both placements work? Yes.
> > > 
> > > I'm cleaning this up now, and have discovered that the reason for the
> > > before-function placement is consistency with static inlines. If I do this:
> > > 
> > > static __always_inline void * kmalloc(size_t size, gfp_t flags) __alloc_size(1)
> > > {
> > > 	...
> > > }
> > > 
> > > GCC is very angry:
> > > 
> > > ./include/linux/slab.h:519:1: error: attributes should be specified before the declarator in a function definition
> > >   519 | static __always_inline void *kmalloc_large(size_t size, gfp_t flags) __alloc_size(1)
> > >       | ^~~~~~
> > > 
> > > It's happy if I treat it as a "return type attribute" in the ordering,
> > > though:
> > > 
> > > static __always_inline void * __alloc_size(1) kmalloc(size_t size, gfp_t flags)
> > > 
> > > I'll do that unless you have a preference for somewhere else...
> > 
> > _please_ put it before the return type on a separate line.
> > 
> > [__attributes]
> > [static inline const] <return type> function(<args...>)
> 
> Somehow Linus wasn't in CC. :P
> 
> Linus, what do you want here? I keep getting conflicting (or
> uncompilable) advice. I'm also trying to prepare a patch for
> Documentation/process/coding-style.rst ...
> 
> Looking through what was written before[1] and through examples in the
> source tree, I find the following categories:
> 
> 1- storage class: static extern inline __always_inline
> 2- storage class attributes/hints/???: __init __cold
> 3- return type: void *
> 4- return type attributes: __must_check __noreturn __assume_aligned(n)
> 5- function attributes: __attribute_const__ __malloc
> 6- function argument attributes: __printf(n, m) __alloc_size(n)
> 
> Everyone seems to basically agree on:
> 
> [storage class] [return type] [return type attributes] [name]([arg1type] [arg1name], ...)
> 
> There is a lot of disagreement over where 5 and 6 should fit in above. And
> there is a lot of confusion over 4 (mixed between before and after the
> function name) and 2 (see below).
> 
> What's currently blocking me is that 6 cannot go after the function
> (for definitions) because it angers GCC (see quoted bit above), but 5
> can (e.g. __attribute_const__).
> 
> Another inconsistency seems to be 2 (mainly section markings like
> __init). Sometimes it's after the storage class and sometimes after the
> return type, but it certainly feels more like a storage class than a
> return type attribute:
> 
> $ git grep 'static __init int' | wc -l
> 349
> $ git grep 'static int __init' | wc -l
> 8402
> 
> But it's clearly positioned like a return type attribute in most of the
> tree. What's correct?

Neither really.  'Correct' is such a difficult concept.
'Preferred' might be better.

btw: there are about another 100 other uses with '__init' as the
initial attribute, mostly in trace.

And I still think that return type attributes like __init, which is
just a __section define, should go before the function storage class
and ideally on a separate line to simplify the parsing of the actual
function declaration.  Attributes like __section, __aligned, __cold,
etc... don't have much value when looking up a function definition.

> Regardless, given the constraints above, it seems like what Linus may
> want is (on "one line", though it will get wrapped in pathological cases
> like kmem_cache_alloc_node_trace):

Pathological is pretty common these days as the function name length
is rather longer now than earlier times.
 
> [storage class] [storage class attributes] [return type] [return type attributes] [function argument attributes] [name]([arg1type] [arg1name], ...) [function attributes]
> 
> Joe appears to want (on two lines):
> 
> [storage class attributes] [function attributes] [function argument attributes]
> [storage class] [return type] [return type attributes] [name]([arg1type] [arg1name], ...)

I would put [return type attributes] on the initial separate line
even though that's not the most common use today.

> I would just like to have an arrangement that won't get NAKed by
> someone. ;)

Bikeshed building dreamer...

btw:

Scouting through kernel code for frequency of use examples really
should have some age of code checking associated to the use.

Older code was far more freeform than more recently written code.

But IMO the desire here is to ask for a bit more uniformity, not
require it.

Re: function prototype element ordering

[Kees Cook]

On Tue, Sep 21, 2021 at 09:24:04PM -0700, Joe Perches wrote:
> On Tue, 2021-09-21 at 19:25 -0700, Kees Cook wrote:
> > [...]
> > Looking through what was written before[1] and through examples in the
> > source tree, I find the following categories:
> > 
> > 1- storage class: static extern inline __always_inline
> > 2- storage class attributes/hints/???: __init __cold
> > 3- return type: void *
> > 4- return type attributes: __must_check __noreturn __assume_aligned(n)
> > 5- function attributes: __attribute_const__ __malloc
> > 6- function argument attributes: __printf(n, m) __alloc_size(n)
> > 
> > Everyone seems to basically agree on:
> > 
> > [storage class] [return type] [return type attributes] [name]([arg1type] [arg1name], ...)
> > 
> > There is a lot of disagreement over where 5 and 6 should fit in above. And
> > there is a lot of confusion over 4 (mixed between before and after the
> > function name) and 2 (see below).
> > 
> > What's currently blocking me is that 6 cannot go after the function
> > (for definitions) because it angers GCC (see quoted bit above), but 5
> > can (e.g. __attribute_const__).
> > 
> > Another inconsistency seems to be 2 (mainly section markings like
> > __init). Sometimes it's after the storage class and sometimes after the
> > return type, but it certainly feels more like a storage class than a
> > return type attribute:
> > 
> > $ git grep 'static __init int' | wc -l
> > 349
> > $ git grep 'static int __init' | wc -l
> > 8402
> > 
> > But it's clearly positioned like a return type attribute in most of the
> > tree. What's correct?
> 
> Neither really.  'Correct' is such a difficult concept.
> 'Preferred' might be better.

Right -- I expect it to be a guideline.

> btw: there are about another 100 other uses with '__init' as the
> initial attribute, mostly in trace.

Hah, yeah.

> And I still think that return type attributes like __init, which is
> just a __section define, should go before the function storage class
> and ideally on a separate line to simplify the parsing of the actual
> function declaration.  Attributes like __section, __aligned, __cold,
> etc... don't have much value when looking up a function definition.
> 
> > Regardless, given the constraints above, it seems like what Linus may
> > want is (on "one line", though it will get wrapped in pathological cases
> > like kmem_cache_alloc_node_trace):
> 
> Pathological is pretty common these days as the function name length
> is rather longer now than earlier times.

Agreed!

> > [storage class] [storage class attributes] [return type] [return type attributes] [function argument attributes] [name]([arg1type] [arg1name], ...) [function attributes]
> > 
> > Joe appears to want (on two lines):
> > 
> > [storage class attributes] [function attributes] [function argument attributes]
> > [storage class] [return type] [return type attributes] [name]([arg1type] [arg1name], ...)
> 
> I would put [return type attributes] on the initial separate line
> even though that's not the most common use today.

I found a few other people wanting separate lines too, so at the risk of
annoying Linus, I guess I'll attempt this (again).

> > I would just like to have an arrangement that won't get NAKed by
> > someone. ;)
> 
> Bikeshed building dreamer...

I just want to know the right place to put stuff. :P

> But IMO the desire here is to ask for a bit more uniformity, not
> require it.

Yeah.

-- 
Kees Cook

Re: function prototype element ordering

[Alexey Dobriyan]

On Tue, Sep 21, 2021 at 07:25:53PM -0700, Kees Cook wrote:
> On Tue, Sep 21, 2021 at 04:45:44PM -0700, Joe Perches wrote:
> > On Tue, 2021-09-21 at 16:37 -0700, Kees Cook wrote:
> > > On Fri, Sep 10, 2021 at 10:23:48AM -0700, Linus Torvalds wrote:
> > > > On Thu, Sep 9, 2021 at 8:10 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> > > > > 
> > > > > +__alloc_size(1)
> > > > >  extern void *vmalloc(unsigned long size);
> > > > [...]
> > > > 
> > > > All of these are added in the wrong place - inconsistent with the very
> > > > compiler documentation the patches add.
> > > > 
> > > > The function attributes are generally added _after_ the function,
> > > > although admittedly we've been quite confused here before.
> > > > 
> > > > But the very compiler documentation you point to in the patch that
> > > > adds these macros gives that as the examples both for gcc and clang:
> > > > 
> > > > + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alloc_005fsize-function-attribute
> > > > + * clang: https://clang.llvm.org/docs/AttributeReference.html#alloc-size
> > > > 
> > > > and honestly I think that is the preferred format because this is
> > > > about the *function*, not about the return type.
> > > > 
> > > > Do both placements work? Yes.
> > > 
> > > I'm cleaning this up now, and have discovered that the reason for the
> > > before-function placement is consistency with static inlines. If I do this:
> > > 
> > > static __always_inline void * kmalloc(size_t size, gfp_t flags) __alloc_size(1)
> > > {
> > > 	...
> > > }
> > > 
> > > GCC is very angry:
> > > 
> > > ./include/linux/slab.h:519:1: error: attributes should be specified before the declarator in a function definition
> > >   519 | static __always_inline void *kmalloc_large(size_t size, gfp_t flags) __alloc_size(1)
> > >       | ^~~~~~
> > > 
> > > It's happy if I treat it as a "return type attribute" in the ordering,
> > > though:
> > > 
> > > static __always_inline void * __alloc_size(1) kmalloc(size_t size, gfp_t flags)
> > > 
> > > I'll do that unless you have a preference for somewhere else...
> > 
> > _please_ put it before the return type on a separate line.
> > 
> > [__attributes]
> > [static inline const] <return type> function(<args...>)
> 
> Somehow Linus wasn't in CC. :P
> 
> Linus, what do you want here? I keep getting conflicting (or
> uncompilable) advice. I'm also trying to prepare a patch for
> Documentation/process/coding-style.rst ...
> 
> Looking through what was written before[1] and through examples in the
> source tree, I find the following categories:
> 
> 1- storage class: static extern inline __always_inline
> 2- storage class attributes/hints/???: __init __cold
> 3- return type: void *
> 4- return type attributes: __must_check __noreturn __assume_aligned(n)
> 5- function attributes: __attribute_const__ __malloc
> 6- function argument attributes: __printf(n, m) __alloc_size(n)
> 
> Everyone seems to basically agree on:
> 
> [storage class] [return type] [return type attributes] [name]([arg1type] [arg1name], ...)
> 
> There is a lot of disagreement over where 5 and 6 should fit in above. And
> there is a lot of confusion over 4 (mixed between before and after the
> function name) and 2 (see below).
> 
> What's currently blocking me is that 6 cannot go after the function
> (for definitions) because it angers GCC (see quoted bit above), but 5
> can (e.g. __attribute_const__).
> 
> Another inconsistency seems to be 2 (mainly section markings like
> __init). Sometimes it's after the storage class and sometimes after the
> return type, but it certainly feels more like a storage class than a
> return type attribute:
> 
> $ git grep 'static __init int' | wc -l
> 349
> $ git grep 'static int __init' | wc -l
> 8402
> 
> But it's clearly positioned like a return type attribute in most of the
> tree. What's correct?
> 
> Regardless, given the constraints above, it seems like what Linus may
> want is (on "one line", though it will get wrapped in pathological cases
> like kmem_cache_alloc_node_trace):
> 
> [storage class] [storage class attributes] [return type] [return type attributes] [function argument attributes] [name]([arg1type] [arg1name], ...) [function attributes]
> 
> Joe appears to want (on two lines):
> 
> [storage class attributes] [function attributes] [function argument attributes]
> [storage class] [return type] [return type attributes] [name]([arg1type] [arg1name], ...)
> 
> I would just like to have an arrangement that won't get NAKed by
> someone. ;) And I'm willing to document it. :)

Attributes should be on their own line, they can be quite lengthy.

	__attribute__((...))
	[static] [inline] T f(A1 arg1, ...)
	{
		...
	}

There will be even more attributes in the future, both added by
compilers and developers (const, pure, WUR), so let's make "prototype lane"
for them.

Same for structures:

	__attribute__((packed))
	struct S {
	};

Kernel practice of hiding attributes under defines (__ro_after_init)
breaks ctags which parses the last identifier before semicolon as object
name. Naturally, it is ctags bug, but placing attributes before
declaration will autmatically unbreak such cases.

Re: function prototype element ordering

[Joe Perches]

On Wed, 2021-09-22 at 10:24 +0300, Alexey Dobriyan wrote:

> Attributes should be on their own line, they can be quite lengthy.
> 
> 	__attribute__((...))
> 	[static] [inline] T f(A1 arg1, ...)
> 	{
> 		...
> 	}
> 
> There will be even more attributes in the future, both added by
> compilers and developers (const, pure, WUR), so let's make "prototype lane"
> for them.
> 
> Same for structures:
> 
> 	__attribute__((packed))
> 	struct S {
> 	};

Do you know if placing attributes like __packed/__aligned() before
definitions would work for all cases for structs/substructs/unions?

Re: function prototype element ordering

[Alexey Dobriyan]

On Wed, Sep 22, 2021 at 01:51:34AM -0700, Joe Perches wrote:
> On Wed, 2021-09-22 at 10:24 +0300, Alexey Dobriyan wrote:
> 
> > Attributes should be on their own line, they can be quite lengthy.
> > 
> > 	__attribute__((...))
> > 	[static] [inline] T f(A1 arg1, ...)
> > 	{
> > 		...
> > 	}
> > 
> > There will be even more attributes in the future, both added by
> > compilers and developers (const, pure, WUR), so let's make "prototype lane"
> > for them.
> > 
> > Same for structures:
> > 
> > 	__attribute__((packed))
> > 	struct S {
> > 	};
> 
> Do you know if placing attributes like __packed/__aligned() before
> definitions would work for all cases for structs/substructs/unions?

Somehow, it doesn't.

But it works for members:

	struct S {
       		__attribute__((aligned(16)))
	        int a;
	};

Re: function prototype element ordering

[Jani Nikula]

On Wed, 22 Sep 2021, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> On Tue, Sep 21, 2021 at 07:25:53PM -0700, Kees Cook wrote:
>> On Tue, Sep 21, 2021 at 04:45:44PM -0700, Joe Perches wrote:
>> > On Tue, 2021-09-21 at 16:37 -0700, Kees Cook wrote:
>> > > On Fri, Sep 10, 2021 at 10:23:48AM -0700, Linus Torvalds wrote:
>> > > > On Thu, Sep 9, 2021 at 8:10 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>> > > > > 
>> > > > > +__alloc_size(1)
>> > > > >  extern void *vmalloc(unsigned long size);
>> > > > [...]
>> > > > 
>> > > > All of these are added in the wrong place - inconsistent with the very
>> > > > compiler documentation the patches add.
>> > > > 
>> > > > The function attributes are generally added _after_ the function,
>> > > > although admittedly we've been quite confused here before.
>> > > > 
>> > > > But the very compiler documentation you point to in the patch that
>> > > > adds these macros gives that as the examples both for gcc and clang:
>> > > > 
>> > > > + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alloc_005fsize-function-attribute
>> > > > + * clang: https://clang.llvm.org/docs/AttributeReference.html#alloc-size
>> > > > 
>> > > > and honestly I think that is the preferred format because this is
>> > > > about the *function*, not about the return type.
>> > > > 
>> > > > Do both placements work? Yes.
>> > > 
>> > > I'm cleaning this up now, and have discovered that the reason for the
>> > > before-function placement is consistency with static inlines. If I do this:
>> > > 
>> > > static __always_inline void * kmalloc(size_t size, gfp_t flags) __alloc_size(1)
>> > > {
>> > > 	...
>> > > }
>> > > 
>> > > GCC is very angry:
>> > > 
>> > > ./include/linux/slab.h:519:1: error: attributes should be specified before the declarator in a function definition
>> > >   519 | static __always_inline void *kmalloc_large(size_t size, gfp_t flags) __alloc_size(1)
>> > >       | ^~~~~~
>> > > 
>> > > It's happy if I treat it as a "return type attribute" in the ordering,
>> > > though:
>> > > 
>> > > static __always_inline void * __alloc_size(1) kmalloc(size_t size, gfp_t flags)
>> > > 
>> > > I'll do that unless you have a preference for somewhere else...
>> > 
>> > _please_ put it before the return type on a separate line.
>> > 
>> > [__attributes]
>> > [static inline const] <return type> function(<args...>)
>> 
>> Somehow Linus wasn't in CC. :P
>> 
>> Linus, what do you want here? I keep getting conflicting (or
>> uncompilable) advice. I'm also trying to prepare a patch for
>> Documentation/process/coding-style.rst ...
>> 
>> Looking through what was written before[1] and through examples in the
>> source tree, I find the following categories:
>> 
>> 1- storage class: static extern inline __always_inline
>> 2- storage class attributes/hints/???: __init __cold
>> 3- return type: void *
>> 4- return type attributes: __must_check __noreturn __assume_aligned(n)
>> 5- function attributes: __attribute_const__ __malloc
>> 6- function argument attributes: __printf(n, m) __alloc_size(n)
>> 
>> Everyone seems to basically agree on:
>> 
>> [storage class] [return type] [return type attributes] [name]([arg1type] [arg1name], ...)
>> 
>> There is a lot of disagreement over where 5 and 6 should fit in above. And
>> there is a lot of confusion over 4 (mixed between before and after the
>> function name) and 2 (see below).
>> 
>> What's currently blocking me is that 6 cannot go after the function
>> (for definitions) because it angers GCC (see quoted bit above), but 5
>> can (e.g. __attribute_const__).
>> 
>> Another inconsistency seems to be 2 (mainly section markings like
>> __init). Sometimes it's after the storage class and sometimes after the
>> return type, but it certainly feels more like a storage class than a
>> return type attribute:
>> 
>> $ git grep 'static __init int' | wc -l
>> 349
>> $ git grep 'static int __init' | wc -l
>> 8402
>> 
>> But it's clearly positioned like a return type attribute in most of the
>> tree. What's correct?
>> 
>> Regardless, given the constraints above, it seems like what Linus may
>> want is (on "one line", though it will get wrapped in pathological cases
>> like kmem_cache_alloc_node_trace):
>> 
>> [storage class] [storage class attributes] [return type] [return type attributes] [function argument attributes] [name]([arg1type] [arg1name], ...) [function attributes]
>> 
>> Joe appears to want (on two lines):
>> 
>> [storage class attributes] [function attributes] [function argument attributes]
>> [storage class] [return type] [return type attributes] [name]([arg1type] [arg1name], ...)
>> 
>> I would just like to have an arrangement that won't get NAKed by
>> someone. ;) And I'm willing to document it. :)
>
> Attributes should be on their own line, they can be quite lengthy.
>
> 	__attribute__((...))
> 	[static] [inline] T f(A1 arg1, ...)
> 	{
> 		...
> 	}
>
> There will be even more attributes in the future, both added by
> compilers and developers (const, pure, WUR), so let's make "prototype lane"
> for them.
>
> Same for structures:
>
> 	__attribute__((packed))
> 	struct S {
> 	};
>
> Kernel practice of hiding attributes under defines (__ro_after_init)
> breaks ctags which parses the last identifier before semicolon as object
> name. Naturally, it is ctags bug, but placing attributes before
> declaration will autmatically unbreak such cases.

git grep seems to suggest __packed is preferred over
__attribute__((packed)), and at the end of the struct declaration
instead of at front:

	struct S {
		/* ... */
        } __packed;

And GNU Global handles this just fine. ;)


BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: function prototype element ordering

[Linus Torvalds]

On Wed, Sep 22, 2021 at 12:24 AM Alexey Dobriyan <adobriyan@gmail.com> wrote:
>
> Attributes should be on their own line, they can be quite lengthy.

No, no no. They really shouldn't.

First off, no normal code should use that "__attribute__(())" syntax
anyway. It's ugly and big, and many of the attributes are
compiler-specific anyway.

So the "quite lengthy" argument is bogus, because the actual names you
should use are things like "__packed" or "__pure" or "__user" etc.

But the "on their own line" is complete garbage to begin with. That
will NEVER be a kernel rule. We should never have a rule that assumes
things are so long that they need to be on multiple lines.

We don't put function return types on their own lines either, even if
some other projects have that rule (just to get function names at the
beginning of lines or some other odd reason).

So no, attributes do not go on their own lines, and they also
generally don't go before the thing they describe.  Your examples are
wrong, and explicitly against kernel rules.

           Linus

Re: function prototype element ordering

[Joe Perches]

On Wed, 2021-09-22 at 14:15 -0700, Linus Torvalds wrote:
> On Wed, Sep 22, 2021 at 12:24 AM Alexey Dobriyan <adobriyan@gmail.com> wrote:
> > 
> > Attributes should be on their own line, they can be quite lengthy.
> 
> No, no no. They really shouldn't.
> 
> First off, no normal code should use that "__attribute__(())" syntax
> anyway. It's ugly and big, and many of the attributes are
> compiler-specific anyway.
> 
> So the "quite lengthy" argument is bogus, because the actual names you
> should use are things like "__packed" or "__pure" or "__user" etc.
> 
> But the "on their own line" is complete garbage to begin with. That
> will NEVER be a kernel rule. We should never have a rule that assumes
> things are so long that they need to be on multiple lines.

I think it's not so much that lines are long, it's more that the
information provided by these markings aren't particularly useful to
a caller/user of a function.

Under what circumstance is a marking like __pure/__cold or __section
useful to someone that just wants to call a particular function?

A secondary reason why these should be separate or at least put
at the begining of a function declaration is compatibility with
existing tools like ctags.

RE: function prototype element ordering

[David Laight]

From: Linus Torvalds
> Sent: 22 September 2021 22:16
...
> We don't put function return types on their own lines either, even if
> some other projects have that rule (just to get function names at the
> beginning of lines or some other odd reason).

If the function name starts at the beginning of a line it is
much easier to grep for the definition.
Trying to find function definitions in the Linux kernel tree
is a PITA - unless they are exported when 'EXPORT.*(function_name)'
will tend to work.

Trying to compile:
static int x(int y) __attribute__((section("x"))) { return y;}
with gcc generates "error: attributes are not allowed on a function-definition".

Putting the attribute anywhere before the function name works fine.
gcc probably accepts:
__inline static __inline int __inline x(void) {return 0;} 

So any of those locations is plausible.
But after the arguments isn't allowed.
So an (extern) function declaration probably should not put them
there - if only for consistency.

I think I'd go for 'first' - optionally on their own line.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: function prototype element ordering

[Linus Torvalds]

On Sat, Sep 25, 2021 at 12:40 PM David Laight <David.Laight@aculab.com> wrote:
>
> If the function name starts at the beginning of a line it is
> much easier to grep for the definition.

That has always been a completely bogus argument. I grep to look up
the type as often as I grep for the function definition, plus it's not
at all unlikely that the "function" is actually a macro wrapper, so
grepping for the beginning of line is just completely wrong.

It's completely wrong for another reason too: it assumes a style of
programming that has never actually been all that common. It's a very
specific pattern to very specific projects, and anybody who learnt
that pattern for their project is going to be completely lost anywhere
else. So don't do it. It's just a bad idea.

So a broken "easier to grep for" is not an excuse for "make the code
harder to read" particularly when it just makes another type of
grepping harder, and it's not actually nearly universal enough to
actually be a useful pattern in the first place.

It's not only never been the pattern in the kernel, but it's generally
not been the pattern anywhere else either. It's literally one of the
broken GNU coding standards - and the fact that almost every other
part of the GNU coding standards were wrong (indentation, placement of
braces, you name it) should give you a hint about how good _that_ one
was.

Here's an exercise for you: go search for C coding examples on the
web, and see how many of them do

    int main(int argc, char **argv)

vs how many of them do

    int
    main(int argc, char **argv)

and then realize that in order for the "grep for ^main" pattern to be
useful, the second version has to not just be more common, it has to
be practically *universal*.

Hint: it isn't even remotely more common, much less universal. In
Debian code search, I had to go to the third page to find any example
at all of people putting the "int" and the "main" on different lines,
and even that one didn't place the "main()" at the beginning of the
line - they had been separated because of other reasons and looked
like this:

int
#ifdef _WIN32
    __cdecl
#endif // _WIN32
    main(int argc, char** argv)

instead.

Maybe Dbian code search isn't the place to go, but I think it proves
my case: the "function name at beginning of line" story is pure
make-believe, and has absolutely no relevance in the real world.

It's a bad straightjacket. Just get over it, and stop perpetuating the
idiotic myth.

If you care so much about grepping for function declarations, and you
use that old-fashioned GNU coding standard policy as an argument, just
be *properly* old-fashioned instead, and use etags or something.

Don't make the rest of us suffer.

Because I grep for functions all the time, and I'd rather have useful
output - which very much includes the type of the function. That's
often one reason _why_ I grep for things in the first place.

Other grep tricks for when the function really is used everywhere, and
you are having trouble finding the definition vs the use:

 - grep in the headers for the type, and actually use the type (either
of the function, or the first argument) as part of the pattern.

   If you really have no idea where it might be, you'll want to start
off with the header grep anyway, to find the macro case (or the inline
case)

   Yeah, splitting the declaration will screw the type information up.
So don't do that, then.

 - if it's so widely used that you find it all over, it's probably
exported. grep for 'EXPORT.*fnname' to see where it is defined.

   We used to (brokenly) export things separately from the definition.
If you find cases of that, let's fix them.

Of course, usually I know roughly where it is defined, so I just limit
the pathnames for 'git grep'.

But the 'add the type of the return value or first argument to the
pattern' is often my second go-to (particularly for the cases where
you might be looking for _multiple_ definitions because it's some
architecture-specific thing, or you have some partial pattern because
every filesystem does their own thing).

Other 'git grep' patterns that often work for kernel sources:

 - looking for a structure declaration? Use

      git grep 'struct name {'

   which mostly works, but obviously depends on coding style so it's
not guaranteed. Good first thing to try, though.

 - use

        git grep '\t\.name\>.*='

   to find things like particular inode operations.

That second case is because we have almost universally converted our
filesystem operation initializers to use that named format (and really
strive to have a policy of constant structures of function pointers
only), and it's really convenient if you are doing VFS changes and
need to find all the places that use a particular VFS interface (eg
".get_acl" or similar).

It used to be a nightmare to find those things back when most of our
initializers were using the traditional unnamed ordered structure
initializers, so this is one area where we've introduced coding style
policies to make it really easy to grep for things (but also much
easier to add new fields and not have to add pointless NULL
initializer elements, of course).

             Linus

RE: function prototype element ordering

[David Laight]

From: Linus Torvalds
> Sent: 26 September 2021 22:04
> 
> On Sat, Sep 25, 2021 at 12:40 PM David Laight <David.Laight@aculab.com> wrote:
> >
> > If the function name starts at the beginning of a line it is
> > much easier to grep for the definition.
> 
> That has always been a completely bogus argument. I grep to look up
> the type as often as I grep for the function definition, plus it's not
> at all unlikely that the "function" is actually a macro wrapper, so
> grepping for the beginning of line is just completely wrong.
> 
> It's completely wrong for another reason too: it assumes a style of
> programming that has never actually been all that common. It's a very
> specific pattern to very specific projects, and anybody who learnt
> that pattern for their project is going to be completely lost anywhere
> else. So don't do it. It's just a bad idea.
> 
> So a broken "easier to grep for" is not an excuse for "make the code
> harder to read" particularly when it just makes another type of
> grepping harder, and it's not actually nearly universal enough to
> actually be a useful pattern in the first place.
> 
> It's not only never been the pattern in the kernel, but it's generally
> not been the pattern anywhere else either. It's literally one of the
> broken GNU coding standards - and the fact that almost every other
> part of the GNU coding standards were wrong (indentation, placement of
> braces, you name it) should give you a hint about how good _that_ one
> was.
> 
> Here's an exercise for you: go search for C coding examples on the
> web, and see how many of them do
> 
>     int main(int argc, char **argv)
> 
> vs how many of them do
> 
>     int
>     main(int argc, char **argv)

It makes a bigger difference with:

struct frobulate *find_frobulate(args)
which is going to need a line break somewhere.
Especially with the (strange) rule about aligning the continued
arguments with the (.

But I didn't expect such a long response :-)

I'm sure the netBSD tree (mostly) puts the function name in column 1.
But after that uses the K&R location for {} (as does Linux).

It true that a lot of 'coding standards' are horrid.
Putting '} else {' on one line is important when reading code.
Especially if the '}' would be at the bottom of the screen,
or worse still turning the page on a fan-fold paper listing to find
a floating 'else' = with no idea which 'if' it goes with.

The modern example of why { and } shouldn't be on their own lines is:
		...
	}
	while (...........................
	{
		...
Is that a loop bottom followed by a code block or
a conditional followed by a loop?

But none of this is related to the location of attributes unless
you need to split long lines and put the attribute before the
function name where you may need.

static struct frobulate *
__inline ....
find_frobulate(....)

Especially if you need #if around the attributes.

	David


	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: function prototype element ordering

[Willy Tarreau]

On Mon, Sep 27, 2021 at 08:21:24AM +0000, David Laight wrote:
> Putting '} else {' on one line is important when reading code.

I used not to like that due to "else if ()" being less readable and less
easy to spot, but the arguments you gave regarding the end of screen are
valid and are similar to my hate of GNU's broken "while ()" on its own
line especially after a "do { }" block where it immediately looks like
an accidental infinite loop.

However:

> But none of this is related to the location of attributes unless
> you need to split long lines and put the attribute before the
> function name where you may need.
> 
> static struct frobulate *
> __inline ....
> find_frobulate(....)

This is exactly the case where I hate to dig into code looking like
that: you build, it fails to find symbol "find_frobulate()", you run
"git grep -w find_frobulate" to figure what file provides it, or even
"grep ^find_frobulate" if you want. And you find it in frobulate.c. You
double-check, you find that frobulate.o was built and linked into your
executable. Despite this it fails to find the symbol. Finally you open
the file to discover this painful "static" two lines above, which made
you waste 3 minutes of your time digging at the wrong place.

*Just* for this reason I'm much more careful to always put the type and
name on the same line nowadays.

> Especially if you need #if around the attributes.

This is the only exception I still have to the rule above. But #if by
definition require multi-line processing anyway and they're not welcome
in the middle of control flows.

Willy

Re: incoming

[Vlastimil Babka]

On 7/19/19 12:56 AM, Andrew Morton wrote:
> 
> The rest of MM and a kernel-wide procfs cleanup.
> 
> 
> 
> Summary of the more significant patches:

Thanks for that!

Perhaps now it would be nice if this went also to linux-mm and lkml, as
mm-commits is sort of hidden.

Vlastimil

Re: incoming

[Vlastimil Babka]

On 7/17/19 1:25 AM, Andrew Morton wrote:
> 
> Most of the rest of MM and just about all of the rest of everything
> else.

Hi,

as I've mentioned at LSF/MM [1], I think it would be nice if mm pull
requests had summaries similar to other subsystems. I see they are now
more structured (thanks!), but they are now probably hitting the limit
of what scripting can do to produce a high-level summary for human
readers (unless patch authors themselves provide a blurb that can be
extracted later?).

So I've tried now to provide an example what I had in mind, below. Maybe
it's too concise - if there were "larger" features in this pull request,
they would probably benefit from more details. I'm CCing the known (to
me) consumers of these mails to judge :) Note I've only covered mm, and
core stuff that I think will be interesting to wide audience (change in
LIST_POISON2 value? I'm sure as hell glad to know about that one :)

Feel free to include this in the merge commit, if you find it useful.

Thanks,
Vlastimil

[1] https://lwn.net/Articles/787705/

-----

- z3fold fixes and enhancements by Henry Burns and Vitaly Wool
- more accurate reclaimed slab caches calculations by Yafang Shao
- fix MAP_UNINITIALIZED UAPI symbol to not depend on config, by
Christoph Hellwig
- !CONFIG_MMU fixes by Christoph Hellwig
- new novmcoredd parameter to omit device dumps from vmcore, by Kairui Song
- new test_meminit module for testing heap and pagealloc initialization,
by Alexander Potapenko
- ioremap improvements for huge mappings, by Anshuman Khandual
- generalize kprobe page fault handling, by Anshuman Khandual
- device-dax hotplug fixes and improvements, by Pavel Tatashin
- enable synchronous DAX fault on powerpc, by Aneesh Kumar K.V
- add pte_devmap() support for arm64, by Robin Murphy
- unify locked_vm accounting with a helper, by Daniel Jordan
- several misc fixes

core/lib
- new typeof_member() macro including some users, by Alexey Dobriyan
- make BIT() and GENMASK() available in asm, by Masahiro Yamada
- changed LIST_POISON2 on x86_64 to 0xdead000000000122 for better code
generation, by Alexey Dobriyan
- rbtree code size optimizations, by Michel Lespinasse
- convert struct pid count to refcount_t, by Joel Fernandes

get_maintainer.pl
- add --no-moderated switch to skip moderated ML's, by Joe Perches

Re: incoming

[Bhaskar Chowdhury]

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



Cool !! 

On 10:47 Wed 17 Jul , Vlastimil Babka wrote:
>On 7/17/19 1:25 AM, Andrew Morton wrote:
>>
>> Most of the rest of MM and just about all of the rest of everything
>> else.
>
>Hi,
>
>as I've mentioned at LSF/MM [1], I think it would be nice if mm pull
>requests had summaries similar to other subsystems. I see they are now
>more structured (thanks!), but they are now probably hitting the limit
>of what scripting can do to produce a high-level summary for human
>readers (unless patch authors themselves provide a blurb that can be
>extracted later?).
>
>So I've tried now to provide an example what I had in mind, below. Maybe
>it's too concise - if there were "larger" features in this pull request,
>they would probably benefit from more details. I'm CCing the known (to
>me) consumers of these mails to judge :) Note I've only covered mm, and
>core stuff that I think will be interesting to wide audience (change in
>LIST_POISON2 value? I'm sure as hell glad to know about that one :)
>
>Feel free to include this in the merge commit, if you find it useful.
>
>Thanks,
>Vlastimil
>
>[1] https://lwn.net/Articles/787705/
>
>-----
>
>- z3fold fixes and enhancements by Henry Burns and Vitaly Wool
>- more accurate reclaimed slab caches calculations by Yafang Shao
>- fix MAP_UNINITIALIZED UAPI symbol to not depend on config, by
>Christoph Hellwig
>- !CONFIG_MMU fixes by Christoph Hellwig
>- new novmcoredd parameter to omit device dumps from vmcore, by Kairui Song
>- new test_meminit module for testing heap and pagealloc initialization,
>by Alexander Potapenko
>- ioremap improvements for huge mappings, by Anshuman Khandual
>- generalize kprobe page fault handling, by Anshuman Khandual
>- device-dax hotplug fixes and improvements, by Pavel Tatashin
>- enable synchronous DAX fault on powerpc, by Aneesh Kumar K.V
>- add pte_devmap() support for arm64, by Robin Murphy
>- unify locked_vm accounting with a helper, by Daniel Jordan
>- several misc fixes
>
>core/lib
>- new typeof_member() macro including some users, by Alexey Dobriyan
>- make BIT() and GENMASK() available in asm, by Masahiro Yamada
>- changed LIST_POISON2 on x86_64 to 0xdead000000000122 for better code
>generation, by Alexey Dobriyan
>- rbtree code size optimizations, by Michel Lespinasse
>- convert struct pid count to refcount_t, by Joel Fernandes
>
>get_maintainer.pl
>- add --no-moderated switch to skip moderated ML's, by Joe Perches
>
>

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

Re: incoming

[Linus Torvalds]

On Wed, Jul 17, 2019 at 1:47 AM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> So I've tried now to provide an example what I had in mind, below.

I'll take it as a trial. I added one-line notes about coda and the
PTRACE_GET_SYSCALL_INFO interface too.

I do hope that eventually I'll just get pull requests, and they'll
have more of a "theme" than this all (*)

           Linus

(*) Although in many ways, the theme for Andrew is "falls through the
cracks otherwise" so I'm not really complaining. This has been working
for years and years.

Re: incoming

[Christian Brauner]

On Wed, Jul 17, 2019 at 09:13:26AM -0700, Linus Torvalds wrote:
> On Wed, Jul 17, 2019 at 1:47 AM Vlastimil Babka <vbabka@suse.cz> wrote:
> >
> > So I've tried now to provide an example what I had in mind, below.
> 
> I'll take it as a trial. I added one-line notes about coda and the
> PTRACE_GET_SYSCALL_INFO interface too.
> 
> I do hope that eventually I'll just get pull requests, and they'll
> have more of a "theme" than this all (*)
> 
>            Linus
> 
> (*) Although in many ways, the theme for Andrew is "falls through the
> cracks otherwise" so I'm not really complaining. This has been working

I put all pid{fd}/clone{3} which is mostly related to pid.c, exit.c,
fork.c into my tree and try to give it a consistent theme for the prs I
sent. And that at least from my perspective that worked and was pretty
easy to coordinate with Andrew. That should hopefully make it a little
easier to theme the -mm tree overall going forward.

Re: incoming

[Vlastimil Babka]

On 7/17/19 6:13 PM, Linus Torvalds wrote:
> On Wed, Jul 17, 2019 at 1:47 AM Vlastimil Babka <vbabka@suse.cz> wrote:
>>
>> So I've tried now to provide an example what I had in mind, below.
> 
> I'll take it as a trial. I added one-line notes about coda and the
> PTRACE_GET_SYSCALL_INFO interface too.

Thanks.

> I do hope that eventually I'll just get pull requests,

Very much agree, that was also discussed at length in the LSF/MM mm
process session I've linked.

> and they'll
> have more of a "theme" than this all (*)

I'll check if the first patch bomb would be more amenable to that, as I
plan to fill in the mm part for 5.3 on LinuxChanges wiki, but for a
merge commit it's too late.

>            Linus
> 
> (*) Although in many ways, the theme for Andrew is "falls through the
> cracks otherwise" so I'm not really complaining. This has been working
> for years and years.

Nevermind the misc stuff that much, but I think mm itself is more
important and deserves what other subsystems have.

Re: incoming

[Linus Torvalds]

On Wed, Sep 9, 2015 at 3:34 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> Subject: lib/: add parse_integer() (replacement for simple_strto*())
> Subject: parse_integer: add runtime testsuite
> Subject: parse-integer: rewrite kstrto*()
> Subject: parse_integer: add checkpatch.pl notice
> Subject: parse_integer: convert scanf()
> Subject: scanf: fix type range overflow
> Subject: parse_integer: convert lib/
> Subject: parse_integer: convert mm/
> Subject: parse_integer: convert fs/
> Subject: parse_integer: convert fs/cachefiles/
> Subject: parse_integer: convert ext2, ext4
> Subject: parse_integer: convert fs/ocfs2/
> Subject: parse_integer: convert fs/9p/
> Subject: parse_integer: convert fs/exofs/

No.

I'm not taking yet another broken "deprecate old interface, replace it
with new-and-improved one, and screw things up in the process".

The whole "kstrto*()" thing was a mistake. We had real bugs brought in
by the conversion to the "better" interface. The "even betterer" new
parse_integer() interface actually looks lik ea real improvement, and
talks about some of the brokenness of the old code, and I was really
wanting to like it, but then I saw the conversions.

The VERY FIRST conversion patch I looked at was buggy. That makes me
angry. The whole *AND*ONLY* point of this whole thing was to get rid
of bugs, and be a obviously safe interface, and then the first
conversion patch proves it wrong.

Let me show you:

        if (isdigit(*str)) {
-               io_tlb_nslabs = simple_strtoul(str, &str, 0);
+               str += parse_integer(str, 0, &io_tlb_nslabs);

and obviously nobody spent even a *second* asking themselves "what if
parse_integer returns an error".

The old code didn't fail catastrophically in the error case. The new one does.

And yes, parse_integer() really can return an error, even despite that
"isdigit(*str)" check. Think about it. Or just read the source code.

I really am very tired indeed of these "trivially obvious
improvements" that are buggy and actually introduce whole new ways to
write buggy code. Yes, the old code could miss an error. But the old
code wouldn't then create invalid pointers like the new code does.

I'm not thrilled about going through the rest of this sequence,
looking for other gotcha's. But I am *really* really tired of this
idiotic "let's make up a new interface that gets things right" and
then absolutely doesn't get it right at all. This is not just an issue
for number parseing - we had similar issues with the completely
moronic and misdesigned crap called "strlcpy()", which was introduced
for similar reasons, and also caused nasty bugs where the old code was
actually correct, and the "converted to better and safer interfaces"
code was actually buggy.

Mixing the error handling and the string update was a mistake.
Although *not* mixing it causes its own set of problems.

But whatever the final resolution to this is, I am *not* taking this
series. No way, no how. I liked the automatic type-based templating it
does, but I *don't* like the breakage that seems to be inevitable in
any large-scale conversion from a previously used historical
interface. People who implement new and improved interfaces always
seem to get that wrong.

              Linus

Re: incoming

[Rasmus Villemoes]

On Thu, Sep 10 2015, Linus Torvalds <torvalds@linux-foundation.org> wrote:

> The VERY FIRST conversion patch I looked at was buggy. That makes me
> angry. The whole *AND*ONLY* point of this whole thing was to get rid
> of bugs, and be a obviously safe interface, and then the first
> conversion patch proves it wrong.
>
> Let me show you:
>
>         if (isdigit(*str)) {
> -               io_tlb_nslabs = simple_strtoul(str, &str, 0);
> +               str += parse_integer(str, 0, &io_tlb_nslabs);
>
> and obviously nobody spent even a *second* asking themselves "what if
> parse_integer returns an error".

[This is going to sound awfully self-glorifying. Oh well.] I did point
that out in another instance (memparse), which I think then got somewhat
fixed in a later version. Since Alexey and I seemed to disagree on what
guiding principles to use when doing the conversions and a number of
other points, I didn't have the energy to go through the entire series,
and the discussion died out.

http://thread.gmane.org/gmane.linux.kernel/1942623/focus=1944193

> I liked the automatic type-based templating it does, but I *don't*
> like the breakage that seems to be inevitable in any large-scale
> conversion from a previously used historical interface.

My words exactly.

Rasmus

incoming

[Andrew Morton]

So this is what I have lined up for the first mm->2.6.22 batch.  I won't be
sending it off for another 12-24 hours yet.  To give people time for final
comment and to give me time to see if it actually works.



- A few serial bits.

- A few pcmcia bits.

- Some of the MM queue.  Includes:

  - An enhancement to /proc/pid/smaps to permit monitoring of a running
    program's working set.

    There's another patchset which builds on this quite a lot from Matt
    Mackall, but it's not quite ready yet.

  - The SLUB allocator.  It's pretty green but I do want to push ahead
    with this pretty aggressively with a view to replacing slab altogether.

    If it ends up not working out then we should remove slub altogether
    again, but I doubt if that will occur.

    If SLUB isn't in good shape by 2.6.22 we should hide it in Kconfig
    to prevent people from hitting known problems.  It'll remain
    EXPERIMENTAL.

  - generic pagetable quicklist management.  We have x86_64 and ia64
    and sparc64 implementations, but I'll only include David's sparc64
    implementation here.  I'll send the x86_64 and ia64 implementations
    through maintainers.

  - Various random MM bits

  - Benh's teach-get_unmapped_area-about-MAP_FIXED changes

  - madvise(MADV_FREE)



  This means I'm holding back Mel's page allocator work, and Andy's
  lumpy-reclaim.

  A shame in a way - I have high hopes for lumpy reclaim against the
  moveable zone, but these things are not to be done lightly.

  A few MM things have been held back awaiting subsystem tree merges
  (probably x86 - I didn't check).


- One little security patch

- the blackfin architecture

- small h8300 update

- small alpha update

- swsusp updates

- m68k bits

- cris udpates

- Lots of UML updates

- v850, xtensa



slab-introduce-krealloc.patch
at91_cf-minor-fix.patch
add-new_id-to-pcmcia-drivers.patch
ide-cs-recognize-2gb-compactflash-from-transcend.patch
serial-driver-pmc-msp71xx.patch
rm9000-serial-driver.patch
serial-define-fixed_port-flag-for-serial_core.patch
serial-use-resource_size_t-for-serial-port-io-addresses.patch
mpsc-serial-driver-tx-locking.patch
8250_pci-fix-pci-must_checks.patch
serial-serial_core-use-pr_debug.patch
add-apply_to_page_range-which-applies-a-function-to-a-pte-range.patch
safer-nr_node_ids-and-nr_node_ids-determination-and-initial.patch
use-zvc-counters-to-establish-exact-size-of-dirtyable-pages.patch
proper-prototype-for-hugetlb_get_unmapped_area.patch
mm-remove-gcc-workaround.patch
slab-ensure-cache_alloc_refill-terminates.patch
mm-make-read_cache_page-synchronous.patch
fs-buffer-dont-pageuptodate-without-page-locked.patch
allow-oom_adj-of-saintly-processes.patch
introduce-config_has_dma.patch
mm-slabc-proper-prototypes.patch
add-pfn_valid_within-helper-for-sub-max_order-hole-detection.patch
mm-simplify-filemap_nopage.patch
add-unitialized_var-macro-for-suppressing-gcc-warnings.patch
i386-add-ptep_test_and_clear_dirtyyoung.patch
i386-use-pte_update_defer-in-ptep_test_and_clear_dirtyyoung.patch
smaps-extract-pmd-walker-from-smaps-code.patch
smaps-add-pages-referenced-count-to-smaps.patch
smaps-add-clear_refs-file-to-clear-reference.patch
readahead-improve-heuristic-detecting-sequential-reads.patch
readahead-code-cleanup.patch
slab-use-num_possible_cpus-in-enable_cpucache.patch
slab-dont-allocate-empty-shared-caches.patch
slab-numa-kmem_cache-diet.patch
do-not-disable-interrupts-when-reading-min_free_kbytes.patch
slab-mark-set_up_list3s-__init.patch
cpusets-allow-tif_memdie-threads-to-allocate-anywhere.patch
i386-use-page-allocator-to-allocate-thread_info-structure.patch
slub-core.patch
make-page-private-usable-in-compound-pages-v1.patch
optimize-compound_head-by-avoiding-a-shared-page.patch
add-virt_to_head_page-and-consolidate-code-in-slab-and-slub.patch
slub-fix-object-tracking.patch
slub-enable-tracking-of-full-slabs.patch
slub-validation-of-slabs-metadata-and-guard-zones.patch
slub-add-min_partial.patch
slub-add-ability-to-list-alloc--free-callers-per-slab.patch
slub-free-slabs-and-sort-partial-slab-lists-in-kmem_cache_shrink.patch
slub-remove-object-activities-out-of-checking-functions.patch
slub-user-documentation.patch
slub-add-slabinfo-tool.patch
quicklists-for-page-table-pages.patch
quicklist-support-for-sparc64.patch
slob-handle-slab_panic-flag.patch
include-kern_-constant-in-printk-calls-in-mm-slabc.patch
mm-madvise-avoid-exclusive-mmap_sem.patch
mm-remove-destroy_dirty_buffers-from-invalidate_bdev.patch
mm-optimize-kill_bdev.patch
mm-optimize-acorn-partition-truncate.patch
slab-allocators-remove-obsolete-slab_must_hwcache_align.patch
kmem_cache-simplify-slab-cache-creation.patch
slab-allocators-remove-multiple-alignment-specifications.patch
fault-injection-fix-failslab-with-config_numa.patch
mm-fix-handling-of-panic_on_oom-when-cpusets-are-in-use.patch
oom-fix-constraint-deadlock.patch
get_unmapped_area-handles-map_fixed-on-powerpc.patch
get_unmapped_area-handles-map_fixed-on-alpha.patch
get_unmapped_area-handles-map_fixed-on-arm.patch
get_unmapped_area-handles-map_fixed-on-frv.patch
get_unmapped_area-handles-map_fixed-on-i386.patch
get_unmapped_area-handles-map_fixed-on-ia64.patch
get_unmapped_area-handles-map_fixed-on-parisc.patch
get_unmapped_area-handles-map_fixed-on-sparc64.patch
get_unmapped_area-handles-map_fixed-on-x86_64.patch
get_unmapped_area-handles-map_fixed-in-hugetlbfs.patch
get_unmapped_area-handles-map_fixed-in-generic-code.patch
get_unmapped_area-doesnt-need-hugetlbfs-hacks-anymore.patch
slab-allocators-remove-slab_debug_initial-flag.patch
slab-allocators-remove-slab_ctor_atomic.patch
slab-allocators-remove-useless-__gfp_no_grow-flag.patch
lazy-freeing-of-memory-through-madv_free.patch
restore-madv_dontneed-to-its-original-linux-behaviour.patch
hugetlbfs-add-null-check-in-hugetlb_zero_setup.patch
slob-fix-page-order-calculation-on-not-4kb-page.patch
page-migration-only-migrate-pages-if-allocation-in-the-highest-zone-is-possible.patch
return-eperm-not-echild-on-security_task_wait-failure.patch
blackfin-arch.patch
driver_bfin_serial_core.patch
blackfin-on-chip-ethernet-mac-controller-driver.patch
blackfin-patch-add-blackfin-support-in-smc91x.patch
blackfin-on-chip-rtc-controller-driver.patch
blackfin-blackfin-on-chip-spi-controller-driver.patch
convert-h8-300-to-generic-timekeeping.patch
h8300-generic-irq.patch
h8300-add-zimage-support.patch
round_up-macro-cleanup-in-arch-alpha-kernel-osf_sysc.patch
alpha-fix-bootp-image-creation.patch
alpha-prctl-macros.patch
srmcons-fix-kmallocgfp_kernel-inside-spinlock.patch
arm26-remove-useless-config-option-generic_bust_spinlock.patch
fix-refrigerator-vs-thaw_process-race.patch
swsusp-use-inline-functions-for-changing-page-flags.patch
swsusp-do-not-use-page-flags.patch
mm-remove-unused-page-flags.patch
swsusp-fix-error-paths-in-snapshot_open.patch
swsusp-use-gfp_kernel-for-creating-basic-data-structures.patch
freezer-remove-pf_nofreeze-from-handle_initrd.patch
swsusp-use-rbtree-for-tracking-allocated-swap.patch
freezer-fix-racy-usage-of-try_to_freeze-in-kswapd.patch
remove-software_suspend.patch
power-management-change-sys-power-disk-display.patch
kconfig-mentioneds-hibernation-not-just-swsusp.patch
swsusp-fix-snapshot_release.patch
swsusp-free-more-memory.patch
remove-unused-header-file-arch-m68k-atari-atasoundh.patch
spin_lock_unlocked-cleanup-in-arch-m68k.patch
remove-unused-header-file-drivers-serial-crisv10h.patch
cris-check-for-memory-allocation.patch
cris-remove-code-related-to-pre-22-kernel.patch
uml-delete-unused-code.patch
uml-formatting-fixes.patch
uml-host_info-tidying.patch
uml-mark-tt-mode-code-for-future-removal.patch
uml-print-coredump-limits.patch
uml-handle-block-device-hotplug-errors.patch
uml-driver-formatting-fixes.patch
uml-driver-formatting-fixes-fix.patch
uml-network-interface-hotplug-error-handling.patch
array_size-check-for-type.patch
uml-move-sigio-testing-to-sigioc.patch
uml-create-archh.patch
uml-create-as-layouth.patch
uml-move-remaining-useful-contents-of-user_utilh.patch
uml-remove-user_utilh.patch
uml-add-missing-__init-declarations.patch
remove-unused-header-file-arch-um-kernel-tt-include-mode_kern-tth.patch
uml-improve-checking-and-diagnostics-of-ethernet-macs.patch
uml-eliminate-temporary-buffer-in-eth_configure.patch
uml-replace-one-element-array-with-zero-element-array.patch
uml-fix-umid-in-xterm-titles.patch
uml-speed-up-exec.patch
uml-no-locking-needed-in-tlsc.patch
uml-tidy-processc.patch
uml-remove-page_size.patch
uml-kernel_thread-shouldnt-panic.patch
uml-tidy-fault-code.patch
uml-kernel-segfaults-should-dump-proper-registers.patch
uml-comment-early-boot-locking.patch
uml-irq-locking-commentary.patch
uml-delete-host_frame_size.patch
uml-drivers-get-release-methods.patch
uml-dump-registers-on-ptrace-or-wait-failure.patch
uml-speed-up-page-table-walking.patch
uml-remove-unused-x86_64-code.patch
uml-start-fixing-os_read_file-and-os_write_file.patch
uml-tidy-libc-code.patch
uml-convert-libc-layer-to-call-read-and-write.patch
uml-batch-i-o-requests.patch
uml-send-pointers-instead-of-structures-to-i-o-thread.patch
uml-send-pointers-instead-of-structures-to-i-o-thread-fix.patch
uml-dump-core-on-panic.patch
uml-dont-try-to-handle-signals-on-initial-process-stack.patch
uml-change-remaining-callers-of-os_read_write_file.patch
uml-formatting-fixes-around-os_read_write_file-callers.patch
uml-remove-debugging-remnants.patch
uml-rename-os_read_write_file_k-back-to-os_read_write_file.patch
uml-aio-deadlock-avoidance.patch
uml-speed-page-fault-path.patch
uml-eliminate-a-piece-of-debugging-code.patch
uml-more-page-fault-path-trimming.patch
uml-only-flush-areas-covered-by-vma.patch
uml-out-of-tmpfs-space-error-clarification.patch
uml-virtualized-time-fix.patch
uml-fix-prototypes.patch
v850-generic-timekeeping-conversion.patch
xtensa-strlcpy-is-smart-enough.patch

Re: incoming

[Benjamin Herrenschmidt]

On Wed, 2007-05-02 at 15:02 -0700, Andrew Morton wrote:
> So this is what I have lined up for the first mm->2.6.22 batch.  I won't be
> sending it off for another 12-24 hours yet.  To give people time for final
> comment and to give me time to see if it actually works.

Thanks.

I have some powerpc bits that depend on that stuff that will go through
Paulus after these show up in git and I've rebased.

Cheers,
Ben.

Re: incoming

[Russell King]

On Wed, May 02, 2007 at 03:02:52PM -0700, Andrew Morton wrote:
> So this is what I have lined up for the first mm->2.6.22 batch.  I won't be
> sending it off for another 12-24 hours yet.  To give people time for final
> comment and to give me time to see if it actually works.

I assume you're going to update this list with my comments I sent
yesterday?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: incoming

[Andrew Morton]

On Thu, 3 May 2007 08:55:43 +0100 Russell King <rmk+lkml@arm.linux.org.uk> wrote:

> On Wed, May 02, 2007 at 03:02:52PM -0700, Andrew Morton wrote:
> > So this is what I have lined up for the first mm->2.6.22 batch.  I won't be
> > sending it off for another 12-24 hours yet.  To give people time for final
> > comment and to give me time to see if it actually works.
> 
> I assume you're going to update this list with my comments I sent
> yesterday?
> 

Serial drivers?  Well you saw me drop a bunch of them.  I now have:

serial-driver-pmc-msp71xx.patch
rm9000-serial-driver.patch
serial-define-fixed_port-flag-for-serial_core.patch
mpsc-serial-driver-tx-locking.patch
serial-serial_core-use-pr_debug.patch

I'll also be holding off on MADV_FREE - Nick has some performance things to
share and I'm assuming they're not as good as he'd like.

Re: incoming

[Greg KH]

On Wed, May 02, 2007 at 03:02:52PM -0700, Andrew Morton wrote:
> - One little security patch

Care to cc: linux-stable with it so we can do a new 2.6.21 release with
it if needed?

thanks,

greg k-h

Re: incoming

[Andrew Morton]

On Fri, 4 May 2007 06:37:28 -0700 Greg KH <greg@kroah.com> wrote:

> On Wed, May 02, 2007 at 03:02:52PM -0700, Andrew Morton wrote:
> > - One little security patch
> 
> Care to cc: linux-stable with it so we can do a new 2.6.21 release with
> it if needed?
> 

Ah.  The patch affects security code, but it doesn't actually address any
insecurity.  I didn't think it was needed for -stable?



From: Roland McGrath <roland@redhat.com>

wait* syscalls return -ECHILD even when an individual PID of a live child
was requested explicitly, when security_task_wait denies the operation. 
This means that something like a broken SELinux policy can produce an
unexpected failure that looks just like a bug with wait or ptrace or
something.

This patch makes do_wait return -EACCES (or other appropriate error returned
from security_task_wait() instead of -ECHILD if some children were ruled out
solely because security_task_wait failed.

[jmorris@namei.org: switch error code to EACCES]
Signed-off-by: Roland McGrath <roland@redhat.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Cc: Chris Wright <chrisw@sous-sol.org>
Cc: James Morris <jmorris@namei.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 kernel/exit.c |   17 +++++++++++++++--
 1 files changed, 15 insertions(+), 2 deletions(-)

diff -puN kernel/exit.c~return-eperm-not-echild-on-security_task_wait-failure kernel/exit.c
--- a/kernel/exit.c~return-eperm-not-echild-on-security_task_wait-failure
+++ a/kernel/exit.c
@@ -1033,6 +1033,8 @@ asmlinkage void sys_exit_group(int error
 
 static int eligible_child(pid_t pid, int options, struct task_struct *p)
 {
+	int err;
+
 	if (pid > 0) {
 		if (p->pid != pid)
 			return 0;
@@ -1066,8 +1068,9 @@ static int eligible_child(pid_t pid, int
 	if (delay_group_leader(p))
 		return 2;
 
-	if (security_task_wait(p))
-		return 0;
+	err = security_task_wait(p);
+	if (err)
+		return err;
 
 	return 1;
 }
@@ -1449,6 +1452,7 @@ static long do_wait(pid_t pid, int optio
 	DECLARE_WAITQUEUE(wait, current);
 	struct task_struct *tsk;
 	int flag, retval;
+	int allowed, denied;
 
 	add_wait_queue(&current->signal->wait_chldexit,&wait);
 repeat:
@@ -1457,6 +1461,7 @@ repeat:
 	 * match our criteria, even if we are not able to reap it yet.
 	 */
 	flag = 0;
+	allowed = denied = 0;
 	current->state = TASK_INTERRUPTIBLE;
 	read_lock(&tasklist_lock);
 	tsk = current;
@@ -1472,6 +1477,12 @@ repeat:
 			if (!ret)
 				continue;
 
+			if (unlikely(ret < 0)) {
+				denied = ret;
+				continue;
+			}
+			allowed = 1;
+
 			switch (p->state) {
 			case TASK_TRACED:
 				/*
@@ -1570,6 +1581,8 @@ check_continued:
 		goto repeat;
 	}
 	retval = -ECHILD;
+	if (unlikely(denied) && !allowed)
+		retval = denied;
 end:
 	current->state = TASK_RUNNING;
 	remove_wait_queue(&current->signal->wait_chldexit,&wait);
_

Re: incoming

[Greg KH]

On Fri, May 04, 2007 at 09:14:34AM -0700, Andrew Morton wrote:
> On Fri, 4 May 2007 06:37:28 -0700 Greg KH <greg@kroah.com> wrote:
> 
> > On Wed, May 02, 2007 at 03:02:52PM -0700, Andrew Morton wrote:
> > > - One little security patch
> > 
> > Care to cc: linux-stable with it so we can do a new 2.6.21 release with
> > it if needed?
> > 
> 
> Ah.  The patch affects security code, but it doesn't actually address any
> insecurity.  I didn't think it was needed for -stable?

Ah, ok, I read "security" as fixing a insecure problem, my mistake :)

thanks,

greg k-h

Re: incoming

[Roland McGrath]

> Ah.  The patch affects security code, but it doesn't actually address any
> insecurity.  I didn't think it was needed for -stable?

I would not recommend it for -stable.  
It is an ABI change for the case of a security refusal.


Thanks,
Roland

Re: incoming

[Greg KH]

On Fri, May 04, 2007 at 11:57:21AM -0700, Roland McGrath wrote:
> > Ah.  The patch affects security code, but it doesn't actually address any
> > insecurity.  I didn't think it was needed for -stable?
> 
> I would not recommend it for -stable.  
> It is an ABI change for the case of a security refusal.

ABI changes are not a problem for -stable, so don't let that stop anyone
:)

thanks,

greg k-h

Re: incoming

[Roland McGrath]

> ABI changes are not a problem for -stable, so don't let that stop anyone
> :)

In fact this is the harmless sort (changes only the error code of a
failure case) that might actually go in if there were any important
reason.  But the smiley stands.


Thanks,
Roland

incoming

[Andrew Morton]

As the commits list probably isn't working at present I'll cc linux-kernel
on this lot.  Fairly cruel, sorry, but I don't like the idea of people not
knowing what's hitting the main tree.



This is the first live test of Linus's git-importing ability.  I'm about
to disappear for 1.5 weeks - hope we'll still have a kernel left when I
get back.

- As we're still a fair way from 2.6.12 and things are still backing up,
  it's a relatively large update.

- Various arch updates

- Big x86_64 update, as discussed

- decent-sized ppc32, ppc64 updates

- big infiniband update

- very nearly the last batch of u32->pm_message_t conversions.  Some
  other bits of this will be sitting out in subsystem trees - this is just
  the stuff which doesn't overlap.

- the important fixes from the md, nfs4 queues

- other random fixes and things we probably want to have in 2.6.12.

- I'd draw especial Linus attention to:

	"fix crash in entry.S restore_all" and
	"pci enumeration on ixp2000: overflow in kernel/resource.c"

Re: incoming

[David Vrabel]

Andrew Morton wrote:
> As the commits list probably isn't working at present I'll cc linux-kernel
> on this lot.  Fairly cruel, sorry, but I don't like the idea of people not
> knowing what's hitting the main tree.

Is there any chance that in the future that these patch sets get posted
all to one thread?  Perhaps as a reply to a summary? 1 thread to ignore
is preferable to 198.

David Vrabel

Re: incoming

[Andrew Morton]

David Vrabel <dvrabel@cantab.net> wrote:
>
> Is there any chance that in the future that these patch sets get posted
>  all to one thread?

I never got around to setting that up, plus the Subject:s pretty quickly
become invisible when they're indented 198 columns in GUI MUAs.

Hopefully we'll have the commits list running next time...

Re: incoming

[David Vrabel]

Andrew Morton wrote:
> David Vrabel <dvrabel@cantab.net> wrote:
> 
>>Is there any chance that in the future that these patch sets get posted
>> all to one thread?
> 
> I never got around to setting that up, plus the Subject:s pretty quickly
> become invisible when they're indented 198 columns in GUI MUAs.

I meant something like this:

[patch 000/100]  Foo-ize the baz.
   [patch 001/100] Frob the baz
   [patch 002/100] baz cleanups
   [patch 003/100] apply foo-ization to baz

Rather than

[patch 000/100] Foo-ize the baz.
   [patch 001/100] Frob the baz
     [patch 002/100] baz cleanups
       [patch 003/100] apply foo-ization to baz

Which would (as you rightly pointed out) be ludicrous.

i.e., all the patches are replys to the summary.

David Vrabel

Re: incoming

[Matthias Urlichs]

Hi,   Andrew Morton schrub am Tue, 12 Apr 2005 04:10:45 -0700:

> David Vrabel <dvrabel@cantab.net> wrote:
>>
>> Is there any chance that in the future that these patch sets get posted
>>  all to one thread?
> 
> I never got around to setting that up, plus the Subject:s pretty quickly
> become invisible when they're indented 198 columns in GUI MUAs.
> 
Umm, what stops you from letting all the parts refer to part zero,
instead of part n-1?

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de

Re: incoming

[Paul Jackson]

Andrew wrote:
> I never got around to setting that up, plus the Subject:s pretty quickly
> become invisible when they're indented 198 columns in GUI MUAs.

My sendpatchset tool should be good for this.  It sends all but the
first message are sent in "Reference" to, and "In-Reply-To" the first
message.

  http://www.speakeasy.org/~pj99/sgi/sendpatchset

I use it when sending out multiple patches in sequence from a quilt
repository.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401

Re: incoming

[Chris Friesen]

Andrew Morton wrote:
> As the commits list probably isn't working at present I'll cc linux-kernel
> on this lot.  Fairly cruel, sorry, but I don't like the idea of people not
> knowing what's hitting the main tree.

I'd like to second the idea of having all the patches be replies to this 
original posting (ie one level of indenting for all patches).  That way 
a threaded view will only have one subject line for all 198 patches.

Chris

Re: incoming

[Russell King]

On Tue, Apr 12, 2005 at 03:23:22AM -0700, Andrew Morton wrote:
> As the commits list probably isn't working at present I'll cc linux-kernel
> on this lot.  Fairly cruel, sorry, but I don't like the idea of people not
> knowing what's hitting the main tree.

I don't see a patch which adds linux/pm.h to linux/sysdev.h, which is
required to fix ARM builds in -rc2 and onwards kernels.

It is my understanding that you have such a patch, and if it isn't
going to be sent, I'd like to send my own fix so that ARM can start
building again in mainline.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: incoming

[Andrew Morton]

Russell King <rmk+lkml@arm.linux.org.uk> wrote:
>
> I don't see a patch which adds linux/pm.h to linux/sysdev.h, which is
>  required to fix ARM builds in -rc2 and onwards kernels.

That fix is buried in [patch 105/198]

Re: incoming

[Russell King]

On Tue, Apr 12, 2005 at 02:08:00PM -0700, Andrew Morton wrote:
> Russell King <rmk+lkml@arm.linux.org.uk> wrote:
> >
> > I don't see a patch which adds linux/pm.h to linux/sysdev.h, which is
> >  required to fix ARM builds in -rc2 and onwards kernels.
> 
> That fix is buried in [patch 105/198]

Great, thanks.  I must have missed it, sorry.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: incoming

[Geert Uytterhoeven]

On Tue, 12 Apr 2005, Andrew Morton wrote:
> As the commits list probably isn't working at present I'll cc linux-kernel
> on this lot.  Fairly cruel, sorry, but I don't like the idea of people not
> knowing what's hitting the main tree.

Is it me, or were really only 117 mails of the 198 sent to lkml?

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: incoming

[Paulo Marques]

Geert Uytterhoeven wrote:
> On Tue, 12 Apr 2005, Andrew Morton wrote:
> 
>>As the commits list probably isn't working at present I'll cc linux-kernel
>>on this lot.  Fairly cruel, sorry, but I don't like the idea of people not
>>knowing what's hitting the main tree.
> 
> 
> Is it me, or were really only 117 mails of the 198 sent to lkml?

(?)

I just double-checked, and I can say that I received all 198 emails from 
vger...

-- 
Paulo Marques - www.grupopie.com

All that is necessary for the triumph of evil is that good men do nothing.
Edmund Burke (1729 - 1797)

Re: incoming

[Lee Revell]

On Thu, 2005-04-14 at 13:48 +0200, Geert Uytterhoeven wrote:
> On Tue, 12 Apr 2005, Andrew Morton wrote:
> > As the commits list probably isn't working at present I'll cc linux-kernel
> > on this lot.  Fairly cruel, sorry, but I don't like the idea of people not
> > knowing what's hitting the main tree.
> 
> Is it me, or were really only 117 mails of the 198 sent to lkml?

The patch bombing seems to have really wedged vger.  It took up to 24
hours to get all the messages.

Looks like Andrew's patch bomb script needs some rate limiting ;-)

Lee

Re: incoming

[Paul Jackson]

> Looks like Andrew's patch bomb script needs some rate limiting ;-)

sendpatchset has that, already builtin ;)

	http://www.speakeasy.org/~pj99/sgi/sendpatchset

Though the 5 second delay might not be enough for someone
publishing at the rate Andrew does.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401