lkml.org 
[lkml]   [2011]   [Mar]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [tip:core/urgent] WARN_ON_SMP(): Add comment to explain ({0;})
From
Date
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




\
 
 \ /
  Last update: 2011-03-29 16:49    [W:0.059 / U:0.176 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site