linux-kernel - Re: [PATCH 1/2] virtio_balloon: Give the balloon its own wakeup source

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <46bb6670-1a61-45e6-af2b-a04b93fa1bbb@redhat.com>
Date: Tue, 19 Mar 2024 09:44:36 +0100
From: David Hildenbrand <david@...hat.com>
To: David Stevens <stevensd@...omium.org>,
 "Michael S . Tsirkin" <mst@...hat.com>
Cc: virtualization@...ts.linux-foundation.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] virtio_balloon: Give the balloon its own wakeup
 source

On 18.03.24 10:10, David Stevens wrote:
> From: David Stevens <stevensd@...omium.org>
> 
> Wakeup sources don't support nesting multiple events, so sharing a
> single object between multiple drivers can result in one driver
> overriding the wakeup event processing period specified by another
> driver. Have the virtio balloon driver use the wakeup source of the
> device it is bound to rather than the wakeup source of the parent
> device, to avoid conflicts with the transport layer.
> 
> Note that although the virtio balloon's virtio_device itself isn't what
> actually wakes up the device, it is responsible for processing wakeup
> events. In the same way that EPOLLWAKEUP uses a dedicated wakeup_source
> to prevent suspend when userspace is processing wakeup events, a
> dedicated wakeup_source is necessary when processing wakeup events in a
> higher layer in the kernel.
> 
> Fixes: b12fbc3f787e ("virtio_balloon: stay awake while adjusting balloon")
> Signed-off-by: David Stevens <stevensd@...omium.org>
> ---
>   drivers/virtio/virtio_balloon.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 1f5b3dd31fcf..7fe7ef5f1c77 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -450,7 +450,7 @@ static void start_update_balloon_size(struct virtio_balloon *vb)
>   	vb->adjustment_signal_pending = true;
>   	if (!vb->adjustment_in_progress) {
>   		vb->adjustment_in_progress = true;
> -		pm_stay_awake(vb->vdev->dev.parent);
> +		pm_stay_awake(&vb->vdev->dev);
>   	}
>   	spin_unlock_irqrestore(&vb->adjustment_lock, flags);
>   
> @@ -462,7 +462,7 @@ static void end_update_balloon_size(struct virtio_balloon *vb)
>   	spin_lock_irq(&vb->adjustment_lock);
>   	if (!vb->adjustment_signal_pending && vb->adjustment_in_progress) {
>   		vb->adjustment_in_progress = false;
> -		pm_relax(vb->vdev->dev.parent);
> +		pm_relax(&vb->vdev->dev);
>   	}
>   	spin_unlock_irq(&vb->adjustment_lock);
>   }
> @@ -1028,6 +1028,7 @@ static int virtballoon_probe(struct virtio_device *vdev)
>   	}
>   
>   	spin_lock_init(&vb->adjustment_lock);

Can we add a comment here why we have to do that?

> +	device_set_wakeup_capable(&vb->vdev->dev, true);
>   
>   	virtio_device_ready(vdev);
>   

Absolutely not an expert on the details, but I assume this is fine.

Acked-by: David Hildenbrand <david@...hat.com>

-- 
Cheers,

David / dhildenb