* [PATCH] ublk: check for unprivileged daemon on each I/O fetch
@ 2025-08-08 15:52 Caleb Sander Mateos
2025-08-08 18:01 ` Uday Shankar
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Caleb Sander Mateos @ 2025-08-08 15:52 UTC (permalink / raw)
To: Ming Lei, Jens Axboe, Caleb Sander Mateos, Uday Shankar
Cc: linux-block, linux-kernel
Commit ab03a61c6614 ("ublk: have a per-io daemon instead of a per-queue
daemon") allowed each ublk I/O to have an independent daemon task.
However, nr_privileged_daemon is only computed based on whether the last
I/O fetched in each ublk queue has an unprivileged daemon task.
Fix this by checking whether every fetched I/O's daemon is privileged.
Change nr_privileged_daemon from a count of queues to a boolean
indicating whether any I/Os have an unprivileged daemon.
Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
Fixes: ab03a61c6614 ("ublk: have a per-io daemon instead of a per-queue daemon")
---
drivers/block/ublk_drv.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 6561d2a561fa..a035070dd690 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -233,11 +233,11 @@ struct ublk_device {
struct ublk_params params;
struct completion completion;
unsigned int nr_queues_ready;
- unsigned int nr_privileged_daemon;
+ bool unprivileged_daemons;
struct mutex cancel_mutex;
bool canceling;
pid_t ublksrv_tgid;
};
@@ -1548,11 +1548,11 @@ static void ublk_reset_ch_dev(struct ublk_device *ub)
ublk_queue_reinit(ub, ublk_get_queue(ub, i));
/* set to NULL, otherwise new tasks cannot mmap io_cmd_buf */
ub->mm = NULL;
ub->nr_queues_ready = 0;
- ub->nr_privileged_daemon = 0;
+ ub->unprivileged_daemons = false;
ub->ublksrv_tgid = -1;
}
static struct gendisk *ublk_get_disk(struct ublk_device *ub)
{
@@ -1978,16 +1978,14 @@ static void ublk_reset_io_flags(struct ublk_device *ub)
/* device can only be started after all IOs are ready */
static void ublk_mark_io_ready(struct ublk_device *ub, struct ublk_queue *ubq)
__must_hold(&ub->mutex)
{
ubq->nr_io_ready++;
- if (ublk_queue_ready(ubq)) {
+ if (ublk_queue_ready(ubq))
ub->nr_queues_ready++;
-
- if (capable(CAP_SYS_ADMIN))
- ub->nr_privileged_daemon++;
- }
+ if (!ub->unprivileged_daemons && !capable(CAP_SYS_ADMIN))
+ ub->unprivileged_daemons = true;
if (ub->nr_queues_ready == ub->dev_info.nr_hw_queues) {
/* now we are ready for handling ublk io request */
ublk_reset_io_flags(ub);
complete_all(&ub->completion);
@@ -2878,12 +2876,12 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
ub->dev_info.ublksrv_pid = ublksrv_pid;
ub->ub_disk = disk;
ublk_apply_params(ub);
- /* don't probe partitions if any one ubq daemon is un-trusted */
- if (ub->nr_privileged_daemon != ub->nr_queues_ready)
+ /* don't probe partitions if any daemon task is un-trusted */
+ if (ub->unprivileged_daemons)
set_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
ublk_get_device(ub);
ub->dev_info.state = UBLK_S_DEV_LIVE;
--
2.45.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ublk: check for unprivileged daemon on each I/O fetch
2025-08-08 15:52 [PATCH] ublk: check for unprivileged daemon on each I/O fetch Caleb Sander Mateos
@ 2025-08-08 18:01 ` Uday Shankar
2025-08-08 18:03 ` Caleb Sander Mateos
2025-08-10 2:20 ` Ming Lei
2025-08-11 14:01 ` Jens Axboe
2 siblings, 1 reply; 6+ messages in thread
From: Uday Shankar @ 2025-08-08 18:01 UTC (permalink / raw)
To: Caleb Sander Mateos; +Cc: Ming Lei, Jens Axboe, linux-block, linux-kernel
On Fri, Aug 08, 2025 at 09:52:15AM -0600, Caleb Sander Mateos wrote:
> Commit ab03a61c6614 ("ublk: have a per-io daemon instead of a per-queue
> daemon") allowed each ublk I/O to have an independent daemon task.
> However, nr_privileged_daemon is only computed based on whether the last
> I/O fetched in each ublk queue has an unprivileged daemon task.
> Fix this by checking whether every fetched I/O's daemon is privileged.
> Change nr_privileged_daemon from a count of queues to a boolean
> indicating whether any I/Os have an unprivileged daemon.
>
> Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> Fixes: ab03a61c6614 ("ublk: have a per-io daemon instead of a per-queue daemon")
Nice catch!
> ---
> drivers/block/ublk_drv.c | 16 +++++++---------
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 6561d2a561fa..a035070dd690 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -233,11 +233,11 @@ struct ublk_device {
>
> struct ublk_params params;
>
> struct completion completion;
> unsigned int nr_queues_ready;
> - unsigned int nr_privileged_daemon;
> + bool unprivileged_daemons;
> struct mutex cancel_mutex;
> bool canceling;
> pid_t ublksrv_tgid;
> };
>
> @@ -1548,11 +1548,11 @@ static void ublk_reset_ch_dev(struct ublk_device *ub)
> ublk_queue_reinit(ub, ublk_get_queue(ub, i));
>
> /* set to NULL, otherwise new tasks cannot mmap io_cmd_buf */
> ub->mm = NULL;
> ub->nr_queues_ready = 0;
> - ub->nr_privileged_daemon = 0;
> + ub->unprivileged_daemons = false;
> ub->ublksrv_tgid = -1;
> }
>
> static struct gendisk *ublk_get_disk(struct ublk_device *ub)
> {
> @@ -1978,16 +1978,14 @@ static void ublk_reset_io_flags(struct ublk_device *ub)
> /* device can only be started after all IOs are ready */
> static void ublk_mark_io_ready(struct ublk_device *ub, struct ublk_queue *ubq)
> __must_hold(&ub->mutex)
> {
> ubq->nr_io_ready++;
> - if (ublk_queue_ready(ubq)) {
> + if (ublk_queue_ready(ubq))
> ub->nr_queues_ready++;
> -
> - if (capable(CAP_SYS_ADMIN))
> - ub->nr_privileged_daemon++;
> - }
> + if (!ub->unprivileged_daemons && !capable(CAP_SYS_ADMIN))
> + ub->unprivileged_daemons = true;
Shorter:
ub->unprivileged_daemons |= !capable(CAP_SYS_ADMIN);
>
> if (ub->nr_queues_ready == ub->dev_info.nr_hw_queues) {
> /* now we are ready for handling ublk io request */
> ublk_reset_io_flags(ub);
> complete_all(&ub->completion);
> @@ -2878,12 +2876,12 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
> ub->dev_info.ublksrv_pid = ublksrv_pid;
> ub->ub_disk = disk;
>
> ublk_apply_params(ub);
>
> - /* don't probe partitions if any one ubq daemon is un-trusted */
> - if (ub->nr_privileged_daemon != ub->nr_queues_ready)
> + /* don't probe partitions if any daemon task is un-trusted */
> + if (ub->unprivileged_daemons)
> set_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
>
> ublk_get_device(ub);
> ub->dev_info.state = UBLK_S_DEV_LIVE;
>
> --
> 2.45.2
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ublk: check for unprivileged daemon on each I/O fetch
2025-08-08 18:01 ` Uday Shankar
@ 2025-08-08 18:03 ` Caleb Sander Mateos
2025-08-08 18:22 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: Caleb Sander Mateos @ 2025-08-08 18:03 UTC (permalink / raw)
To: Uday Shankar; +Cc: Ming Lei, Jens Axboe, linux-block, linux-kernel
On Fri, Aug 8, 2025 at 2:01 PM Uday Shankar <ushankar@purestorage.com> wrote:
>
> On Fri, Aug 08, 2025 at 09:52:15AM -0600, Caleb Sander Mateos wrote:
> > Commit ab03a61c6614 ("ublk: have a per-io daemon instead of a per-queue
> > daemon") allowed each ublk I/O to have an independent daemon task.
> > However, nr_privileged_daemon is only computed based on whether the last
> > I/O fetched in each ublk queue has an unprivileged daemon task.
> > Fix this by checking whether every fetched I/O's daemon is privileged.
> > Change nr_privileged_daemon from a count of queues to a boolean
> > indicating whether any I/Os have an unprivileged daemon.
> >
> > Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> > Fixes: ab03a61c6614 ("ublk: have a per-io daemon instead of a per-queue daemon")
>
> Nice catch!
>
> > ---
> > drivers/block/ublk_drv.c | 16 +++++++---------
> > 1 file changed, 7 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > index 6561d2a561fa..a035070dd690 100644
> > --- a/drivers/block/ublk_drv.c
> > +++ b/drivers/block/ublk_drv.c
> > @@ -233,11 +233,11 @@ struct ublk_device {
> >
> > struct ublk_params params;
> >
> > struct completion completion;
> > unsigned int nr_queues_ready;
> > - unsigned int nr_privileged_daemon;
> > + bool unprivileged_daemons;
> > struct mutex cancel_mutex;
> > bool canceling;
> > pid_t ublksrv_tgid;
> > };
> >
> > @@ -1548,11 +1548,11 @@ static void ublk_reset_ch_dev(struct ublk_device *ub)
> > ublk_queue_reinit(ub, ublk_get_queue(ub, i));
> >
> > /* set to NULL, otherwise new tasks cannot mmap io_cmd_buf */
> > ub->mm = NULL;
> > ub->nr_queues_ready = 0;
> > - ub->nr_privileged_daemon = 0;
> > + ub->unprivileged_daemons = false;
> > ub->ublksrv_tgid = -1;
> > }
> >
> > static struct gendisk *ublk_get_disk(struct ublk_device *ub)
> > {
> > @@ -1978,16 +1978,14 @@ static void ublk_reset_io_flags(struct ublk_device *ub)
> > /* device can only be started after all IOs are ready */
> > static void ublk_mark_io_ready(struct ublk_device *ub, struct ublk_queue *ubq)
> > __must_hold(&ub->mutex)
> > {
> > ubq->nr_io_ready++;
> > - if (ublk_queue_ready(ubq)) {
> > + if (ublk_queue_ready(ubq))
> > ub->nr_queues_ready++;
> > -
> > - if (capable(CAP_SYS_ADMIN))
> > - ub->nr_privileged_daemon++;
> > - }
> > + if (!ub->unprivileged_daemons && !capable(CAP_SYS_ADMIN))
> > + ub->unprivileged_daemons = true;
>
> Shorter:
>
> ub->unprivileged_daemons |= !capable(CAP_SYS_ADMIN);
I was trying to avoid the capable() call if unprivileged_daemons was
already set. But maybe that's not a common case and it's not worth
optimizing?
Best,
Caleb
>
> >
> > if (ub->nr_queues_ready == ub->dev_info.nr_hw_queues) {
> > /* now we are ready for handling ublk io request */
> > ublk_reset_io_flags(ub);
> > complete_all(&ub->completion);
> > @@ -2878,12 +2876,12 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub,
> > ub->dev_info.ublksrv_pid = ublksrv_pid;
> > ub->ub_disk = disk;
> >
> > ublk_apply_params(ub);
> >
> > - /* don't probe partitions if any one ubq daemon is un-trusted */
> > - if (ub->nr_privileged_daemon != ub->nr_queues_ready)
> > + /* don't probe partitions if any daemon task is un-trusted */
> > + if (ub->unprivileged_daemons)
> > set_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
> >
> > ublk_get_device(ub);
> > ub->dev_info.state = UBLK_S_DEV_LIVE;
> >
> > --
> > 2.45.2
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ublk: check for unprivileged daemon on each I/O fetch
2025-08-08 18:03 ` Caleb Sander Mateos
@ 2025-08-08 18:22 ` Jens Axboe
0 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2025-08-08 18:22 UTC (permalink / raw)
To: Caleb Sander Mateos, Uday Shankar; +Cc: Ming Lei, linux-block, linux-kernel
On 8/8/25 12:03 PM, Caleb Sander Mateos wrote:
> On Fri, Aug 8, 2025 at 2:01?PM Uday Shankar <ushankar@purestorage.com> wrote:
>>
>> On Fri, Aug 08, 2025 at 09:52:15AM -0600, Caleb Sander Mateos wrote:
>>> Commit ab03a61c6614 ("ublk: have a per-io daemon instead of a per-queue
>>> daemon") allowed each ublk I/O to have an independent daemon task.
>>> However, nr_privileged_daemon is only computed based on whether the last
>>> I/O fetched in each ublk queue has an unprivileged daemon task.
>>> Fix this by checking whether every fetched I/O's daemon is privileged.
>>> Change nr_privileged_daemon from a count of queues to a boolean
>>> indicating whether any I/Os have an unprivileged daemon.
>>>
>>> Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
>>> Fixes: ab03a61c6614 ("ublk: have a per-io daemon instead of a per-queue daemon")
>>
>> Nice catch!
>>
>>> ---
>>> drivers/block/ublk_drv.c | 16 +++++++---------
>>> 1 file changed, 7 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
>>> index 6561d2a561fa..a035070dd690 100644
>>> --- a/drivers/block/ublk_drv.c
>>> +++ b/drivers/block/ublk_drv.c
>>> @@ -233,11 +233,11 @@ struct ublk_device {
>>>
>>> struct ublk_params params;
>>>
>>> struct completion completion;
>>> unsigned int nr_queues_ready;
>>> - unsigned int nr_privileged_daemon;
>>> + bool unprivileged_daemons;
>>> struct mutex cancel_mutex;
>>> bool canceling;
>>> pid_t ublksrv_tgid;
>>> };
>>>
>>> @@ -1548,11 +1548,11 @@ static void ublk_reset_ch_dev(struct ublk_device *ub)
>>> ublk_queue_reinit(ub, ublk_get_queue(ub, i));
>>>
>>> /* set to NULL, otherwise new tasks cannot mmap io_cmd_buf */
>>> ub->mm = NULL;
>>> ub->nr_queues_ready = 0;
>>> - ub->nr_privileged_daemon = 0;
>>> + ub->unprivileged_daemons = false;
>>> ub->ublksrv_tgid = -1;
>>> }
>>>
>>> static struct gendisk *ublk_get_disk(struct ublk_device *ub)
>>> {
>>> @@ -1978,16 +1978,14 @@ static void ublk_reset_io_flags(struct ublk_device *ub)
>>> /* device can only be started after all IOs are ready */
>>> static void ublk_mark_io_ready(struct ublk_device *ub, struct ublk_queue *ubq)
>>> __must_hold(&ub->mutex)
>>> {
>>> ubq->nr_io_ready++;
>>> - if (ublk_queue_ready(ubq)) {
>>> + if (ublk_queue_ready(ubq))
>>> ub->nr_queues_ready++;
>>> -
>>> - if (capable(CAP_SYS_ADMIN))
>>> - ub->nr_privileged_daemon++;
>>> - }
>>> + if (!ub->unprivileged_daemons && !capable(CAP_SYS_ADMIN))
>>> + ub->unprivileged_daemons = true;
>>
>> Shorter:
>>
>> ub->unprivileged_daemons |= !capable(CAP_SYS_ADMIN);
>
> I was trying to avoid the capable() call if unprivileged_daemons was
> already set. But maybe that's not a common case and it's not worth
> optimizing?
Definitely worth it, you did the right thing.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ublk: check for unprivileged daemon on each I/O fetch
2025-08-08 15:52 [PATCH] ublk: check for unprivileged daemon on each I/O fetch Caleb Sander Mateos
2025-08-08 18:01 ` Uday Shankar
@ 2025-08-10 2:20 ` Ming Lei
2025-08-11 14:01 ` Jens Axboe
2 siblings, 0 replies; 6+ messages in thread
From: Ming Lei @ 2025-08-10 2:20 UTC (permalink / raw)
To: Caleb Sander Mateos; +Cc: Jens Axboe, Uday Shankar, linux-block, linux-kernel
On Fri, Aug 08, 2025 at 09:52:15AM -0600, Caleb Sander Mateos wrote:
> Commit ab03a61c6614 ("ublk: have a per-io daemon instead of a per-queue
> daemon") allowed each ublk I/O to have an independent daemon task.
> However, nr_privileged_daemon is only computed based on whether the last
> I/O fetched in each ublk queue has an unprivileged daemon task.
> Fix this by checking whether every fetched I/O's daemon is privileged.
> Change nr_privileged_daemon from a count of queues to a boolean
> indicating whether any I/Os have an unprivileged daemon.
>
> Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
> Fixes: ab03a61c6614 ("ublk: have a per-io daemon instead of a per-queue daemon")
Looks fine,
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Thanks,
Ming
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ublk: check for unprivileged daemon on each I/O fetch
2025-08-08 15:52 [PATCH] ublk: check for unprivileged daemon on each I/O fetch Caleb Sander Mateos
2025-08-08 18:01 ` Uday Shankar
2025-08-10 2:20 ` Ming Lei
@ 2025-08-11 14:01 ` Jens Axboe
2 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2025-08-11 14:01 UTC (permalink / raw)
To: Ming Lei, Uday Shankar, Caleb Sander Mateos; +Cc: linux-block, linux-kernel
On Fri, 08 Aug 2025 09:52:15 -0600, Caleb Sander Mateos wrote:
> Commit ab03a61c6614 ("ublk: have a per-io daemon instead of a per-queue
> daemon") allowed each ublk I/O to have an independent daemon task.
> However, nr_privileged_daemon is only computed based on whether the last
> I/O fetched in each ublk queue has an unprivileged daemon task.
> Fix this by checking whether every fetched I/O's daemon is privileged.
> Change nr_privileged_daemon from a count of queues to a boolean
> indicating whether any I/Os have an unprivileged daemon.
>
> [...]
Applied, thanks!
[1/1] ublk: check for unprivileged daemon on each I/O fetch
commit: 5058a62875e1916e5133a1639f0207ea2148c0bc
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-08-11 14:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-08 15:52 [PATCH] ublk: check for unprivileged daemon on each I/O fetch Caleb Sander Mateos
2025-08-08 18:01 ` Uday Shankar
2025-08-08 18:03 ` Caleb Sander Mateos
2025-08-08 18:22 ` Jens Axboe
2025-08-10 2:20 ` Ming Lei
2025-08-11 14:01 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).