On 6/30/22 05:57, Mark Hemment wrote:
Hi Khalid,
On Wed, 29 Jun 2022 at 23:54, Khalid Aziz <khalid.aziz@xxxxxxxxxx> wrote:
Memory pages shared between processes require a page table entry
(PTE) for each process. Each of these PTE consumes consume some of
the memory and as long as number of mappings being maintained is
small enough, this space consumed by page tables is not
objectionable. When very few memory pages are shared between
processes, the number of page table entries (PTEs) to maintain is
mostly constrained by the number of pages of memory on the system.
As the number of shared pages and the number of times pages are
shared goes up, amount of memory consumed by page tables starts to
become significant. This issue does not apply to threads. Any number
of threads can share the same pages inside a process while sharing
the same PTEs. Extending this same model to sharing pages across
processes can eliminate this issue for sharing across processes as
well.
Some of the field deployments commonly see memory pages shared
across 1000s of processes. On x86_64, each page requires a PTE that
is only 8 bytes long which is very small compared to the 4K page
size. When 2000 processes map the same page in their address space,
each one of them requires 8 bytes for its PTE and together that adds
up to 8K of memory just to hold the PTEs for one 4K page. On a
database server with 300GB SGA, a system crash was seen with
out-of-memory condition when 1500+ clients tried to share this SGA
even though the system had 512GB of memory. On this server, in the
worst case scenario of all 1500 processes mapping every page from
SGA would have required 878GB+ for just the PTEs. If these PTEs
could be shared, amount of memory saved is very significant.
This patch series implements a mechanism in kernel to allow
userspace processes to opt into sharing PTEs. It adds a new
in-memory filesystem - msharefs. A file created on msharefs creates
a new shared region where all processes sharing that region will
share the PTEs as well. A process can create a new file on msharefs
and then mmap it which assigns a starting address and size to this
mshare'd region. Another process that has the right permission to
open the file on msharefs can then mmap this file in its address
space at same virtual address and size and share this region through
shared PTEs. An unlink() on the file marks the mshare'd region for
deletion once there are no more users of the region. When the mshare
region is deleted, all the pages used by the region are freed.
Noting the flexibility of 'mshare' has been reduced from v1. The
earlier version allowed msharing of named mappings, while this patch
is only for anonymous mappings.
Any plans to support named mappings? If not, I guess *someone* will
want it (eventually). Minor, as the patch does not introduce new
syscalls, but having an API which is flexible for both named and anon
mappings would be good (this is a nit, not a strong suggestion).
I apologize for not clarifying this. The initial mmap() call looks like an anonymous mapping but one could easily call
mremap later and map any other objects in the same address space which remains shared until the mshare region is torn
down. It is my intent to support mapping any objects in mshare region.
The cover letter details the problem being solved and the API, but
gives no details of the implementation. A paragraph on the use of a
mm_struct per-msharefs file would be helpful.
Good point. I will do that next time.
I've only quickly scanned the patchset; not in enough detail to
comment on each patch, but a few observations.
o I was expecting to see mprotect() against a mshared vma to either
be disallowed or code to support the splitting of a mshared vma. I
didn't see either.msharefs_delmm
Since mshare region is intended to support multiple objects being mapped in the region and different protections on
different parts of region, mprotect should be supported and should handle splitting the mshare'd vmas. Until basic code
is solid, it would make sense to prevent splitting vmas and add that on later. I will add this code.
o For the case where the mshare file has been closed/unmmap but not
unlinked, a 'mshare_data' structure will leaked when the inode is
evicted.
You are right. mshare_evict_inode() needs to call msharefs_delmm() to clean up.
o The alignment requirement is PGDIR_SIZE, which is very large.
Should/could this be PMD_SIZE?
Yes, PGDIR_SIZE is large. It works for the database folks who requested this feature but PMD might be more versatile. I
have been thinking about switching to PMD since that will make it easier to move hugetlbfs page table sharing code over
to this code.
o mshare should be a conditional feature (CONFIG_MSHARE ?).
I can do that. I was reluctant to add yet another CONFIG option. Since this feature is activated explicitly by userspace
code, is it necessary to make it a config option?
I might get a chance do a finer grain review later/tomorrow.
API
===
mshare does not introduce a new API. It instead uses existing APIs
to implement page table sharing. The steps to use this feature are:
1. Mount msharefs on /sys/fs/mshare -
mount -t msharefs msharefs /sys/fs/mshare
2. mshare regions have alignment and size requirements. Start
address for the region must be aligned to an address boundary and
be a multiple of fixed size. This alignment and size requirement
can be obtained by reading the file /sys/fs/mshare/mshare_info
which returns a number in text format. mshare regions must be
aligned to this boundary and be a multiple of this size.
3. For the process creating mshare region:
a. Create a file on /sys/fs/mshare, for example -
fd = open("/sys/fs/mshare/shareme",
O_RDWR|O_CREAT|O_EXCL, 0600);
b. mmap this file to establish starting address and size -
mmap((void *)TB(2), BUF_SIZE, PROT_READ | PROT_WRITE,
MAP_SHARED, fd, 0);
c. Write and read to mshared region normally.
4. For processes attaching to mshare'd region:
a. Open the file on msharefs, for example -
fd = open("/sys/fs/mshare/shareme", O_RDWR);
b. Get information about mshare'd region from the file:
struct mshare_info {
unsigned long start;
unsigned long size;
} m_info;
read(fd, &m_info, sizeof(m_info));
c. mmap the mshare'd region -
mmap(m_info.start, m_info.size,
PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
5. To delete the mshare region -
unlink("/sys/fs/mshare/shareme");
Example Code
============
Snippet of the code that a donor process would run looks like below:
-----------------
fd = open("/sys/fs/mshare/mshare_info", O_RDONLY);
read(fd, req, 128);
alignsize = atoi(req);
close(fd);
fd = open("/sys/fs/mshare/shareme", O_RDWR|O_CREAT|O_EXCL, 0600);
start = alignsize * 4;
size = alignsize * 2;
addr = mmap((void *)start, size, PROT_READ | PROT_WRITE,
MAP_SHARED | MAP_ANONYMOUS, 0, 0);
Typo, missing 'fd'; MAP_SHARED | MAP_ANONYMOUS, fd, 0)
Yes, you are right. I will fix that.
Thanks, Mark! I really appreciate your taking time to review this code.
--
Khalid
