On Thu, Apr 16, 2020 at 08:25:01PM +0300, Jarkko Sakkinen wrote: > 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@xxxxxxxxx> > > > > > 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 Finally squashed. /Jarkko