Re: [RFC PATCH 0/4] SGX shmem backing store issue

Dave Hansen
2022-05-02T21:33:04Z

On 5/2/22 10:11, Reinette Chatre wrote:
> My goal was to prevent the page fault handler from doing a "is the PCMD
> page empty" if the reclaimer has a reference to it. Even if the PCMD page
> is empty when the page fault handler checks it the page is expected to
> get data right when reclaimer can get the mutex back since the reclaimer
> already has a reference to the page.

I think shmem_truncate_range() might be the wrong operation. It
destroys data and, in the end, we don't want to destroy data.
Filesystems and the page cache already have nice ways to keep from
destroying data, we just need to use them.

First, I think set_page_dirty(pcmd_page) before the EWB is a good start.
That's what filesystems do before important data that needs to be saved
goes into pages.

Second, I think we need behavior like POSIX_FADV_DONTNEED, not
FALLOC_FL_PUNCH_HOLE. The DONTNEED operations end up in
mapping_evict_folio(), which has both page refcount *and* dirty page
checks. That means that either elevating a refcount or set_page_dirty()
will thwart DONTNEED-like behavior.

There are two basic things we need to do:

1. Prevent page from being truncated around EWB time
2. Prevent unreferenced page with all zeros from staying in shmem
forever or taking up swap space

On the EWB (write to PCMD side) I think something like this works:

sgx_encl_get_backing()
get_page(pcmd_page)

...
lock_page(pcmd_page);
// check for truncation since sgx_encl_get_backing()
if (pcmd_page->mapping != shmem)
goto retry;
// double check this is OK under lock_page():
set_page_dirty(pcmd_page);
__sgx_encl_ewb();
unlock_page(pcmd_page);

That's basically what filesystems do. Get the page from the page cache,
lock it, then make sure it is consistent. If not, retry.

On the "free" / read in (ELDU) side:

// get pcmd_page ref
lock_page(pcmd_page);
// truncation is not a concern because that's only done
// on the read-in side, here, where we hold encl->lock

memset();
if (!memchr_inv())
// clear the way for DONTNEED:
ClearPageDirty(pcmd_page);
unlock_page(pcmd_page);
// drop pcmd_page ref
...
POSIX_FADV_DONTNEED

There's one downside to this: it's _possible_ that an transient
get_page() would block POSIX_FADV_DONTNEED. Then the zeroed page would
stick around forever, or at least until the next ELDU operation did
another memchr_inv().

I went looking around for some of those and could not find any that I
*know* apply to shmem.

This doesn't feel like a great solution; it's more complicated than I
would like. Any other ideas?