ublk invert part scan bit logic

ublk invert part scan bit logic

[Jens Axboe]

Hi,

For ublk, there's this logic in in ublk_ctrl_start_dev():

/* 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);
}

where the

clear_bit(GD_SUPPRESS_PART_SCAN, &disk->state);

seems reversed? Why is GD_SUPPRESS_PART_SCAN being cleared if
UBLK_F_NO_AUTO_PART_SCAN is set? Added in:

8443e2087e70 ("ublk: add UBLK_F_NO_AUTO_PART_SCAN feature flag")

-- 
Jens Axboe

Re: ublk invert part scan bit logic

[Ming Lei]

On Thu, Feb 12, 2026 at 04:05:27AM -0700, Jens Axboe wrote:
> Hi,
> 
> For ublk, there's this logic in in ublk_ctrl_start_dev():
> 
> /* 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);
> }
> 
> where the
> 
> clear_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
> 
> seems reversed? Why is GD_SUPPRESS_PART_SCAN being cleared if
> UBLK_F_NO_AUTO_PART_SCAN is set? Added in:
> 
> 8443e2087e70 ("ublk: add UBLK_F_NO_AUTO_PART_SCAN feature flag")

Yeah, the interface is designed in this way: partition scan is not
done during add disk, and allowed since then on. The selftest code
is written in same way too.

If GD_SUPPRESS_PART_SCAN isn't cleared, userspace can't probe partitions
any more.

However, if you think the interface isn't good, we still can change it
before 7.0 release.


Thanks
Ming

Re: ublk invert part scan bit logic

[Jens Axboe]

On 2/12/26 5:42 AM, Ming Lei wrote:
> On Thu, Feb 12, 2026 at 04:05:27AM -0700, Jens Axboe wrote:
>> Hi,
>>
>> For ublk, there's this logic in in ublk_ctrl_start_dev():
>>
>> /* 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);
>> }
>>
>> where the
>>
>> clear_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
>>
>> seems reversed? Why is GD_SUPPRESS_PART_SCAN being cleared if
>> UBLK_F_NO_AUTO_PART_SCAN is set? Added in:
>>
>> 8443e2087e70 ("ublk: add UBLK_F_NO_AUTO_PART_SCAN feature flag")
> 
> Yeah, the interface is designed in this way: partition scan is not
> done during add disk, and allowed since then on. The selftest code
> is written in same way too.
> 
> If GD_SUPPRESS_PART_SCAN isn't cleared, userspace can't probe partitions
> any more.
> 
> However, if you think the interface isn't good, we still can change it
> before 7.0 release.

What I mean is, if UBLK_F_NO_AUTO_PART_SCAN is set, should we not
either leave the disk->state alone, or _set_ GD_SUPPRESS_PART_SCAN?
I might be confused here, but the current implementation doesn't
make much sense to me! If it is correct, then a comment to that
effect would be good imho.

-- 
Jens Axboe

Re: ublk invert part scan bit logic

[Ming Lei]

On Thu, Feb 12, 2026 at 05:48:40AM -0700, Jens Axboe wrote:
> On 2/12/26 5:42 AM, Ming Lei wrote:
> > On Thu, Feb 12, 2026 at 04:05:27AM -0700, Jens Axboe wrote:
> >> Hi,
> >>
> >> For ublk, there's this logic in in ublk_ctrl_start_dev():
> >>
> >> /* 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);
> >> }
> >>
> >> where the
> >>
> >> clear_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
> >>
> >> seems reversed? Why is GD_SUPPRESS_PART_SCAN being cleared if
> >> UBLK_F_NO_AUTO_PART_SCAN is set? Added in:
> >>
> >> 8443e2087e70 ("ublk: add UBLK_F_NO_AUTO_PART_SCAN feature flag")
> > 
> > Yeah, the interface is designed in this way: partition scan is not
> > done during add disk, and allowed since then on. The selftest code
> > is written in same way too.
> > 
> > If GD_SUPPRESS_PART_SCAN isn't cleared, userspace can't probe partitions
> > any more.
> > 
> > However, if you think the interface isn't good, we still can change it
> > before 7.0 release.
> 
> What I mean is, if UBLK_F_NO_AUTO_PART_SCAN is set, should we not
> either leave the disk->state alone, or _set_ GD_SUPPRESS_PART_SCAN?

UBLK_F_NO_AUTO_PART_SCAN means partition scanning isn't done automatically
from add_disk(), but it still allow userspace to send ioctl(BLKRRPART) for
probing partition since disk is added.

If GD_SUPPRESS_PART_SCAN is set, ioctl(BLKRRPART) can't succeed any more.

> I might be confused here, but the current implementation doesn't
> make much sense to me! If it is correct, then a comment to that
> effect would be good imho.

I admit it is a little confusing, will send a patch to document this
behavior if no one objects the UAPI.


Thanks,
Ming

Re: ublk invert part scan bit logic

[Hannes Reinecke]

On 2/12/26 14:02, Ming Lei wrote:
> On Thu, Feb 12, 2026 at 05:48:40AM -0700, Jens Axboe wrote:
>> On 2/12/26 5:42 AM, Ming Lei wrote:
>>> On Thu, Feb 12, 2026 at 04:05:27AM -0700, Jens Axboe wrote:
>>>> Hi,
>>>>
>>>> For ublk, there's this logic in in ublk_ctrl_start_dev():
>>>>
>>>> /* 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);
>>>> }
>>>>
>>>> where the
>>>>
>>>> clear_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
>>>>
>>>> seems reversed? Why is GD_SUPPRESS_PART_SCAN being cleared if
>>>> UBLK_F_NO_AUTO_PART_SCAN is set? Added in:
>>>>
>>>> 8443e2087e70 ("ublk: add UBLK_F_NO_AUTO_PART_SCAN feature flag")
>>>
>>> Yeah, the interface is designed in this way: partition scan is not
>>> done during add disk, and allowed since then on. The selftest code
>>> is written in same way too.
>>>
>>> If GD_SUPPRESS_PART_SCAN isn't cleared, userspace can't probe partitions
>>> any more.
>>>
>>> However, if you think the interface isn't good, we still can change it
>>> before 7.0 release.
>>
>> What I mean is, if UBLK_F_NO_AUTO_PART_SCAN is set, should we not
>> either leave the disk->state alone, or _set_ GD_SUPPRESS_PART_SCAN?
> 
> UBLK_F_NO_AUTO_PART_SCAN means partition scanning isn't done automatically
> from add_disk(), but it still allow userspace to send ioctl(BLKRRPART) for
> probing partition since disk is added.
> 
> If GD_SUPPRESS_PART_SCAN is set, ioctl(BLKRRPART) can't succeed any more.
> 
>> I might be confused here, but the current implementation doesn't
>> make much sense to me! If it is correct, then a comment to that
>> effect would be good imho.
> 
> I admit it is a little confusing, will send a patch to document this
> behavior if no one objects the UAPI.
> 
Is there a specific reason to continue to allow BLKRRPART if the
UBLK_F_NO_AUTO_PART_SCAN is set? Wouldn't is be better to have a
simple 'UBLK_F_NO_PART_SCAN', which directly maps to GD_SUPPRESS_PART_SCAN?
Would we lose something substantial in that case?

I actually would _like_ to be able to suppress partition scan
completely for ublk devices...

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

Re: ublk invert part scan bit logic

[Ming Lei]

On Thu, Feb 12, 2026 at 03:04:53PM +0100, Hannes Reinecke wrote:
> On 2/12/26 14:02, Ming Lei wrote:
> > On Thu, Feb 12, 2026 at 05:48:40AM -0700, Jens Axboe wrote:
> > > On 2/12/26 5:42 AM, Ming Lei wrote:
> > > > On Thu, Feb 12, 2026 at 04:05:27AM -0700, Jens Axboe wrote:
> > > > > Hi,
> > > > > 
> > > > > For ublk, there's this logic in in ublk_ctrl_start_dev():
> > > > > 
> > > > > /* 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);
> > > > > }
> > > > > 
> > > > > where the
> > > > > 
> > > > > clear_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
> > > > > 
> > > > > seems reversed? Why is GD_SUPPRESS_PART_SCAN being cleared if
> > > > > UBLK_F_NO_AUTO_PART_SCAN is set? Added in:
> > > > > 
> > > > > 8443e2087e70 ("ublk: add UBLK_F_NO_AUTO_PART_SCAN feature flag")
> > > > 
> > > > Yeah, the interface is designed in this way: partition scan is not
> > > > done during add disk, and allowed since then on. The selftest code
> > > > is written in same way too.
> > > > 
> > > > If GD_SUPPRESS_PART_SCAN isn't cleared, userspace can't probe partitions
> > > > any more.
> > > > 
> > > > However, if you think the interface isn't good, we still can change it
> > > > before 7.0 release.
> > > 
> > > What I mean is, if UBLK_F_NO_AUTO_PART_SCAN is set, should we not
> > > either leave the disk->state alone, or _set_ GD_SUPPRESS_PART_SCAN?
> > 
> > UBLK_F_NO_AUTO_PART_SCAN means partition scanning isn't done automatically
> > from add_disk(), but it still allow userspace to send ioctl(BLKRRPART) for
> > probing partition since disk is added.
> > 
> > If GD_SUPPRESS_PART_SCAN is set, ioctl(BLKRRPART) can't succeed any more.
> > 
> > > I might be confused here, but the current implementation doesn't
> > > make much sense to me! If it is correct, then a comment to that
> > > effect would be good imho.
> > 
> > I admit it is a little confusing, will send a patch to document this
> > behavior if no one objects the UAPI.
> > 
> Is there a specific reason to continue to allow BLKRRPART if the
> UBLK_F_NO_AUTO_PART_SCAN is set? Wouldn't is be better to have a

No, I just want to allow future ioctl(BLKRRPART).

> simple 'UBLK_F_NO_PART_SCAN', which directly maps to GD_SUPPRESS_PART_SCAN?
> Would we lose something substantial in that case?

If there is, we may add new feature flag for it.

> 
> I actually would _like_ to be able to suppress partition scan
> completely for ublk devices...

Fine, we can change UBLK_F_NO_AUTO_PART_SCAN to align with GD_SUPPRESS_PART_SCAN.



Thanks,
Ming

Re: ublk invert part scan bit logic

[Hannes Reinecke]

On 2/12/26 15:22, Ming Lei wrote:
> On Thu, Feb 12, 2026 at 03:04:53PM +0100, Hannes Reinecke wrote:
>> On 2/12/26 14:02, Ming Lei wrote:
>>> On Thu, Feb 12, 2026 at 05:48:40AM -0700, Jens Axboe wrote:
[ .. ]
>>>> I might be confused here, but the current implementation doesn't
>>>> make much sense to me! If it is correct, then a comment to that
>>>> effect would be good imho.
>>>
>>> I admit it is a little confusing, will send a patch to document this
>>> behavior if no one objects the UAPI.
>>>
>> Is there a specific reason to continue to allow BLKRRPART if the
>> UBLK_F_NO_AUTO_PART_SCAN is set? Wouldn't is be better to have a
> 
> No, I just want to allow future ioctl(BLKRRPART).
> 
>> simple 'UBLK_F_NO_PART_SCAN', which directly maps to GD_SUPPRESS_PART_SCAN?
>> Would we lose something substantial in that case?
> 
> If there is, we may add new feature flag for it.
> 
>>
>> I actually would _like_ to be able to suppress partition scan
>> completely for ublk devices...
> 
> Fine, we can change UBLK_F_NO_AUTO_PART_SCAN to align with GD_SUPPRESS_PART_SCAN.
> 
yes, please. That would make the code easier to understand and we 
wouldn't need to have any 'special' handling (or documentation)
here.

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

Re: ublk invert part scan bit logic

[Alexander Atanasov]

On 12.02.26 16:22, Ming Lei wrote:
> On Thu, Feb 12, 2026 at 03:04:53PM +0100, Hannes Reinecke wrote:
>> On 2/12/26 14:02, Ming Lei wrote:
>>> On Thu, Feb 12, 2026 at 05:48:40AM -0700, Jens Axboe wrote:
>>>> On 2/12/26 5:42 AM, Ming Lei wrote:
>>>>> On Thu, Feb 12, 2026 at 04:05:27AM -0700, Jens Axboe wrote:

>>>
>> Is there a specific reason to continue to allow BLKRRPART if the
>> UBLK_F_NO_AUTO_PART_SCAN is set? Wouldn't is be better to have a
> 
> No, I just want to allow future ioctl(BLKRRPART).
> 
>> simple 'UBLK_F_NO_PART_SCAN', which directly maps to GD_SUPPRESS_PART_SCAN?
>> Would we lose something substantial in that case?
> 
> If there is, we may add new feature flag for it.
> 
>>
>> I actually would _like_ to be able to suppress partition scan
>> completely for ublk devices...
> 
> Fine, we can change UBLK_F_NO_AUTO_PART_SCAN to align with GD_SUPPRESS_PART_SCAN.


These are two different things, does not make sense to merge them.

Use case is you have a backing files which are not ready/accessible -
you create a device with UBLK_F_NO_AUTO_PART_SCAN.
Do some preparations using the device and when files are ready
read the partitions.

If there is a use case to disable partitions on a trusted devices then a
new flag to map to GD_SUPPRESS_PART_SCAN makes sense.
And that is in case using UBLK_F_NO_AUTO_PART_SCAN to suppress initial
auto scan is not enough.


-- 
have fun,
alex

Re: ublk invert part scan bit logic

[Alexander Atanasov]

Hi,

On 12.02.26 13:05, Jens Axboe wrote:
> Hi,
> 
> For ublk, there's this logic in in ublk_ctrl_start_dev():
> 
> /* 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);
> }
> 
> where the
> 
> clear_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
> 
> seems reversed? Why is GD_SUPPRESS_PART_SCAN being cleared if
> UBLK_F_NO_AUTO_PART_SCAN is set? Added in:
> 
> 8443e2087e70 ("ublk: add UBLK_F_NO_AUTO_PART_SCAN feature flag")
> 

There is more to this :
a comment few lines above states that:
	 * For unprivileged daemons, keep GD_SUPPRESS_PART_SCAN set
	 * permanently.
	 */
	set_bit(GD_SUPPRESS_PART_SCAN, &disk->state);

next in ublk_partition_scan_work it is
if (WARN_ON_ONCE(!test_and_clear_bit(GD_SUPPRESS_PART_SCAN,
it is not used anywhere else but at least for debug it is good to stay
and align with the comment.

So if i've untwisted it right:
do not clear the bit before scanning and invert logic to clear the
bit only(enable) scan only if UBLK_F_NO_AUTO_PART_SCAN is not requested
by the user for a trusted daemon.

Ming - what do you think?

-- 
have fun,
alex

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 3c918db4905c..0d6d840b4f21 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -2361,7 +2361,7 @@ 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,
+       if (WARN_ON_ONCE(test_bit(GD_SUPPRESS_PART_SCAN,
                                              &disk->state)))
                 goto out;

@@ -4429,13 +4429,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) {
+       /* Skip partition scan if disabled by user only for trusted 
daemons */
+       if (!ub->unprivileged_daemons &&
+           !(ub->dev_info.flags & UBLK_F_NO_AUTO_PART_SCAN)) {
+               /* Enable partition scanning */
                 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);
+               schedule_work(&ub->partition_scan_work);
         }

  out_put_cdev:

Re: ublk invert part scan bit logic

[Ming Lei]

On Thu, Feb 12, 2026 at 03:08:17PM +0200, Alexander Atanasov wrote:
> Hi,
> 
> On 12.02.26 13:05, Jens Axboe wrote:
> > Hi,
> > 
> > For ublk, there's this logic in in ublk_ctrl_start_dev():
> > 
> > /* 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);
> > }
> > 
> > where the
> > 
> > clear_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
> > 
> > seems reversed? Why is GD_SUPPRESS_PART_SCAN being cleared if
> > UBLK_F_NO_AUTO_PART_SCAN is set? Added in:
> > 
> > 8443e2087e70 ("ublk: add UBLK_F_NO_AUTO_PART_SCAN feature flag")
> > 
> 
> There is more to this :
> a comment few lines above states that:
> 	 * For unprivileged daemons, keep GD_SUPPRESS_PART_SCAN set
> 	 * permanently.
> 	 */
> 	set_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
> 
> next in ublk_partition_scan_work it is
> if (WARN_ON_ONCE(!test_and_clear_bit(GD_SUPPRESS_PART_SCAN,
> it is not used anywhere else but at least for debug it is good to stay
> and align with the comment.

Actually ioctl(BLKRRPART) does require CAP_SYS_ADMIN, so either we
modify above comment or not clear GD_SUPPRESS_PART_SCAN for
ub->unprivileged_daemons.

> 
> So if i've untwisted it right:
> do not clear the bit before scanning and invert logic to clear the
> bit only(enable) scan only if UBLK_F_NO_AUTO_PART_SCAN is not requested
> by the user for a trusted daemon.
> 
> Ming - what do you think?
> 
> -- 
> have fun,
> alex
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 3c918db4905c..0d6d840b4f21 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -2361,7 +2361,7 @@ 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,
> +       if (WARN_ON_ONCE(test_bit(GD_SUPPRESS_PART_SCAN,
>                                              &disk->state)))

This way will fail future ioctl(BLKRRPART).

>                 goto out;
> 
> @@ -4429,13 +4429,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) {
> +       /* Skip partition scan if disabled by user only for trusted daemons
> */
> +       if (!ub->unprivileged_daemons &&
> +           !(ub->dev_info.flags & UBLK_F_NO_AUTO_PART_SCAN)) {

The above change looks fine.

> +               /* Enable partition scanning */
>                 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);
> +               schedule_work(&ub->partition_scan_work);

We shouldn't or can't schedule the part scan work in case of
ub->unprivileged_daemons.

Thanks,
Ming

Re: ublk invert part scan bit logic

[Alexander Atanasov]

On 12.02.26 15:17, Ming Lei wrote:

Cooked up a patch - i think i've hit some of the points in the thread.

https://lore.kernel.org/linux-block/20260212152553.1737373-1-alex@zazolabs.com/T/#u

-- 
have fun,
alex