linux-kernel - Re: [virtio-dev] Re: [PATCH v1 1/2] virtio-balloon: tweak config

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]

Date:   Fri, 04 Jan 2019 13:14:27 +0800
From:   Wei Wang <wei.w.wang@...el.com>
To:     "Michael S. Tsirkin" <mst@...hat.com>
CC:     virtio-dev@...ts.oasis-open.org, linux-kernel@...r.kernel.org,
        virtualization@...ts.linux-foundation.org, kvm@...r.kernel.org,
        cohuck@...hat.com, borntraeger@...ibm.com, pbonzini@...hat.com,
        dgilbert@...hat.com, pasic@...ux.ibm.com
Subject: Re: [virtio-dev] Re: [PATCH v1 1/2] virtio-balloon: tweak config_changed
 implementation

On 01/04/2019 12:42 AM, Michael S. Tsirkin wrote:
> On Thu, Jan 03, 2019 at 01:31:01PM +0800, Wei Wang wrote:
>> virtio-ccw has deadlock issues with reading config registers inside the
>> interrupt context, so we tweak the virtballoon_changed implementation
>> by moving the config read operations into the related workqueue contexts.
>>
>> Signed-off-by: Wei Wang <wei.w.wang@...el.com>
>> ---
>>   drivers/virtio/virtio_balloon.c | 54 ++++++++++++++++++++---------------------
>>   1 file changed, 26 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
>> index 728ecd1..9a82a11 100644
>> --- a/drivers/virtio/virtio_balloon.c
>> +++ b/drivers/virtio/virtio_balloon.c
>> @@ -394,33 +394,15 @@ static void virtballoon_changed(struct virtio_device *vdev)
>>   {
>>   	struct virtio_balloon *vb = vdev->priv;
>>   	unsigned long flags;
>> -	s64 diff = towards_target(vb);
>> -
>> -	if (diff) {
>> -		spin_lock_irqsave(&vb->stop_update_lock, flags);
>> -		if (!vb->stop_update)
>> -			queue_work(system_freezable_wq,
>> -				   &vb->update_balloon_size_work);
>> -		spin_unlock_irqrestore(&vb->stop_update_lock, flags);
>> -	}
>>   
>> -	if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
>> -		virtio_cread(vdev, struct virtio_balloon_config,
>> -			     free_page_report_cmd_id, &vb->cmd_id_received);
> There's one problem with this approach:
>
> previously updating the cmd_id_received here would immediately
> stop the report in send_free_pages.
>
> With this approach we are waiting for the wq to schedule,
> which might be blocked waiting for report to complete.
> So host can no longer quickly stop the report in progress.
>
> A simple work-around would be to set some kind of flag whenever there
> is a change interrupt, then have send_free_pages test it
> and re-read cmd_id_received.
>
> Needs to be an atomic I guess ...
>

Yes, sounds better..will update the patch.

Best,
Wei