* [PATCH] ublk: cleanup flag handling and fix UBLK_F_NO_AUTO_PART_SCAN
@ 2026-02-12 15:25 Alexander Atanasov
2026-02-13 3:48 ` Ming Lei
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Atanasov @ 2026-02-12 15:25 UTC (permalink / raw)
To: Ming Lei, Jens Axboe; +Cc: linux-block, Alexander Atanasov, Yoav Cohen
Currently the code misuse GD_SUPPRESS_PART_SCAN flag
as it tries to use it as a switch for the auto partition scan.
When UBLK_F_NO_AUTO_PART_SCAN is not passed a proper
auto partition scanning is not done because the first
call to ublk_partition_scan_work will clear the bit
GD_SUPPRESS_PART_SCAN but not actually load the partitions.
How it should work:
Unprivileged daemons - never scan partitions, they have
GD_SUPPRESS_PART_SCAN always set - they are marked as devices
without partitions. While ioctl(BLKRRPART) does require
CAP_SYS_ADMIN, block layer have its own a check in
disk_has_partscan(...) if partitions are supported.
Trusted daemons - auto scan partitions by default
and they do not have the bit set, so they do have partitions.
For trusted daemons auto partitions scan is performed but it can
be skipped if user have set UBLK_F_NO_AUTO_PART_SCAN flag.
Rework the code to work as described above:
- remove checks in ublk_partition_scan_work and rely on
the caller to schedule the work only if it has to be done.
- keep GD_SUPPRESS_PART_SCAN always set on unprivileged
- keep GD_SUPPRESS_PART_SCAN always clear on trusted
- Properly handle user request to skip auto partitioning scanning
Fixes: 8443e2087e70 ("ublk: add UBLK_F_NO_AUTO_PART_SCAN feature flag")
Signed-off-by: Alexander Atanasov <alex@zazolabs.com>
---
drivers/block/ublk_drv.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
Cc: Yoav Cohen <yoav@nvidia.com>
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 3c918db4905c..e662c6e8e722 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -2361,14 +2361,9 @@ static void ublk_partition_scan_work(struct work_struct *work)
if (!disk)
return;
- if (WARN_ON_ONCE(!test_and_clear_bit(GD_SUPPRESS_PART_SCAN,
- &disk->state)))
- goto out;
-
mutex_lock(&disk->open_mutex);
bdev_disk_changed(disk, false);
mutex_unlock(&disk->open_mutex);
-out:
ublk_put_disk(disk);
}
@@ -4429,12 +4424,11 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
set_bit(UB_STATE_USED, &ub->state);
- /* Skip partition scan if disabled by user */
- if (ub->dev_info.flags & UBLK_F_NO_AUTO_PART_SCAN) {
+ if (!ub->unprivileged_daemons) {
+ /* Enable partition scanning for trusted daemons */
clear_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
- } else {
- /* Schedule async partition scan for trusted daemons */
- if (!ub->unprivileged_daemons)
+ /* Skip auto partition scan if requested by user */
+ if (!(ub->dev_info.flags & UBLK_F_NO_AUTO_PART_SCAN))
schedule_work(&ub->partition_scan_work);
}
base-commit: 72f4d6fca699a1e35b39c5e5dacac2926d254135
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ublk: cleanup flag handling and fix UBLK_F_NO_AUTO_PART_SCAN
2026-02-12 15:25 [PATCH] ublk: cleanup flag handling and fix UBLK_F_NO_AUTO_PART_SCAN Alexander Atanasov
@ 2026-02-13 3:48 ` Ming Lei
2026-02-13 6:56 ` Hannes Reinecke
2026-02-13 11:05 ` Alexander Atanasov
0 siblings, 2 replies; 6+ messages in thread
From: Ming Lei @ 2026-02-13 3:48 UTC (permalink / raw)
To: Alexander Atanasov; +Cc: Jens Axboe, linux-block, Yoav Cohen
On Thu, Feb 12, 2026 at 03:25:52PM +0000, Alexander Atanasov wrote:
> Currently the code misuse GD_SUPPRESS_PART_SCAN flag
> as it tries to use it as a switch for the auto partition scan.
> When UBLK_F_NO_AUTO_PART_SCAN is not passed a proper
> auto partition scanning is not done because the first
> call to ublk_partition_scan_work will clear the bit
> GD_SUPPRESS_PART_SCAN but not actually load the partitions.
>
> How it should work:
> Unprivileged daemons - never scan partitions, they have
> GD_SUPPRESS_PART_SCAN always set - they are marked as devices
> without partitions. While ioctl(BLKRRPART) does require
> CAP_SYS_ADMIN, block layer have its own a check in
> disk_has_partscan(...) if partitions are supported.
>
> Trusted daemons - auto scan partitions by default
> and they do not have the bit set, so they do have partitions.
>
> For trusted daemons auto partitions scan is performed but it can
> be skipped if user have set UBLK_F_NO_AUTO_PART_SCAN flag.
>
> Rework the code to work as described above:
> - remove checks in ublk_partition_scan_work and rely on
> the caller to schedule the work only if it has to be done.
> - keep GD_SUPPRESS_PART_SCAN always set on unprivileged
> - keep GD_SUPPRESS_PART_SCAN always clear on trusted
> - Properly handle user request to skip auto partitioning scanning
>
> Fixes: 8443e2087e70 ("ublk: add UBLK_F_NO_AUTO_PART_SCAN feature flag")
> Signed-off-by: Alexander Atanasov <alex@zazolabs.com>
> ---
> drivers/block/ublk_drv.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
> Cc: Yoav Cohen <yoav@nvidia.com>
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 3c918db4905c..e662c6e8e722 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -2361,14 +2361,9 @@ static void ublk_partition_scan_work(struct work_struct *work)
> if (!disk)
> return;
>
> - if (WARN_ON_ONCE(!test_and_clear_bit(GD_SUPPRESS_PART_SCAN,
> - &disk->state)))
> - goto out;
> -
It is wrong to remove clearing GD_SUPPRESS_PART_SCAN, otherwise ioctl(BLKRRPART)
can't succeed.
> mutex_lock(&disk->open_mutex);
> bdev_disk_changed(disk, false);
> mutex_unlock(&disk->open_mutex);
> -out:
> ublk_put_disk(disk);
> }
>
> @@ -4429,12 +4424,11 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
>
> set_bit(UB_STATE_USED, &ub->state);
>
> - /* Skip partition scan if disabled by user */
> - if (ub->dev_info.flags & UBLK_F_NO_AUTO_PART_SCAN) {
> + if (!ub->unprivileged_daemons) {
> + /* Enable partition scanning for trusted daemons */
> clear_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
> - } else {
> - /* Schedule async partition scan for trusted daemons */
> - if (!ub->unprivileged_daemons)
> + /* Skip auto partition scan if requested by user */
> + if (!(ub->dev_info.flags & UBLK_F_NO_AUTO_PART_SCAN))
> schedule_work(&ub->partition_scan_work);
We may switch to disallow partition scan permanently for UBLK_F_NO_AUTO_PART_SCAN
per discussion in the following thread:
https://lore.kernel.org/linux-block/0535f4dd-ada3-414a-84c6-7abc232aa670@kernel.dk/
So the change may can be simplified as below, meantime
selftests/ublk/test_part_01.sh need to be updated.
Alexander, please let us know if you'd like work this way?
diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
index a88876756805..d6f479fff75b 100644
--- a/include/uapi/linux/ublk_cmd.h
+++ b/include/uapi/linux/ublk_cmd.h
@@ -367,8 +367,8 @@
*/
#define UBLK_F_SAFE_STOP_DEV (1ULL << 17)
-/* Disable automatic partition scanning when device is started */
-#define UBLK_F_NO_AUTO_PART_SCAN (1ULL << 18)
+/* Disable partition scanning */
+#define UBLK_F_NO_PART_SCAN (1ULL << 18)
/* device state */
#define UBLK_S_DEV_DEAD 0
[root@ktest-40 linux]# git diff
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index c13cda58a7c6..f1a104222fb1 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -81,7 +81,7 @@
| (IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY) ? UBLK_F_INTEGRITY : 0) \
| UBLK_F_SAFE_STOP_DEV \
| UBLK_F_BATCH_IO \
- | UBLK_F_NO_AUTO_PART_SCAN)
+ | UBLK_F_NO_PART_SCAN)
#define UBLK_F_ALL_RECOVERY_FLAGS (UBLK_F_USER_RECOVERY \
| UBLK_F_USER_RECOVERY_REISSUE \
@@ -4438,14 +4438,13 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
set_bit(UB_STATE_USED, &ub->state);
- /* Skip partition scan if disabled by user */
- if (ub->dev_info.flags & UBLK_F_NO_AUTO_PART_SCAN) {
- clear_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
- } else {
- /* Schedule async partition scan for trusted daemons */
- if (!ub->unprivileged_daemons)
- schedule_work(&ub->partition_scan_work);
- }
+ /*
+ * Only schedule async partition scan in case of
+ * !UBLK_F_NO_PART_SCAN and trusted daemons
+ */
+ if (!(ub->dev_info.flags & UBLK_F_NO_PART_SCAN) &&
+ !ub->unprivileged_daemons)
+ schedule_work(&ub->partition_scan_work);
out_put_cdev:
if (ret) {
diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
index a88876756805..d6f479fff75b 100644
--- a/include/uapi/linux/ublk_cmd.h
+++ b/include/uapi/linux/ublk_cmd.h
@@ -367,8 +367,8 @@
*/
#define UBLK_F_SAFE_STOP_DEV (1ULL << 17)
-/* Disable automatic partition scanning when device is started */
-#define UBLK_F_NO_AUTO_PART_SCAN (1ULL << 18)
+/* Disable partition scanning */
+#define UBLK_F_NO_PART_SCAN (1ULL << 18)
/* device state */
#define UBLK_S_DEV_DEAD 0
Thanks,
Ming
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ublk: cleanup flag handling and fix UBLK_F_NO_AUTO_PART_SCAN
2026-02-13 3:48 ` Ming Lei
@ 2026-02-13 6:56 ` Hannes Reinecke
2026-02-13 11:05 ` Alexander Atanasov
1 sibling, 0 replies; 6+ messages in thread
From: Hannes Reinecke @ 2026-02-13 6:56 UTC (permalink / raw)
To: Ming Lei, Alexander Atanasov; +Cc: Jens Axboe, linux-block, Yoav Cohen
On 2/13/26 04:48, Ming Lei wrote:
> On Thu, Feb 12, 2026 at 03:25:52PM +0000, Alexander Atanasov wrote:
>> Currently the code misuse GD_SUPPRESS_PART_SCAN flag
>> as it tries to use it as a switch for the auto partition scan.
>> When UBLK_F_NO_AUTO_PART_SCAN is not passed a proper
>> auto partition scanning is not done because the first
>> call to ublk_partition_scan_work will clear the bit
>> GD_SUPPRESS_PART_SCAN but not actually load the partitions.
>>
>> How it should work:
>> Unprivileged daemons - never scan partitions, they have
>> GD_SUPPRESS_PART_SCAN always set - they are marked as devices
>> without partitions. While ioctl(BLKRRPART) does require
>> CAP_SYS_ADMIN, block layer have its own a check in
>> disk_has_partscan(...) if partitions are supported.
>>
>> Trusted daemons - auto scan partitions by default
>> and they do not have the bit set, so they do have partitions.
>>
>> For trusted daemons auto partitions scan is performed but it can
>> be skipped if user have set UBLK_F_NO_AUTO_PART_SCAN flag.
>>
>> Rework the code to work as described above:
>> - remove checks in ublk_partition_scan_work and rely on
>> the caller to schedule the work only if it has to be done.
>> - keep GD_SUPPRESS_PART_SCAN always set on unprivileged
>> - keep GD_SUPPRESS_PART_SCAN always clear on trusted
>> - Properly handle user request to skip auto partitioning scanning
>>
>> Fixes: 8443e2087e70 ("ublk: add UBLK_F_NO_AUTO_PART_SCAN feature flag")
>> Signed-off-by: Alexander Atanasov <alex@zazolabs.com>
>> ---
>> drivers/block/ublk_drv.c | 14 ++++----------
>> 1 file changed, 4 insertions(+), 10 deletions(-)
>>
>> Cc: Yoav Cohen <yoav@nvidia.com>
>>
>> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
>> index 3c918db4905c..e662c6e8e722 100644
>> --- a/drivers/block/ublk_drv.c
>> +++ b/drivers/block/ublk_drv.c
>> @@ -2361,14 +2361,9 @@ static void ublk_partition_scan_work(struct work_struct *work)
>> if (!disk)
>> return;
>>
>> - if (WARN_ON_ONCE(!test_and_clear_bit(GD_SUPPRESS_PART_SCAN,
>> - &disk->state)))
>> - goto out;
>> -
>
> It is wrong to remove clearing GD_SUPPRESS_PART_SCAN, otherwise ioctl(BLKRRPART)
> can't succeed.
>
>> mutex_lock(&disk->open_mutex);
>> bdev_disk_changed(disk, false);
>> mutex_unlock(&disk->open_mutex);
>> -out:
>> ublk_put_disk(disk);
>> }
>>
>> @@ -4429,12 +4424,11 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
>>
>> set_bit(UB_STATE_USED, &ub->state);
>>
>> - /* Skip partition scan if disabled by user */
>> - if (ub->dev_info.flags & UBLK_F_NO_AUTO_PART_SCAN) {
>> + if (!ub->unprivileged_daemons) {
>> + /* Enable partition scanning for trusted daemons */
>> clear_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
>> - } else {
>> - /* Schedule async partition scan for trusted daemons */
>> - if (!ub->unprivileged_daemons)
>> + /* Skip auto partition scan if requested by user */
>> + if (!(ub->dev_info.flags & UBLK_F_NO_AUTO_PART_SCAN))
>> schedule_work(&ub->partition_scan_work);
>
> We may switch to disallow partition scan permanently for UBLK_F_NO_AUTO_PART_SCAN
> per discussion in the following thread:
>
> https://lore.kernel.org/linux-block/0535f4dd-ada3-414a-84c6-7abc232aa670@kernel.dk/
>
> So the change may can be simplified as below, meantime
> selftests/ublk/test_part_01.sh need to be updated.
>
> Alexander, please let us know if you'd like work this way?
>
> diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
> index a88876756805..d6f479fff75b 100644
> --- a/include/uapi/linux/ublk_cmd.h
> +++ b/include/uapi/linux/ublk_cmd.h
> @@ -367,8 +367,8 @@
> */
> #define UBLK_F_SAFE_STOP_DEV (1ULL << 17)
>
> -/* Disable automatic partition scanning when device is started */
> -#define UBLK_F_NO_AUTO_PART_SCAN (1ULL << 18)
> +/* Disable partition scanning */
> +#define UBLK_F_NO_PART_SCAN (1ULL << 18)
>
> /* device state */
> #define UBLK_S_DEV_DEAD 0
> [root@ktest-40 linux]# git diff
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index c13cda58a7c6..f1a104222fb1 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -81,7 +81,7 @@
> | (IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY) ? UBLK_F_INTEGRITY : 0) \
> | UBLK_F_SAFE_STOP_DEV \
> | UBLK_F_BATCH_IO \
> - | UBLK_F_NO_AUTO_PART_SCAN)
> + | UBLK_F_NO_PART_SCAN)
>
> #define UBLK_F_ALL_RECOVERY_FLAGS (UBLK_F_USER_RECOVERY \
> | UBLK_F_USER_RECOVERY_REISSUE \
> @@ -4438,14 +4438,13 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
>
> set_bit(UB_STATE_USED, &ub->state);
>
> - /* Skip partition scan if disabled by user */
> - if (ub->dev_info.flags & UBLK_F_NO_AUTO_PART_SCAN) {
> - clear_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
> - } else {
> - /* Schedule async partition scan for trusted daemons */
> - if (!ub->unprivileged_daemons)
> - schedule_work(&ub->partition_scan_work);
> - }
> + /*
> + * Only schedule async partition scan in case of
> + * !UBLK_F_NO_PART_SCAN and trusted daemons
> + */
> + if (!(ub->dev_info.flags & UBLK_F_NO_PART_SCAN) &&
> + !ub->unprivileged_daemons)
> + schedule_work(&ub->partition_scan_work);
>
> out_put_cdev:
> if (ret) {
> diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
> index a88876756805..d6f479fff75b 100644
> --- a/include/uapi/linux/ublk_cmd.h
> +++ b/include/uapi/linux/ublk_cmd.h
> @@ -367,8 +367,8 @@
> */
> #define UBLK_F_SAFE_STOP_DEV (1ULL << 17)
>
> -/* Disable automatic partition scanning when device is started */
> -#define UBLK_F_NO_AUTO_PART_SCAN (1ULL << 18)
> +/* Disable partition scanning */
> +#define UBLK_F_NO_PART_SCAN (1ULL << 18)
>
> /* device state */
> #define UBLK_S_DEV_DEAD 0
>
>
Thanks, that is precisely what I had in mind.
You can add:
Reviewed-by: Hannes Reinecke <hare@suse.de>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ublk: cleanup flag handling and fix UBLK_F_NO_AUTO_PART_SCAN
2026-02-13 3:48 ` Ming Lei
2026-02-13 6:56 ` Hannes Reinecke
@ 2026-02-13 11:05 ` Alexander Atanasov
2026-02-20 3:58 ` Ming Lei
1 sibling, 1 reply; 6+ messages in thread
From: Alexander Atanasov @ 2026-02-13 11:05 UTC (permalink / raw)
To: Ming Lei; +Cc: Jens Axboe, linux-block, Hannes Reinecke, Yoav Cohen
On 13.02.26 5:48, Ming Lei wrote:
> On Thu, Feb 12, 2026 at 03:25:52PM +0000, Alexander Atanasov wrote:
>> Currently the code misuse GD_SUPPRESS_PART_SCAN flag
>> as it tries to use it as a switch for the auto partition scan.
>> When UBLK_F_NO_AUTO_PART_SCAN is not passed a proper
>> auto partition scanning is not done because the first
>> call to ublk_partition_scan_work will clear the bit
>> GD_SUPPRESS_PART_SCAN but not actually load the partitions.
>>
>> How it should work:
>> Unprivileged daemons - never scan partitions, they have
>> GD_SUPPRESS_PART_SCAN always set - they are marked as devices
>> without partitions. While ioctl(BLKRRPART) does require
>> CAP_SYS_ADMIN, block layer have its own a check in
>> disk_has_partscan(...) if partitions are supported.
>>
>> Trusted daemons - auto scan partitions by default
>> and they do not have the bit set, so they do have partitions.
>>
>> For trusted daemons auto partitions scan is performed but it can
>> be skipped if user have set UBLK_F_NO_AUTO_PART_SCAN flag.
>>
>> Rework the code to work as described above:
>> - remove checks in ublk_partition_scan_work and rely on
>> the caller to schedule the work only if it has to be done.
>> - keep GD_SUPPRESS_PART_SCAN always set on unprivileged
>> - keep GD_SUPPRESS_PART_SCAN always clear on trusted
>> - Properly handle user request to skip auto partitioning scanning
>>
>> Fixes: 8443e2087e70 ("ublk: add UBLK_F_NO_AUTO_PART_SCAN feature flag")
>> Signed-off-by: Alexander Atanasov <alex@zazolabs.com>
>> ---
>> drivers/block/ublk_drv.c | 14 ++++----------
>> 1 file changed, 4 insertions(+), 10 deletions(-)
>>
>> Cc: Yoav Cohen <yoav@nvidia.com>
>>
>> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
>> index 3c918db4905c..e662c6e8e722 100644
>> --- a/drivers/block/ublk_drv.c
>> +++ b/drivers/block/ublk_drv.c
>> @@ -2361,14 +2361,9 @@ static void ublk_partition_scan_work(struct work_struct *work)
>> if (!disk)
>> return;
>>
>> - if (WARN_ON_ONCE(!test_and_clear_bit(GD_SUPPRESS_PART_SCAN,
>> - &disk->state)))
>> - goto out;
>> -
>
> It is wrong to remove clearing GD_SUPPRESS_PART_SCAN, otherwise ioctl(BLKRRPART)
> can't succeed.
If you agree with how should it work in the message then it is not a
problems. GD_SUPPRESS_PART_SCAN is set only on unprivileged - no
partitions on them. Trusted have it always clear - they
schedule the scan and get their partitions automatically.
If user passes the UBLK_F_NO_AUTO_PART_SCAN, then the scan is not
scheduled and it is up to the user to reread partitions.
This ublk_partition_scan_work is used inside ublk_drv only,
ioctl uses different code path. Double checked and the only
way to reach it is thru ublk_ctrl_add_dev and from ublk_ctrl_uring_cmd.
>
>> mutex_lock(&disk->open_mutex);
>> bdev_disk_changed(disk, false);
>> mutex_unlock(&disk->open_mutex);
>> -out:
>> ublk_put_disk(disk);
>> }
>>
>> @@ -4429,12 +4424,11 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
>>
>> set_bit(UB_STATE_USED, &ub->state);
>>
>> - /* Skip partition scan if disabled by user */
>> - if (ub->dev_info.flags & UBLK_F_NO_AUTO_PART_SCAN) {
>> + if (!ub->unprivileged_daemons) {
>> + /* Enable partition scanning for trusted daemons */
>> clear_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
>> - } else {
>> - /* Schedule async partition scan for trusted daemons */
>> - if (!ub->unprivileged_daemons)
>> + /* Skip auto partition scan if requested by user */
>> + if (!(ub->dev_info.flags & UBLK_F_NO_AUTO_PART_SCAN))
>> schedule_work(&ub->partition_scan_work);
>
> We may switch to disallow partition scan permanently for UBLK_F_NO_AUTO_PART_SCAN
> per discussion in the following thread:
>
> https://lore.kernel.org/linux-block/0535f4dd-ada3-414a-84c6-7abc232aa670@kernel.dk/
>
> So the change may can be simplified as below, meantime
> selftests/ublk/test_part_01.sh need to be updated.
Ok, tests and docs are next as Jens requested.
>
> Alexander, please let us know if you'd like work this way?
https://lore.kernel.org/linux-block/c95b3de4-f4f8-4b35-a3e4-a83da98ced0b@zazolabs.com/T/#mbb410ceb3f4c6f23e6c566911dfc833365ce9cdc
in short - having partitions and auto scan are two different features,
why merge them?
If we want to cover all cases i thing we should go like this:
/* Enable partitions support on device */
#define UBLK_F_PARTITIONS (1ULL << 18)
/* Do not perform automatic partition scanning when device is added */
#define UBLK_F_NO_AUTO_PART_SCAN (1ULL << 19)
Added value is to have partitions enabled/disabled explicitly for
trusted devices. Where by default they will be disabled.
Also may be change from UBLK_F_NO_AUTO_PART_SCAN to
UBLK_F_AUTO_PART_SCAN - to make it explicit instead and be like
the rest of the flags - all are enable flags
Then these variants become possible:
- unprivileged without partitions (can we add security checks to allow
partitions for them too?)
- trusted with or without partitions (UBLK_F_PARTITIONS bound to
GD_SUPPRESS_PART_SCAN)
- If it is a device with partitions then UBLK_F_NO_AUTO_PART_SCAN
controls if initial auto scan is performed or not.
Having them separate can allow emulation of removable media where data
is not available at device creation time but is known when available by
external event so you can load partitions. Two step device bring up,
i.e. partition scan at device creation time will block you, but you know
when data is available from an external event, and then you can load
partitions.
What do you think?
I'be added CC Yoav since i think he requested that feature in first
place. I beleive Hannes may have a different use case, so let's get some
more input on this.
> diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
> index a88876756805..d6f479fff75b 100644
> --- a/include/uapi/linux/ublk_cmd.h
> +++ b/include/uapi/linux/ublk_cmd.h
> @@ -367,8 +367,8 @@
> */
> #define UBLK_F_SAFE_STOP_DEV (1ULL << 17)
>
> -/* Disable automatic partition scanning when device is started */
> -#define UBLK_F_NO_AUTO_PART_SCAN (1ULL << 18)
> +/* Disable partition scanning */
> +#define UBLK_F_NO_PART_SCAN (1ULL << 18)
>
> /* device state */
> #define UBLK_S_DEV_DEAD 0
> [root@ktest-40 linux]# git diff
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index c13cda58a7c6..f1a104222fb1 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -81,7 +81,7 @@
> | (IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY) ? UBLK_F_INTEGRITY : 0) \
> | UBLK_F_SAFE_STOP_DEV \
> | UBLK_F_BATCH_IO \
> - | UBLK_F_NO_AUTO_PART_SCAN)
> + | UBLK_F_NO_PART_SCAN)
>
> #define UBLK_F_ALL_RECOVERY_FLAGS (UBLK_F_USER_RECOVERY \
> | UBLK_F_USER_RECOVERY_REISSUE \
> @@ -4438,14 +4438,13 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
>
> set_bit(UB_STATE_USED, &ub->state);
>
> - /* Skip partition scan if disabled by user */
> - if (ub->dev_info.flags & UBLK_F_NO_AUTO_PART_SCAN) {
> - clear_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
> - } else {
> - /* Schedule async partition scan for trusted daemons */
> - if (!ub->unprivileged_daemons)
> - schedule_work(&ub->partition_scan_work);
> - }
> + /*
> + * Only schedule async partition scan in case of
> + * !UBLK_F_NO_PART_SCAN and trusted daemons
> + */
> + if (!(ub->dev_info.flags & UBLK_F_NO_PART_SCAN) &&
> + !ub->unprivileged_daemons)
> + schedule_work(&ub->partition_scan_work);
a few lines above
set_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
is done uncoditionally.
schedule_work(&ub->partition_scan_work); without clearing
the bit first won't load the partitions just clear the bit and return.
This is a bug in the original commit, not a result of any of the
proposed changes.
--
have fun,
alex
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ublk: cleanup flag handling and fix UBLK_F_NO_AUTO_PART_SCAN
2026-02-13 11:05 ` Alexander Atanasov
@ 2026-02-20 3:58 ` Ming Lei
2026-02-21 13:00 ` Alexander Atanasov
0 siblings, 1 reply; 6+ messages in thread
From: Ming Lei @ 2026-02-20 3:58 UTC (permalink / raw)
To: Alexander Atanasov; +Cc: Jens Axboe, linux-block, Hannes Reinecke, Yoav Cohen
Hi Alexander,
On Fri, Feb 13, 2026 at 01:05:40PM +0200, Alexander Atanasov wrote:
> On 13.02.26 5:48, Ming Lei wrote:
> > On Thu, Feb 12, 2026 at 03:25:52PM +0000, Alexander Atanasov wrote:
> > > Currently the code misuse GD_SUPPRESS_PART_SCAN flag
> > > as it tries to use it as a switch for the auto partition scan.
> > > When UBLK_F_NO_AUTO_PART_SCAN is not passed a proper
> > > auto partition scanning is not done because the first
> > > call to ublk_partition_scan_work will clear the bit
> > > GD_SUPPRESS_PART_SCAN but not actually load the partitions.
> > >
> > > How it should work:
> > > Unprivileged daemons - never scan partitions, they have
> > > GD_SUPPRESS_PART_SCAN always set - they are marked as devices
> > > without partitions. While ioctl(BLKRRPART) does require
> > > CAP_SYS_ADMIN, block layer have its own a check in
> > > disk_has_partscan(...) if partitions are supported.
> > >
> > > Trusted daemons - auto scan partitions by default
> > > and they do not have the bit set, so they do have partitions.
> > >
> > > For trusted daemons auto partitions scan is performed but it can
> > > be skipped if user have set UBLK_F_NO_AUTO_PART_SCAN flag.
> > >
> > > Rework the code to work as described above:
> > > - remove checks in ublk_partition_scan_work and rely on
> > > the caller to schedule the work only if it has to be done.
> > > - keep GD_SUPPRESS_PART_SCAN always set on unprivileged
> > > - keep GD_SUPPRESS_PART_SCAN always clear on trusted
> > > - Properly handle user request to skip auto partitioning scanning
> > >
> > > Fixes: 8443e2087e70 ("ublk: add UBLK_F_NO_AUTO_PART_SCAN feature flag")
> > > Signed-off-by: Alexander Atanasov <alex@zazolabs.com>
> > > ---
> > > drivers/block/ublk_drv.c | 14 ++++----------
> > > 1 file changed, 4 insertions(+), 10 deletions(-)
> > >
> > > Cc: Yoav Cohen <yoav@nvidia.com>
> > >
> > > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > > index 3c918db4905c..e662c6e8e722 100644
> > > --- a/drivers/block/ublk_drv.c
> > > +++ b/drivers/block/ublk_drv.c
> > > @@ -2361,14 +2361,9 @@ static void ublk_partition_scan_work(struct work_struct *work)
> > > if (!disk)
> > > return;
> > > - if (WARN_ON_ONCE(!test_and_clear_bit(GD_SUPPRESS_PART_SCAN,
> > > - &disk->state)))
> > > - goto out;
> > > -
> >
> > It is wrong to remove clearing GD_SUPPRESS_PART_SCAN, otherwise ioctl(BLKRRPART)
> > can't succeed.
>
> If you agree with how should it work in the message then it is not a
> problems. GD_SUPPRESS_PART_SCAN is set only on unprivileged - no
> partitions on them. Trusted have it always clear - they
> schedule the scan and get their partitions automatically.
> If user passes the UBLK_F_NO_AUTO_PART_SCAN, then the scan is not scheduled
> and it is up to the user to reread partitions.
> This ublk_partition_scan_work is used inside ublk_drv only,
> ioctl uses different code path. Double checked and the only
> way to reach it is thru ublk_ctrl_add_dev and from ublk_ctrl_uring_cmd.
>
>
> >
> > > mutex_lock(&disk->open_mutex);
> > > bdev_disk_changed(disk, false);
> > > mutex_unlock(&disk->open_mutex);
> > > -out:
> > > ublk_put_disk(disk);
> > > }
> > > @@ -4429,12 +4424,11 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
> > > set_bit(UB_STATE_USED, &ub->state);
> > > - /* Skip partition scan if disabled by user */
> > > - if (ub->dev_info.flags & UBLK_F_NO_AUTO_PART_SCAN) {
> > > + if (!ub->unprivileged_daemons) {
> > > + /* Enable partition scanning for trusted daemons */
> > > clear_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
> > > - } else {
> > > - /* Schedule async partition scan for trusted daemons */
> > > - if (!ub->unprivileged_daemons)
> > > + /* Skip auto partition scan if requested by user */
> > > + if (!(ub->dev_info.flags & UBLK_F_NO_AUTO_PART_SCAN))
> > > schedule_work(&ub->partition_scan_work);
> >
> > We may switch to disallow partition scan permanently for UBLK_F_NO_AUTO_PART_SCAN
> > per discussion in the following thread:
> >
> > https://lore.kernel.org/linux-block/0535f4dd-ada3-414a-84c6-7abc232aa670@kernel.dk/
> >
> > So the change may can be simplified as below, meantime
> > selftests/ublk/test_part_01.sh need to be updated.
>
> Ok, tests and docs are next as Jens requested.
>
> >
> > Alexander, please let us know if you'd like work this way?
>
> https://lore.kernel.org/linux-block/c95b3de4-f4f8-4b35-a3e4-a83da98ced0b@zazolabs.com/T/#mbb410ceb3f4c6f23e6c566911dfc833365ce9cdc
>
> in short - having partitions and auto scan are two different features, why
> merge them?
Yoav's original requirement has been addressed by scheduling scan work in
wq already.
So I think it makes more sense to switch to UBLK_F_NO_PART_SCAN.
>
> If we want to cover all cases i thing we should go like this:
>
> /* Enable partitions support on device */
> #define UBLK_F_PARTITIONS (1ULL << 18)
>
> /* Do not perform automatic partition scanning when device is added */
> #define UBLK_F_NO_AUTO_PART_SCAN (1ULL << 19)
>
> Added value is to have partitions enabled/disabled explicitly for trusted
> devices. Where by default they will be disabled.
>
> Also may be change from UBLK_F_NO_AUTO_PART_SCAN to UBLK_F_AUTO_PART_SCAN -
> to make it explicit instead and be like
> the rest of the flags - all are enable flags
>
> Then these variants become possible:
> - unprivileged without partitions (can we add security checks to allow
> partitions for them too?)
> - trusted with or without partitions (UBLK_F_PARTITIONS bound to
> GD_SUPPRESS_PART_SCAN)
> - If it is a device with partitions then UBLK_F_NO_AUTO_PART_SCAN controls
> if initial auto scan is performed or not.
>
> Having them separate can allow emulation of removable media where data is
> not available at device creation time but is known when available by
> external event so you can load partitions. Two step device bring up, i.e.
> partition scan at device creation time will block you, but you know when
> data is available from an external event, and then you can load partitions.
>
> What do you think?
>
> I'be added CC Yoav since i think he requested that feature in first place. I
> beleive Hannes may have a different use case, so let's get some more input
> on this.
Yoav's original requirement is to not scan partition during ADD_DEV
command for avoiding hang, which is addressed already by scheduling scan work in wq
context.
So I think it is fine to change UBLK_F_NO_AUTO_PART_SCAN into
UBLK_F_NO_PART_SCAN, which can be implemented by GENHD_FL_NO_PART, as you
mentioned.
Thanks,
Ming
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ublk: cleanup flag handling and fix UBLK_F_NO_AUTO_PART_SCAN
2026-02-20 3:58 ` Ming Lei
@ 2026-02-21 13:00 ` Alexander Atanasov
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Atanasov @ 2026-02-21 13:00 UTC (permalink / raw)
To: Ming Lei; +Cc: Jens Axboe, linux-block, Hannes Reinecke, Yoav Cohen
Hello,
> On 20 Feb 2026, at 5:58, Ming Lei <ming.lei@redhat.com> wrote:
>
> Hi Alexander,
>
> On Fri, Feb 13, 2026 at 01:05:40PM +0200, Alexander Atanasov wrote:
>> On 13.02.26 5:48, Ming Lei wrote:
>>> On Thu, Feb 12, 2026 at 03:25:52PM +0000, Alexander Atanasov wrote:
>>>> Currently the code misuse GD_SUPPRESS_PART_SCAN flag
[snip]
>>
>>>
>>> Alexander, please let us know if you'd like work this way?
>>
>> https://lore.kernel.org/linux-block/c95b3de4-f4f8-4b35-a3e4-a83da98ced0b@zazolabs.com/T/#mbb410ceb3f4c6f23e6c566911dfc833365ce9cdc
>>
>> in short - having partitions and auto scan are two different features, why
>> merge them?
>
> Yoav's original requirement has been addressed by scheduling scan work in
> wq already.
>
> So I think it makes more sense to switch to UBLK_F_NO_PART_SCAN.
Okay.
>
>>
>> If we want to cover all cases i thing we should go like this:
>>
>> /* Enable partitions support on device */
>> #define UBLK_F_PARTITIONS (1ULL << 18)
>>
>> /* Do not perform automatic partition scanning when device is added */
>> #define UBLK_F_NO_AUTO_PART_SCAN (1ULL << 19)
>>
>> Added value is to have partitions enabled/disabled explicitly for trusted
>> devices. Where by default they will be disabled.
>>
>> Also may be change from UBLK_F_NO_AUTO_PART_SCAN to UBLK_F_AUTO_PART_SCAN -
>> to make it explicit instead and be like
>> the rest of the flags - all are enable flags
>>
>> Then these variants become possible:
>> - unprivileged without partitions (can we add security checks to allow
>> partitions for them too?)
>> - trusted with or without partitions (UBLK_F_PARTITIONS bound to
>> GD_SUPPRESS_PART_SCAN)
>> - If it is a device with partitions then UBLK_F_NO_AUTO_PART_SCAN controls
>> if initial auto scan is performed or not.
>>
>> Having them separate can allow emulation of removable media where data is
>> not available at device creation time but is known when available by
>> external event so you can load partitions. Two step device bring up, i.e.
>> partition scan at device creation time will block you, but you know when
>> data is available from an external event, and then you can load partitions.
>>
>> What do you think?
>>
>> I'be added CC Yoav since i think he requested that feature in first place. I
>> beleive Hannes may have a different use case, so let's get some more input
>> on this.
>
> Yoav's original requirement is to not scan partition during ADD_DEV
> command for avoiding hang, which is addressed already by scheduling scan work in wq
> context.
Okay.
>
> So I think it is fine to change UBLK_F_NO_AUTO_PART_SCAN into
> UBLK_F_NO_PART_SCAN, which can be implemented by GENHD_FL_NO_PART, as you
> mentioned.
Okay then I will switch to use GENHD_FL_NO_PART.
UBLK_F_NO_PARTITIONS/UBLK_F_PARTITIONS will be a better flag name than
UBLK_F_NO_PART_SCAN - since it will not be about scanning any more.
Partitions will be disabled completely - by default for untrusted and
if requested for privileged.
And if privileged have partitions - we need UBLK_F_AUTO_PART_SCAN
or UBLK_F_NO_AUTO_PART_SCAN option to explicitly request or suppress
the initial auto scan.
Regards,
Alexander Atanasov
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-02-21 13:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-12 15:25 [PATCH] ublk: cleanup flag handling and fix UBLK_F_NO_AUTO_PART_SCAN Alexander Atanasov
2026-02-13 3:48 ` Ming Lei
2026-02-13 6:56 ` Hannes Reinecke
2026-02-13 11:05 ` Alexander Atanasov
2026-02-20 3:58 ` Ming Lei
2026-02-21 13:00 ` Alexander Atanasov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox