Re: [PATCH] mmc: make number of mmcblk minors configurable

[Olof Johansson <olof@lixom.net>]

Hi,

On Wed, Aug 18, 2010 at 05:16:36PM -0700, Andrew Morton wrote:
> On Tue, 17 Aug 2010 23:13:33 -0500
> Olof Johansson <olof@lixom.net> wrote:
> 
> > The old limit of number of minor numbers per mmcblk device was hardcoded
> > at 8. This isn't enough for some of the more elaborate partitioning
> > schemes, for example those used by Chrome OS.
> > 
> > Since there might be a bunch of systems out there with static /dev
> > contents that relies on the old numbering scheme, let's make it a
> > build-time option with the default set to the previous 8.
> > 
> 
> How does the numbering scheme change?  What's the user-visible effect
> of setting CONFIG_MMC_BLOCK_MINORS to, say, 16?

mmcblk0 will be 0, mmcblk0p1 1, etc. mmcblk1 (second device) will be
bumped up to start at minor 16 instead of 8 before, which is why I didn't
just bump up the static default and went with a config option instead.

FWIW, udev will happily keep creating device nodes for higher partitions,
so without this patch you end up with overlapping device entries where
mmcblk1p1 and mmcblk0p9 will be the same major/minor.

> > ---
> >  drivers/mmc/card/Kconfig |   10 ++++++++++
> >  drivers/mmc/card/block.c |   20 ++++++++------------
> >  2 files changed, 18 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/mmc/card/Kconfig b/drivers/mmc/card/Kconfig
> > index 3f2a912..e939dcf 100644
> > --- a/drivers/mmc/card/Kconfig
> > +++ b/drivers/mmc/card/Kconfig
> > @@ -14,6 +14,16 @@ config MMC_BLOCK
> >  	  mount the filesystem. Almost everyone wishing MMC support
> >  	  should say Y or M here.
> >  
> > +config MMC_BLOCK_MINORS
> > +	int "Number of minors per block device"
> > +	range 4 32
> > +	default 8
> > +	help
> > +	  Number of minors per block device. One is needed for every
> > +	  partition (plus one for the whole device).
> > +	  Default is 8 to be backwards compatible with previous
> > +	  hardcoded device numbering.
> 
> If possible it would be better to do this with a kernel boot (or
> modprobe) parameter.  That permits vendors to ship packaged kernels
> with full functionality, etc.  Possible?  The default value of the
> runtime-settable parameter could be Kconfigurable is there's a real
> need for that.

I considered just doing a runtime option, but it would be suboptimal in
our case (Chrome OS) since it would mean that we had yet another kernel
arg we always have to include on all platforms that boot from mmc devices.
Modprobe args won't work there because the driver is statically linked in
(and no ramdisk).

I can definitely change it to take the default from the Kconfig and make
it possible to override with boot/modprobe parameters though. Revised
patch to follow.

> >  config MMC_BLOCK_BOUNCE
> >  	bool "Use bounce buffer for simple hosts"
> >  	depends on MMC_BLOCK
> > diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> > index d545f79..524e232 100644
> > --- a/drivers/mmc/card/block.c
> > +++ b/drivers/mmc/card/block.c
> > @@ -45,13 +45,9 @@
> >  
> >  MODULE_ALIAS("mmc:block");
> >  
> > -/*
> > - * max 8 partitions per card
> > - */
> > -#define MMC_SHIFT	3
> > -#define MMC_NUM_MINORS	(256 >> MMC_SHIFT)
> > +#define MMC_MAX_DEVICES	((255 + CONFIG_MMC_BLOCK_MINORS) / CONFIG_MMC_BLOCK_MINORS)
> 
> So if I increase CONFIG_MMC_BLOCK_MINORS, I can no longer support as
> many devices?  That'd be worth mentioning somewhere?

Yep, I'll add a comment and clarify it in the Kconfig help.

> fwiw, the above could use DIV_ROUND_UP().

Good point, will do.


-Olof