All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>,
	kvm@vger.kernel.org, virtualization@lists.osdl.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] vhost: rcu annotation fixup
Date: Tue, 18 Jan 2011 11:02:33 -0800	[thread overview]
Message-ID: <20110118190232.GM2193@linux.vnet.ibm.com> (raw)
In-Reply-To: <20110118175500.GA6935@redhat.com>

On Tue, Jan 18, 2011 at 07:55:00PM +0200, Michael S. Tsirkin wrote:
> On Tue, Jan 18, 2011 at 09:48:34AM -0800, Paul E. McKenney wrote:
> > On Tue, Jan 18, 2011 at 01:08:45PM +0200, Michael S. Tsirkin wrote:
> > > When built with rcu checks enabled, vhost triggers
> > > bogus warnings as vhost features are read without
> > > dev->mutex sometimes.
> > > Fixing it properly is not trivial as vhost.h does not
> > > know which lockdep classes it will be used under.
> > > Disable the warning by stubbing out the check for now.
> > > 
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > >  drivers/vhost/vhost.h |    4 +---
> > >  1 files changed, 1 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > > index 2af44b7..2d03a31 100644
> > > --- a/drivers/vhost/vhost.h
> > > +++ b/drivers/vhost/vhost.h
> > > @@ -173,9 +173,7 @@ static inline int vhost_has_feature(struct vhost_dev *dev, int bit)
> > >  {
> > >  	unsigned acked_features;
> > > 
> > > -	acked_features =
> > > -		rcu_dereference_index_check(dev->acked_features,
> > > -					    lockdep_is_held(&dev->mutex));
> > > +	acked_features = rcu_dereference_index_check(dev->acked_features, 1);
> > 
> > Ouch!!!
> > 
> > Could you please at least add a comment?
> 
> Yes, OK.
> 
> > Alternatively, pass in the lock that is held and check for that?  Given
> > that this is a static inline, the compiler should be able to optimize
> > the argument away when !PROVE_RCU, correct?
> > 
> > 							Thanx, Paul
> 
> Hopefully, yes. We don't always have a lock: the idea was
> to create a lockdep for these cases. But we can't pass
> the pointer to that ...

I suppose you could pass a pointer to the lockdep map structure.
Not sure if this makes sense, but it would handle the situation.

Alternatively, create a helper function that checks the possibilities
and screams if none of them are in effect.

							Thanx, Paul

> > >  	return acked_features & (1 << bit);
> > >  }
> > > 
> > > -- 
> > > 1.7.3.2.91.g446ac
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > Please read the FAQ at  http://www.tux.org/lkml/

  reply	other threads:[~2011-01-18 19:02 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-18 11:08 [PATCH] vhost: rcu annotation fixup Michael S. Tsirkin
2011-01-18 17:48 ` Paul E. McKenney
2011-01-18 17:55   ` Michael S. Tsirkin
2011-01-18 19:02     ` Paul E. McKenney [this message]
2011-01-18 20:10       ` Michael S. Tsirkin
2011-01-18 20:28         ` Paul E. McKenney
2011-01-19  0:40 ` Mel Gorman
2011-01-19  5:18   ` Michael S. Tsirkin

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=20110118190232.GM2193@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=jasowang@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=virtualization@lists.osdl.org \
    /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.