Re: [PATCH 1/3] pmem: Initial version of persistent memory driver

[Ross Zwisler <ross.zwisler@linux.intel.com>]

On Thu, 2015-03-26 at 09:06 +0100, Christoph Hellwig wrote:
> On Wed, Mar 25, 2015 at 02:21:53PM -0600, Ross Zwisler wrote:
> > What needed to be fixed with the partition support?  I used to have real
> > numbers for first_minor and passed into alloc_disk(), but simplified it based
> > on code found in this commit in the nvme driver:
> > 
> > 469071a37afc NVMe: Dynamically allocate partition numbers
> > 
> > This has worked fine for me - is there some test case in which it breaks?
> 
> Yes, if CONFIG_DEBUG_BLOCK_EXT_DEVT isn't set that code doesn't work at all.

I can't figure out a use case that breaks when using dynamically allocated
minors without CONFIG_DEBUG_BLOCK_EXT_DEVT.  The patch that I've been testing
against is at the bottom of this mail.

Here are the minors that I get when creating a bunch of partitions using the
current code with PMEM_MINORS=16, with CONFIG_DEBUG_BLOCK_EXT_DEVT turned off:

pmem0      249:0    0 63.5G  0 rom  
├─pmem0p1  249:1    0    1G  0 part 
├─pmem0p2  249:2    0    1G  0 part 
├─pmem0p3  249:3    0    1G  0 part 
├─pmem0p4  249:4    0    1G  0 part 
├─pmem0p5  249:5    0    1G  0 part 
├─pmem0p6  249:6    0    1G  0 part 
├─pmem0p7  249:7    0    1G  0 part 
├─pmem0p8  249:8    0    1G  0 part 
├─pmem0p9  249:9    0    1G  0 part 
├─pmem0p10 249:10   0    1G  0 part 
├─pmem0p11 249:11   0    1G  0 part 
├─pmem0p12 249:12   0    1G  0 part 
├─pmem0p13 249:13   0    1G  0 part 
├─pmem0p14 249:14   0    1G  0 part 
├─pmem0p15 249:15   0    1G  0 part 
├─pmem0p16 259:0    0    1G  0 part 
├─pmem0p17 259:1    0    1G  0 part 
└─pmem0p18 259:2    0    1G  0 part 

With dynamic minor allocation, with CONFIG_DEBUG_BLOCK_EXT_DEVT turned off:

pmem0      259:0    0 63.5G  0 rom  
├─pmem0p1  259:1    0    1G  0 part 
├─pmem0p2  259:2    0    1G  0 part 
├─pmem0p3  259:3    0    1G  0 part 
├─pmem0p4  259:4    0    1G  0 part 
├─pmem0p5  259:5    0    1G  0 part 
├─pmem0p6  259:6    0    1G  0 part 
├─pmem0p7  259:7    0    1G  0 part 
├─pmem0p8  259:8    0    1G  0 part 
├─pmem0p9  259:9    0    1G  0 part 
├─pmem0p10 259:10   0    1G  0 part 
├─pmem0p11 259:11   0    1G  0 part 
├─pmem0p12 259:12   0    1G  0 part 
├─pmem0p13 259:13   0    1G  0 part 
├─pmem0p14 259:14   0    1G  0 part 
├─pmem0p15 259:15   0    1G  0 part 
├─pmem0p16 259:16   0    1G  0 part 
├─pmem0p17 259:17   0    1G  0 part 
└─pmem0p18 259:18   0    1G  0 part

And with CONFIG_DEBUG_BLOCK_EXT_DEVT turned on:

pmem0      259:262144  0 63.5G  0 rom  
├─pmem0p1  259:786432  0    1G  0 part 
├─pmem0p2  259:131072  0    1G  0 part 
├─pmem0p3  259:655360  0    1G  0 part 
├─pmem0p4  259:393216  0    1G  0 part 
├─pmem0p5  259:917504  0    1G  0 part 
├─pmem0p6  259:65536   0    1G  0 part 
├─pmem0p7  259:589824  0    1G  0 part 
├─pmem0p8  259:327680  0    1G  0 part 
├─pmem0p9  259:851968  0    1G  0 part 
├─pmem0p10 259:196608  0    1G  0 part 
├─pmem0p11 259:720896  0    1G  0 part 
├─pmem0p12 259:458752  0    1G  0 part 
├─pmem0p13 259:983040  0    1G  0 part 
├─pmem0p14 259:32768   0    1G  0 part 
├─pmem0p15 259:557056  0    1G  0 part 
├─pmem0p16 259:294912  0    1G  0 part 
├─pmem0p17 259:819200  0    1G  0 part 
└─pmem0p18 259:163840  0    1G  0 part

With CONFIG_DEBUG_BLOCK_EXT_DEVT the minors are all mangled due to
blk_mangle_minor(), but I think that all three configs work?

Was there maybe confusion between that config option and the GENHD_FL_EXT_DEVT
gendisk flag, which AFAIK are independent?

Is there a use case that breaks when using dynamic minors without
CONFIG_DEBUG_BLOCK_EXT_DEVT?

Thanks,
- Ross

--- >8 ---
>From 6202dc7c1ef765faebb905161860c6b9ab19cc8a Mon Sep 17 00:00:00 2001
From: Ross Zwisler <ross.zwisler@linux.intel.com>
Date: Mon, 4 May 2015 10:26:54 -0600
Subject: [PATCH] pmem: Dynamically allocate partition numbers

Dynamically allocate minor numbers for partitions instead of statically
preallocating them.

Inspired by this commit:

469071a37afc NVMe: Dynamically allocate partition numbers

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 drivers/block/nd/pmem.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/block/nd/pmem.c b/drivers/block/nd/pmem.c
index 900dad61a6b9..b977def8981e 100644
--- a/drivers/block/nd/pmem.c
+++ b/drivers/block/nd/pmem.c
@@ -26,8 +26,6 @@
 #include <linux/nd.h>
 #include "nd.h"
 
-#define PMEM_MINORS		16
-
 struct pmem_device {
 	struct request_queue	*pmem_queue;
 	struct gendisk		*pmem_disk;
@@ -185,12 +183,12 @@ static struct pmem_device *pmem_alloc(struct device *dev, struct resource *res)
 	blk_queue_max_hw_sectors(pmem->pmem_queue, 1024);
 	blk_queue_bounce_limit(pmem->pmem_queue, BLK_BOUNCE_ANY);
 
-	disk = alloc_disk(PMEM_MINORS);
+	disk = alloc_disk(0);
 	if (!disk)
 		goto out_free_queue;
 
 	disk->major		= pmem_major;
-	disk->first_minor	= PMEM_MINORS * pmem->id;
+	disk->first_minor	= 0;
 	disk->fops		= &pmem_fops;
 	disk->private_data	= pmem;
 	disk->queue		= pmem->pmem_queue;
-- 
1.9.3