Re: [PATCHv3 1/2] vhost: return task creation error instead of NULL

Mike Christie
2025-02-28T18:35:16Z

On 2/27/25 5:06 PM, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
>
> Lets callers distinguish why the vhost task creation failed. No one
> currently cares why it failed, so no real runtime change from this
> patch, but that will not be the case for long.
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> arch/x86/kvm/mmu/mmu.c | 2 +-
> drivers/vhost/vhost.c | 2 +-
> kernel/vhost_task.c | 4 ++--
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index d4ac4a1f8b81b..18ca1ea6dc240 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -7471,7 +7471,7 @@ static void kvm_mmu_start_lpage_recovery(struct once *once)
> kvm_nx_huge_page_recovery_worker_kill,
> kvm, "kvm-nx-lpage-recovery");
>
> - if (!nx_thread)
> + if (IS_ERR(nx_thread))
> return;
>
> vhost_task_start(nx_thread);
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 9ac25d08f473e..63612faeab727 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -666,7 +666,7 @@ static struct vhost_worker *vhost_worker_create(struct vhost_dev *dev)
>
> vtsk = vhost_task_create(vhost_run_work_list, vhost_worker_killed,
> worker, name);
> - if (!vtsk)
> + if (IS_ERR(vtsk))
> goto free_worker;
>
> mutex_init(&worker->mutex);
> diff --git a/kernel/vhost_task.c b/kernel/vhost_task.c
> index 8800f5acc0071..2ef2e1b800916 100644
> --- a/kernel/vhost_task.c
> +++ b/kernel/vhost_task.c
> @@ -133,7 +133,7 @@ struct vhost_task *vhost_task_create(bool (*fn)(void *),
>
> vtsk = kzalloc(sizeof(*vtsk), GFP_KERNEL);
> if (!vtsk)
> - return NULL;
> + return ERR_PTR(-ENOMEM);
> init_completion(&vtsk->exited);
> mutex_init(&vtsk->exit_mutex);
> vtsk->data = arg;
> @@ -145,7 +145,7 @@ struct vhost_task *vhost_task_create(bool (*fn)(void *),
> tsk = copy_process(NULL, 0, NUMA_NO_NODE, &args);
> if (IS_ERR(tsk)) {
> kfree(vtsk);
> - return NULL;
> + return ERR_PTR(PTR_ERR(tsk));
> }
>
> vtsk->task = tsk;

The vhost task parts look ok to me.

Reviewed-by: Mike Christie <michael.christie@oracle.com>