2023-03-22 19:16:56

by Mike Christie

[permalink] [raw]
Subject: [PATCH 1/1] vhost_task: Fix vhost_task_create return value

vhost_task_create is supposed to return the vhost_task or NULL on
failure. This fixes it to return the correct value when the allocation
of the struct fails.

Fixes: 77feab3c4156 ("vhost_task: Allow vhost layer to use copy_process") # mainline only
Reported-by: [email protected]
Signed-off-by: Mike Christie <[email protected]>
---
kernel/vhost_task.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/vhost_task.c b/kernel/vhost_task.c
index 4b8aff160640..b7cbd66f889e 100644
--- a/kernel/vhost_task.c
+++ b/kernel/vhost_task.c
@@ -88,7 +88,7 @@ struct vhost_task *vhost_task_create(int (*fn)(void *), void *arg,

vtsk = kzalloc(sizeof(*vtsk), GFP_KERNEL);
if (!vtsk)
- return ERR_PTR(-ENOMEM);
+ return NULL;
init_completion(&vtsk->exited);
vtsk->data = arg;
vtsk->fn = fn;
--
2.25.1


2023-03-23 07:45:22

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [PATCH 1/1] vhost_task: Fix vhost_task_create return value

On Wed, Mar 22, 2023 at 01:56:05PM -0500, Mike Christie wrote:
> vhost_task_create is supposed to return the vhost_task or NULL on
> failure. This fixes it to return the correct value when the allocation
> of the struct fails.
>
> Fixes: 77feab3c4156 ("vhost_task: Allow vhost layer to use copy_process") # mainline only
> Reported-by: [email protected]
> Signed-off-by: Mike Christie <[email protected]>

Acked-by: Michael S. Tsirkin <[email protected]>

The affected patch is not upstream yet, right?
I don't know if the tree in question allows rebases - linux-next
does. So ideally it would be squashed to avoid issues during bisect.
Still it's error path so I guess not a tragedy even without squashing.

> ---
> kernel/vhost_task.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/vhost_task.c b/kernel/vhost_task.c
> index 4b8aff160640..b7cbd66f889e 100644
> --- a/kernel/vhost_task.c
> +++ b/kernel/vhost_task.c
> @@ -88,7 +88,7 @@ struct vhost_task *vhost_task_create(int (*fn)(void *), void *arg,
>
> vtsk = kzalloc(sizeof(*vtsk), GFP_KERNEL);
> if (!vtsk)
> - return ERR_PTR(-ENOMEM);
> + return NULL;
> init_completion(&vtsk->exited);
> vtsk->data = arg;
> vtsk->fn = fn;
> --
> 2.25.1

2023-03-23 08:52:26

by Stefano Garzarella

[permalink] [raw]
Subject: Re: [PATCH 1/1] vhost_task: Fix vhost_task_create return value

On Wed, Mar 22, 2023 at 01:56:05PM -0500, Mike Christie wrote:
>vhost_task_create is supposed to return the vhost_task or NULL on
>failure. This fixes it to return the correct value when the allocation
>of the struct fails.
>
>Fixes: 77feab3c4156 ("vhost_task: Allow vhost layer to use copy_process") # mainline only
>Reported-by: [email protected]
>Signed-off-by: Mike Christie <[email protected]>
>---
> kernel/vhost_task.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Stefano Garzarella <[email protected]>

>
>diff --git a/kernel/vhost_task.c b/kernel/vhost_task.c
>index 4b8aff160640..b7cbd66f889e 100644
>--- a/kernel/vhost_task.c
>+++ b/kernel/vhost_task.c
>@@ -88,7 +88,7 @@ struct vhost_task *vhost_task_create(int (*fn)(void *), void *arg,
>
> vtsk = kzalloc(sizeof(*vtsk), GFP_KERNEL);
> if (!vtsk)
>- return ERR_PTR(-ENOMEM);
>+ return NULL;
> init_completion(&vtsk->exited);
> vtsk->data = arg;
> vtsk->fn = fn;
>--
>2.25.1
>

2023-03-23 10:48:22

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH 1/1] vhost_task: Fix vhost_task_create return value


On Wed, 22 Mar 2023 13:56:05 -0500, Mike Christie wrote:
> vhost_task_create is supposed to return the vhost_task or NULL on
> failure. This fixes it to return the correct value when the allocation
> of the struct fails.
>
>

Applied, thanks!

tree: git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git
branch: kernel.user_worker

[1/1] vhost_task: Fix vhost_task_create return value
commit: cbc0fc3d37b88182c1454da02002cd7d99b4a28e

2023-03-23 10:51:12

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH 1/1] vhost_task: Fix vhost_task_create return value

On Thu, Mar 23, 2023 at 03:37:19AM -0400, Michael S. Tsirkin wrote:
> On Wed, Mar 22, 2023 at 01:56:05PM -0500, Mike Christie wrote:
> > vhost_task_create is supposed to return the vhost_task or NULL on
> > failure. This fixes it to return the correct value when the allocation
> > of the struct fails.
> >
> > Fixes: 77feab3c4156 ("vhost_task: Allow vhost layer to use copy_process") # mainline only
> > Reported-by: [email protected]
> > Signed-off-by: Mike Christie <[email protected]>
>
> Acked-by: Michael S. Tsirkin <[email protected]>
>
> The affected patch is not upstream yet, right?
> I don't know if the tree in question allows rebases - linux-next
> does. So ideally it would be squashed to avoid issues during bisect.
> Still it's error path so I guess not a tragedy even without squashing.

I tend to not rebase once stuff has been in linux-next but I make
exceptions as long as it's before -rc4. For now I've put the patch on
top (see the other mail I sent) but if it's really important I can
squash it after the weekend (I'll be mostly afk until then.).

2023-03-23 11:45:36

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [PATCH 1/1] vhost_task: Fix vhost_task_create return value

On Thu, Mar 23, 2023 at 11:44:45AM +0100, Christian Brauner wrote:
> On Thu, Mar 23, 2023 at 03:37:19AM -0400, Michael S. Tsirkin wrote:
> > On Wed, Mar 22, 2023 at 01:56:05PM -0500, Mike Christie wrote:
> > > vhost_task_create is supposed to return the vhost_task or NULL on
> > > failure. This fixes it to return the correct value when the allocation
> > > of the struct fails.
> > >
> > > Fixes: 77feab3c4156 ("vhost_task: Allow vhost layer to use copy_process") # mainline only
> > > Reported-by: [email protected]
> > > Signed-off-by: Mike Christie <[email protected]>
> >
> > Acked-by: Michael S. Tsirkin <[email protected]>
> >
> > The affected patch is not upstream yet, right?
> > I don't know if the tree in question allows rebases - linux-next
> > does. So ideally it would be squashed to avoid issues during bisect.
> > Still it's error path so I guess not a tragedy even without squashing.
>
> I tend to not rebase once stuff has been in linux-next but I make
> exceptions as long as it's before -rc4. For now I've put the patch on
> top (see the other mail I sent) but if it's really important I can
> squash it after the weekend (I'll be mostly afk until then.).

Hard to say how important, but I'd prefer that, yes.

--
MST

2023-03-23 11:51:34

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH 1/1] vhost_task: Fix vhost_task_create return value

On Thu, Mar 23, 2023 at 07:43:04AM -0400, Michael S. Tsirkin wrote:
> On Thu, Mar 23, 2023 at 11:44:45AM +0100, Christian Brauner wrote:
> > On Thu, Mar 23, 2023 at 03:37:19AM -0400, Michael S. Tsirkin wrote:
> > > On Wed, Mar 22, 2023 at 01:56:05PM -0500, Mike Christie wrote:
> > > > vhost_task_create is supposed to return the vhost_task or NULL on
> > > > failure. This fixes it to return the correct value when the allocation
> > > > of the struct fails.
> > > >
> > > > Fixes: 77feab3c4156 ("vhost_task: Allow vhost layer to use copy_process") # mainline only
> > > > Reported-by: [email protected]
> > > > Signed-off-by: Mike Christie <[email protected]>
> > >
> > > Acked-by: Michael S. Tsirkin <[email protected]>
> > >
> > > The affected patch is not upstream yet, right?
> > > I don't know if the tree in question allows rebases - linux-next
> > > does. So ideally it would be squashed to avoid issues during bisect.
> > > Still it's error path so I guess not a tragedy even without squashing.
> >
> > I tend to not rebase once stuff has been in linux-next but I make
> > exceptions as long as it's before -rc4. For now I've put the patch on
> > top (see the other mail I sent) but if it's really important I can
> > squash it after the weekend (I'll be mostly afk until then.).
>
> Hard to say how important, but I'd prefer that, yes.

Ok, fold the fixup into

e297cd54b3f8 vhost_task: Allow vhost layer to use copy_process

the series is now at:

tree: git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git
branch: kernel.user_worker

1a5f8090c6de vhost: move worker thread fields to new struct
e297cd54b3f8 vhost_task: Allow vhost layer to use copy_process
89c8e98d8cfb fork: allow kernel code to call copy_process
094717586bf7 fork: Add kernel_clone_args flag to ignore signals
11f3f500ec8a fork: add kernel_clone_args flag to not dup/clone files
54e6842d0775 fork/vm: Move common PF_IO_WORKER behavior to new flag
c81cc5819faf kernel: Make io_thread and kthread bit fields
73e0c116594d kthread: Pass in the thread's name during creation
cf587db2ee02 kernel: Allow a kernel thread's name to be set in copy_process
e0a98139c162 csky: Remove kernel_thread declaration

2023-03-23 13:18:33

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [PATCH 1/1] vhost_task: Fix vhost_task_create return value

On Thu, Mar 23, 2023 at 12:50:49PM +0100, Christian Brauner wrote:
> On Thu, Mar 23, 2023 at 07:43:04AM -0400, Michael S. Tsirkin wrote:
> > On Thu, Mar 23, 2023 at 11:44:45AM +0100, Christian Brauner wrote:
> > > On Thu, Mar 23, 2023 at 03:37:19AM -0400, Michael S. Tsirkin wrote:
> > > > On Wed, Mar 22, 2023 at 01:56:05PM -0500, Mike Christie wrote:
> > > > > vhost_task_create is supposed to return the vhost_task or NULL on
> > > > > failure. This fixes it to return the correct value when the allocation
> > > > > of the struct fails.
> > > > >
> > > > > Fixes: 77feab3c4156 ("vhost_task: Allow vhost layer to use copy_process") # mainline only
> > > > > Reported-by: [email protected]
> > > > > Signed-off-by: Mike Christie <[email protected]>
> > > >
> > > > Acked-by: Michael S. Tsirkin <[email protected]>
> > > >
> > > > The affected patch is not upstream yet, right?
> > > > I don't know if the tree in question allows rebases - linux-next
> > > > does. So ideally it would be squashed to avoid issues during bisect.
> > > > Still it's error path so I guess not a tragedy even without squashing.
> > >
> > > I tend to not rebase once stuff has been in linux-next but I make
> > > exceptions as long as it's before -rc4. For now I've put the patch on
> > > top (see the other mail I sent) but if it's really important I can
> > > squash it after the weekend (I'll be mostly afk until then.).
> >
> > Hard to say how important, but I'd prefer that, yes.
>
> Ok, fold the fixup into
>
> e297cd54b3f8 vhost_task: Allow vhost layer to use copy_process
>
> the series is now at:
>
> tree: git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git
> branch: kernel.user_worker
>
> 1a5f8090c6de vhost: move worker thread fields to new struct
> e297cd54b3f8 vhost_task: Allow vhost layer to use copy_process
> 89c8e98d8cfb fork: allow kernel code to call copy_process
> 094717586bf7 fork: Add kernel_clone_args flag to ignore signals
> 11f3f500ec8a fork: add kernel_clone_args flag to not dup/clone files
> 54e6842d0775 fork/vm: Move common PF_IO_WORKER behavior to new flag
> c81cc5819faf kernel: Make io_thread and kthread bit fields
> 73e0c116594d kthread: Pass in the thread's name during creation
> cf587db2ee02 kernel: Allow a kernel thread's name to be set in copy_process
> e0a98139c162 csky: Remove kernel_thread declaration

Thanks a lot! Mike could you give it a spin to make sure all is well?

--
MST

2023-03-23 15:53:01

by Mike Christie

[permalink] [raw]
Subject: Re: [PATCH 1/1] vhost_task: Fix vhost_task_create return value

On 3/23/23 7:47 AM, Michael S. Tsirkin wrote:
> On Thu, Mar 23, 2023 at 12:50:49PM +0100, Christian Brauner wrote:
>> On Thu, Mar 23, 2023 at 07:43:04AM -0400, Michael S. Tsirkin wrote:
>>> On Thu, Mar 23, 2023 at 11:44:45AM +0100, Christian Brauner wrote:
>>>> On Thu, Mar 23, 2023 at 03:37:19AM -0400, Michael S. Tsirkin wrote:
>>>>> On Wed, Mar 22, 2023 at 01:56:05PM -0500, Mike Christie wrote:
>>>>>> vhost_task_create is supposed to return the vhost_task or NULL on
>>>>>> failure. This fixes it to return the correct value when the allocation
>>>>>> of the struct fails.
>>>>>>
>>>>>> Fixes: 77feab3c4156 ("vhost_task: Allow vhost layer to use copy_process") # mainline only
>>>>>> Reported-by: [email protected]
>>>>>> Signed-off-by: Mike Christie <[email protected]>
>>>>>
>>>>> Acked-by: Michael S. Tsirkin <[email protected]>
>>>>>
>>>>> The affected patch is not upstream yet, right?
>>>>> I don't know if the tree in question allows rebases - linux-next
>>>>> does. So ideally it would be squashed to avoid issues during bisect.
>>>>> Still it's error path so I guess not a tragedy even without squashing.
>>>>
>>>> I tend to not rebase once stuff has been in linux-next but I make
>>>> exceptions as long as it's before -rc4. For now I've put the patch on
>>>> top (see the other mail I sent) but if it's really important I can
>>>> squash it after the weekend (I'll be mostly afk until then.).
>>>
>>> Hard to say how important, but I'd prefer that, yes.
>>
>> Ok, fold the fixup into
>>
>> e297cd54b3f8 vhost_task: Allow vhost layer to use copy_process
>>
>> the series is now at:
>>
>> tree: git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git
>> branch: kernel.user_worker
>>
>> 1a5f8090c6de vhost: move worker thread fields to new struct
>> e297cd54b3f8 vhost_task: Allow vhost layer to use copy_process
>> 89c8e98d8cfb fork: allow kernel code to call copy_process
>> 094717586bf7 fork: Add kernel_clone_args flag to ignore signals
>> 11f3f500ec8a fork: add kernel_clone_args flag to not dup/clone files
>> 54e6842d0775 fork/vm: Move common PF_IO_WORKER behavior to new flag
>> c81cc5819faf kernel: Make io_thread and kthread bit fields
>> 73e0c116594d kthread: Pass in the thread's name during creation
>> cf587db2ee02 kernel: Allow a kernel thread's name to be set in copy_process
>> e0a98139c162 csky: Remove kernel_thread declaration
>
> Thanks a lot! Mike could you give it a spin to make sure all is well?

Yeah, I'll review and retest over the weekend. If I hit any issues
I'll let you guys know by monday.

2023-03-25 23:06:42

by Mike Christie

[permalink] [raw]
Subject: Re: [PATCH 1/1] vhost_task: Fix vhost_task_create return value

On 3/23/23 7:47 AM, Michael S. Tsirkin wrote:
> On Thu, Mar 23, 2023 at 12:50:49PM +0100, Christian Brauner wrote:
>> On Thu, Mar 23, 2023 at 07:43:04AM -0400, Michael S. Tsirkin wrote:
>>> On Thu, Mar 23, 2023 at 11:44:45AM +0100, Christian Brauner wrote:
>>>> On Thu, Mar 23, 2023 at 03:37:19AM -0400, Michael S. Tsirkin wrote:
>>>>> On Wed, Mar 22, 2023 at 01:56:05PM -0500, Mike Christie wrote:
>>>>>> vhost_task_create is supposed to return the vhost_task or NULL on
>>>>>> failure. This fixes it to return the correct value when the allocation
>>>>>> of the struct fails.
>>>>>>
>>>>>> Fixes: 77feab3c4156 ("vhost_task: Allow vhost layer to use copy_process") # mainline only
>>>>>> Reported-by: [email protected]
>>>>>> Signed-off-by: Mike Christie <[email protected]>
>>>>>
>>>>> Acked-by: Michael S. Tsirkin <[email protected]>
>>>>>
>>>>> The affected patch is not upstream yet, right?
>>>>> I don't know if the tree in question allows rebases - linux-next
>>>>> does. So ideally it would be squashed to avoid issues during bisect.
>>>>> Still it's error path so I guess not a tragedy even without squashing.
>>>>
>>>> I tend to not rebase once stuff has been in linux-next but I make
>>>> exceptions as long as it's before -rc4. For now I've put the patch on
>>>> top (see the other mail I sent) but if it's really important I can
>>>> squash it after the weekend (I'll be mostly afk until then.).
>>>
>>> Hard to say how important, but I'd prefer that, yes.
>>
>> Ok, fold the fixup into
>>
>> e297cd54b3f8 vhost_task: Allow vhost layer to use copy_process
>>
>> the series is now at:
>>
>> tree: git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git
>> branch: kernel.user_worker
>>
>> 1a5f8090c6de vhost: move worker thread fields to new struct
>> e297cd54b3f8 vhost_task: Allow vhost layer to use copy_process
>> 89c8e98d8cfb fork: allow kernel code to call copy_process
>> 094717586bf7 fork: Add kernel_clone_args flag to ignore signals
>> 11f3f500ec8a fork: add kernel_clone_args flag to not dup/clone files
>> 54e6842d0775 fork/vm: Move common PF_IO_WORKER behavior to new flag
>> c81cc5819faf kernel: Make io_thread and kthread bit fields
>> 73e0c116594d kthread: Pass in the thread's name during creation
>> cf587db2ee02 kernel: Allow a kernel thread's name to be set in copy_process
>> e0a98139c162 csky: Remove kernel_thread declaration
>
> Thanks a lot! Mike could you give it a spin to make sure all is well?

Reviewed and tested the patches and it's all ok.