* Nullblk configfs oddities @ 2022-04-18 21:38 Josef Bacik 2022-04-18 21:54 ` Chaitanya Kulkarni 0 siblings, 1 reply; 11+ messages in thread From: Josef Bacik @ 2022-04-18 21:38 UTC (permalink / raw) To: bvanassche; +Cc: linux-block Hello, I'm trying to add a test to fsperf and it requires the use of nullblk. I'm trying to use the configfs thing, and it's doing some odd things. My basic reproducer is modprobe null_blk mkdir /sys/kernel/config/nullb/nullb0 echo some shit into the config echo 1 > /sys/kernel/config/nullb/nullb0/power Now null_blk apparently defaults to nr_devices == 1, so it creates nullb0 on modprobe. But this doesn't show up in the configfs directory. There's no way to find this out until when I try to mkfs my nullb0 and it doesn't work. The above steps gets my device created at /dev/nullb1, but there's no actual way to figure out that's what happened. If I do something like /sys/kernel/config/nullb/nullbfsperf I still just get nullb<number>, I don't get my fancy name. I don't really care what the solution looks like, I either need to be able to create a random name and have a symlink automatically created so I can get to the right device node (which I don't think can be done without udev, so I'm not fan of this) or I need a way to figure out from configfs which device my thing actually got created as. It would also be nice if /sys/kernel/config/nullb/nullb0 was populated at modinfo time so I know it's there already, then I could do something silly like get the next number and use that as my device. Thanks, Josef ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Nullblk configfs oddities 2022-04-18 21:38 Nullblk configfs oddities Josef Bacik @ 2022-04-18 21:54 ` Chaitanya Kulkarni 2022-04-18 22:02 ` Josef Bacik 2022-04-18 22:14 ` Jens Axboe 0 siblings, 2 replies; 11+ messages in thread From: Chaitanya Kulkarni @ 2022-04-18 21:54 UTC (permalink / raw) To: Josef Bacik; +Cc: linux-block, bvanassche On 4/18/22 14:38, Josef Bacik wrote: > Hello, > > I'm trying to add a test to fsperf and it requires the use of nullblk. I'm > trying to use the configfs thing, and it's doing some odd things. My basic > reproducer is > > modprobe null_blk > mkdir /sys/kernel/config/nullb/nullb0 > echo some shit into the config > echo 1 > /sys/kernel/config/nullb/nullb0/power > > Now null_blk apparently defaults to nr_devices == 1, so it creates nullb0 on > modprobe. But this doesn't show up in the configfs directory. There's no way > to find this out until when I try to mkfs my nullb0 and it doesn't work. The > above steps gets my device created at /dev/nullb1, but there's no actual way to > figure out that's what happened. If I do something like > /sys/kernel/config/nullb/nullbfsperf I still just get nullb<number>, I don't get > my fancy name. > when you load module with default module parameter it will create a default device with no memory backed mode, that will not be visible in the configfs. So you need to load the module with nr_devices=0 that will prevent the null_blk to create the default device which is not memory backed and not present in the configfs:- linux-block (for-next) # modprobe null_blk linux-block (for-next) # lsblk NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT sda 8:0 0 50G 0 disk ├─sda1 8:1 0 1G 0 part /boot └─sda2 8:2 0 49G 0 part /home sdb 8:16 0 100G 0 disk /mnt/data sr0 11:0 1 1024M 0 rom nullb0 250:0 0 250G 0 disk <------------------- zram0 251:0 0 8G 0 disk [SWAP] vda 252:0 0 512M 0 disk nvme0n1 259:0 0 1G 0 disk linux-block (for-next) # tree config config └── nullb └── features 1 directory, 1 file linux-block (for-next) # modprobe -r null_blk linux-block (for-next) # modprobe null_blk nr_devices=0 linux-block (for-next) # lsblk NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT sda 8:0 0 50G 0 disk ├─sda1 8:1 0 1G 0 part /boot └─sda2 8:2 0 49G 0 part /home sdb 8:16 0 100G 0 disk /mnt/data sr0 11:0 1 1024M 0 rom zram0 251:0 0 8G 0 disk [SWAP] vda 252:0 0 512M 0 disk nvme0n1 259:0 0 1G 0 disk linux-block (for-next) # tree config config └── nullb └── features 1 directory, 1 file linux-block (for-next) # -ck ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Nullblk configfs oddities 2022-04-18 21:54 ` Chaitanya Kulkarni @ 2022-04-18 22:02 ` Josef Bacik 2022-04-18 22:15 ` Chaitanya Kulkarni 2022-04-18 22:14 ` Jens Axboe 1 sibling, 1 reply; 11+ messages in thread From: Josef Bacik @ 2022-04-18 22:02 UTC (permalink / raw) To: Chaitanya Kulkarni; +Cc: linux-block, bvanassche On Mon, Apr 18, 2022 at 5:54 PM Chaitanya Kulkarni <chaitanyak@nvidia.com> wrote: > > On 4/18/22 14:38, Josef Bacik wrote: > > Hello, > > > > I'm trying to add a test to fsperf and it requires the use of nullblk. I'm > > trying to use the configfs thing, and it's doing some odd things. My basic > > reproducer is > > > > modprobe null_blk > > mkdir /sys/kernel/config/nullb/nullb0 > > echo some shit into the config > > echo 1 > /sys/kernel/config/nullb/nullb0/power > > > > Now null_blk apparently defaults to nr_devices == 1, so it creates nullb0 on > > modprobe. But this doesn't show up in the configfs directory. There's no way > > to find this out until when I try to mkfs my nullb0 and it doesn't work. The > > above steps gets my device created at /dev/nullb1, but there's no actual way to > > figure out that's what happened. If I do something like > > /sys/kernel/config/nullb/nullbfsperf I still just get nullb<number>, I don't get > > my fancy name. > > > > when you load module with default module parameter it will create a > default device with no memory backed mode, that will not be visible in > the configfs. > > So you need to load the module with nr_devices=0 that will prevent the > null_blk to create the default device which is not memory backed and not > present in the configfs:- > Yup I know what it's doing, I'm raising this as it's weird and took me a bit to work out what was happening, and it annoyed me. It's not anything I can't work around, but the UX kinda sucks. Thanks, Josef ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Nullblk configfs oddities 2022-04-18 22:02 ` Josef Bacik @ 2022-04-18 22:15 ` Chaitanya Kulkarni 0 siblings, 0 replies; 11+ messages in thread From: Chaitanya Kulkarni @ 2022-04-18 22:15 UTC (permalink / raw) To: Josef Bacik; +Cc: linux-block, bvanassche >> when you load module with default module parameter it will create a >> default device with no memory backed mode, that will not be visible in >> the configfs. >> >> So you need to load the module with nr_devices=0 that will prevent the >> null_blk to create the default device which is not memory backed and not >> present in the configfs:- >> > > Yup I know what it's doing, I'm raising this as it's weird and took me > a bit to work out what was happening, and it annoyed me. It's not > anything I can't work around, but the UX kinda sucks. Thanks, > > Josef hmmm, how about a following patch, this will create memory backed default devices at the time of loading the module ? # git diff diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c index c441a4972064..d45c2f3f5692 100644 --- a/drivers/block/null_blk/main.c +++ b/drivers/block/null_blk/main.c @@ -148,7 +148,7 @@ static const struct kernel_param_ops null_queue_mode_param_ops = { device_param_cb(queue_mode, &null_queue_mode_param_ops, &g_queue_mode, 0444); MODULE_PARM_DESC(queue_mode, "Block interface to use (0=bio,1=rq,2=multiqueue)"); -static int g_gb = 250; +static int g_gb = 1; module_param_named(gb, g_gb, int, 0444); MODULE_PARM_DESC(gb, "Size in GB"); @@ -168,6 +168,10 @@ static bool g_blocking; module_param_named(blocking, g_blocking, bool, 0444); MODULE_PARM_DESC(blocking, "Register as a blocking blk-mq driver device"); +static bool g_memory_backed; +module_param_named(memory_backed, g_memory_backed, bool, 0444); +MODULE_PARM_DESC(memory_backed, "memory backed device, default:false"); + static bool shared_tags; module_param(shared_tags, bool, 0444); MODULE_PARM_DESC(shared_tags, "Share tag set between devices for blk-mq"); @@ -657,6 +661,8 @@ static struct nullb_device *null_alloc_dev(void) dev->zone_max_open = g_zone_max_open; dev->zone_max_active = g_zone_max_active; dev->virt_boundary = g_virt_boundary; + dev->memory_backed = g_memory_backed; + return dev; } This passed the fio verification test when loaded with newly added module parameter memory_backed=1:- + modprobe -r null_blk + modprobe null_blk + tree config/ config/ └── nullb └── features 1 directory, 1 file + lsblk + grep null nullb0 250:0 0 1G 0 disk + fio fio/verify.fio --filename=/dev/nullb0 --output=/tmp/fio.log verify: bad header offset 668917760, wanted 0 at file /dev/nullb0 offset 0, length 4096 (requested block: offset=0, length=4096) verify: bad header offset 746139648, wanted 4096 at file /dev/nullb0 offset 4096, length 4096 (requested block: offset=4096, length=4096) verify: bad header offset 705691648, wanted 8192 at file /dev/nullb0 offset 8192, length 4096 (requested block: offset=8192, length=4096) verify: bad header offset 363917312, wanted 12288 at file /dev/nullb0 offset 12288, length 4096 (requested block: offset=12288, length=4096) + grep err= /tmp/fio.log write-and-verify: (groupid=0, jobs=1): err=84 (file:io_u.c:2141, func=io_u_queued_complete, error=Invalid or incomplete multibyte or wide character): pid=21015: Mon Apr 18 15:14:36 2022 + modprobe -r null_blk + modprobe -r null_blk + modprobe null_blk memory_backed=1 + tree config/ config/ └── nullb └── features 1 directory, 1 file + lsblk + grep null nullb0 250:0 0 1G 0 disk + fio fio/verify.fio --filename=/dev/nullb0 --output=/tmp/fio.log + grep err= /tmp/fio.log write-and-verify: (groupid=0, jobs=1): err= 0: pid=21025: Mon Apr 18 15:14:38 2022 + modprobe -r null_blk -ck ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: Nullblk configfs oddities 2022-04-18 21:54 ` Chaitanya Kulkarni 2022-04-18 22:02 ` Josef Bacik @ 2022-04-18 22:14 ` Jens Axboe 2022-04-18 22:21 ` Chaitanya Kulkarni 1 sibling, 1 reply; 11+ messages in thread From: Jens Axboe @ 2022-04-18 22:14 UTC (permalink / raw) To: Chaitanya Kulkarni, Josef Bacik; +Cc: linux-block, bvanassche On 4/18/22 3:54 PM, Chaitanya Kulkarni wrote: > On 4/18/22 14:38, Josef Bacik wrote: >> Hello, >> >> I'm trying to add a test to fsperf and it requires the use of nullblk. I'm >> trying to use the configfs thing, and it's doing some odd things. My basic >> reproducer is >> >> modprobe null_blk >> mkdir /sys/kernel/config/nullb/nullb0 >> echo some shit into the config >> echo 1 > /sys/kernel/config/nullb/nullb0/power >> >> Now null_blk apparently defaults to nr_devices == 1, so it creates nullb0 on >> modprobe. But this doesn't show up in the configfs directory. There's no way >> to find this out until when I try to mkfs my nullb0 and it doesn't work. The >> above steps gets my device created at /dev/nullb1, but there's no actual way to >> figure out that's what happened. If I do something like >> /sys/kernel/config/nullb/nullbfsperf I still just get nullb<number>, I don't get >> my fancy name. >> > > when you load module with default module parameter it will create a > default device with no memory backed mode, that will not be visible in > the configfs. Right, the problem is really that pre-configured devices (via nr_devices being bigger than 0, which is the default) don't show up in configfs. That, to me, is the real issue here, because it means you need to know which ones are already setup before doing mkdir for a new one. On top of that, it's also odd that they don't show up there to begin with. -- Jens Axboe ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Nullblk configfs oddities 2022-04-18 22:14 ` Jens Axboe @ 2022-04-18 22:21 ` Chaitanya Kulkarni 2022-04-18 22:24 ` Jens Axboe 0 siblings, 1 reply; 11+ messages in thread From: Chaitanya Kulkarni @ 2022-04-18 22:21 UTC (permalink / raw) To: Jens Axboe, Josef Bacik; +Cc: linux-block, bvanassche On 4/18/22 15:14, Jens Axboe wrote: > On 4/18/22 3:54 PM, Chaitanya Kulkarni wrote: >> On 4/18/22 14:38, Josef Bacik wrote: >>> Hello, >>> >>> I'm trying to add a test to fsperf and it requires the use of nullblk. I'm >>> trying to use the configfs thing, and it's doing some odd things. My basic >>> reproducer is >>> >>> modprobe null_blk >>> mkdir /sys/kernel/config/nullb/nullb0 >>> echo some shit into the config >>> echo 1 > /sys/kernel/config/nullb/nullb0/power >>> >>> Now null_blk apparently defaults to nr_devices == 1, so it creates nullb0 on >>> modprobe. But this doesn't show up in the configfs directory. There's no way >>> to find this out until when I try to mkfs my nullb0 and it doesn't work. The >>> above steps gets my device created at /dev/nullb1, but there's no actual way to >>> figure out that's what happened. If I do something like >>> /sys/kernel/config/nullb/nullbfsperf I still just get nullb<number>, I don't get >>> my fancy name. >>> >> >> when you load module with default module parameter it will create a >> default device with no memory backed mode, that will not be visible in >> the configfs. > > Right, the problem is really that pre-configured devices (via nr_devices > being bigger than 0, which is the default) don't show up in configfs. > That, to me, is the real issue here, because it means you need to know > which ones are already setup before doing mkdir for a new one. > > On top of that, it's also odd that they don't show up there to begin > with. > it is indeed confusing, maybe we need to find a way to populate the configfs when loading the module? but I'm not sure if that is the right approach since configs ideally should be populated by user. OTOH we can make the memory_backed module param [1] so user can tentatively not use configfs and only rely on default configuration ? -ck [1] https://marc.info/?l=linux-block&m=165032010206884&w=2 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Nullblk configfs oddities 2022-04-18 22:21 ` Chaitanya Kulkarni @ 2022-04-18 22:24 ` Jens Axboe 2022-04-18 22:28 ` Josef Bacik 2022-04-19 4:23 ` Damien Le Moal 0 siblings, 2 replies; 11+ messages in thread From: Jens Axboe @ 2022-04-18 22:24 UTC (permalink / raw) To: Chaitanya Kulkarni, Josef Bacik; +Cc: linux-block, bvanassche On 4/18/22 4:21 PM, Chaitanya Kulkarni wrote: > On 4/18/22 15:14, Jens Axboe wrote: >> On 4/18/22 3:54 PM, Chaitanya Kulkarni wrote: >>> On 4/18/22 14:38, Josef Bacik wrote: >>>> Hello, >>>> >>>> I'm trying to add a test to fsperf and it requires the use of nullblk. I'm >>>> trying to use the configfs thing, and it's doing some odd things. My basic >>>> reproducer is >>>> >>>> modprobe null_blk >>>> mkdir /sys/kernel/config/nullb/nullb0 >>>> echo some shit into the config >>>> echo 1 > /sys/kernel/config/nullb/nullb0/power >>>> >>>> Now null_blk apparently defaults to nr_devices == 1, so it creates nullb0 on >>>> modprobe. But this doesn't show up in the configfs directory. There's no way >>>> to find this out until when I try to mkfs my nullb0 and it doesn't work. The >>>> above steps gets my device created at /dev/nullb1, but there's no actual way to >>>> figure out that's what happened. If I do something like >>>> /sys/kernel/config/nullb/nullbfsperf I still just get nullb<number>, I don't get >>>> my fancy name. >>>> >>> >>> when you load module with default module parameter it will create a >>> default device with no memory backed mode, that will not be visible in >>> the configfs. >> >> Right, the problem is really that pre-configured devices (via nr_devices >> being bigger than 0, which is the default) don't show up in configfs. >> That, to me, is the real issue here, because it means you need to know >> which ones are already setup before doing mkdir for a new one. >> >> On top of that, it's also odd that they don't show up there to begin >> with. >> > > it is indeed confusing, maybe we need to find a way to populate the > configfs when loading the module? but I'm not sure if that is > the right approach since configs ideally should be populated by > user. > > OTOH we can make the memory_backed module param [1] so user can > tentatively not use configfs and only rely on default configuration ? Arguably configfs should just be disabled if loading with nr_devices larger than 0, as it's a mess of an API as it stands. But probably too late for that. The fact that we also have an option that's specific to configfs just makes it even worse. I don't know much about configfs, but pre-populating with the configured devices and options would be ideal and completely solve this. I think that would be the best solution given the current situation. Not that it's THAT important, null_blk is a developer tool and as such can have some sharper and rouger edges. Still would be nice to make it saner, though. -- Jens Axboe ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Nullblk configfs oddities 2022-04-18 22:24 ` Jens Axboe @ 2022-04-18 22:28 ` Josef Bacik 2022-04-19 4:23 ` Damien Le Moal 1 sibling, 0 replies; 11+ messages in thread From: Josef Bacik @ 2022-04-18 22:28 UTC (permalink / raw) To: Jens Axboe; +Cc: Chaitanya Kulkarni, linux-block, bvanassche On Mon, Apr 18, 2022 at 6:24 PM Jens Axboe <axboe@kernel.dk> wrote: > > On 4/18/22 4:21 PM, Chaitanya Kulkarni wrote: > > On 4/18/22 15:14, Jens Axboe wrote: > >> On 4/18/22 3:54 PM, Chaitanya Kulkarni wrote: > >>> On 4/18/22 14:38, Josef Bacik wrote: > >>>> Hello, > >>>> > >>>> I'm trying to add a test to fsperf and it requires the use of nullblk. I'm > >>>> trying to use the configfs thing, and it's doing some odd things. My basic > >>>> reproducer is > >>>> > >>>> modprobe null_blk > >>>> mkdir /sys/kernel/config/nullb/nullb0 > >>>> echo some shit into the config > >>>> echo 1 > /sys/kernel/config/nullb/nullb0/power > >>>> > >>>> Now null_blk apparently defaults to nr_devices == 1, so it creates nullb0 on > >>>> modprobe. But this doesn't show up in the configfs directory. There's no way > >>>> to find this out until when I try to mkfs my nullb0 and it doesn't work. The > >>>> above steps gets my device created at /dev/nullb1, but there's no actual way to > >>>> figure out that's what happened. If I do something like > >>>> /sys/kernel/config/nullb/nullbfsperf I still just get nullb<number>, I don't get > >>>> my fancy name. > >>>> > >>> > >>> when you load module with default module parameter it will create a > >>> default device with no memory backed mode, that will not be visible in > >>> the configfs. > >> > >> Right, the problem is really that pre-configured devices (via nr_devices > >> being bigger than 0, which is the default) don't show up in configfs. > >> That, to me, is the real issue here, because it means you need to know > >> which ones are already setup before doing mkdir for a new one. > >> > >> On top of that, it's also odd that they don't show up there to begin > >> with. > >> > > > > it is indeed confusing, maybe we need to find a way to populate the > > configfs when loading the module? but I'm not sure if that is > > the right approach since configs ideally should be populated by > > user. > > > > OTOH we can make the memory_backed module param [1] so user can > > tentatively not use configfs and only rely on default configuration ? > > Arguably configfs should just be disabled if loading with nr_devices > larger than 0, as it's a mess of an API as it stands. But probably too > late for that. The fact that we also have an option that's specific to > configfs just makes it even worse. > > I don't know much about configfs, but pre-populating with the configured > devices and options would be ideal and completely solve this. I think > that would be the best solution given the current situation. Not that > it's THAT important, null_blk is a developer tool and as such can have > some sharper and rouger edges. Still would be nice to make it saner, > though. > I'd settle for a sysfs link to the device node that got created, so I can maybe just do mdkir nullb/lolmine dev=$(find nullb/lolmine --maxdepth 1 -type l) devname=$(basename $dev) so I know what my device actually got created as. Thanks, Josef ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Nullblk configfs oddities 2022-04-18 22:24 ` Jens Axboe 2022-04-18 22:28 ` Josef Bacik @ 2022-04-19 4:23 ` Damien Le Moal 2022-04-19 14:49 ` Bart Van Assche 2022-05-03 20:20 ` Josef Bacik 1 sibling, 2 replies; 11+ messages in thread From: Damien Le Moal @ 2022-04-19 4:23 UTC (permalink / raw) To: Jens Axboe, Chaitanya Kulkarni, Josef Bacik; +Cc: linux-block, bvanassche On 4/19/22 07:24, Jens Axboe wrote: > On 4/18/22 4:21 PM, Chaitanya Kulkarni wrote: >> On 4/18/22 15:14, Jens Axboe wrote: >>> On 4/18/22 3:54 PM, Chaitanya Kulkarni wrote: >>>> On 4/18/22 14:38, Josef Bacik wrote: >>>>> Hello, >>>>> >>>>> I'm trying to add a test to fsperf and it requires the use of nullblk. I'm >>>>> trying to use the configfs thing, and it's doing some odd things. My basic >>>>> reproducer is >>>>> >>>>> modprobe null_blk >>>>> mkdir /sys/kernel/config/nullb/nullb0 >>>>> echo some shit into the config >>>>> echo 1 > /sys/kernel/config/nullb/nullb0/power >>>>> >>>>> Now null_blk apparently defaults to nr_devices == 1, so it creates nullb0 on >>>>> modprobe. But this doesn't show up in the configfs directory. There's no way >>>>> to find this out until when I try to mkfs my nullb0 and it doesn't work. The >>>>> above steps gets my device created at /dev/nullb1, but there's no actual way to >>>>> figure out that's what happened. If I do something like >>>>> /sys/kernel/config/nullb/nullbfsperf I still just get nullb<number>, I don't get >>>>> my fancy name. >>>>> >>>> >>>> when you load module with default module parameter it will create a >>>> default device with no memory backed mode, that will not be visible in >>>> the configfs. >>> >>> Right, the problem is really that pre-configured devices (via nr_devices >>> being bigger than 0, which is the default) don't show up in configfs. >>> That, to me, is the real issue here, because it means you need to know >>> which ones are already setup before doing mkdir for a new one. >>> >>> On top of that, it's also odd that they don't show up there to begin >>> with. >>> >> >> it is indeed confusing, maybe we need to find a way to populate the >> configfs when loading the module? but I'm not sure if that is >> the right approach since configs ideally should be populated by >> user. >> >> OTOH we can make the memory_backed module param [1] so user can >> tentatively not use configfs and only rely on default configuration ? > > Arguably configfs should just be disabled if loading with nr_devices > larger than 0, as it's a mess of an API as it stands. But probably too > late for that. The fact that we also have an option that's specific to > configfs just makes it even worse. > > I don't know much about configfs, but pre-populating with the configured > devices and options would be ideal and completely solve this. I think > that would be the best solution given the current situation. Not that > it's THAT important, null_blk is a developer tool and as such can have > some sharper and rouger edges. Still would be nice to make it saner, > though. > I came up with this. It does prepopulate configfs nullb directory with the devices created on modprobe. But... doing an rmmod now always gives a "rmmod: ERROR: Module null_blk is in use" because configfs takes a ref on nullblk module for each entry. So the user now must do an rmdir of all prepopulated devices before doing rmmod. Not ideal. And messing up with the module references is probably not a good idea... diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c index 1aa4897685f6..98933555b59f 100644 --- a/drivers/block/null_blk/main.c +++ b/drivers/block/null_blk/main.c @@ -9,6 +9,7 @@ #include <linux/sched.h> #include <linux/fs.h> #include <linux/init.h> +#include <linux/namei.h> #include "null_blk.h" #define FREE_BATCH 16 @@ -667,6 +668,7 @@ static void null_free_dev(struct nullb_device *dev) null_free_zoned_dev(dev); badblocks_exit(&dev->badblocks); + kfree(dev->config_path); kfree(dev); } @@ -2088,12 +2090,121 @@ static int null_add_dev(struct nullb_device *dev) return rv; } +#ifdef CONFIG_CONFIGFS_FS + +static int nullb_create_dev(int idx) +{ + char disk_name[DISK_NAME_LEN]; + struct nullb_device *dev; + struct config_item *item; + struct dentry *dentry; + struct path parent; + const char *path; + int ret; + + /* Use configfs to allocate the device */ + sprintf(disk_name, "nullb%d", idx); + path = kasprintf(GFP_KERNEL, "/sys/kernel/config/nullb/%s", disk_name); + if (!path) + return -ENOMEM; + + dentry = kern_path_create(AT_FDCWD, path, &parent, LOOKUP_DIRECTORY); + if (IS_ERR(dentry)) { + ret = PTR_ERR(dentry); + goto free; + } + + ret = vfs_mkdir(mnt_user_ns(parent.mnt), d_inode(parent.dentry), + dentry, 0755); + if (ret) + goto done; + + /* Start the device */ + item = config_group_find_item(&nullb_subsys.su_group, disk_name); + if (!item) { + pr_err("Device %s not powered up\n", disk_name); + goto done; + } + + dev = to_nullb_device(item); + set_bit(NULLB_DEV_FL_UP, &dev->flags); + ret = null_add_dev(dev); + if (ret) { + clear_bit(NULLB_DEV_FL_UP, &dev->flags); + goto put; + } + + set_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags); + dev->power = true; + dev->config_path = path; + path = NULL; + +put: + config_item_put(item); +done: + done_path_create(&parent, dentry); +free: + kfree(path); + + return ret; +} + +static void nullb_destroy_dev(struct nullb *nullb) +{ + struct nullb_device *dev = nullb->dev; + struct dentry *dentry; + struct path parent; + + dentry = kern_path_create(AT_FDCWD, dev->config_path, &parent, LOOKUP_DIRECTORY); + if (IS_ERR(dentry)) { + pr_err("Lookup %s failed %ld\n", + dev->config_path, PTR_ERR(dentry)); + return; + } + + if (d_really_is_positive(dentry)) { + vfs_rmdir(mnt_user_ns(parent.mnt), d_inode(parent.dentry), + dentry); + dput(dentry); + } + + done_path_create(&parent, dentry); +} + +#else + +static int nullb_create_dev(int idx) +{ + struct nullb_device *dev; + + dev = null_alloc_dev(); + if (!dev) + return -ENOMEM; + + ret = null_add_dev(dev); + if (ret) { + null_free_dev(dev); + return ret; + } + + return 0; +} + +static int nullb_destroy_dev(struct nullb *nullb) +{ + struct nullb_device *dev = nullb->dev; + + null_del_dev(nullb); + null_free_dev(dev); +} + +#endif + static int __init null_init(void) { int ret = 0; unsigned int i; struct nullb *nullb; - struct nullb_device *dev; if (g_bs > PAGE_SIZE) { pr_warn("invalid block size\n"); @@ -2151,16 +2262,9 @@ static int __init null_init(void) } for (i = 0; i < nr_devices; i++) { - dev = null_alloc_dev(); - if (!dev) { - ret = -ENOMEM; - goto err_dev; - } - ret = null_add_dev(dev); - if (ret) { - null_free_dev(dev); + ret = nullb_create_dev(i); + if (ret) goto err_dev; - } } pr_info("module loaded\n"); @@ -2169,9 +2273,7 @@ static int __init null_init(void) err_dev: while (!list_empty(&nullb_list)) { nullb = list_entry(nullb_list.next, struct nullb, list); - dev = nullb->dev; - null_del_dev(nullb); - null_free_dev(dev); + nullb_destroy_dev(nullb); } unregister_blkdev(null_major, "nullb"); err_conf: diff --git a/drivers/block/null_blk/null_blk.h b/drivers/block/null_blk/null_blk.h index 78eb56b0ca55..ecdc22e74d35 100644 --- a/drivers/block/null_blk/null_blk.h +++ b/drivers/block/null_blk/null_blk.h @@ -61,6 +61,7 @@ struct nullb_zone { struct nullb_device { struct nullb *nullb; struct config_item item; + const char *config_path; struct radix_tree_root data; /* data stored in the disk */ struct radix_tree_root cache; /* disk cache data */ unsigned long flags; /* device flags */ -- Damien Le Moal Western Digital Research ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: Nullblk configfs oddities 2022-04-19 4:23 ` Damien Le Moal @ 2022-04-19 14:49 ` Bart Van Assche 2022-05-03 20:20 ` Josef Bacik 1 sibling, 0 replies; 11+ messages in thread From: Bart Van Assche @ 2022-04-19 14:49 UTC (permalink / raw) To: Damien Le Moal, Jens Axboe, Chaitanya Kulkarni, Josef Bacik, Christoph Hellwig Cc: linux-block On 4/18/22 21:23, Damien Le Moal wrote: > + ret = vfs_mkdir(mnt_user_ns(parent.mnt), d_inode(parent.dentry), > + dentry, 0755); Christoph, if I remember correctly functionality has been added recently to configfs for creating directories from inside the kernel without calling vfs_mkdir()? Thanks, Bart. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Nullblk configfs oddities 2022-04-19 4:23 ` Damien Le Moal 2022-04-19 14:49 ` Bart Van Assche @ 2022-05-03 20:20 ` Josef Bacik 1 sibling, 0 replies; 11+ messages in thread From: Josef Bacik @ 2022-05-03 20:20 UTC (permalink / raw) To: Damien Le Moal; +Cc: Jens Axboe, Chaitanya Kulkarni, linux-block, bvanassche On Tue, Apr 19, 2022 at 01:23:22PM +0900, Damien Le Moal wrote: > On 4/19/22 07:24, Jens Axboe wrote: > > On 4/18/22 4:21 PM, Chaitanya Kulkarni wrote: > >> On 4/18/22 15:14, Jens Axboe wrote: > >>> On 4/18/22 3:54 PM, Chaitanya Kulkarni wrote: > >>>> On 4/18/22 14:38, Josef Bacik wrote: > >>>>> Hello, > >>>>> > >>>>> I'm trying to add a test to fsperf and it requires the use of nullblk. I'm > >>>>> trying to use the configfs thing, and it's doing some odd things. My basic > >>>>> reproducer is > >>>>> > >>>>> modprobe null_blk > >>>>> mkdir /sys/kernel/config/nullb/nullb0 > >>>>> echo some shit into the config > >>>>> echo 1 > /sys/kernel/config/nullb/nullb0/power > >>>>> > >>>>> Now null_blk apparently defaults to nr_devices == 1, so it creates nullb0 on > >>>>> modprobe. But this doesn't show up in the configfs directory. There's no way > >>>>> to find this out until when I try to mkfs my nullb0 and it doesn't work. The > >>>>> above steps gets my device created at /dev/nullb1, but there's no actual way to > >>>>> figure out that's what happened. If I do something like > >>>>> /sys/kernel/config/nullb/nullbfsperf I still just get nullb<number>, I don't get > >>>>> my fancy name. > >>>>> > >>>> > >>>> when you load module with default module parameter it will create a > >>>> default device with no memory backed mode, that will not be visible in > >>>> the configfs. > >>> > >>> Right, the problem is really that pre-configured devices (via nr_devices > >>> being bigger than 0, which is the default) don't show up in configfs. > >>> That, to me, is the real issue here, because it means you need to know > >>> which ones are already setup before doing mkdir for a new one. > >>> > >>> On top of that, it's also odd that they don't show up there to begin > >>> with. > >>> > >> > >> it is indeed confusing, maybe we need to find a way to populate the > >> configfs when loading the module? but I'm not sure if that is > >> the right approach since configs ideally should be populated by > >> user. > >> > >> OTOH we can make the memory_backed module param [1] so user can > >> tentatively not use configfs and only rely on default configuration ? > > > > Arguably configfs should just be disabled if loading with nr_devices > > larger than 0, as it's a mess of an API as it stands. But probably too > > late for that. The fact that we also have an option that's specific to > > configfs just makes it even worse. > > > > I don't know much about configfs, but pre-populating with the configured > > devices and options would be ideal and completely solve this. I think > > that would be the best solution given the current situation. Not that > > it's THAT important, null_blk is a developer tool and as such can have > > some sharper and rouger edges. Still would be nice to make it saner, > > though. > > > > I came up with this. It does prepopulate configfs nullb directory with the > devices created on modprobe. But... doing an rmmod now always gives a > "rmmod: ERROR: Module null_blk is in use" because configfs takes a ref on > nullblk module for each entry. So the user now must do an rmdir of all > prepopulated devices before doing rmmod. Not ideal. And messing up with > the module references is probably not a good idea... > I meant to reply to this previously, I tried this and it worked fine for me. I'll leave it up to you guys if the new behavior is acceptable to you, but for me this worked. Thanks, Josef ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-05-03 20:21 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-18 21:38 Nullblk configfs oddities Josef Bacik 2022-04-18 21:54 ` Chaitanya Kulkarni 2022-04-18 22:02 ` Josef Bacik 2022-04-18 22:15 ` Chaitanya Kulkarni 2022-04-18 22:14 ` Jens Axboe 2022-04-18 22:21 ` Chaitanya Kulkarni 2022-04-18 22:24 ` Jens Axboe 2022-04-18 22:28 ` Josef Bacik 2022-04-19 4:23 ` Damien Le Moal 2022-04-19 14:49 ` Bart Van Assche 2022-05-03 20:20 ` Josef Bacik
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.