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.