From: Cornelia Huck <cohuck@redhat.com>
To: Halil Pasic <pasic@linux.ibm.com>
Cc: "Wang, Wei W" <wei.w.wang@intel.com>,
Christian Borntraeger <borntraeger@de.ibm.com>,
"virtio-dev@lists.oasis-open.org"
<virtio-dev@lists.oasis-open.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"virtualization@lists.linux-foundation.org"
<virtualization@lists.linux-foundation.org>,
"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
"mst@redhat.com" <mst@redhat.com>,
"pbonzini@redhat.com" <pbonzini@redhat.com>,
"dgilbert@redhat.com" <dgilbert@redhat.com>
Subject: Re: [virtio-dev] RE: [PATCH v1 0/2] Virtio: fix some vq allocation issues
Date: Wed, 2 Jan 2019 14:23:38 +0100 [thread overview]
Message-ID: <20190102142338.15bfe876.cohuck@redhat.com> (raw)
In-Reply-To: <20190102105314.0b4e2485.cohuck@redhat.com>
On Wed, 2 Jan 2019 10:53:14 +0100
Cornelia Huck <cohuck@redhat.com> wrote:
> On Tue, 1 Jan 2019 00:40:19 +0100
> Halil Pasic <pasic@linux.ibm.com> wrote:
> > As I said, at the moment I don't have a preference regarding the fix,
> > partly because I'm not sure if "reading config inside the handler" is OK
> > or not. Maybe Connie or Michael can help us here. I'm however sure that
> > commit 86a5597 "virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT"
> > breaks virtio-balloon with the ccw transport (i.e. effectively breaks
> > virtio-balloon on s390): it used to work before and does not work
> > after.
>
> Yes, that's unfortunate.
>
> >
> > AFAICT tweaking the balloon code may be simpler than tweaking the
> > virtio-ccw (transport code). ccw_io_helper() relies on getting
> > an interrupt when the issued IO is done. If virtio-ccw is buggy, it
> > needs to be fixed, but I'm not sure it is.
>
> I would not call virtio-ccw buggy, but it has some constraints that
> virtio-pci apparently doesn't have (and which did not show up so far;
> e.g. virtio-blk schedules a work item on config change, so there's no
> deadlock there.)
>
> One way to get out of that constraint (don't interact with the config
> space directly in the config changed handler) would be to schedule a
> work item in virtio-ccw that calls virtio_config_changed() for the
> device. My understanding is that delaying the notification to a work
> queue would be fine.
Unfortunately, calling virtio_config_changed() from a work item is not
enough: That function takes the config_lock, and the virtio-ccw code to
get the config both needs to allocate some memory and call schedule :/
The best option really seems to be
- have virtio-balloon move the handling of the config change onto a
workqueue or something like that, and
- document that you cannot read/write the virtio config space from an
atomic context
Unless someone has a better idea?
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
WARNING: multiple messages have this Message-ID (diff)
From: Cornelia Huck <cohuck@redhat.com>
To: Halil Pasic <pasic@linux.ibm.com>
Cc: "Wang, Wei W" <wei.w.wang@intel.com>,
Christian Borntraeger <borntraeger@de.ibm.com>,
"virtio-dev@lists.oasis-open.org"
<virtio-dev@lists.oasis-open.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"virtualization@lists.linux-foundation.org"
<virtualization@lists.linux-foundation.org>,
"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
"mst@redhat.com" <mst@redhat.com>,
"pbonzini@redhat.com" <pbonzini@redhat.com>,
"dgilbert@redhat.com" <dgilbert@redhat.com>
Subject: Re: RE: [PATCH v1 0/2] Virtio: fix some vq allocation issues
Date: Wed, 2 Jan 2019 14:23:38 +0100 [thread overview]
Message-ID: <20190102142338.15bfe876.cohuck@redhat.com> (raw)
In-Reply-To: <20190102105314.0b4e2485.cohuck@redhat.com>
On Wed, 2 Jan 2019 10:53:14 +0100
Cornelia Huck <cohuck@redhat.com> wrote:
> On Tue, 1 Jan 2019 00:40:19 +0100
> Halil Pasic <pasic@linux.ibm.com> wrote:
> > As I said, at the moment I don't have a preference regarding the fix,
> > partly because I'm not sure if "reading config inside the handler" is OK
> > or not. Maybe Connie or Michael can help us here. I'm however sure that
> > commit 86a5597 "virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT"
> > breaks virtio-balloon with the ccw transport (i.e. effectively breaks
> > virtio-balloon on s390): it used to work before and does not work
> > after.
>
> Yes, that's unfortunate.
>
> >
> > AFAICT tweaking the balloon code may be simpler than tweaking the
> > virtio-ccw (transport code). ccw_io_helper() relies on getting
> > an interrupt when the issued IO is done. If virtio-ccw is buggy, it
> > needs to be fixed, but I'm not sure it is.
>
> I would not call virtio-ccw buggy, but it has some constraints that
> virtio-pci apparently doesn't have (and which did not show up so far;
> e.g. virtio-blk schedules a work item on config change, so there's no
> deadlock there.)
>
> One way to get out of that constraint (don't interact with the config
> space directly in the config changed handler) would be to schedule a
> work item in virtio-ccw that calls virtio_config_changed() for the
> device. My understanding is that delaying the notification to a work
> queue would be fine.
Unfortunately, calling virtio_config_changed() from a work item is not
enough: That function takes the config_lock, and the virtio-ccw code to
get the config both needs to allocate some memory and call schedule :/
The best option really seems to be
- have virtio-balloon move the handling of the config change onto a
workqueue or something like that, and
- document that you cannot read/write the virtio config space from an
atomic context
Unless someone has a better idea?
WARNING: multiple messages have this Message-ID (diff)
From: Cornelia Huck <cohuck@redhat.com>
To: Halil Pasic <pasic@linux.ibm.com>
Cc: "Wang, Wei W" <wei.w.wang@intel.com>,
Christian Borntraeger <borntraeger@de.ibm.com>,
"virtio-dev@lists.oasis-open.org"
<virtio-dev@lists.oasis-open.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"virtualization@lists.linux-foundation.org"
<virtualization@lists.linux-foundation.org>,
"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
"mst@redhat.com" <mst@redhat.com>,
"pbonzini@redhat.com" <pbonzini@redhat.com>,
"dgilbert@redhat.com" <dgilbert@redhat.com>
Subject: Re: [virtio-dev] RE: [PATCH v1 0/2] Virtio: fix some vq allocation issues
Date: Wed, 2 Jan 2019 14:23:38 +0100 [thread overview]
Message-ID: <20190102142338.15bfe876.cohuck@redhat.com> (raw)
In-Reply-To: <20190102105314.0b4e2485.cohuck@redhat.com>
On Wed, 2 Jan 2019 10:53:14 +0100
Cornelia Huck <cohuck@redhat.com> wrote:
> On Tue, 1 Jan 2019 00:40:19 +0100
> Halil Pasic <pasic@linux.ibm.com> wrote:
> > As I said, at the moment I don't have a preference regarding the fix,
> > partly because I'm not sure if "reading config inside the handler" is OK
> > or not. Maybe Connie or Michael can help us here. I'm however sure that
> > commit 86a5597 "virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT"
> > breaks virtio-balloon with the ccw transport (i.e. effectively breaks
> > virtio-balloon on s390): it used to work before and does not work
> > after.
>
> Yes, that's unfortunate.
>
> >
> > AFAICT tweaking the balloon code may be simpler than tweaking the
> > virtio-ccw (transport code). ccw_io_helper() relies on getting
> > an interrupt when the issued IO is done. If virtio-ccw is buggy, it
> > needs to be fixed, but I'm not sure it is.
>
> I would not call virtio-ccw buggy, but it has some constraints that
> virtio-pci apparently doesn't have (and which did not show up so far;
> e.g. virtio-blk schedules a work item on config change, so there's no
> deadlock there.)
>
> One way to get out of that constraint (don't interact with the config
> space directly in the config changed handler) would be to schedule a
> work item in virtio-ccw that calls virtio_config_changed() for the
> device. My understanding is that delaying the notification to a work
> queue would be fine.
Unfortunately, calling virtio_config_changed() from a work item is not
enough: That function takes the config_lock, and the virtio-ccw code to
get the config both needs to allocate some memory and call schedule :/
The best option really seems to be
- have virtio-balloon move the handling of the config change onto a
workqueue or something like that, and
- document that you cannot read/write the virtio config space from an
atomic context
Unless someone has a better idea?
next prev parent reply other threads:[~2019-01-02 13:23 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-28 2:26 [virtio-dev] [PATCH v1 0/2] Virtio: fix some vq allocation issues Wei Wang
2018-12-28 2:26 ` Wei Wang
2018-12-28 2:26 ` [PATCH v1 1/2] virtio_pci: use queue idx instead of array idx to set up the vq Wei Wang
2018-12-28 2:26 ` [virtio-dev] " Wei Wang
2018-12-28 2:26 ` Wei Wang
2019-01-03 9:57 ` [virtio-dev] " Cornelia Huck
2019-01-03 9:57 ` Cornelia Huck
2019-01-03 9:57 ` Cornelia Huck
2019-01-09 16:29 ` Christian Borntraeger
2019-01-09 16:29 ` [virtio-dev] " Christian Borntraeger
2019-01-09 16:29 ` Christian Borntraeger
2018-12-28 2:26 ` [PATCH v1 2/2] virtio: don't allocate vqs when names[i] = NULL Wei Wang
2018-12-28 2:26 ` [virtio-dev] " Wei Wang
2018-12-28 2:26 ` Wei Wang
2019-01-03 9:59 ` Cornelia Huck
2019-01-03 9:59 ` [virtio-dev] " Cornelia Huck
2019-01-03 9:59 ` Cornelia Huck
2019-01-09 16:30 ` Christian Borntraeger
2019-01-09 16:30 ` [virtio-dev] " Christian Borntraeger
2019-01-09 16:30 ` Christian Borntraeger
2018-12-28 7:57 ` [PATCH v1 0/2] Virtio: fix some vq allocation issues Christian Borntraeger
2018-12-28 7:57 ` [virtio-dev] " Christian Borntraeger
2018-12-28 7:57 ` Christian Borntraeger
2018-12-29 2:45 ` [virtio-dev] " Wang, Wei W
2018-12-29 2:45 ` Wang, Wei W
2018-12-30 6:06 ` [virtio-dev] " Halil Pasic
2018-12-30 6:06 ` Halil Pasic
2018-12-31 6:03 ` [virtio-dev] " Wang, Wei W
2018-12-31 6:03 ` Wang, Wei W
2018-12-31 6:03 ` Wang, Wei W
2018-12-31 23:40 ` [virtio-dev] " Halil Pasic
2018-12-31 23:40 ` Halil Pasic
2018-12-31 23:40 ` Halil Pasic
2019-01-02 9:53 ` [virtio-dev] " Cornelia Huck
2019-01-02 9:53 ` Cornelia Huck
2019-01-02 9:53 ` Cornelia Huck
2019-01-02 13:23 ` [virtio-dev] " Cornelia Huck
2019-01-02 13:23 ` Cornelia Huck [this message]
2019-01-02 13:23 ` Cornelia Huck
2019-01-02 13:23 ` Cornelia Huck
2019-01-02 15:59 ` [virtio-dev] " Halil Pasic
2019-01-02 15:59 ` Halil Pasic
2019-01-02 15:59 ` Halil Pasic
2019-01-02 18:02 ` [virtio-dev] " Cornelia Huck
2019-01-02 18:02 ` Cornelia Huck
2019-01-02 18:02 ` Cornelia Huck
2019-01-02 18:02 ` Cornelia Huck
2019-01-02 19:13 ` [virtio-dev] " Halil Pasic
2019-01-02 19:13 ` Halil Pasic
2019-01-02 19:13 ` Halil Pasic
2019-01-02 13:56 ` [virtio-dev] " Halil Pasic
2019-01-02 13:56 ` Halil Pasic
2019-01-02 13:56 ` Halil Pasic
2019-01-02 9:53 ` [virtio-dev] " Cornelia Huck
2018-12-29 2:45 ` Wang, Wei W
2019-01-03 6:18 ` [virtio-dev] " Wei Wang
2019-01-03 6:18 ` Wei Wang
2019-01-03 6:18 ` Wei Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190102142338.15bfe876.cohuck@redhat.com \
--to=cohuck@redhat.com \
--cc=borntraeger@de.ibm.com \
--cc=dgilbert@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mst@redhat.com \
--cc=pasic@linux.ibm.com \
--cc=pbonzini@redhat.com \
--cc=virtio-dev@lists.oasis-open.org \
--cc=virtualization@lists.linux-foundation.org \
--cc=wei.w.wang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.