All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ming Lei <ming.lei@redhat.com>
To: Caleb Sander Mateos <csander@purestorage.com>
Cc: Alexander Atanasov <alex@zazolabs.com>,
	Jens Axboe <axboe@kernel.dk>,
	linux-block@vger.kernel.org
Subject: Re: [PATCH 1/4] ublk: cleanup flag handling and fix UBLK_F_NO_AUTO_PART_SCAN
Date: Tue, 24 Feb 2026 17:31:57 +0800	[thread overview]
Message-ID: <aZ1wDStqgU5vjyFR@fedora> (raw)
In-Reply-To: <CADUfDZp-jX98mfERO0OUoMePfws5yRtvfmLfzD2tzfi+HDPyLQ@mail.gmail.com>

On Mon, Feb 23, 2026 at 07:59:54AM -0800, Caleb Sander Mateos wrote:
> On Mon, Feb 23, 2026 at 4:17 AM Alexander Atanasov <alex@zazolabs.com> wrote:
> >
> > Currently the code misuse GD_SUPPRESS_PART_SCAN flag
> > as it tries to use it as a switch for the auto partition scan.
> > To fully disable creation of partitions GENHD_FL_NO_PART must
> > be used, switch code to use it instead GD_SUPPRESS_PART_SCAN.
> >
> > Using the new flags rules for partitions become:
> >  - Unprivileged daemons - never scan partitions, they have
> > GENHD_FL_NO_PART always set - they are devices without partitions.
> >
> >  - Trusted daemons - by default have partitions disabled.
> > Partitions can be enabled via UBLK_F_PARTITIONS flag and
> > automatic initial scan can be requested via UBLK_F_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.
> > - set GENHD_FL_NO_PART on unprivileged devices
> > - set GENHD_FL_NO_PART depending on UBLK_F_PARTITIONS on trusted
> > devices.
> >
> > 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      | 34 +++++++++++++++++++---------------
> >  include/uapi/linux/ublk_cmd.h |  7 +++++--
> >  2 files changed, 24 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > index 3c918db4905c..d0c646eb9435 100644
> > --- a/drivers/block/ublk_drv.c
> > +++ b/drivers/block/ublk_drv.c
> > @@ -81,7 +81,8 @@
> >                 | (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_PARTITIONS \
> > +               | UBLK_F_AUTO_PART_SCAN)
> >
> >  #define UBLK_F_ALL_RECOVERY_FLAGS (UBLK_F_USER_RECOVERY \
> >                 | UBLK_F_USER_RECOVERY_REISSUE \
> > @@ -2361,14 +2362,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);
> >  }
> >
> > @@ -4409,9 +4405,13 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
> >          * wait while holding ub->mutex, which can deadlock with other
> >          * operations that need the mutex. Defer partition scan to async
> >          * work.
> > -        * For unprivileged daemons, keep GD_SUPPRESS_PART_SCAN set
> > -        * permanently.
> > +        * For unprivileged daemons, set GENHD_FL_NO_PART to
> > +        * disable partitions.
> >          */
> > +       if (ub->unprivileged_daemons)
> > +               disk->flags |= GENHD_FL_NO_PART;
> > +
> > +       /* Disable partition scans */
> >         set_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
> >
> >         ublk_get_device(ub);
> > @@ -4429,13 +4429,17 @@ 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);
> > +       if (!ub->unprivileged_daemons) {
> > +               if (!(ub->dev_info.flags & UBLK_F_PARTITIONS)) {
> > +                       /* disable partitions on trusted device */
> > +                       disk->flags |= GENHD_FL_NO_PART;
> > +               } else {
> > +                       /* Enable partition scan on device */
> > +                       clear_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
> > +                       /* Schedule auto partition scan if requested */
> > +                       if (ub->dev_info.flags & UBLK_F_AUTO_PART_SCAN)
> > +                               schedule_work(&ub->partition_scan_work);
> > +               }
> >         }
> >
> >  out_put_cdev:
> > diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
> > index a88876756805..467ecaa20ac1 100644
> > --- a/include/uapi/linux/ublk_cmd.h
> > +++ b/include/uapi/linux/ublk_cmd.h
> > @@ -367,8 +367,11 @@
> >   */
> >  #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)
> > +/* Enable partitions on the device */
> > +#define UBLK_F_PARTITIONS (1ULL << 18)
> 
> This is not backwards compatible. Existing ublk servers expect their
> devices to support partitioning. If you want to introduce a feature
> flag to control this, it needs to be opt-out rather than opt-in.
> UBLK_F_AUTO_PART_SCAN probably also needs to remain as
> UBLK_F_NO_AUTO_PART_SCAN.

UBLK_F_AUTO_PART_SCAN is just merged in 7.0-rc, which isn't released yet,
so it is fine to change it before 7.0 formal release, IMO.


Thanks,
Ming


  parent reply	other threads:[~2026-02-24  9:32 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-23 12:15 [PATCH 0/4] ublk: rework partition handling Alexander Atanasov
2026-02-23 12:15 ` [PATCH 1/4] ublk: cleanup flag handling and fix UBLK_F_NO_AUTO_PART_SCAN Alexander Atanasov
2026-02-23 12:29   ` Hannes Reinecke
2026-02-23 15:59   ` Caleb Sander Mateos
2026-02-23 16:23     ` Alexander Atanasov
2026-02-23 16:42       ` Caleb Sander Mateos
2026-02-23 17:19         ` Alexander Atanasov
2026-02-24  9:31     ` Ming Lei [this message]
2026-02-24  9:34   ` Ming Lei
     [not found]     ` <182C75BB-3CBC-4627-9F15-AE8FE675260C@zazolabs.com>
2026-02-24 11:04       ` Ming Lei
2026-02-23 12:15 ` [PATCH 2/4] selftests: ublk: rework partitions tests for new flags Alexander Atanasov
2026-02-23 12:29   ` Hannes Reinecke
2026-02-23 12:15 ` [PATCH 3/4] ublk: document partition flags, kublk and update external links Alexander Atanasov
2026-02-23 12:30   ` Hannes Reinecke
2026-02-23 12:15 ` [PATCH 4/4] selftests: ublk: cleanup kublk.h Alexander Atanasov
2026-02-23 12:30   ` Hannes Reinecke

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aZ1wDStqgU5vjyFR@fedora \
    --to=ming.lei@redhat.com \
    --cc=alex@zazolabs.com \
    --cc=axboe@kernel.dk \
    --cc=csander@purestorage.com \
    --cc=linux-block@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.