Messages in this thread Patch in this message | | | Subject | Re: [tip:core/urgent] WARN_ON_SMP(): Add comment to explain ({0;}) | From | Steven Rostedt <> | Date | Tue, 29 Mar 2011 10:47:28 -0400 |
| |
On Tue, 2011-03-29 at 07:32 -0700, H. Peter Anvin wrote: > I think the question is what are the expected semantics if WARN_ON_SMP > is used in an if... especially for UP.
Well, actually this whole patchset was created because we added the semantic if (WARN_ON_SMP(x)).
This patch: https://lkml.org/lkml/2011/3/11/463
Added the following code:
+static void __unqueue_futex(struct futex_q *q) +{ + struct futex_hash_bucket *hb; + + if (WARN_ON(!q->lock_ptr || !spin_is_locked(q->lock_ptr) + || plist_node_empty(&q->list))) + return; + + hb = container_of(q->lock_ptr, struct futex_hash_bucket, lock); + plist_del(&q->list, &hb->chain); +}
Which on !CONFIG_SMP broke, because that "|| !spin_is_locked()" always return true and we trigger the WARN_ON() as well as returning early, breaking the code, and triggering bugs, as we never do the "plist_del()".
To fix this, I added: https://lkml.org/lkml/2011/3/17/312
Which does this change:
@@ -785,8 +785,8 @@ static void __unqueue_futex(struct futex_q *q) { struct futex_hash_bucket *hb; - if (WARN_ON(!q->lock_ptr || !spin_is_locked(q->lock_ptr) - || plist_node_empty(&q->list))) + if (WARN_ON_SMP(!q->lock_ptr || !spin_is_locked(q->lock_ptr)) + || WARN_ON(plist_node_empty(&q->list))) return; hb = container_of(q->lock_ptr, struct futex_hash_bucket, lock);
But as WARN_ON_SMP() currently had no if () context, I needed to first make the change of: https://lkml.org/lkml/2011/3/17/313 #ifdef CONFIG_SMP # define WARN_ON_SMP(x) WARN_ON(x) #else -# define WARN_ON_SMP(x) do { } while (0) +# define WARN_ON_SMP(x) ({0;}) #endif But if this is such a problem, we can revert all this and do:
@@ -785,8 +785,11 @@ static void __unqueue_futex(struct futex_q *q) { struct futex_hash_bucket *hb; - if (WARN_ON(!q->lock_ptr || !spin_is_locked(q->lock_ptr) - || plist_node_empty(&q->list))) +#ifdef CONFIG_SMP + if (WARN_ON(!q->lock_ptr || !spin_is_locked(q->lock_ptr)) + return; +#endif + if (WARN_ON(plist_node_empty(&q->list))) return; hb = container_of(q->lock_ptr, struct futex_hash_bucket, lock);
And leave the WARN_ON_SMP() alone.
-- Steve
| |