diff mbox

Revert "mmc: block: don't use parameter prefix if built as module"

Message ID 1455206051-8633-1-git-send-email-ulf.hansson@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Ulf Hansson Feb. 11, 2016, 3:54 p.m. UTC
This reverts commit 829b6962f7e3cfc06f7c5c26269fd47ad48cf503.

Revert this change as it causes a sysfs path to change and therefore
introduces and ABI regression. More precisely Android's vold is not being
able to access /sys/module/mmcblk/parameters/perdev_minors any more, since
the path becomes changed to: "/sys/module/mmc_block/..."

Fixes: 829b6962f7e3 ("mmc: block: don't use parameter prefix if built as
module")
Reported-by: John Stultz <john.stultz@linaro.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/card/block.c | 3 ---
 1 file changed, 3 deletions(-)

Comments

Ulf Hansson Feb. 11, 2016, 3:57 p.m. UTC | #1
On 11 February 2016 at 16:54, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> This reverts commit 829b6962f7e3cfc06f7c5c26269fd47ad48cf503.
>
> Revert this change as it causes a sysfs path to change and therefore
> introduces and ABI regression. More precisely Android's vold is not being
> able to access /sys/module/mmcblk/parameters/perdev_minors any more, since
> the path becomes changed to: "/sys/module/mmc_block/..."
>
> Fixes: 829b6962f7e3 ("mmc: block: don't use parameter prefix if built as
> module")
> Reported-by: John Stultz <john.stultz@linaro.org>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

John, I noticed your regression report. Thanks!

I decided to send this patch and I have already queued it for fixes.

Sorry for not thinking clear when I applied the earlier patch.

Kind regards
Uffe

> ---
>  drivers/mmc/card/block.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 951641a..fe207e5 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -47,13 +47,10 @@
>  #include "queue.h"
>
>  MODULE_ALIAS("mmc:block");
> -
> -#ifdef KERNEL
>  #ifdef MODULE_PARAM_PREFIX
>  #undef MODULE_PARAM_PREFIX
>  #endif
>  #define MODULE_PARAM_PREFIX "mmcblk."
> -#endif
>
>  #define INAND_CMD38_ARG_EXT_CSD  113
>  #define INAND_CMD38_ARG_ERASE    0x00
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg Kroah-Hartman Feb. 11, 2016, 5:19 p.m. UTC | #2
On Thu, Feb 11, 2016 at 04:54:11PM +0100, Ulf Hansson wrote:
> This reverts commit 829b6962f7e3cfc06f7c5c26269fd47ad48cf503.
> 
> Revert this change as it causes a sysfs path to change and therefore
> introduces and ABI regression. More precisely Android's vold is not being
> able to access /sys/module/mmcblk/parameters/perdev_minors any more, since
> the path becomes changed to: "/sys/module/mmc_block/..."
> 
> Fixes: 829b6962f7e3 ("mmc: block: don't use parameter prefix if built as
> module")
> Reported-by: John Stultz <john.stultz@linaro.org>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Please also add a "cc: stable..." tag to the patch so it gets picked up
in stable kernel releases.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
John Stultz Feb. 11, 2016, 9:52 p.m. UTC | #3
On Thu, Feb 11, 2016 at 7:57 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 11 February 2016 at 16:54, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> This reverts commit 829b6962f7e3cfc06f7c5c26269fd47ad48cf503.
>>
>> Revert this change as it causes a sysfs path to change and therefore
>> introduces and ABI regression. More precisely Android's vold is not being
>> able to access /sys/module/mmcblk/parameters/perdev_minors any more, since
>> the path becomes changed to: "/sys/module/mmc_block/..."
>>
>> Fixes: 829b6962f7e3 ("mmc: block: don't use parameter prefix if built as
>> module")
>> Reported-by: John Stultz <john.stultz@linaro.org>
>> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>
> John, I noticed your regression report. Thanks!
>
> I decided to send this patch and I have already queued it for fixes.
>
> Sorry for not thinking clear when I applied the earlier patch.

Don't worry, those sorts of changes are subtle. I've seen a lot of
funny sysfs path breakage recently, but usually they are in paths that
are device specific (often related to the paths I think changed in the
dts - ie: lots of paths now have .../soc/... in them), so its not too
hard to just fix it for the device.

This one was more painful because it was generic path across all
devices. So I'm just glad to have caught it before the new behavior
became established and we'd be a bit stuck having to support both.

thanks
-john
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson Feb. 12, 2016, 10:06 a.m. UTC | #4
On 11 February 2016 at 18:19, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, Feb 11, 2016 at 04:54:11PM +0100, Ulf Hansson wrote:
>> This reverts commit 829b6962f7e3cfc06f7c5c26269fd47ad48cf503.
>>
>> Revert this change as it causes a sysfs path to change and therefore
>> introduces and ABI regression. More precisely Android's vold is not being
>> able to access /sys/module/mmcblk/parameters/perdev_minors any more, since
>> the path becomes changed to: "/sys/module/mmc_block/..."
>>
>> Fixes: 829b6962f7e3 ("mmc: block: don't use parameter prefix if built as
>> module")
>> Reported-by: John Stultz <john.stultz@linaro.org>
>> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>
> Please also add a "cc: stable..." tag to the patch so it gets picked up
> in stable kernel releases.

Doesn't the Fixes tag take care of that?

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg Kroah-Hartman Feb. 12, 2016, 4:32 p.m. UTC | #5
On Fri, Feb 12, 2016 at 11:06:03AM +0100, Ulf Hansson wrote:
> On 11 February 2016 at 18:19, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Thu, Feb 11, 2016 at 04:54:11PM +0100, Ulf Hansson wrote:
> >> This reverts commit 829b6962f7e3cfc06f7c5c26269fd47ad48cf503.
> >>
> >> Revert this change as it causes a sysfs path to change and therefore
> >> introduces and ABI regression. More precisely Android's vold is not being
> >> able to access /sys/module/mmcblk/parameters/perdev_minors any more, since
> >> the path becomes changed to: "/sys/module/mmc_block/..."
> >>
> >> Fixes: 829b6962f7e3 ("mmc: block: don't use parameter prefix if built as
> >> module")
> >> Reported-by: John Stultz <john.stultz@linaro.org>
> >> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> >
> > Please also add a "cc: stable..." tag to the patch so it gets picked up
> > in stable kernel releases.
> 
> Doesn't the Fixes tag take care of that?

Not at all, never rely on that, please read
Documentation/stable_kernel_rules.txt for how to properly tag a patch
for a stable release.

Sometimes I get bored and look at patches with only a fixes: tag on them
to see how bad the maintainer is messing up and then do their work for
them, but that's rare these days...

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson March 3, 2016, 1:37 p.m. UTC | #6
On 12 February 2016 at 17:32, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Feb 12, 2016 at 11:06:03AM +0100, Ulf Hansson wrote:
>> On 11 February 2016 at 18:19, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > On Thu, Feb 11, 2016 at 04:54:11PM +0100, Ulf Hansson wrote:
>> >> This reverts commit 829b6962f7e3cfc06f7c5c26269fd47ad48cf503.
>> >>
>> >> Revert this change as it causes a sysfs path to change and therefore
>> >> introduces and ABI regression. More precisely Android's vold is not being
>> >> able to access /sys/module/mmcblk/parameters/perdev_minors any more, since
>> >> the path becomes changed to: "/sys/module/mmc_block/..."
>> >>
>> >> Fixes: 829b6962f7e3 ("mmc: block: don't use parameter prefix if built as
>> >> module")
>> >> Reported-by: John Stultz <john.stultz@linaro.org>
>> >> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> >
>> > Please also add a "cc: stable..." tag to the patch so it gets picked up
>> > in stable kernel releases.
>>
>> Doesn't the Fixes tag take care of that?
>
> Not at all, never rely on that, please read
> Documentation/stable_kernel_rules.txt for how to properly tag a patch
> for a stable release.
>
> Sometimes I get bored and look at patches with only a fixes: tag on them
> to see how bad the maintainer is messing up and then do their work for
> them, but that's rare these days...

That's sounds like you do this entirely manually, I doubt you have
time for that? :-)

So, isn't it quite simple to automate this thing, as all the
information you need (ideally) is to know what commit is being fixed.
Right?

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg Kroah-Hartman March 3, 2016, 3:58 p.m. UTC | #7
On Thu, Mar 03, 2016 at 02:37:04PM +0100, Ulf Hansson wrote:
> On 12 February 2016 at 17:32, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Fri, Feb 12, 2016 at 11:06:03AM +0100, Ulf Hansson wrote:
> >> On 11 February 2016 at 18:19, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> > On Thu, Feb 11, 2016 at 04:54:11PM +0100, Ulf Hansson wrote:
> >> >> This reverts commit 829b6962f7e3cfc06f7c5c26269fd47ad48cf503.
> >> >>
> >> >> Revert this change as it causes a sysfs path to change and therefore
> >> >> introduces and ABI regression. More precisely Android's vold is not being
> >> >> able to access /sys/module/mmcblk/parameters/perdev_minors any more, since
> >> >> the path becomes changed to: "/sys/module/mmc_block/..."
> >> >>
> >> >> Fixes: 829b6962f7e3 ("mmc: block: don't use parameter prefix if built as
> >> >> module")
> >> >> Reported-by: John Stultz <john.stultz@linaro.org>
> >> >> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> >> >
> >> > Please also add a "cc: stable..." tag to the patch so it gets picked up
> >> > in stable kernel releases.
> >>
> >> Doesn't the Fixes tag take care of that?
> >
> > Not at all, never rely on that, please read
> > Documentation/stable_kernel_rules.txt for how to properly tag a patch
> > for a stable release.
> >
> > Sometimes I get bored and look at patches with only a fixes: tag on them
> > to see how bad the maintainer is messing up and then do their work for
> > them, but that's rare these days...
> 
> That's sounds like you do this entirely manually, I doubt you have
> time for that? :-)

Right now, no, I don't, and because of that, I just ignored a few
hundred patches that had this tag on it but not a stable@ tag.  Most of
them probably were not relevant for a stable release, based on the usual
numbers, but possibly some were.  Oh well.

> So, isn't it quite simple to automate this thing, as all the
> information you need (ideally) is to know what commit is being fixed.
> Right?

Yes, and I have it semi-automated, but still you need to look at each
patch manually to ensure that they really do meet the stable kernel
rules.  That's not something that anyone has figured out how to automate
(if so, I would love it as my work here would be trivial!)

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 951641a..fe207e5 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -47,13 +47,10 @@ 
 #include "queue.h"
 
 MODULE_ALIAS("mmc:block");
-
-#ifdef KERNEL
 #ifdef MODULE_PARAM_PREFIX
 #undef MODULE_PARAM_PREFIX
 #endif
 #define MODULE_PARAM_PREFIX "mmcblk."
-#endif
 
 #define INAND_CMD38_ARG_EXT_CSD  113
 #define INAND_CMD38_ARG_ERASE    0x00