Re: ublk invert part scan bit logic

[Ming Lei <ming.lei@redhat.com>]

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