Linux-Sgx Archive on lore.kernel.org
 help / color / Atom feed
From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: linux-sgx@vger.kernel.org, Haitao Huang <haitao.huang@linux.intel.com>
Subject: Re: [PATCH v4] x86/sgx: Fix deadlock and race conditions between fork() and EPC reclaim
Date: Thu, 16 Apr 2020 20:24:59 +0300
Message-ID: <20200416172459.GF199110@linux.intel.com> (raw)
In-Reply-To: <20200416042810.GC10076@linux.intel.com>

On Wed, Apr 15, 2020 at 09:28:10PM -0700, Sean Christopherson wrote:
> On Tue, Apr 14, 2020 at 09:45:28PM +0300, Jarkko Sakkinen wrote:
> > On Tue, Apr 14, 2020 at 10:17:49AM +0300, Jarkko Sakkinen wrote:
> > > On Mon, Apr 13, 2020 at 09:32:34PM -0700, Sean Christopherson wrote:
> > > > On Mon, Apr 06, 2020 at 11:56:26PM +0300, Jarkko Sakkinen wrote:
> > > > > From: Sean Christopherson <sean.j.christopherson@intel.com>
> > > >   	spin_lock(&encl->mm_lock);
> > > > > +
> > > > >  	list_add_rcu(&encl_mm->list, &encl->mm_list);
> > > > > -	spin_unlock(&encl->mm_lock);
> > > > >  
> > > > > -	synchronize_srcu(&encl->srcu);
> > > > > +	/* Even if the CPU does not reorder writes, a compiler might. */
> > > > 
> > > > The preferred (by maintainers) style of comment for smp_wmb()/smp_rmb()
> > > > comments is to explicitly call out the associated reader/writer.  If you
> > > > want to go with a minimal comment, my vote is for something like:
> > > > 
> > > > 	/*
> > > > 	 * Add to list before updating version.  Pairs the with smp_rmb() in
> > > > 	 * sgx_reclaimer_block().
> > > > 	 */
> > > > 
> > > > And if you want to go really spartan, I'd take:
> > > > 
> > > > 	/* Pairs with smp_rmb() in sgx_reclaimer_block(). */
> > > > 
> > > > over a generic comment about the compiler reordering instructions.
> > > 
> > > Thaks Sean, makes sense, I'll go with your "spartan" suggestion.
> > 
> > Updated, ready to squash?
> 
> Any objection to using the spartan comment for the smb_rmb() in
> sgx_reclaimer_block() as well?

For sure. I think here the role of the comment is to help with
the navigation.

/Jarkko
the navigation.

  reply index

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-06 20:56 Jarkko Sakkinen
2020-04-14  4:32 ` Sean Christopherson
2020-04-14  7:17   ` Jarkko Sakkinen
2020-04-14 18:45     ` Jarkko Sakkinen
2020-04-16  4:28       ` Sean Christopherson
2020-04-16 17:24         ` Jarkko Sakkinen [this message]
2020-04-16 17:39           ` 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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=20200416172459.GF199110@linux.intel.com \
    --to=jarkko.sakkinen@linux.intel.com \
    --cc=haitao.huang@linux.intel.com \
    --cc=linux-sgx@vger.kernel.org \
    --cc=sean.j.christopherson@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Sgx Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-sgx/0 linux-sgx/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-sgx linux-sgx/ https://lore.kernel.org/linux-sgx \
		linux-sgx@vger.kernel.org
	public-inbox-index linux-sgx

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-sgx


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git