mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Dave Hansen <dave.hansen@intel.com>
To: Greg KH <gregkh@linuxfoundation.org>,
	Reinette Chatre <reinette.chatre@intel.com>
Cc: dave.hansen@linux.intel.com, jarkko@kernel.org,
	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
Date: Thu, 4 Nov 2021 12:04:55 -0700	[thread overview]
Message-ID: <460c7c74-0921-03a4-ded0-afa31d9c8aa6@intel.com> (raw)
In-Reply-To: <YYQsc0kktaOdOXb0@kroah.com>

On 11/4/21 11:54 AM, Greg KH wrote:
>>  static bool sgx_should_reclaim(unsigned long watermark)
>>  {
>> -	return sgx_nr_free_pages < watermark && !list_empty(&sgx_active_page_list);
>> +	return atomic_long_read(&sgx_nr_free_pages) < watermark &&
>> +	       !list_empty(&sgx_active_page_list);
> What prevents the value from changing right after you test this?  Why is
> an atomic value somehow solving the problem?

Nothing.  It's fundamentally racy, and that's OK.

Just like the core VM, being under the watermark is an indication that
the kernel is low on pages (SGX pages in this case).  It means we should
try SGX reclaim.  Let's say there's a race and a bunch of pages are
freed.  Reclaim will run once iteration then stop.

We could make an argument that the sgx_reclaim_pages() loop should check
sgx_nr_free_pages in a few places to ensure it doesn't unnecessarily
reclaim too much memory.  That's something to look at, but it's beyond
the scope of this very simple bug fix.

  reply	other threads:[~2021-11-04 19:05 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-04 18:28 Reinette Chatre
2021-11-04 18:36 ` Luck, Tony
2021-11-04 18:44   ` Reinette Chatre
2021-11-04 18:54 ` Greg KH
2021-11-04 19:04   ` Dave Hansen [this message]
2021-11-04 20:57   ` Reinette Chatre
2021-11-05  7:10     ` Greg KH
2021-11-08 19:19       ` Reinette Chatre
2021-11-07 16:45 ` Jarkko Sakkinen
2021-11-07 16:47   ` Jarkko Sakkinen
2021-11-08 19:48     ` Reinette Chatre
2021-11-08 20:12       ` Jarkko Sakkinen
2021-11-08 20:56         ` Reinette Chatre
2021-11-09  1:30           ` 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=460c7c74-0921-03a4-ded0-afa31d9c8aa6@intel.com \
    --to=dave.hansen@intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hpa@zytor.com \
    --cc=jarkko@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sgx@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=reinette.chatre@intel.com \
    --cc=seanjc@google.com \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    /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
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

all inboxes | Powered by JetHome®