linux-kernel - Re: [PATCH] KVM: x86: Handle the kthread worker using the new API

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b2b9e65c-42c1-9a88-e8c4-f40bad53d2df@redhat.com>
Date:   Wed, 19 Oct 2016 13:52:19 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Petr Mladek <pmladek@...e.com>,
        Radim Krčmář <rkrcmar@...hat.com>
Cc:     x86@...nel.org, Tejun Heo <tj@...nel.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] KVM: x86: Handle the kthread worker using the new API



On 19/10/2016 13:50, Petr Mladek wrote:
> Use the new API to create and destroy the "kvm-pit" kthread
> worker. The API hides some implementation details.
> 
> In particular, kthread_create_worker() allocates and initializes
> struct kthread_worker. It runs the kthread the right way
> and stores task_struct into the worker structure.
> 
> kthread_destroy_worker() flushes all pending works, stops
> the kthread and frees the structure.
> 
> This patch does not change the existing behavior except for
> dynamically allocating struct kthread_worker and storing
> only the pointer of this structure.
> 
> It is compile tested only because I did not find an easy
> way how to run the code. Well, it should be pretty safe
> given the nature of the change.

Running old Linux guests (early 2.6) will trigger it.  Let us take this
through the KVM tree, we will take care of testing.

Thanks,

Paolo

> Signed-off-by: Petr Mladek <pmladek@...e.com>
> ---
>  arch/x86/kvm/i8254.c | 15 ++++++---------
>  arch/x86/kvm/i8254.h |  3 +--
>  2 files changed, 7 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
> index 16a7134eedac..a78b445ce411 100644
> --- a/arch/x86/kvm/i8254.c
> +++ b/arch/x86/kvm/i8254.c
> @@ -212,7 +212,7 @@ static void kvm_pit_ack_irq(struct kvm_irq_ack_notifier *kian)
>  	 */
>  	smp_mb();
>  	if (atomic_dec_if_positive(&ps->pending) > 0)
> -		kthread_queue_work(&pit->worker, &pit->expired);
> +		kthread_queue_work(pit->worker, &pit->expired);
>  }
>  
>  void __kvm_migrate_pit_timer(struct kvm_vcpu *vcpu)
> @@ -272,7 +272,7 @@ static enum hrtimer_restart pit_timer_fn(struct hrtimer *data)
>  	if (atomic_read(&ps->reinject))
>  		atomic_inc(&ps->pending);
>  
> -	kthread_queue_work(&pt->worker, &pt->expired);
> +	kthread_queue_work(pt->worker, &pt->expired);
>  
>  	if (ps->is_periodic) {
>  		hrtimer_add_expires_ns(&ps->timer, ps->period);
> @@ -667,10 +667,8 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
>  	pid_nr = pid_vnr(pid);
>  	put_pid(pid);
>  
> -	kthread_init_worker(&pit->worker);
> -	pit->worker_task = kthread_run(kthread_worker_fn, &pit->worker,
> -				       "kvm-pit/%d", pid_nr);
> -	if (IS_ERR(pit->worker_task))
> +	pit->worker = kthread_create_worker(0, "kvm-pit/%d", pid_nr);
> +	if (IS_ERR(pit->worker))
>  		goto fail_kthread;
>  
>  	kthread_init_work(&pit->expired, pit_do_work);
> @@ -713,7 +711,7 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
>  fail_register_pit:
>  	mutex_unlock(&kvm->slots_lock);
>  	kvm_pit_set_reinject(pit, false);
> -	kthread_stop(pit->worker_task);
> +	kthread_destroy_worker(pit->worker);
>  fail_kthread:
>  	kvm_free_irq_source_id(kvm, pit->irq_source_id);
>  fail_request:
> @@ -730,8 +728,7 @@ void kvm_free_pit(struct kvm *kvm)
>  		kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS, &pit->speaker_dev);
>  		kvm_pit_set_reinject(pit, false);
>  		hrtimer_cancel(&pit->pit_state.timer);
> -		kthread_flush_work(&pit->expired);
> -		kthread_stop(pit->worker_task);
> +		kthread_destroy_worker(pit->worker);
>  		kvm_free_irq_source_id(kvm, pit->irq_source_id);
>  		kfree(pit);
>  	}
> diff --git a/arch/x86/kvm/i8254.h b/arch/x86/kvm/i8254.h
> index 2f5af0798326..600bee9dcbbd 100644
> --- a/arch/x86/kvm/i8254.h
> +++ b/arch/x86/kvm/i8254.h
> @@ -44,8 +44,7 @@ struct kvm_pit {
>  	struct kvm_kpit_state pit_state;
>  	int irq_source_id;
>  	struct kvm_irq_mask_notifier mask_notifier;
> -	struct kthread_worker worker;
> -	struct task_struct *worker_task;
> +	struct kthread_worker *worker;
>  	struct kthread_work expired;
>  };
>  
>