archive mirror
 help / color / mirror / Atom feed
From: Dave Hansen <>
To: Jarkko Sakkinen <>
Cc: Sean Christopherson <>,,,,,
	Haitao Huang <>,
	Thomas Gleixner <>,
	Ingo Molnar <>, Borislav Petkov <>,, "H. Peter Anvin" <>,
	Jethro Beekman <>
Subject: Re: [PATCH v4] x86/sgx: Fix the call order of synchronize_srcu() in sgx_release()
Date: Mon, 25 Jan 2021 07:49:04 -0800	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

Haitao managed to create another splat over the weekend.  It was, indeed:

> WARNING: CPU: 3 PID: 7620 at kernel/rcu/srcutree.c:374 cleanup_srcu_struct+0xed/0x100

which is:

>         if (WARN_ON(!srcu_get_delay(ssp)))
>                 return; /* Just leak it! */

That check means that there is an outstanding "expedited" grace period.
 The fact that it's expedited is not important.  This:

describes the reasoning behind the warning:

	If the struct srcu_struct is dynamically allocated, then
	cleanup_srcu_struct() must be called before it is freed ... the
	caller must take care to ensure that all SRCU read-side critical
	sections have completed (and that no more will commence) before
	calling cleanup_srcu_struct().

synchronize_srcu() will (obviously) wait for the grace period to
complete.  Calling it will shut up the warning for sure, most of the time.

The required sequence of events is in here:


I suspect that the mmu notifier's synchronize_srcu() is run in parallel
very close to when cleanup_srcu_struct() is called.  This violates the
"prevent any further calls to synchronize_srcu" rule.

So, while I suspect that adding a synchronize_srcu() is *part* of the
correct solution, I'm still not convinced that the
sgx_mmu_notifier_release() code is correct.

  reply	other threads:[~2021-01-25 15:50 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-15  1:46 [PATCH v4] x86/sgx: Fix the call order of synchronize_srcu() in sgx_release() jarkko
2021-01-15  7:18 ` Borislav Petkov
2021-01-16  5:12   ` Jarkko Sakkinen
2021-01-18 18:57     ` Borislav Petkov
2021-01-20 14:43       ` Jarkko Sakkinen
2021-01-20 17:34         ` Dave Hansen
2021-01-21  0:26           ` Jarkko Sakkinen
2021-01-22 18:20             ` Haitao Huang
2021-01-20 17:35 ` Sean Christopherson
2021-01-21  0:29   ` Jarkko Sakkinen
2021-01-21  1:19     ` Dave Hansen
2021-01-21 12:55       ` Jarkko Sakkinen
2021-01-21 18:19         ` Dave Hansen
2021-01-22 16:56   ` Dave Hansen
2021-01-23  8:58     ` Jarkko Sakkinen
2021-01-25 15:49       ` Dave Hansen [this message]
2021-01-27 17:31         ` Jarkko Sakkinen

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \

* 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).