* Re: [PATCH 1/1] nvme: fix implicit bool to flags conversion
2025-05-20 15:14 ` [PATCH 1/1] nvme: fix implicit bool to flags conversion Pavel Begunkov
@ 2025-05-20 15:21 ` Jens Axboe
2025-05-20 15:38 ` Anuj gupta
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Jens Axboe @ 2025-05-20 15:21 UTC (permalink / raw)
To: Pavel Begunkov, Keith Busch, Christoph Hellwig, Sagi Grimberg; +Cc: linux-nvme
On 5/20/25 9:14 AM, Pavel Begunkov wrote:
> nvme_map_user_request() takes flags as the last argument, but
> nvme_uring_cmd_io() shoves a bool "vec" into it. It behaves as
> expected because bool is converted to 0/1 and NVME_IOCTL_VEC is
> defined as 1, but it's better to pass flags explicitly.
LGTM:
Reviewed-by: Jens Axboe <axboe@kernel.dk>
--
Jens Axboe
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] nvme: fix implicit bool to flags conversion
2025-05-20 15:14 ` [PATCH 1/1] nvme: fix implicit bool to flags conversion Pavel Begunkov
2025-05-20 15:21 ` Jens Axboe
@ 2025-05-20 15:38 ` Anuj gupta
2025-05-20 15:42 ` Caleb Sander Mateos
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Anuj gupta @ 2025-05-20 15:38 UTC (permalink / raw)
To: Pavel Begunkov
Cc: Keith Busch, Christoph Hellwig, Sagi Grimberg, linux-nvme,
Jens Axboe
Looks good to me.
Reviewed-by: Anuj Gupta <anuj20.g@samsung.com>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] nvme: fix implicit bool to flags conversion
2025-05-20 15:14 ` [PATCH 1/1] nvme: fix implicit bool to flags conversion Pavel Begunkov
2025-05-20 15:21 ` Jens Axboe
2025-05-20 15:38 ` Anuj gupta
@ 2025-05-20 15:42 ` Caleb Sander Mateos
2025-05-20 15:46 ` Keith Busch
2025-05-20 15:51 ` Kanchan Joshi
` (2 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Caleb Sander Mateos @ 2025-05-20 15:42 UTC (permalink / raw)
To: Pavel Begunkov
Cc: Keith Busch, Christoph Hellwig, Sagi Grimberg, linux-nvme,
Jens Axboe
On Tue, May 20, 2025 at 8:28 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> nvme_map_user_request() takes flags as the last argument, but
> nvme_uring_cmd_io() shoves a bool "vec" into it. It behaves as
> expected because bool is converted to 0/1 and NVME_IOCTL_VEC is
> defined as 1, but it's better to pass flags explicitly.
>
> Fixes: 7b7fdb8e2dbc1 ("nvme: replace the "bool vec" arguments with flags in the ioctl path")
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
Keith had an earlier patch to fix this, but looks like the series
never got merged:
https://lore.kernel.org/linux-nvme/20250224182128.2042061-4-kbusch@meta.com/
This approach also looks fine to me, but not sure if it will
complicate some of the changes Keith wanted to make.
Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] nvme: fix implicit bool to flags conversion
2025-05-20 15:42 ` Caleb Sander Mateos
@ 2025-05-20 15:46 ` Keith Busch
2025-05-21 9:29 ` Pavel Begunkov
0 siblings, 1 reply; 9+ messages in thread
From: Keith Busch @ 2025-05-20 15:46 UTC (permalink / raw)
To: Caleb Sander Mateos
Cc: Pavel Begunkov, Christoph Hellwig, Sagi Grimberg, linux-nvme,
Jens Axboe
On Tue, May 20, 2025 at 08:42:36AM -0700, Caleb Sander Mateos wrote:
> On Tue, May 20, 2025 at 8:28 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
> >
> > nvme_map_user_request() takes flags as the last argument, but
> > nvme_uring_cmd_io() shoves a bool "vec" into it. It behaves as
> > expected because bool is converted to 0/1 and NVME_IOCTL_VEC is
> > defined as 1, but it's better to pass flags explicitly.
> >
> > Fixes: 7b7fdb8e2dbc1 ("nvme: replace the "bool vec" arguments with flags in the ioctl path")
> > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>
> Keith had an earlier patch to fix this, but looks like the series
> never got merged:
> https://lore.kernel.org/linux-nvme/20250224182128.2042061-4-kbusch@meta.com/
> This approach also looks fine to me, but not sure if it will
> complicate some of the changes Keith wanted to make.
Thanks, but no need to wait on me for this. I will revisit these
cleanups someday once I can arrange for fewer distractions, which may
not be any time soon.
Patch looks good to me too.
Reviewed-by: Keith Busch <kbusch@kernel.org>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] nvme: fix implicit bool to flags conversion
2025-05-20 15:46 ` Keith Busch
@ 2025-05-21 9:29 ` Pavel Begunkov
0 siblings, 0 replies; 9+ messages in thread
From: Pavel Begunkov @ 2025-05-21 9:29 UTC (permalink / raw)
To: Keith Busch, Caleb Sander Mateos
Cc: Christoph Hellwig, Sagi Grimberg, linux-nvme, Jens Axboe
On 5/20/25 16:46, Keith Busch wrote:
> On Tue, May 20, 2025 at 08:42:36AM -0700, Caleb Sander Mateos wrote:
>> On Tue, May 20, 2025 at 8:28 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>>>
>>> nvme_map_user_request() takes flags as the last argument, but
>>> nvme_uring_cmd_io() shoves a bool "vec" into it. It behaves as
>>> expected because bool is converted to 0/1 and NVME_IOCTL_VEC is
>>> defined as 1, but it's better to pass flags explicitly.
>>>
>>> Fixes: 7b7fdb8e2dbc1 ("nvme: replace the "bool vec" arguments with flags in the ioctl path")
>>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>>
>> Keith had an earlier patch to fix this, but looks like the series
>> never got merged:
>> https://lore.kernel.org/linux-nvme/20250224182128.2042061-4-kbusch@meta.com/
>> This approach also looks fine to me, but not sure if it will
>> complicate some of the changes Keith wanted to make.
Aha, so it was caught before, that's good.
> Thanks, but no need to wait on me for this. I will revisit these
> cleanups someday once I can arrange for fewer distractions, which may
> not be any time soon.
>
> Patch looks good to me too.
>
> Reviewed-by: Keith Busch <kbusch@kernel.org>
In which case it'd make sense to take this patch, but no objection
in either case.
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] nvme: fix implicit bool to flags conversion
2025-05-20 15:14 ` [PATCH 1/1] nvme: fix implicit bool to flags conversion Pavel Begunkov
` (2 preceding siblings ...)
2025-05-20 15:42 ` Caleb Sander Mateos
@ 2025-05-20 15:51 ` Kanchan Joshi
2025-05-20 19:17 ` Chaitanya Kulkarni
2025-05-21 11:09 ` Christoph Hellwig
5 siblings, 0 replies; 9+ messages in thread
From: Kanchan Joshi @ 2025-05-20 15:51 UTC (permalink / raw)
To: Pavel Begunkov, Keith Busch, Christoph Hellwig, Sagi Grimberg
Cc: linux-nvme, Jens Axboe
Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] nvme: fix implicit bool to flags conversion
2025-05-20 15:14 ` [PATCH 1/1] nvme: fix implicit bool to flags conversion Pavel Begunkov
` (3 preceding siblings ...)
2025-05-20 15:51 ` Kanchan Joshi
@ 2025-05-20 19:17 ` Chaitanya Kulkarni
2025-05-21 11:09 ` Christoph Hellwig
5 siblings, 0 replies; 9+ messages in thread
From: Chaitanya Kulkarni @ 2025-05-20 19:17 UTC (permalink / raw)
To: Pavel Begunkov, Keith Busch, Christoph Hellwig, Sagi Grimberg
Cc: linux-nvme@lists.infradead.org, Jens Axboe
On 5/20/25 08:14, Pavel Begunkov wrote:
> nvme_map_user_request() takes flags as the last argument, but
> nvme_uring_cmd_io() shoves a bool "vec" into it. It behaves as
> expected because bool is converted to 0/1 and NVME_IOCTL_VEC is
> defined as 1, but it's better to pass flags explicitly.
>
> Fixes: 7b7fdb8e2dbc1 ("nvme: replace the "bool vec" arguments with flags in the ioctl path")
> Signed-off-by: Pavel Begunkov<asml.silence@gmail.com>
Looks good.
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
-ck
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] nvme: fix implicit bool to flags conversion
2025-05-20 15:14 ` [PATCH 1/1] nvme: fix implicit bool to flags conversion Pavel Begunkov
` (4 preceding siblings ...)
2025-05-20 19:17 ` Chaitanya Kulkarni
@ 2025-05-21 11:09 ` Christoph Hellwig
5 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2025-05-21 11:09 UTC (permalink / raw)
To: Pavel Begunkov
Cc: Keith Busch, Christoph Hellwig, Sagi Grimberg, linux-nvme,
Jens Axboe
Thanks,
applied to nvme-6.16.
^ permalink raw reply [flat|nested] 9+ messages in thread