Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1168227 > unrolled thread

[PATCH v2 03/14] notifiers: Assert that RCU is watching in notify_die

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 03/14] notifiers: Assert that RCU is watching in notify_die Andy Lutomirski <luto@kernel.org> - 2015-06-18 21:20 +0200
    Re: [PATCH v2 03/14] notifiers: Assert that RCU is watching in  notify_die Borislav Petkov <bp@alien8.de> - 2015-06-22 13:40 +0200
      Re: [PATCH v2 03/14] notifiers: Assert that RCU is watching in notify_die Andy Lutomirski <luto@amacapital.net> - 2015-06-22 21:50 +0200

#1168227 — [PATCH v2 03/14] notifiers: Assert that RCU is watching in notify_die

Low-level arch entries often call notify_die, and it's easy for arch
code to fail to exit an RCU quiescent state first.  Assert that
we're not quiescent in notify_die.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 kernel/notifier.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/notifier.c b/kernel/notifier.c
index ae9fc7cc360e..980e4330fb59 100644
--- a/kernel/notifier.c
+++ b/kernel/notifier.c
@@ -544,6 +544,8 @@ int notrace notify_die(enum die_val val, const char *str,
 		.signr	= sig,
 
 	};
+	rcu_lockdep_assert(rcu_is_watching(),
+			   "notify_die called but RCU thinks we're quiescent");
 	return atomic_notifier_call_chain(&die_chain, val, &args);
 }
 NOKPROBE_SYMBOL(notify_die);
-- 
2.4.3

--
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/

[toc] | [next] | [standalone]

#1169864 — Re: [PATCH v2 03/14] notifiers: Assert that RCU is watching in notify_die

On Thu, Jun 18, 2015 at 12:08:35PM -0700, Andy Lutomirski wrote:
> Low-level arch entries often call notify_die, and it's easy for arch
> code to fail to exit an RCU quiescent state first.  Assert that
> we're not quiescent in notify_die.
> 
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  kernel/notifier.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/notifier.c b/kernel/notifier.c
> index ae9fc7cc360e..980e4330fb59 100644
> --- a/kernel/notifier.c
> +++ b/kernel/notifier.c
> @@ -544,6 +544,8 @@ int notrace notify_die(enum die_val val, const char *str,
>  		.signr	= sig,
>  
>  	};
> +	rcu_lockdep_assert(rcu_is_watching(),
> +			   "notify_die called but RCU thinks we're quiescent");
>  	return atomic_notifier_call_chain(&die_chain, val, &args);
>  }

Ok, we're about to die and we will prepend what would be a more
important splat possibly hinting at the problem is with a lockdep splat.

I think we should do the assertion and make the rcu_lockdep splat come
last I but don't see how to do this easily from all the notify_die()
call sites.

Or am I missing something...?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1170182

On Mon, Jun 22, 2015 at 11:15 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Mon, Jun 22, 2015 at 10:37:39AM -0700, Andy Lutomirski wrote:
>> But if we OOPS, we'll OOPS after the lockdep splat and the lockdep
>> splat will scroll off the screen, right?  Am I missing something here?
>
> No, you're not.
>
>> notify_die is called before the actual OOPS code is invoked in traps.c.
>
> Yes, and with this assertion, you get to potentially print two
> dump_stack()'s back-to-back instead of the one from traps.c.
>
> And if the machine is about to be wedged solid soon anyway, we want to
> dump as less (not-so-important) blurb to serial/console as possible. And
> in this case, my suspicion is not that the lockdep splat will scroll
> off the screen but that we might freeze before we even issue the whole
> thing.
>
> That's why I think we should be conservative and make the lockdep splat
> come out second, if possible.

That'll annoy people using regular consoles, though.

I think this scenario isn't that likely. If we dereference a NULL
pointer, then we really should rcu watching before we actually oops in
the page fault code.  Similarly, if we take a non-fixed-up GPF, we
should have rcu watching in the early part of do_general_protection.

I'd be all for skipping the assertion entirely if we're going to OOPS,
but we don't know whether we're actually OOPSing when notify_die is
called.  We could individually instrument everything, or we could just
drop this patch entirely, but it has helped me catch some goofs while
developing all this code.

--Andy

>
> Am I making more sense now?
>
> --
> Regards/Gruss,
>     Boris.
>
> ECO tip #101: Trim your mails when you reply.
> --



-- 
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web