[RFC] mmc_block: Allow more than 8 partitions per card

Commit Message

John Stultz Oct. 16, 2015, 6:40 p.m. UTC

From: Colin Cross <[e-mail]>

It is quite common for Android devices to utilize more
then 8 partitions on internal eMMC storage. This patch,
which has been carried for quite awhile in the AOSP common
tree is necessary in order to support such configurations,
so I wanted to submit it for consideration upstream.

This patch sets the GENHD_FL_EXT_DEVT flag, which will
allocate minor number in major 259 for partitions past
disk->minors.

It also removes the use of disk_devt to determine devidx
from md->disk. md->disk->first_minor is always initialized
from devidx and can always be used to recover it.

Thoughts or feedback would be greatly appreciated.

Cc: Ulf Hansson <[e-mail]>
Cc: Adrian Hunter <[e-mail]>
Cc: Ben Hutchings <[e-mail]>
Cc: Chuanxiao Dong <[e-mail]>
Cc: Shawn Lin <[e-mail]>
Cc: Android Kernel Team <[e-mail]>
Cc: [e-mail]
Signed-off-by: Colin Cross <[e-mail]>
[jstultz: Added context to commit message]
Signed-off-by: John Stultz <[e-mail]>
---
 drivers/mmc/card/block.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Austin S. Hemmelgarn Oct. 16, 2015, 7:03 p.m. UTC | #1

On 2015-10-16 14:40, John Stultz wrote:
> From: Colin Cross <[e-mail]>
>
> It is quite common for Android devices to utilize more
> then 8 partitions on internal eMMC storage. This patch,
> which has been carried for quite awhile in the AOSP common
> tree is necessary in order to support such configurations,
> so I wanted to submit it for consideration upstream.
Isn't this what CONFIG_MMC_BLOCK_MINORS is for?  It does limit you to 
256 minors total, and therefore the number of supported MMC's is equal 
to 256/CONFIG_MMC_BLOCK_MINORS, but I've never heard of an Android 
device with support for more than 4 MMC/SD cards (including eMMC's), and 
I would seriously question anyone who has the need for more than 32 
partitions on the root device for a phone/tablet/television.

Arnd Bergmann Oct. 16, 2015, 7:08 p.m. UTC | #2

On Friday 16 October 2015 15:03:29 Austin S Hemmelgarn wrote:
> On 2015-10-16 14:40, John Stultz wrote:
> > From: Colin Cross <[e-mail]>
> >
> > It is quite common for Android devices to utilize more
> > then 8 partitions on internal eMMC storage. This patch,
> > which has been carried for quite awhile in the AOSP common
> > tree is necessary in order to support such configurations,
> > so I wanted to submit it for consideration upstream.
>
> Isn't this what CONFIG_MMC_BLOCK_MINORS is for?  It does limit you to 
> 256 minors total, and therefore the number of supported MMC's is equal 
> to 256/CONFIG_MMC_BLOCK_MINORS, but I've never heard of an Android 
> device with support for more than 4 MMC/SD cards (including eMMC's), and 
> I would seriously question anyone who has the need for more than 32 
> partitions on the root device for a phone/tablet/television.

The 256 minor limit is really arbitrary, we should not be limited by
that, and you should have have to decide at compile time whether you
might need many partitions or many devices, even if we believe that
nobody will ever need both at the same time.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Austin S. Hemmelgarn Oct. 16, 2015, 7:16 p.m. UTC | #3

On 2015-10-16 15:08, Arnd Bergmann wrote:
> On Friday 16 October 2015 15:03:29 Austin S Hemmelgarn wrote:
>> On 2015-10-16 14:40, John Stultz wrote:
>>> From: Colin Cross <[e-mail]>
>>>
>>> It is quite common for Android devices to utilize more
>>> then 8 partitions on internal eMMC storage. This patch,
>>> which has been carried for quite awhile in the AOSP common
>>> tree is necessary in order to support such configurations,
>>> so I wanted to submit it for consideration upstream.
>>
>> Isn't this what CONFIG_MMC_BLOCK_MINORS is for?  It does limit you to
>> 256 minors total, and therefore the number of supported MMC's is equal
>> to 256/CONFIG_MMC_BLOCK_MINORS, but I've never heard of an Android
>> device with support for more than 4 MMC/SD cards (including eMMC's), and
>> I would seriously question anyone who has the need for more than 32
>> partitions on the root device for a phone/tablet/television.
>
> The 256 minor limit is really arbitrary, we should not be limited by
> that, and you should have have to decide at compile time whether you
> might need many partitions or many devices, even if we believe that
> nobody will ever need both at the same time.
I didn't mean to imply that I disagree with any of that, I was just 
trying to get the point across that framing the problem this is trying 
to solve in such limited terms really isn't the best idea, although I 
could have gone about doing so in a better way.

John Stultz Oct. 16, 2015, 7:31 p.m. UTC | #4

On Fri, Oct 16, 2015 at 12:03 PM, Austin S Hemmelgarn
<[e-mail]> wrote:
> On 2015-10-16 14:40, John Stultz wrote:
>>
>> From: Colin Cross <[e-mail]>
>>
>> It is quite common for Android devices to utilize more
>> then 8 partitions on internal eMMC storage. This patch,
>> which has been carried for quite awhile in the AOSP common
>> tree is necessary in order to support such configurations,
>> so I wanted to submit it for consideration upstream.
>
> Isn't this what CONFIG_MMC_BLOCK_MINORS is for?  It does limit you to 256
> minors total, and therefore the number of supported MMC's is equal to
> 256/CONFIG_MMC_BLOCK_MINORS, but I've never heard of an Android device with
> support for more than 4 MMC/SD cards (including eMMC's), and I would
> seriously question anyone who has the need for more than 32 partitions on
> the root device for a phone/tablet/television.

So thanks, CONFIG_MMC_BLOCK_MINORS is something I wasn't aware of, and
it does seem to allow me to boot on my device w/o the proposed patch!

That said, my fairly old device does have 30-partitions now, and from
Linux Plumber's[1] it seems like Android devices are only going to
have more partitions in the future. So as Arnd commented, if the
proposed patch provides a less limited solution, it might still be
worth consideration.  I'll give it a day or so for folks to provide
any more commentary, and if no other objections are brought up, I'll
re-submit with an improve rational for the patch that includes this
info.

Thanks for the feedback!
-john

[1]: https://linuxplumbersconf.org/2015/ocw//system/presentations/3387/original/Android%20system,%20partitions%20and%20customization.pdf
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index c742cfd..564436e 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -172,11 +172,7 @@  static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk)
 
 static inline int mmc_get_devidx(struct gendisk *disk)
 {
-	int devmaj = MAJOR(disk_devt(disk));
-	int devidx = MINOR(disk_devt(disk)) / perdev_minors;
-
-	if (!devmaj)
-		devidx = disk->first_minor / perdev_minors;
+	int devidx = disk->first_minor / perdev_minors;
 	return devidx;
 }
 
@@ -2162,6 +2158,7 @@  static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	md->disk->queue = md->queue.queue;
 	md->disk->driverfs_dev = parent;
 	set_disk_ro(md->disk, md->read_only || default_ro);
+	md->disk->flags = GENHD_FL_EXT_DEVT;
 	if (area_type & (MMC_BLK_DATA_AREA_RPMB | MMC_BLK_DATA_AREA_BOOT))
 		md->disk->flags |= GENHD_FL_NO_PART_SCAN;