Re: [PATCH] Hugetlb pages should not be reserved by shmat() if SHM_NORESERVE

[Prakash Sangappa <prakash.sangappa@oracle.com>]

> On Jan 21, 2024, at 2:32 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> On Fri, 19 Jan 2024 17:17:52 -0800 Prakash Sangappa <prakash.sangappa@oracle.com> wrote:
> 
>> For shared memory of type SHM_HUGETLB, hugetlb pages are reserved in
>> shmget() call. If SHM_NORESERVE flags is specified then the hugetlb
>> pages are not reserved. However when the shared memory is attached
>> with the shmat() call the hugetlb pages are getting reserved incorrectly
>> for SHM_HUGETLB shared memory created with SHM_NORESERVE.
>> 
>> Ensure that the hugetlb pages are no reserved for SHM_HUGETLB shared
>> memory in the shmat() call.
> 
> Thanks.

Sent a v2 patch with slightly modified fix.

> 
> What are the userspace-visible effects of this change?

This is a bug. Following test shows the issue

$ cat shmhtb.c
#include <stdlib.h>
#include <sys/mman.h>
#include <stdio.h>
#include <sys/ipc.h>
#include <sys/shm.h>
#include <unistd.h>
#include <errno.h>

#define SHMSZ (10*1024*1024)
#define SKEY 41321234

int main()
{
int shmflags = 0660 | IPC_CREAT | SHM_HUGETLB | SHM_NORESERVE;
int shmid;

shmid = shmget(SKEY, SHMSZ, shmflags);

if (shmid < 0)
{  printf("shmat: shmget() failed, %d\n", errno);
return 1;
}

printf("After shmget\n");
system("cat /proc/meminfo | grep -i hugepages_”);

shmat(shmid, NULL, 0);

printf("After shmat\n");
system("cat /proc/meminfo | grep -i hugepages_");

shmctl(shmid, IPC_RMID, NULL);

return 0;
}


# sysctl -w vm.nr_hugepages=20
#./shmhtb
After shmget
HugePages_Total:      20
HugePages_Free:       20
HugePages_Rsvd:        0
HugePages_Surp:        0
After shmat
HugePages_Total:      20
HugePages_Free:       20
HugePages_Rsvd:        5 <--
HugePages_Surp:        0

> 
> Based on that, is a -stable backport desirable?

I think so. The issue is reproducible on older kernel versions. Reproduced on v4.18

> 
> And can we please identify a suitable Fixes: target for this?

Should it be mentioned in the patch?

-Prakash

>