public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Gregory Haskins <ghaskins@novell.com>
Cc: avi@redhat.com, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, mtosatti@redhat.com,
	paulmck@linux.vnet.ibm.com, markmc@redhat.com
Subject: Re: [PATCH] kvm: remove in_range from kvm_io_device
Date: Thu, 25 Jun 2009 14:08:12 +0300	[thread overview]
Message-ID: <20090625110812.GA6487@redhat.com> (raw)
In-Reply-To: <20090624084901.GB22865@redhat.com>

On Wed, Jun 24, 2009 at 11:49:01AM +0300, Michael S. Tsirkin wrote:
> On Tue, Jun 23, 2009 at 09:43:31PM -0400, Gregory Haskins wrote:
> > Michael S. Tsirkin wrote:
> > > Remove in_range from kvm_io_device and ask read/write callbacks, if
> > > supplied, to perform range checks internally. This allows aliasing
> > > (mostly for in-kernel virtio), as well as better error handling by
> > > making it possible to pass errors up to userspace. And it's enough to
> > > look at the diffstat to see that it's a better API anyway.
> > >
> > > While we are at it, document locking rules for kvm_io_device.
> > >   
> > 
> > Hi Michael,
> > 
> > I just tried to apply this to kvm.git/master, and it blew up really
> > badly.  What tree should I be using?
> 
> Ugh, this is against 2.6.30. I'll post kvm.git version soon.
> 

I went ahead and tried to rebase it, to find that it conflicts with
recent patch 35b3038961f94e83557944ae0d30c8fa0b5012cf merged in kvm.git:
'KVM: switch irq injection/acking data structures to irq_lock'
which now does this:
	lock
	find
	unlock
	write

I thought for a while that it might make sense to start small and just
add in_range parameter for starters ...
However, I just realised that this only works because devices are not
added or removed dynamically.

The long term fix is to switch to SRCU for bus management.  But if we
need to do this for iosignalfd anyway, in_range removal becomes possible
again.

Short term it might be also possible to go back to keeping kvm lock
across both find and read - since the lock is taken, we don't
really win anything currently if we drop the lock earlier.

Comments?

-- 
MST

  reply	other threads:[~2009-06-25 11:09 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-23 15:00 [PATCH] kvm: remove in_range from kvm_io_device Michael S. Tsirkin
2009-06-23 15:21 ` Gregory Haskins
2009-06-23 15:31   ` Michael S. Tsirkin
2009-06-23 15:44     ` Gregory Haskins
2009-06-23 15:56       ` Michael S. Tsirkin
2009-06-23 16:14         ` Gregory Haskins
2009-06-24  1:43 ` Gregory Haskins
2009-06-24  8:49   ` Michael S. Tsirkin
2009-06-25 11:08     ` Michael S. Tsirkin [this message]
2009-06-25 11:27       ` Gregory Haskins
2009-06-25 11:54         ` Michael S. Tsirkin
2009-06-25 12:08           ` Gregory Haskins
2009-06-25 12:37             ` Michael S. Tsirkin
2009-06-25 13:02               ` Gregory Haskins
2009-06-25 13:16                 ` Michael S. Tsirkin
2009-06-25 13:19                   ` Gregory Haskins
2009-06-28 12:07                     ` Avi Kivity
2009-06-25 15:45       ` Gregory Haskins

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=20090625110812.GA6487@redhat.com \
    --to=mst@redhat.com \
    --cc=avi@redhat.com \
    --cc=ghaskins@novell.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=markmc@redhat.com \
    --cc=mtosatti@redhat.com \
    --cc=paulmck@linux.vnet.ibm.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox