On Tue, Jan 14, 2020 at 05:16:57PM +0800, Ming Lei wrote: > On Mon, Jan 13, 2020 at 09:43:23PM +0800, Zhiqiang Liu wrote: > > In brd_init func, rd_nr num of brd_device are firstly allocated > > and add in brd_devices, then brd_devices are traversed to add each > > brd_device by calling add_disk func. When allocating brd_device, > > the disk->first_minor is set to i * max_part, if rd_nr * max_part > > is larger than MINORMASK, two different brd_device may have the same > > devt, then only one of them can be successfully added. > > It is just because disk->first_minor is >= 0x100000, then same dev_t > can be allocated in blk_alloc_devt(). > > MKDEV(disk->major, disk->first_minor + part->partno) > > But block layer does support extended dynamic devt allocation, and brd > sets flag of GENHD_FL_EXT_DEVT too. > > So I think the correct fix is to fallback to extended dynamic allocation > when running out of consecutive minor space. > > How about the following approach? > > And of course, ext devt allocation may fail too, but that is another > generic un-solved issue: error handling isn't done for adding disk. > > diff --git a/drivers/block/brd.c b/drivers/block/brd.c > index a8730cc4db10..9aa7ce7c9abf 100644 > --- a/drivers/block/brd.c > +++ b/drivers/block/brd.c > @@ -398,7 +398,16 @@ static struct brd_device *brd_alloc(int i) > if (!disk) > goto out_free_queue; > disk->major = RAMDISK_MAJOR; > - disk->first_minor = i * max_part; > + > + /* > + * Clear .minors when running out of consecutive minor space since > + * GENHD_FL_EXT_DEVT is set, and we can allocate from extended devt > + */ > + if ((i * disk->minors) & ~MINORMASK) > + disk->minors = 0; > + else > + disk->first_minor = i * disk->minors; > + > disk->fops = &brd_fops; > disk->private_data = brd; > disk->flags = GENHD_FL_EXT_DEVT; But still suggest to limit 'max_part' <= 256, and the name is actually misleading, which just reserves consecutive minors. However, I don't think it is a good idea to add limit on device number. Thanks, Ming
