Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 8 Nov 2021 22:12:50 +0200
From:   Jarkko Sakkinen <jarkko@kernel.org>
To:     Reinette Chatre <reinette.chatre@intel.com>
Cc:     dave.hansen@linux.intel.com, tglx@linutronix.de, bp@alien8.de,
        mingo@redhat.com, linux-sgx@vger.kernel.org, x86@kernel.org,
        seanjc@google.com, tony.luck@intel.com, hpa@zytor.com,
        linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH] x86/sgx: Fix free page accounting
Message-ID: <YYmEwobYw+jGBSwV@iki.fi>
References: <373992d869cd356ce9e9afe43ef4934b70d604fd.1636049678.git.reinette.chatre@intel.com>
 <6e51fdacc2c1d834258f00ad8cc268b8d782eca7.camel@kernel.org>
 <2a0b84575733e4aaee13926387d997c35ac23130.camel@kernel.org>
 <d7a6dedb-03c5-fad1-e112-c912473c7214@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <d7a6dedb-03c5-fad1-e112-c912473c7214@intel.com>
Precedence: bulk

On Mon, Nov 08, 2021 at 11:48:18AM -0800, Reinette Chatre wrote:
> Hi Jarkko,
> 
> On 11/7/2021 8:47 AM, Jarkko Sakkinen wrote:
> > On Sun, 2021-11-07 at 18:45 +0200, Jarkko Sakkinen wrote:
> > > On Thu, 2021-11-04 at 11:28 -0700, Reinette Chatre wrote:
> > > > The consequence of sgx_nr_free_pages not being protected is that
> > > > its value may not accurately reflect the actual number of free
> > > > pages on the system, impacting the availability of free pages in
> > > > support of many flows. The problematic scenario is when the
> > > > reclaimer never runs because it believes there to be sufficient
> > > > free pages while any attempt to allocate a page fails because there
> > > > are no free pages available. The worst scenario observed was a
> > > > user space hang because of repeated page faults caused by
> > > > no free pages ever made available.
> > > 
> > > Can you go in detail with the "concrete scenario" in the commit
> > > message? It does not have to describe all the possible scenarios
> > > but at least one sequence of events.
> 
> 
> I provided significant detail regarding the "concrete scenario" in a
> separate response to Greg:
> https://lore.kernel.org/lkml/a636290d-db04-be16-1c86-a8dcc3719b39@intel.com/
> 
> That message details the test that was run (the test hangs before the fix
> and can complete after the fix), the traces captured at the time the test
> hung, analysis of the traces with root cause of why the system is hung,
> traces after fix applied demonstrating why user space is able to make
> progress and explaining why the test can complete.

For me that sequence looks like something that you could "abstract"
a bit and get a rough description of the concurrency scenario.

It is as important in this type of patch, as the code change itself,
not least because it helps with maintaining in the future to have
that info in some level of detail in the commit log.

/Jarkko