Re: [PATCH] x86/sgx: Fix free page accounting

[Reinette Chatre]

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

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.

Unfortunately the traces I provided wrapped and are not easy to read. 
The essential message from the two traces are that the first trace 
(before the fix) shows that the system is stuck (almost 100%) in the SGX 
page fault handler and not able to make any progress and user space 
hangs. The second trace (after the fix) shows that the system splits its 
time between the SGX page fault handler and the reclaimer enabling user 
space to make progress and the test can complete.

> I.e. I don't have anything fundamentally against changing it to
> atomic but the commit message is completely lacking the stimulus
> of changing anything.

The problem needing to be fixed is that sgx_nr_free_pages is not updated 
safely on systems with more than one NUMA node.

Reinette