Re: [PATCH] sysfs: add sysfs_create/remove_groups()

2013-08-21 Thread Anthony Foiani
Greg Kroah-Hartman  writes:

> + * sysfs_create_groups - given a directory kobject, create a bunch of 
> attribute groups
> + * @kobj:The kobject to create the group on
> + * @groups:  The attribute groups to create, NULL terminated
> + *
> + * This function creates a bunch of attribute groups.  If an error occurs 
> when
> + * creating a group, all previously created groups will be removed, unwinding
> + * everything back to the original state when this function was called.
> + * It will explicitly warn and error if any of the attribute files being
> + * created already exist.
> + *
> + * Returns 0 on success or error code from sysfs_create_groups on error.

"... from sysfs_create_group on error"
   ^ (note singular, not plural)

Otherwise it's a bit of a tautology...

Maybe rename to "sysfs_(create|remove)_multiple_groups" (or
..._many_groups) to avoid this kind of one-char difference?

> + * sysfs_remove_groups - remove a list of groups
> + *
> + * kobj: The kobject for the groups to be removed from
> + * groups:   NULL terminated list of groups to be removed
> + *
> + * If groups is not NULL, the all groups will be removed from the kobject

"If groups is not NULL, remove listed groups from the kobject"

(I also played around with using pointer arithmetic instead of array
accesses for these two functions; it cut a few lines of code, but not
enough to bother, I don't think.)

Thanks,
Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] sysfs: add sysfs_create/remove_groups()

2013-08-21 Thread Anthony Foiani
Greg Kroah-Hartman gre...@linuxfoundation.org writes:

 + * sysfs_create_groups - given a directory kobject, create a bunch of 
 attribute groups
 + * @kobj:The kobject to create the group on
 + * @groups:  The attribute groups to create, NULL terminated
 + *
 + * This function creates a bunch of attribute groups.  If an error occurs 
 when
 + * creating a group, all previously created groups will be removed, unwinding
 + * everything back to the original state when this function was called.
 + * It will explicitly warn and error if any of the attribute files being
 + * created already exist.
 + *
 + * Returns 0 on success or error code from sysfs_create_groups on error.

... from sysfs_create_group on error
   ^ (note singular, not plural)

Otherwise it's a bit of a tautology...

Maybe rename to sysfs_(create|remove)_multiple_groups (or
..._many_groups) to avoid this kind of one-char difference?

 + * sysfs_remove_groups - remove a list of groups
 + *
 + * kobj: The kobject for the groups to be removed from
 + * groups:   NULL terminated list of groups to be removed
 + *
 + * If groups is not NULL, the all groups will be removed from the kobject

If groups is not NULL, remove listed groups from the kobject

(I also played around with using pointer arithmetic instead of array
accesses for these two functions; it cut a few lines of code, but not
enough to bother, I don't think.)

Thanks,
Tony
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: trouble building 'perf' for 3.9.7

2013-06-30 Thread Anthony Foiani

Greetings --

I wrote:

> Anyway, this worked fine for 2.6.late, as well as 3.0 stable and 3.4
> stable.  However, I wanted the improvements to 'usbip', so I switched
> to 3.9; currently I'm using 3.9.7.
>
> Using exactly the same scripts (see below), perf now fails to build:

I have to eat some of those words.

I tried switching back to 3.4, and the build still failed.  I'm
mystified, as I thought I had done a full rebuild with that kernel,
but maybe I hadn't (and the build succeeded due to bits left over from
earlier kernels).

The final fix (for 3.4 -- testing with 3.9 now) was to *not* point the
perf build at the target ("staging") headers.  Instead, perf finds the
sysroot headers installed as a part of the cross-compile toolchain.

This left issues with elfutils, but I changed that build step to put
them in a custom subdirectory.  That way, I could configure perf to
use that location without pulling in the sanitized headers.

Sorry if anyone wasted their time chasing down my carelessness.

(Although, to be fair, 'perf' is the only package out of about 20 that
fails when I explicitly point the build at the target's "include"
directory.)

Thanks again!

Best regards,
Anthony Foiani
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: trouble building 'perf' for 3.9.7

2013-06-30 Thread Anthony Foiani

Greetings --

I wrote:

 Anyway, this worked fine for 2.6.late, as well as 3.0 stable and 3.4
 stable.  However, I wanted the improvements to 'usbip', so I switched
 to 3.9; currently I'm using 3.9.7.

 Using exactly the same scripts (see below), perf now fails to build:

I have to eat some of those words.

I tried switching back to 3.4, and the build still failed.  I'm
mystified, as I thought I had done a full rebuild with that kernel,
but maybe I hadn't (and the build succeeded due to bits left over from
earlier kernels).

The final fix (for 3.4 -- testing with 3.9 now) was to *not* point the
perf build at the target (staging) headers.  Instead, perf finds the
sysroot headers installed as a part of the cross-compile toolchain.

This left issues with elfutils, but I changed that build step to put
them in a custom subdirectory.  That way, I could configure perf to
use that location without pulling in the sanitized headers.

Sorry if anyone wasted their time chasing down my carelessness.

(Although, to be fair, 'perf' is the only package out of about 20 that
fails when I explicitly point the build at the target's include
directory.)

Thanks again!

Best regards,
Anthony Foiani
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


trouble building 'perf' for 3.9.7

2013-06-29 Thread Anthony Foiani

Greetings.

I'm working on an embedded project (developing on x86-64, targeting ppc32).

My build system goes through and builds targets in a certain order:

  u-boot
  kernel (uImage, modules, headers)
  elftools
  perf
  ... lots more ...

(Yes, if I had understood how all the parts fit together 4 years ago,
I would use open embedded or yocto or something else.  Oh well.)

Anyway, this worked fine for 2.6.late, as well as 3.0 stable and 3.4
stable.  However, I wanted the improvements to 'usbip', so I switched
to 3.9; currently I'm using 3.9.7.

Using exactly the same scripts (see below), perf now fails to build:

In file included from util/include/linux/list.h:4:0,
 from util/parse-events.h:7,
 from perf.c:15:
util/include/../../../../include/linux/list.h:24:42: error: 'struct 
list_head' declared inside parameter list [-Werror]
util/include/../../../../include/linux/list.h:24:42: error: its scope is 
only this definition or declaration, which is probably not what you want 
[-Werror]

This seems to be fallout from the UAPI split, which came in about 3.7 or so:

commit 607ca46e97a1b6594b29647d98a32d545c24bdff
Author: David Howells 
Date:   Sat Oct 13 10:46:48 2012 +0100

UAPI: (Scripted) Disintegrate include/linux

Signed-off-by: David Howells 
Acked-by: Arnd Bergmann 
Acked-by: Thomas Gleixner 
Acked-by: Michael Kerrisk 
Acked-by: Paul E. McKenney 
Acked-by: Dave Jones 

Looking through the preprocessed source, the problem seems to be that
'perf' eventually asks for .  Before the UAPI split,
this file included the list_head structure; aftwerwards, it didn't.

I've tried various combinations of cpp flags to get the code to look
in the unpacked source first, before looking in the installed headers,
but I can't seem to make that work, either.

(Among other issues is that I need to use the elfutils built for this
project; if I try to move my project's include directory to the end of
the search list with "-idirafter", then the elfutils for the
cross-compiler itself are caught; if the include directory is any
earlier, perf picks up the sanitized linux/ directory.)

There are other reports of similar breakage:

  http://marc.info/?l=linux-kernel=135095162403283
  Re: tools/vm build fails

  http://lkml.indiana.edu/hypermail/linux/kernel/1212.1/02045.html
  Commit 607ca46e97a1b6594b29647d98a32d545c24bdff breaks building 
tools/hv/hv_kvp_daemon.c

And it has similar pattern (although for not exactly the same reasons) as 

  http://lkml.indiana.edu/hypermail/linux/kernel/1008.1/00601.html
  perf build broke by list_head changes...

Anyway, my scripts are fairly straightforward.  After my cross-compile
toolchain is built, I have the following environment variables:

  TARGET_TUPLE=powerpc-e300c3-linux-gnu
  PLATFORM_STAGE=/opt/cross/stage
  PLATFORM_BOOT=/opt/cross/boot

I build the kernel like so, and this works fine:

version=3.9.7-ajf-mumble
build=$( cd ../build ; pwd )
arch=${TARGET_TUPLE%%-*}
 
yes "" | make O="$build" ARCH="$arch" CROSS_COMPILE="$TARGET_TUPLE"- \
 oldconfig || exit $?
 
rm -rf "$PLATFORM_STAGE"/lib/modules
make O="$build" ARCH="$arch" CROSS_COMPILE="$TARGET_TUPLE"- \
 $MAKE_PARALLEL_ARGS uImage modules || exit $?

cp "$build/arch/$arch/boot/uImage" "$PLATFORM_BOOT" || exit $?
cp "$build/vmlinux""$PLATFORM_BOOT" || exit $?
make O="$build" ARCH="$arch" CROSS_COMPILE="$TARGET_TUPLE"- \
 INSTALL_MOD_PATH="$PLATFORM_STAGE" \
 INSTALL_HDR_PATH="$PLATFORM_STAGE/include/linux-$version" \
 headers_install modules_install || exit $?

( cd "$PLATFORM_STAGE/include"
rmdir linux
ln -sf "linux-$version"/include/* . )

(Yes, I'm aware I should be using 'olddefconfig' and 'set -e'.  The
joys of hindsight.)

To build perf, I do something similar (but in a freshly-unpacked copy
of the kernel tree -- an artifact of my desire to be able to restart
the build process at any stage):

build=$( cd ../build ; pwd )
arch=${TARGET_TUPLE%%-*}
 
yes "" | make O="$build" ARCH="$arch" CROSS_COMPILE="$TARGET_TUPLE"- \
 oldconfig || exit $?
 
(
cd tools/perf
make O="$build" ARCH="$arch" CROSS_COMPILE="$TARGET_TUPLE"- V=1 \
 EXTRA_CFLAGS="-I $PLATFORM_STAGE/include -I 
$PLATFORM_STAGE/include/elfutils" \
 LDFLAGS="-L$PLATFORM_STAGE/lib -lz -lbz2" \
 DESTDIR="$PLATFORM_STAGE" \
 install || exit $?
)

Again, this all worked fine for 3.4.

Is there an obvious fix for 3.9?

Many thanks in advance.

Best regards,
Anthony Foiani
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


trouble building 'perf' for 3.9.7

2013-06-29 Thread Anthony Foiani

Greetings.

I'm working on an embedded project (developing on x86-64, targeting ppc32).

My build system goes through and builds targets in a certain order:

  u-boot
  kernel (uImage, modules, headers)
  elftools
  perf
  ... lots more ...

(Yes, if I had understood how all the parts fit together 4 years ago,
I would use open embedded or yocto or something else.  Oh well.)

Anyway, this worked fine for 2.6.late, as well as 3.0 stable and 3.4
stable.  However, I wanted the improvements to 'usbip', so I switched
to 3.9; currently I'm using 3.9.7.

Using exactly the same scripts (see below), perf now fails to build:

In file included from util/include/linux/list.h:4:0,
 from util/parse-events.h:7,
 from perf.c:15:
util/include/../../../../include/linux/list.h:24:42: error: 'struct 
list_head' declared inside parameter list [-Werror]
util/include/../../../../include/linux/list.h:24:42: error: its scope is 
only this definition or declaration, which is probably not what you want 
[-Werror]

This seems to be fallout from the UAPI split, which came in about 3.7 or so:

commit 607ca46e97a1b6594b29647d98a32d545c24bdff
Author: David Howells dhowe...@redhat.com
Date:   Sat Oct 13 10:46:48 2012 +0100

UAPI: (Scripted) Disintegrate include/linux

Signed-off-by: David Howells dhowe...@redhat.com
Acked-by: Arnd Bergmann a...@arndb.de
Acked-by: Thomas Gleixner t...@linutronix.de
Acked-by: Michael Kerrisk mtk.manpa...@gmail.com
Acked-by: Paul E. McKenney paul...@linux.vnet.ibm.com
Acked-by: Dave Jones da...@redhat.com

Looking through the preprocessed source, the problem seems to be that
'perf' eventually asks for linux/types.h.  Before the UAPI split,
this file included the list_head structure; aftwerwards, it didn't.

I've tried various combinations of cpp flags to get the code to look
in the unpacked source first, before looking in the installed headers,
but I can't seem to make that work, either.

(Among other issues is that I need to use the elfutils built for this
project; if I try to move my project's include directory to the end of
the search list with -idirafter, then the elfutils for the
cross-compiler itself are caught; if the include directory is any
earlier, perf picks up the sanitized linux/ directory.)

There are other reports of similar breakage:

  http://marc.info/?l=linux-kernelm=135095162403283
  Re: tools/vm build fails

  http://lkml.indiana.edu/hypermail/linux/kernel/1212.1/02045.html
  Commit 607ca46e97a1b6594b29647d98a32d545c24bdff breaks building 
tools/hv/hv_kvp_daemon.c

And it has similar pattern (although for not exactly the same reasons) as 

  http://lkml.indiana.edu/hypermail/linux/kernel/1008.1/00601.html
  perf build broke by list_head changes...

Anyway, my scripts are fairly straightforward.  After my cross-compile
toolchain is built, I have the following environment variables:

  TARGET_TUPLE=powerpc-e300c3-linux-gnu
  PLATFORM_STAGE=/opt/cross/stage
  PLATFORM_BOOT=/opt/cross/boot

I build the kernel like so, and this works fine:

version=3.9.7-ajf-mumble
build=$( cd ../build ; pwd )
arch=${TARGET_TUPLE%%-*}
 
yes  | make O=$build ARCH=$arch CROSS_COMPILE=$TARGET_TUPLE- \
 oldconfig || exit $?
 
rm -rf $PLATFORM_STAGE/lib/modules
make O=$build ARCH=$arch CROSS_COMPILE=$TARGET_TUPLE- \
 $MAKE_PARALLEL_ARGS uImage modules || exit $?

cp $build/arch/$arch/boot/uImage $PLATFORM_BOOT || exit $?
cp $build/vmlinux$PLATFORM_BOOT || exit $?
make O=$build ARCH=$arch CROSS_COMPILE=$TARGET_TUPLE- \
 INSTALL_MOD_PATH=$PLATFORM_STAGE \
 INSTALL_HDR_PATH=$PLATFORM_STAGE/include/linux-$version \
 headers_install modules_install || exit $?

( cd $PLATFORM_STAGE/include
rmdir linux
ln -sf linux-$version/include/* . )

(Yes, I'm aware I should be using 'olddefconfig' and 'set -e'.  The
joys of hindsight.)

To build perf, I do something similar (but in a freshly-unpacked copy
of the kernel tree -- an artifact of my desire to be able to restart
the build process at any stage):

build=$( cd ../build ; pwd )
arch=${TARGET_TUPLE%%-*}
 
yes  | make O=$build ARCH=$arch CROSS_COMPILE=$TARGET_TUPLE- \
 oldconfig || exit $?
 
(
cd tools/perf
make O=$build ARCH=$arch CROSS_COMPILE=$TARGET_TUPLE- V=1 \
 EXTRA_CFLAGS=-I $PLATFORM_STAGE/include -I 
$PLATFORM_STAGE/include/elfutils \
 LDFLAGS=-L$PLATFORM_STAGE/lib -lz -lbz2 \
 DESTDIR=$PLATFORM_STAGE \
 install || exit $?
)

Again, this all worked fine for 3.4.

Is there an obvious fix for 3.9?

Many thanks in advance.

Best regards,
Anthony Foiani
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Documentation: Enumerate the guidelines for stable patches.

2012-11-30 Thread Anthony Foiani
Greg KH  writes:

> Is this really needed?  For the large majority of the stable
> patches, specifically enumerating this isn't a big deal, it's a tiny
> patch, and if you think I'll remember to tell you which specific
> clause you didn't follow, then you think I have more spare time than
> I really do.

Yeah, it was largely tongue-in-cheek.  :)

Although a hint of "not upstream" would have been helpful.  You
obviously have the checklist in your head, and you (presumably) have a
your formletter automated; it just seems that, as busy as the
maintainers are, trading an extra 2s of your time for an hour of a
contributor's time would sometimes be the right thing.

Regardless, you're the one doing the work, not me.  (And you're an
outright angel, compared to some other high-level maintainers, so I
should simply count myself lucky.)

> Sorry, but no, I don't think this patch is ok, especially that S15
> clause, nice try :)

Can't fault a guy for trying.  (Or, hopefully, can't fault him much...)

Have a good weekend,
t.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/1] v3.0.x: mtd: check partition count not partition array pointer

2012-11-30 Thread Anthony Foiani

Greg --

Thanks for the very quick response.

Greg KH  writes:
> This is an obvious one, it needs to be upstream first.
>
> Or if not, a whole lot of explaination saying that you know it
> isn't, and why it isn't, and why it isn't applicable there,

I thought that I did provide exactly this information, when I
indicated that this function no longer even existed in upstream.

The replacement in upstream was to fold this functionality into
another function -- but I found that function much more complex and
difficult to follow.

So I went with the simple fix.

> including agreement from the subsystem maintainers saying that they
> agree with you.

I also don't have that.  I might post it to the mtd list, to see if
they have any interest in either approving it as-is for 3.0 series, or
doing the backport of the current code.

Thanks again for your time.

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] Documentation: Enumerate the guidelines for stable patches.

2012-11-30 Thread Anthony Foiani

From: Anthony Foiani 

Having recently received a formletter rejection on a stable patch, I
found it difficult to determine exactly which guideline I had missed.

Numbering the guidelines will allow the stable maintainer to quickly
and easily indicate which guidelines have not been followed.

The actual changes are only the addition of clause numbering, and the
wishful thinking added to S15.

Signed-Off-By: Anthony Foiani 
---
 Documentation/stable_kernel_rules.txt | 185 --
 1 file changed, 108 insertions(+), 77 deletions(-)

diff --git a/Documentation/stable_kernel_rules.txt 
b/Documentation/stable_kernel_rules.txt
index b0714d8..9a6dc59 100644
--- a/Documentation/stable_kernel_rules.txt
+++ b/Documentation/stable_kernel_rules.txt
@@ -1,95 +1,126 @@
 Everything you ever wanted to know about Linux -stable releases.
 
-Rules on what kind of patches are accepted, and which ones are not, into the
-"-stable" tree:
-
- - It must be obviously correct and tested.
- - It cannot be bigger than 100 lines, with context.
- - It must fix only one thing.
- - It must fix a real bug that bothers people (not a, "This could be a
-   problem..." type thing).
- - It must fix a problem that causes a build error (but not for things
-   marked CONFIG_BROKEN), an oops, a hang, data corruption, a real
-   security issue, or some "oh, that's not good" issue.  In short, something
-   critical.
- - Serious issues as reported by a user of a distribution kernel may also
-   be considered if they fix a notable performance or interactivity issue.
-   As these fixes are not as obvious and have a higher risk of a subtle
-   regression they should only be submitted by a distribution kernel
-   maintainer and include an addendum linking to a bugzilla entry if it
-   exists and additional information on the user-visible impact.
- - New device IDs and quirks are also accepted.
- - No "theoretical race condition" issues, unless an explanation of how the
-   race can be exploited is also provided.
- - It cannot contain any "trivial" fixes in it (spelling changes,
-   whitespace cleanups, etc).
- - It must follow the Documentation/SubmittingPatches rules.
- - It or an equivalent fix must already exist in Linus' tree (upstream).
+Rules on what kind of patches are accepted, and which ones are not,
+into the "-stable" tree.
 
+S1.  It must be obviously correct and tested.
+
+S2.  It cannot be bigger than 100 lines, with context.
+
+S3.  It must fix only one thing.
+
+S4.  It must fix a real bug that bothers people (not a, "This could be
+ a problem..." type thing).
+
+S5.  It must fix a problem that causes a build error (but not for
+ things marked CONFIG_BROKEN), an oops, a hang, data corruption, a
+ real security issue, or some "oh, that's not good" issue.  In
+ short, something critical.
+
+S6.  Serious issues as reported by a user of a distribution kernel may
+ also be considered if they fix a notable performance or
+ interactivity issue.  As these fixes are not as obvious and have
+ a higher risk of a subtle regression they should only be
+ submitted by a distribution kernel maintainer and include an
+ addendum linking to a bugzilla entry if it exists and additional
+ information on the user-visible impact.
+
+S7.  New device IDs and quirks are also accepted.
+
+S8.  No "theoretical race condition" issues, unless an explanation of
+ how the race can be exploited is also provided.
+
+S9.  It cannot contain any "trivial" fixes in it (spelling changes,
+ whitespace cleanups, etc).
+
+S10. It must follow the Documentation/SubmittingPatches rules.
+
+S11. It or an equivalent fix must already exist in Linus' tree
+ (upstream).
 
 Procedure for submitting patches to the -stable tree:
 
- - Send the patch, after verifying that it follows the above rules, to
-   sta...@vger.kernel.org.  You must note the upstream commit ID in the
-   changelog of your submission, as well as the kernel version you wish
-   it to be applied to.
- - To have the patch automatically included in the stable tree, add the tag
- Cc: sta...@vger.kernel.org
-   in the sign-off area. Once the patch is merged it will be applied to
-   the stable tree without anything else needing to be done by the author
-   or subsystem maintainer.
- - If the patch requires other patches as prerequisites which can be
-   cherry-picked than this can be specified in the following format in
-   the sign-off area:
-
- Cc:  # 3.3.x: a1f84a3: sched: Check for idle
- Cc:  # 3.3.x: 1b9508f: sched: Rate-limit newidle
- Cc:  # 3.3.x: fd21073: sched: Fix affinity logic
- Cc:  # 3.3.x
-Signed-off-by: Ingo Molnar 
-
-   The tag sequence has the meaning of:
- git cherry-pick a1f84a3
- git cherry-pick 1b9508f
- git cherry-pick fd21073
- git cherry-pick 
-
- - The sender will r

Re: [PATCH 1/1] v3.0.x: mtd: check partition count not partition array pointer

2012-11-30 Thread Anthony Foiani
Greg KH  writes:
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
> for how to do this properly.

My checklist against stable_kernel_rules.txt is below.

I could only find two reasons why you are saying this is incorrect:

  1. There is no matching upstream commit; or

  2. I did not CC: stable in the signed-off region.

Please let me know which it is, so I can either drop the subject (for
reasons indicated further below), or respin the patch.

Thanks,
Tony

| - It must be obviously correct and tested.

Check.

| - It cannot be bigger than 100 lines, with context.

Check: 1 changed line, 2 active lines in the patch, ~10 lines total.

| - It must fix only one thing.

Check.

| - It must fix a real bug that bothers people (not a, "This could be
|   a problem..." type thing).

It bothered me (made my embedded project not boot correctly).

| - It must fix a problem that causes a build error (but not for
|   things marked CONFIG_BROKEN), an oops, a hang, data corruption, a
|   real security issue, or some "oh, that's not good" issue.  In
|   short, something critical.

It repaired a change in behavior; for my project that was a "oh,
that's not good" issue.

It booted, but in such a way that it failed to expose a critical
system resource correctly.

| - Serious issues as reported by a user of a distribution kernel [...]

Not applicable.

| - New device IDs and quirks are also accepted.

Not applicable.

| - No "theoretical race condition" issues [...]

Not applicable.

| - It cannot contain any "trivial" fixes in it [...]

Check.

| - It must follow the Documentation/SubmittingPatches rules.

As far as I can tell, this patch follows those rules. 

| - It or an equivalent fix must already exist in Linus' tree (upstream).

As mentioned in the original submission:

>> The current kernel has retired this function; I have not examined
>> its replacement to see if it has the same issue.

You can either use this 1-line patch, or you can have someone else
backport the changes made in the mainline kernel.  That someone will
not be me, sadly.

| Procedure for submitting patches to the -stable tree:
|
| - Send the patch, after verifying that it follows the above rules,
|   to sta...@vger.kernel.org.

I thought I did this, but I'm guessing that you're complaining about:

|   You must note the upstream commit ID in the changelog of your
|   submission.

As mentioned above, there is no corresponding upstream commit, because
that function got removed.

I thought that my contribution was more in the spirit of the stable
series ("small, obvious, correct"); more so than would be the
backporting of the upstream fix.

But it's your call; if you disagree, then you disagree.  My patch will
be here if other people need it.

| - To have the patch automatically included in the stable tree, add
|   the tag
|
| Cc: sta...@vger.kernel.org
|
|   in the sign-off area. Once the patch is merged it will be applied
|   to the stable tree without anything else needing to be done by the
|   author or subsystem maintainer.

I would be happy to resubmit with that modification, if you find the
other aspects of the patch acceptable.

With only the formletter response, I'm unable to determine which bits
you dislike.

| - If the patch requires other patches as prerequisites which can be
|   cherry-picked than this can be specified in the following format
|   [...]

Not applicable

| - The sender will receive an ACK when the patch has been accepted
|   into the queue, or a NAK if the patch is rejected.  This response
|   might take a few days, according to the developer's schedules.

I received a NAK, and it was timely enough -- thank you.  :)

| - If accepted, the patch will be added to the -stable queue, for
|   review by other developers and by the relevant subsystem
|   maintainer.

Presumably not applicable until you tell me why it was rejected; if it
is repairable, I'll be happy to resubmit.

| - Security patches should not be sent to this alias, but instead to the
|   documented secur...@kernel.org address.

Not applicable.

...and I'll not belabor the point.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/1] v3.0.x: mtd: check partition count not partition array pointer

2012-11-30 Thread Anthony Foiani
Greg KH g...@kroah.com writes:
 This is not the correct way to submit patches for inclusion in the
 stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
 for how to do this properly.

My checklist against stable_kernel_rules.txt is below.

I could only find two reasons why you are saying this is incorrect:

  1. There is no matching upstream commit; or

  2. I did not CC: stable in the signed-off region.

Please let me know which it is, so I can either drop the subject (for
reasons indicated further below), or respin the patch.

Thanks,
Tony

| - It must be obviously correct and tested.

Check.

| - It cannot be bigger than 100 lines, with context.

Check: 1 changed line, 2 active lines in the patch, ~10 lines total.

| - It must fix only one thing.

Check.

| - It must fix a real bug that bothers people (not a, This could be
|   a problem... type thing).

It bothered me (made my embedded project not boot correctly).

| - It must fix a problem that causes a build error (but not for
|   things marked CONFIG_BROKEN), an oops, a hang, data corruption, a
|   real security issue, or some oh, that's not good issue.  In
|   short, something critical.

It repaired a change in behavior; for my project that was a oh,
that's not good issue.

It booted, but in such a way that it failed to expose a critical
system resource correctly.

| - Serious issues as reported by a user of a distribution kernel [...]

Not applicable.

| - New device IDs and quirks are also accepted.

Not applicable.

| - No theoretical race condition issues [...]

Not applicable.

| - It cannot contain any trivial fixes in it [...]

Check.

| - It must follow the Documentation/SubmittingPatches rules.

As far as I can tell, this patch follows those rules. 

| - It or an equivalent fix must already exist in Linus' tree (upstream).

As mentioned in the original submission:

 The current kernel has retired this function; I have not examined
 its replacement to see if it has the same issue.

You can either use this 1-line patch, or you can have someone else
backport the changes made in the mainline kernel.  That someone will
not be me, sadly.

| Procedure for submitting patches to the -stable tree:
|
| - Send the patch, after verifying that it follows the above rules,
|   to sta...@vger.kernel.org.

I thought I did this, but I'm guessing that you're complaining about:

|   You must note the upstream commit ID in the changelog of your
|   submission.

As mentioned above, there is no corresponding upstream commit, because
that function got removed.

I thought that my contribution was more in the spirit of the stable
series (small, obvious, correct); more so than would be the
backporting of the upstream fix.

But it's your call; if you disagree, then you disagree.  My patch will
be here if other people need it.

| - To have the patch automatically included in the stable tree, add
|   the tag
|
| Cc: sta...@vger.kernel.org
|
|   in the sign-off area. Once the patch is merged it will be applied
|   to the stable tree without anything else needing to be done by the
|   author or subsystem maintainer.

I would be happy to resubmit with that modification, if you find the
other aspects of the patch acceptable.

With only the formletter response, I'm unable to determine which bits
you dislike.

| - If the patch requires other patches as prerequisites which can be
|   cherry-picked than this can be specified in the following format
|   [...]

Not applicable

| - The sender will receive an ACK when the patch has been accepted
|   into the queue, or a NAK if the patch is rejected.  This response
|   might take a few days, according to the developer's schedules.

I received a NAK, and it was timely enough -- thank you.  :)

| - If accepted, the patch will be added to the -stable queue, for
|   review by other developers and by the relevant subsystem
|   maintainer.

Presumably not applicable until you tell me why it was rejected; if it
is repairable, I'll be happy to resubmit.

| - Security patches should not be sent to this alias, but instead to the
|   documented secur...@kernel.org address.

Not applicable.

...and I'll not belabor the point.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] Documentation: Enumerate the guidelines for stable patches.

2012-11-30 Thread Anthony Foiani

From: Anthony Foiani anthony.foi...@gmail.com

Having recently received a formletter rejection on a stable patch, I
found it difficult to determine exactly which guideline I had missed.

Numbering the guidelines will allow the stable maintainer to quickly
and easily indicate which guidelines have not been followed.

The actual changes are only the addition of clause numbering, and the
wishful thinking added to S15.

Signed-Off-By: Anthony Foiani anthony.foi...@gmail.com
---
 Documentation/stable_kernel_rules.txt | 185 --
 1 file changed, 108 insertions(+), 77 deletions(-)

diff --git a/Documentation/stable_kernel_rules.txt 
b/Documentation/stable_kernel_rules.txt
index b0714d8..9a6dc59 100644
--- a/Documentation/stable_kernel_rules.txt
+++ b/Documentation/stable_kernel_rules.txt
@@ -1,95 +1,126 @@
 Everything you ever wanted to know about Linux -stable releases.
 
-Rules on what kind of patches are accepted, and which ones are not, into the
--stable tree:
-
- - It must be obviously correct and tested.
- - It cannot be bigger than 100 lines, with context.
- - It must fix only one thing.
- - It must fix a real bug that bothers people (not a, This could be a
-   problem... type thing).
- - It must fix a problem that causes a build error (but not for things
-   marked CONFIG_BROKEN), an oops, a hang, data corruption, a real
-   security issue, or some oh, that's not good issue.  In short, something
-   critical.
- - Serious issues as reported by a user of a distribution kernel may also
-   be considered if they fix a notable performance or interactivity issue.
-   As these fixes are not as obvious and have a higher risk of a subtle
-   regression they should only be submitted by a distribution kernel
-   maintainer and include an addendum linking to a bugzilla entry if it
-   exists and additional information on the user-visible impact.
- - New device IDs and quirks are also accepted.
- - No theoretical race condition issues, unless an explanation of how the
-   race can be exploited is also provided.
- - It cannot contain any trivial fixes in it (spelling changes,
-   whitespace cleanups, etc).
- - It must follow the Documentation/SubmittingPatches rules.
- - It or an equivalent fix must already exist in Linus' tree (upstream).
+Rules on what kind of patches are accepted, and which ones are not,
+into the -stable tree.
 
+S1.  It must be obviously correct and tested.
+
+S2.  It cannot be bigger than 100 lines, with context.
+
+S3.  It must fix only one thing.
+
+S4.  It must fix a real bug that bothers people (not a, This could be
+ a problem... type thing).
+
+S5.  It must fix a problem that causes a build error (but not for
+ things marked CONFIG_BROKEN), an oops, a hang, data corruption, a
+ real security issue, or some oh, that's not good issue.  In
+ short, something critical.
+
+S6.  Serious issues as reported by a user of a distribution kernel may
+ also be considered if they fix a notable performance or
+ interactivity issue.  As these fixes are not as obvious and have
+ a higher risk of a subtle regression they should only be
+ submitted by a distribution kernel maintainer and include an
+ addendum linking to a bugzilla entry if it exists and additional
+ information on the user-visible impact.
+
+S7.  New device IDs and quirks are also accepted.
+
+S8.  No theoretical race condition issues, unless an explanation of
+ how the race can be exploited is also provided.
+
+S9.  It cannot contain any trivial fixes in it (spelling changes,
+ whitespace cleanups, etc).
+
+S10. It must follow the Documentation/SubmittingPatches rules.
+
+S11. It or an equivalent fix must already exist in Linus' tree
+ (upstream).
 
 Procedure for submitting patches to the -stable tree:
 
- - Send the patch, after verifying that it follows the above rules, to
-   sta...@vger.kernel.org.  You must note the upstream commit ID in the
-   changelog of your submission, as well as the kernel version you wish
-   it to be applied to.
- - To have the patch automatically included in the stable tree, add the tag
- Cc: sta...@vger.kernel.org
-   in the sign-off area. Once the patch is merged it will be applied to
-   the stable tree without anything else needing to be done by the author
-   or subsystem maintainer.
- - If the patch requires other patches as prerequisites which can be
-   cherry-picked than this can be specified in the following format in
-   the sign-off area:
-
- Cc: sta...@vger.kernel.org # 3.3.x: a1f84a3: sched: Check for idle
- Cc: sta...@vger.kernel.org # 3.3.x: 1b9508f: sched: Rate-limit newidle
- Cc: sta...@vger.kernel.org # 3.3.x: fd21073: sched: Fix affinity logic
- Cc: sta...@vger.kernel.org # 3.3.x
-Signed-off-by: Ingo Molnar mi...@elte.hu
-
-   The tag sequence has the meaning of:
- git cherry-pick a1f84a3
- git cherry-pick 1b9508f
- git cherry-pick fd21073
- git cherry-pick

Re: [PATCH 1/1] v3.0.x: mtd: check partition count not partition array pointer

2012-11-30 Thread Anthony Foiani

Greg --

Thanks for the very quick response.

Greg KH g...@kroah.com writes:
 This is an obvious one, it needs to be upstream first.

 Or if not, a whole lot of explaination saying that you know it
 isn't, and why it isn't, and why it isn't applicable there,

I thought that I did provide exactly this information, when I
indicated that this function no longer even existed in upstream.

The replacement in upstream was to fold this functionality into
another function -- but I found that function much more complex and
difficult to follow.

So I went with the simple fix.

 including agreement from the subsystem maintainers saying that they
 agree with you.

I also don't have that.  I might post it to the mtd list, to see if
they have any interest in either approving it as-is for 3.0 series, or
doing the backport of the current code.

Thanks again for your time.

Tony
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Documentation: Enumerate the guidelines for stable patches.

2012-11-30 Thread Anthony Foiani
Greg KH gre...@linuxfoundation.org writes:

 Is this really needed?  For the large majority of the stable
 patches, specifically enumerating this isn't a big deal, it's a tiny
 patch, and if you think I'll remember to tell you which specific
 clause you didn't follow, then you think I have more spare time than
 I really do.

Yeah, it was largely tongue-in-cheek.  :)

Although a hint of not upstream would have been helpful.  You
obviously have the checklist in your head, and you (presumably) have a
your formletter automated; it just seems that, as busy as the
maintainers are, trading an extra 2s of your time for an hour of a
contributor's time would sometimes be the right thing.

Regardless, you're the one doing the work, not me.  (And you're an
outright angel, compared to some other high-level maintainers, so I
should simply count myself lucky.)

 Sorry, but no, I don't think this patch is ok, especially that S15
 clause, nice try :)

Can't fault a guy for trying.  (Or, hopefully, can't fault him much...)

Have a good weekend,
t.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 1/1] v3.0.x: mtd: check partition count not partition array pointer

2012-11-07 Thread Anthony Foiani

mtd: check partition count not partition array pointer

The documentation claims that "nr_parts" is the determining factor,
while the code originally tested whether "parts" is non-null.

In at least one driver (fsl_elbc_nand), parts is never initialized to
0; even though nr_parts is correctly 0, add_mtd_partitions still tries
to create 0 partitions.)

Make the code adhere to the documentation.

A quick scan of all uses in the 3.0.51 kernel show that they correctly
rely on nr_parts rather than parts.

The current kernel has retired this function; I have not examined its
replacement to see if it has the same issue.

Signed-Off-By: Anthony Foiani 
---
 drivers/mtd/mtdcore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index c510aff..ac624df 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -446,7 +446,7 @@ int mtd_device_register(struct mtd_info *master,
const struct mtd_partition *parts,
int nr_parts)
 {
-   return parts ? add_mtd_partitions(master, parts, nr_parts) :
+   return nr_parts ? add_mtd_partitions(master, parts, nr_parts) :
add_mtd_device(master);
 }
 EXPORT_SYMBOL_GPL(mtd_device_register);
--
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 1/1] v3.0.x: mtd: check partition count not partition array pointer

2012-11-07 Thread Anthony Foiani

mtd: check partition count not partition array pointer

The documentation claims that nr_parts is the determining factor,
while the code originally tested whether parts is non-null.

In at least one driver (fsl_elbc_nand), parts is never initialized to
0; even though nr_parts is correctly 0, add_mtd_partitions still tries
to create 0 partitions.)

Make the code adhere to the documentation.

A quick scan of all uses in the 3.0.51 kernel show that they correctly
rely on nr_parts rather than parts.

The current kernel has retired this function; I have not examined its
replacement to see if it has the same issue.

Signed-Off-By: Anthony Foiani anthony.foi...@gmail.com
---
 drivers/mtd/mtdcore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index c510aff..ac624df 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -446,7 +446,7 @@ int mtd_device_register(struct mtd_info *master,
const struct mtd_partition *parts,
int nr_parts)
 {
-   return parts ? add_mtd_partitions(master, parts, nr_parts) :
+   return nr_parts ? add_mtd_partitions(master, parts, nr_parts) :
add_mtd_device(master);
 }
 EXPORT_SYMBOL_GPL(mtd_device_register);
--
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/