All of lore.kernel.org
 help / color / mirror / Atom feed
From: Halil Pasic <pasic@linux.ibm.com>
To: Cornelia Huck <cohuck@redhat.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 20:13:41 +0100	[thread overview]
Message-ID: <20190102201341.28bf9378@oc2783563651> (raw)
In-Reply-To: <20190102190233.69b267fb.cohuck@redhat.com>

On Wed, 2 Jan 2019 19:02:33 +0100
Cornelia Huck <cohuck@redhat.com> wrote:

> On Wed, 2 Jan 2019 16:59:19 +0100
> Halil Pasic <pasic@linux.ibm.com> wrote:
> 
> > On Wed, 2 Jan 2019 14:23:38 +0100
> > Cornelia Huck <cohuck@redhat.com> wrote:
> > 
> > > 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:  
> 
> > > > > 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?
> > >   
> > 
> > I wonder, would making config_lock a mutex suffice?
> 
> Unless I'm mistaken, you can't take a mutex in an interrupt path.
>

I was too focused on virtio-ccw. We have the workqueue now, so it would
not be a problem for us, but for the other transports. Grrr


---------------------------------------------------------------------
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: Halil Pasic <pasic@linux.ibm.com>
To: Cornelia Huck <cohuck@redhat.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 20:13:41 +0100	[thread overview]
Message-ID: <20190102201341.28bf9378@oc2783563651> (raw)
In-Reply-To: <20190102190233.69b267fb.cohuck@redhat.com>

On Wed, 2 Jan 2019 19:02:33 +0100
Cornelia Huck <cohuck@redhat.com> wrote:

> On Wed, 2 Jan 2019 16:59:19 +0100
> Halil Pasic <pasic@linux.ibm.com> wrote:
> 
> > On Wed, 2 Jan 2019 14:23:38 +0100
> > Cornelia Huck <cohuck@redhat.com> wrote:
> > 
> > > 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:  
> 
> > > > > 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?
> > >   
> > 
> > I wonder, would making config_lock a mutex suffice?
> 
> Unless I'm mistaken, you can't take a mutex in an interrupt path.
>

I was too focused on virtio-ccw. We have the workqueue now, so it would
not be a problem for us, but for the other transports. Grrr

WARNING: multiple messages have this Message-ID (diff)
From: Halil Pasic <pasic@linux.ibm.com>
To: Cornelia Huck <cohuck@redhat.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 20:13:41 +0100	[thread overview]
Message-ID: <20190102201341.28bf9378@oc2783563651> (raw)
In-Reply-To: <20190102190233.69b267fb.cohuck@redhat.com>

On Wed, 2 Jan 2019 19:02:33 +0100
Cornelia Huck <cohuck@redhat.com> wrote:

> On Wed, 2 Jan 2019 16:59:19 +0100
> Halil Pasic <pasic@linux.ibm.com> wrote:
> 
> > On Wed, 2 Jan 2019 14:23:38 +0100
> > Cornelia Huck <cohuck@redhat.com> wrote:
> > 
> > > 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:  
> 
> > > > > 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?
> > >   
> > 
> > I wonder, would making config_lock a mutex suffice?
> 
> Unless I'm mistaken, you can't take a mutex in an interrupt path.
>

I was too focused on virtio-ccw. We have the workqueue now, so it would
not be a problem for us, but for the other transports. Grrr


  reply	other threads:[~2019-01-02 19:13 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   ` [virtio-dev] " Christian Borntraeger
2019-01-09 16:29     ` 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   ` [virtio-dev] " Cornelia Huck
2019-01-03  9:59     ` Cornelia Huck
2019-01-03  9:59   ` Cornelia Huck
2019-01-09 16:30   ` [virtio-dev] " Christian Borntraeger
2019-01-09 16:30     ` 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  9:53             ` Cornelia Huck
2019-01-02 13:23             ` [virtio-dev] " Cornelia Huck
2019-01-02 13:23             ` Cornelia Huck
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                   ` Halil Pasic [this message]
2019-01-02 19:13                     ` [virtio-dev] " 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
2018-12-29  2:45   ` Wang, Wei W
2019-01-03  6:18   ` Wei Wang
2019-01-03  6:18   ` [virtio-dev] " 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=20190102201341.28bf9378@oc2783563651 \
    --to=pasic@linux.ibm.com \
    --cc=borntraeger@de.ibm.com \
    --cc=cohuck@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.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.