Thread (8 messages) 8 messages, 3 authors, 2011-01-19

Re: [PATCH] vhost: rcu annotation fixup

From: Paul E. McKenney <hidden>
Date: 2011-01-18 20:31:18
Also in: kvm, lkml, netdev

On Tue, Jan 18, 2011 at 10:10:31PM +0200, Michael S. Tsirkin wrote:
On Tue, Jan 18, 2011 at 11:02:33AM -0800, Paul E. McKenney wrote:
quoted
On Tue, Jan 18, 2011 at 07:55:00PM +0200, Michael S. Tsirkin wrote:
quoted
On Tue, Jan 18, 2011 at 09:48:34AM -0800, Paul E. McKenney wrote:
quoted
On Tue, Jan 18, 2011 at 01:08:45PM +0200, Michael S. Tsirkin wrote:
quoted
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.
quoted
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.
Will it compile with lockdep disabled too? What will the pointer be?
One (crude) approach would be to make the pointer void* if lockdep
is disabled.
quoted
Alternatively, create a helper function that checks the possibilities
and screams if none of them are in effect.

							Thanx, Paul
The problem here is the callee needs to know about all callers.
As does the guy reading the code.  ;-)

							Thanx, Paul
quoted
quoted
quoted
quoted
 	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/
--
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/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help