All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -tip 0/4] ipc/shm: moar updates for v4.11
@ 2017-02-09 20:52 Davidlohr Bueso
  2017-02-09 20:52 ` [PATCH 1/4] ipc/shm: do not check for MAP_POPULATE Davidlohr Bueso
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Davidlohr Bueso @ 2017-02-09 20:52 UTC (permalink / raw)
  To: akpm; +Cc: manfred, dave, linux-kernel

Hi,

Here are some more updates and fixes I noticed while going through
more sysv shm code after the recent shmat(2) fix.

I know it's a bit late in the game, but please consider for v4.11.

Passes ltp tests.

Thanks!

Davidlohr Bueso (4):
  ipc/shm: do not check for MAP_POPULATE
  ipc/shm: some shmat cleanups
  sysv,ipc: cacheline align kern_ipc_perm
  mm,hugetlb: compute page_size_log properly

 include/linux/ipc.h |  7 +++----
 include/linux/sem.h |  3 +--
 ipc/shm.c           | 18 +++++++-----------
 mm/mmap.c           |  2 +-
 4 files changed, 12 insertions(+), 18 deletions(-)

-- 
2.6.6

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/4] ipc/shm: do not check for MAP_POPULATE
  2017-02-09 20:52 [PATCH -tip 0/4] ipc/shm: moar updates for v4.11 Davidlohr Bueso
@ 2017-02-09 20:52 ` Davidlohr Bueso
  2017-02-10  2:15   ` Hugh Dickins
  2017-02-09 20:53 ` [PATCH 2/4] ipc/shm: some shmat cleanups Davidlohr Bueso
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Davidlohr Bueso @ 2017-02-09 20:52 UTC (permalink / raw)
  To: akpm; +Cc: manfred, dave, linux-kernel, Davidlohr Bueso

We do not support prefaulting functionality in sysv shm,
nor MAP_NONBLOCK for that matter. Drop the pointless check
for populate in do_shmat().

Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 ipc/shm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/ipc/shm.c b/ipc/shm.c
index 06ea9ef7f54a..6b3769967789 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -1234,8 +1234,6 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg,
 		err = (long)addr;
 invalid:
 	up_write(&current->mm->mmap_sem);
-	if (populate)
-		mm_populate(addr, populate);
 
 out_fput:
 	fput(file);
-- 
2.6.6

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 2/4] ipc/shm: some shmat cleanups
  2017-02-09 20:52 [PATCH -tip 0/4] ipc/shm: moar updates for v4.11 Davidlohr Bueso
  2017-02-09 20:52 ` [PATCH 1/4] ipc/shm: do not check for MAP_POPULATE Davidlohr Bueso
@ 2017-02-09 20:53 ` Davidlohr Bueso
  2017-02-09 20:53 ` [PATCH 3/4] sysv,ipc: cacheline align kern_ipc_perm Davidlohr Bueso
  2017-02-09 20:53   ` Davidlohr Bueso
  3 siblings, 0 replies; 18+ messages in thread
From: Davidlohr Bueso @ 2017-02-09 20:53 UTC (permalink / raw)
  To: akpm; +Cc: manfred, dave, linux-kernel, Davidlohr Bueso

... cleans up early flag and address minutia.

Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 ipc/shm.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/ipc/shm.c b/ipc/shm.c
index 6b3769967789..9c960241e214 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -1095,11 +1095,11 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg,
 	      ulong *raddr, unsigned long shmlba)
 {
 	struct shmid_kernel *shp;
-	unsigned long addr;
+	unsigned long addr = (unsigned long)shmaddr;
 	unsigned long size;
 	struct file *file;
 	int    err;
-	unsigned long flags;
+	unsigned long flags = MAP_SHARED;
 	unsigned long prot;
 	int acc_mode;
 	struct ipc_namespace *ns;
@@ -1111,7 +1111,8 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg,
 	err = -EINVAL;
 	if (shmid < 0)
 		goto out;
-	else if ((addr = (ulong)shmaddr)) {
+
+	if (addr) {
 		if (addr & (shmlba - 1)) {
 			/*
 			 * Round down to the nearest multiple of shmlba.
@@ -1126,13 +1127,10 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg,
 #endif
 					goto out;
 		}
-		flags = MAP_SHARED | MAP_FIXED;
-	} else {
-		if ((shmflg & SHM_REMAP))
-			goto out;
 
-		flags = MAP_SHARED;
-	}
+		flags |= MAP_FIXED;
+	} else if ((shmflg & SHM_REMAP))
+		goto out;
 
 	if (shmflg & SHM_RDONLY) {
 		prot = PROT_READ;
-- 
2.6.6

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 3/4] sysv,ipc: cacheline align kern_ipc_perm
  2017-02-09 20:52 [PATCH -tip 0/4] ipc/shm: moar updates for v4.11 Davidlohr Bueso
  2017-02-09 20:52 ` [PATCH 1/4] ipc/shm: do not check for MAP_POPULATE Davidlohr Bueso
  2017-02-09 20:53 ` [PATCH 2/4] ipc/shm: some shmat cleanups Davidlohr Bueso
@ 2017-02-09 20:53 ` Davidlohr Bueso
  2017-02-09 20:53   ` Davidlohr Bueso
  3 siblings, 0 replies; 18+ messages in thread
From: Davidlohr Bueso @ 2017-02-09 20:53 UTC (permalink / raw)
  To: akpm; +Cc: manfred, dave, linux-kernel, Davidlohr Bueso

Assign 'struct kern_ipc_perm' its own cacheline to avoid false
sharing with sysv ipc calls. While the structure itself is rather
read-mostly throughout the lifespan of ipc, the spinlock causes
most of the invalidations. One example is 31a7c4746e9 (ipc/sem.c:
cacheline align the ipc spinlock for semaphores). Therefore,
extend this to all ipc.

The effect of cacheline alignment on sems can be seen in sembench,
which deals mostly with semtimedop wait/wakes is seen to improve raw
throughput (worker loops) between 8 to 12% on a 24-core x86 with
over 4 threads.

Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 include/linux/ipc.h | 7 +++----
 include/linux/sem.h | 3 +--
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/include/linux/ipc.h b/include/linux/ipc.h
index 9d84942ae2e5..71fd92d81b26 100644
--- a/include/linux/ipc.h
+++ b/include/linux/ipc.h
@@ -8,8 +8,7 @@
 #define IPCMNI 32768  /* <= MAX_INT limit for ipc arrays (including sysctl changes) */
 
 /* used by in-kernel data structures */
-struct kern_ipc_perm
-{
+struct kern_ipc_perm {
 	spinlock_t	lock;
 	bool		deleted;
 	int		id;
@@ -18,9 +17,9 @@ struct kern_ipc_perm
 	kgid_t		gid;
 	kuid_t		cuid;
 	kgid_t		cgid;
-	umode_t		mode; 
+	umode_t		mode;
 	unsigned long	seq;
 	void		*security;
-};
+} ____cacheline_aligned_in_smp;
 
 #endif /* _LINUX_IPC_H */
diff --git a/include/linux/sem.h b/include/linux/sem.h
index 4fc222f8755d..9edec926e9d9 100644
--- a/include/linux/sem.h
+++ b/include/linux/sem.h
@@ -10,8 +10,7 @@ struct task_struct;
 
 /* One sem_array data structure for each set of semaphores in the system. */
 struct sem_array {
-	struct kern_ipc_perm	____cacheline_aligned_in_smp
-				sem_perm;	/* permissions .. see ipc.h */
+	struct kern_ipc_perm	sem_perm;	/* permissions .. see ipc.h */
 	time_t			sem_ctime;	/* last change time */
 	struct sem		*sem_base;	/* ptr to first semaphore in array */
 	struct list_head	pending_alter;	/* pending operations */
-- 
2.6.6

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 4/4] mm,hugetlb: compute page_size_log properly
  2017-02-09 20:52 [PATCH -tip 0/4] ipc/shm: moar updates for v4.11 Davidlohr Bueso
@ 2017-02-09 20:53   ` Davidlohr Bueso
  2017-02-09 20:53 ` [PATCH 2/4] ipc/shm: some shmat cleanups Davidlohr Bueso
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 18+ messages in thread
From: Davidlohr Bueso @ 2017-02-09 20:53 UTC (permalink / raw)
  To: akpm; +Cc: manfred, dave, linux-kernel, linux-mm, Davidlohr Bueso

The SHM_HUGE_* stuff  was introduced in:

   42d7395feb5 (mm: support more pagesizes for MAP_HUGETLB/SHM_HUGETLB)

It unnecessarily adds another layer, specific to sysv shm, without
anything special about it: the macros are identical to the MAP_HUGE_*
stuff, which in turn does correctly describe the hugepage subsystem.

One example of the problems with extra layers what this patch fixes:
mmap_pgoff() should never be using SHM_HUGE_* logic. It is obviously
harmless but it would still be grand to get rid of it -- although
now in the manpages I don't see that happening.

Cc: linux-mm@kvack.org
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 mm/mmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index 499b988b1639..40b29aca18c1 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1479,7 +1479,7 @@ SYSCALL_DEFINE6(mmap_pgoff, unsigned long, addr, unsigned long, len,
 		struct user_struct *user = NULL;
 		struct hstate *hs;
 
-		hs = hstate_sizelog((flags >> MAP_HUGE_SHIFT) & SHM_HUGE_MASK);
+		hs = hstate_sizelog((flags >> MAP_HUGE_SHIFT) & MAP_HUGE_MASK);
 		if (!hs)
 			return -EINVAL;
 
-- 
2.6.6

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 4/4] mm,hugetlb: compute page_size_log properly
@ 2017-02-09 20:53   ` Davidlohr Bueso
  0 siblings, 0 replies; 18+ messages in thread
From: Davidlohr Bueso @ 2017-02-09 20:53 UTC (permalink / raw)
  To: akpm; +Cc: manfred, dave, linux-kernel, linux-mm, Davidlohr Bueso

The SHM_HUGE_* stuff  was introduced in:

   42d7395feb5 (mm: support more pagesizes for MAP_HUGETLB/SHM_HUGETLB)

It unnecessarily adds another layer, specific to sysv shm, without
anything special about it: the macros are identical to the MAP_HUGE_*
stuff, which in turn does correctly describe the hugepage subsystem.

One example of the problems with extra layers what this patch fixes:
mmap_pgoff() should never be using SHM_HUGE_* logic. It is obviously
harmless but it would still be grand to get rid of it -- although
now in the manpages I don't see that happening.

Cc: linux-mm@kvack.org
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 mm/mmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index 499b988b1639..40b29aca18c1 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1479,7 +1479,7 @@ SYSCALL_DEFINE6(mmap_pgoff, unsigned long, addr, unsigned long, len,
 		struct user_struct *user = NULL;
 		struct hstate *hs;
 
-		hs = hstate_sizelog((flags >> MAP_HUGE_SHIFT) & SHM_HUGE_MASK);
+		hs = hstate_sizelog((flags >> MAP_HUGE_SHIFT) & MAP_HUGE_MASK);
 		if (!hs)
 			return -EINVAL;
 
-- 
2.6.6

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] ipc/shm: do not check for MAP_POPULATE
  2017-02-09 20:52 ` [PATCH 1/4] ipc/shm: do not check for MAP_POPULATE Davidlohr Bueso
@ 2017-02-10  2:15   ` Hugh Dickins
  2017-02-13 18:14     ` Davidlohr Bueso
  0 siblings, 1 reply; 18+ messages in thread
From: Hugh Dickins @ 2017-02-10  2:15 UTC (permalink / raw)
  To: Davidlohr Bueso; +Cc: akpm, manfred, linux-kernel, Davidlohr Bueso

On Thu, 9 Feb 2017, Davidlohr Bueso wrote:

> We do not support prefaulting functionality in sysv shm,
> nor MAP_NONBLOCK for that matter. Drop the pointless check
> for populate in do_shmat().

I haven't checked, but are you sure that "populated" does nothing
when the attacher had previously called mlockall(MCL_FUTURE)?

Hugh

> 
> Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
> ---
>  ipc/shm.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/ipc/shm.c b/ipc/shm.c
> index 06ea9ef7f54a..6b3769967789 100644
> --- a/ipc/shm.c
> +++ b/ipc/shm.c
> @@ -1234,8 +1234,6 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg,
>  		err = (long)addr;
>  invalid:
>  	up_write(&current->mm->mmap_sem);
> -	if (populate)
> -		mm_populate(addr, populate);
>  
>  out_fput:
>  	fput(file);
> -- 
> 2.6.6

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/4] mm,hugetlb: compute page_size_log properly
  2017-02-09 20:53   ` Davidlohr Bueso
@ 2017-02-10 10:20     ` Michal Hocko
  -1 siblings, 0 replies; 18+ messages in thread
From: Michal Hocko @ 2017-02-10 10:20 UTC (permalink / raw)
  To: Davidlohr Bueso; +Cc: akpm, manfred, linux-kernel, linux-mm, Davidlohr Bueso

On Thu 09-02-17 12:53:02, Davidlohr Bueso wrote:
> The SHM_HUGE_* stuff  was introduced in:
> 
>    42d7395feb5 (mm: support more pagesizes for MAP_HUGETLB/SHM_HUGETLB)
> 
> It unnecessarily adds another layer, specific to sysv shm, without
> anything special about it: the macros are identical to the MAP_HUGE_*
> stuff, which in turn does correctly describe the hugepage subsystem.
> 
> One example of the problems with extra layers what this patch fixes:
> mmap_pgoff() should never be using SHM_HUGE_* logic. It is obviously
> harmless but it would still be grand to get rid of it -- although
> now in the manpages I don't see that happening.

Can we just drop SHM_HUGE_MASK altogether? It is not exported in uapi
headers AFAICS.

> 
> Cc: linux-mm@kvack.org
> Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
> ---
>  mm/mmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 499b988b1639..40b29aca18c1 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1479,7 +1479,7 @@ SYSCALL_DEFINE6(mmap_pgoff, unsigned long, addr, unsigned long, len,
>  		struct user_struct *user = NULL;
>  		struct hstate *hs;
>  
> -		hs = hstate_sizelog((flags >> MAP_HUGE_SHIFT) & SHM_HUGE_MASK);
> +		hs = hstate_sizelog((flags >> MAP_HUGE_SHIFT) & MAP_HUGE_MASK);
>  		if (!hs)
>  			return -EINVAL;
>  
> -- 
> 2.6.6
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/4] mm,hugetlb: compute page_size_log properly
@ 2017-02-10 10:20     ` Michal Hocko
  0 siblings, 0 replies; 18+ messages in thread
From: Michal Hocko @ 2017-02-10 10:20 UTC (permalink / raw)
  To: Davidlohr Bueso; +Cc: akpm, manfred, linux-kernel, linux-mm, Davidlohr Bueso

On Thu 09-02-17 12:53:02, Davidlohr Bueso wrote:
> The SHM_HUGE_* stuff  was introduced in:
> 
>    42d7395feb5 (mm: support more pagesizes for MAP_HUGETLB/SHM_HUGETLB)
> 
> It unnecessarily adds another layer, specific to sysv shm, without
> anything special about it: the macros are identical to the MAP_HUGE_*
> stuff, which in turn does correctly describe the hugepage subsystem.
> 
> One example of the problems with extra layers what this patch fixes:
> mmap_pgoff() should never be using SHM_HUGE_* logic. It is obviously
> harmless but it would still be grand to get rid of it -- although
> now in the manpages I don't see that happening.

Can we just drop SHM_HUGE_MASK altogether? It is not exported in uapi
headers AFAICS.

> 
> Cc: linux-mm@kvack.org
> Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
> ---
>  mm/mmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 499b988b1639..40b29aca18c1 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1479,7 +1479,7 @@ SYSCALL_DEFINE6(mmap_pgoff, unsigned long, addr, unsigned long, len,
>  		struct user_struct *user = NULL;
>  		struct hstate *hs;
>  
> -		hs = hstate_sizelog((flags >> MAP_HUGE_SHIFT) & SHM_HUGE_MASK);
> +		hs = hstate_sizelog((flags >> MAP_HUGE_SHIFT) & MAP_HUGE_MASK);
>  		if (!hs)
>  			return -EINVAL;
>  
> -- 
> 2.6.6
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/4] mm,hugetlb: compute page_size_log properly
  2017-02-10 10:20     ` Michal Hocko
@ 2017-02-10 16:51       ` Davidlohr Bueso
  -1 siblings, 0 replies; 18+ messages in thread
From: Davidlohr Bueso @ 2017-02-10 16:51 UTC (permalink / raw)
  To: Michal Hocko; +Cc: akpm, manfred, linux-kernel, linux-mm, Davidlohr Bueso

On Fri, 10 Feb 2017, Michal Hocko wrote:

>On Thu 09-02-17 12:53:02, Davidlohr Bueso wrote:
>> The SHM_HUGE_* stuff  was introduced in:
>>
>>    42d7395feb5 (mm: support more pagesizes for MAP_HUGETLB/SHM_HUGETLB)
>>
>> It unnecessarily adds another layer, specific to sysv shm, without
>> anything special about it: the macros are identical to the MAP_HUGE_*
>> stuff, which in turn does correctly describe the hugepage subsystem.
>>
>> One example of the problems with extra layers what this patch fixes:
>> mmap_pgoff() should never be using SHM_HUGE_* logic. It is obviously
>> harmless but it would still be grand to get rid of it -- although
>> now in the manpages I don't see that happening.
>
>Can we just drop SHM_HUGE_MASK altogether? It is not exported in uapi
>headers AFAICS.

Yeah that was my original idea, however I noticed that shmget.2 mentions
kernel internals as part of SHM_HUGE_{2MB,1GB}, ie: SHM_HUGE_SHIFT. So
dropping _MASK doesn't make sense if we are going to keep _SHIFT.

Thanks,
Davidlohr

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/4] mm,hugetlb: compute page_size_log properly
@ 2017-02-10 16:51       ` Davidlohr Bueso
  0 siblings, 0 replies; 18+ messages in thread
From: Davidlohr Bueso @ 2017-02-10 16:51 UTC (permalink / raw)
  To: Michal Hocko; +Cc: akpm, manfred, linux-kernel, linux-mm, Davidlohr Bueso

On Fri, 10 Feb 2017, Michal Hocko wrote:

>On Thu 09-02-17 12:53:02, Davidlohr Bueso wrote:
>> The SHM_HUGE_* stuff  was introduced in:
>>
>>    42d7395feb5 (mm: support more pagesizes for MAP_HUGETLB/SHM_HUGETLB)
>>
>> It unnecessarily adds another layer, specific to sysv shm, without
>> anything special about it: the macros are identical to the MAP_HUGE_*
>> stuff, which in turn does correctly describe the hugepage subsystem.
>>
>> One example of the problems with extra layers what this patch fixes:
>> mmap_pgoff() should never be using SHM_HUGE_* logic. It is obviously
>> harmless but it would still be grand to get rid of it -- although
>> now in the manpages I don't see that happening.
>
>Can we just drop SHM_HUGE_MASK altogether? It is not exported in uapi
>headers AFAICS.

Yeah that was my original idea, however I noticed that shmget.2 mentions
kernel internals as part of SHM_HUGE_{2MB,1GB}, ie: SHM_HUGE_SHIFT. So
dropping _MASK doesn't make sense if we are going to keep _SHIFT.

Thanks,
Davidlohr

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] ipc/shm: do not check for MAP_POPULATE
  2017-02-10  2:15   ` Hugh Dickins
@ 2017-02-13 18:14     ` Davidlohr Bueso
  0 siblings, 0 replies; 18+ messages in thread
From: Davidlohr Bueso @ 2017-02-13 18:14 UTC (permalink / raw)
  To: Hugh Dickins; +Cc: akpm, manfred, linux-kernel, Davidlohr Bueso

On Thu, 09 Feb 2017, Hugh Dickins wrote:

>I haven't checked, but are you sure that "populated" does nothing
>when the attacher had previously called mlockall(MCL_FUTURE)?

I checked and you are certainly right. Andrew, please do not
consider this patch, it's bogus.

Thanks,
Davidlohr

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/4] mm,hugetlb: compute page_size_log properly
  2017-02-10 16:51       ` Davidlohr Bueso
@ 2017-02-20 16:11         ` Michal Hocko
  -1 siblings, 0 replies; 18+ messages in thread
From: Michal Hocko @ 2017-02-20 16:11 UTC (permalink / raw)
  To: Davidlohr Bueso; +Cc: akpm, manfred, linux-kernel, linux-mm, Davidlohr Bueso

Sorry for a late reply, I wasn't online last week

On Fri 10-02-17 08:51:11, Davidlohr Bueso wrote:
> On Fri, 10 Feb 2017, Michal Hocko wrote:
> 
> > On Thu 09-02-17 12:53:02, Davidlohr Bueso wrote:
> > > The SHM_HUGE_* stuff  was introduced in:
> > > 
> > >    42d7395feb5 (mm: support more pagesizes for MAP_HUGETLB/SHM_HUGETLB)
> > > 
> > > It unnecessarily adds another layer, specific to sysv shm, without
> > > anything special about it: the macros are identical to the MAP_HUGE_*
> > > stuff, which in turn does correctly describe the hugepage subsystem.
> > > 
> > > One example of the problems with extra layers what this patch fixes:
> > > mmap_pgoff() should never be using SHM_HUGE_* logic. It is obviously
> > > harmless but it would still be grand to get rid of it -- although
> > > now in the manpages I don't see that happening.
> > 
> > Can we just drop SHM_HUGE_MASK altogether? It is not exported in uapi
> > headers AFAICS.
> 
> Yeah that was my original idea, however I noticed that shmget.2 mentions
> kernel internals as part of SHM_HUGE_{2MB,1GB}, ie: SHM_HUGE_SHIFT. So
> dropping _MASK doesn't make sense if we are going to keep _SHIFT.

I am not sure I understand.
$ git grep SHM_HUGE_ include/uapi/
$

So there doesn't seem to be any user visible constant. The man page
mentiones is but I do not really see how is the userspace supposed to
use it.
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/4] mm,hugetlb: compute page_size_log properly
@ 2017-02-20 16:11         ` Michal Hocko
  0 siblings, 0 replies; 18+ messages in thread
From: Michal Hocko @ 2017-02-20 16:11 UTC (permalink / raw)
  To: Davidlohr Bueso; +Cc: akpm, manfred, linux-kernel, linux-mm, Davidlohr Bueso

Sorry for a late reply, I wasn't online last week

On Fri 10-02-17 08:51:11, Davidlohr Bueso wrote:
> On Fri, 10 Feb 2017, Michal Hocko wrote:
> 
> > On Thu 09-02-17 12:53:02, Davidlohr Bueso wrote:
> > > The SHM_HUGE_* stuff  was introduced in:
> > > 
> > >    42d7395feb5 (mm: support more pagesizes for MAP_HUGETLB/SHM_HUGETLB)
> > > 
> > > It unnecessarily adds another layer, specific to sysv shm, without
> > > anything special about it: the macros are identical to the MAP_HUGE_*
> > > stuff, which in turn does correctly describe the hugepage subsystem.
> > > 
> > > One example of the problems with extra layers what this patch fixes:
> > > mmap_pgoff() should never be using SHM_HUGE_* logic. It is obviously
> > > harmless but it would still be grand to get rid of it -- although
> > > now in the manpages I don't see that happening.
> > 
> > Can we just drop SHM_HUGE_MASK altogether? It is not exported in uapi
> > headers AFAICS.
> 
> Yeah that was my original idea, however I noticed that shmget.2 mentions
> kernel internals as part of SHM_HUGE_{2MB,1GB}, ie: SHM_HUGE_SHIFT. So
> dropping _MASK doesn't make sense if we are going to keep _SHIFT.

I am not sure I understand.
$ git grep SHM_HUGE_ include/uapi/
$

So there doesn't seem to be any user visible constant. The man page
mentiones is but I do not really see how is the userspace supposed to
use it.
-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/4] mm,hugetlb: compute page_size_log properly
  2017-02-20 16:11         ` Michal Hocko
@ 2017-02-22 16:03           ` Davidlohr Bueso
  -1 siblings, 0 replies; 18+ messages in thread
From: Davidlohr Bueso @ 2017-02-22 16:03 UTC (permalink / raw)
  To: Michal Hocko; +Cc: akpm, manfred, linux-kernel, linux-mm, Davidlohr Bueso

On Mon, 20 Feb 2017, Michal Hocko wrote:

>I am not sure I understand.
>$ git grep SHM_HUGE_ include/uapi/
>$
>
>So there doesn't seem to be any user visible constant. The man page
>mentiones is but I do not really see how is the userspace supposed to
>use it.

Yeah, userspace is not supposed to use it, it's just there because
the manpage describes kernel internals. I'm not really a big fan
of touching manpages (and ipc is already full of corner cases),
but I guess nobody can really complain if we rip out all the
SHM_HUGE_ stuff.

Thanks,
Davidlohr

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/4] mm,hugetlb: compute page_size_log properly
@ 2017-02-22 16:03           ` Davidlohr Bueso
  0 siblings, 0 replies; 18+ messages in thread
From: Davidlohr Bueso @ 2017-02-22 16:03 UTC (permalink / raw)
  To: Michal Hocko; +Cc: akpm, manfred, linux-kernel, linux-mm, Davidlohr Bueso

On Mon, 20 Feb 2017, Michal Hocko wrote:

>I am not sure I understand.
>$ git grep SHM_HUGE_ include/uapi/
>$
>
>So there doesn't seem to be any user visible constant. The man page
>mentiones is but I do not really see how is the userspace supposed to
>use it.

Yeah, userspace is not supposed to use it, it's just there because
the manpage describes kernel internals. I'm not really a big fan
of touching manpages (and ipc is already full of corner cases),
but I guess nobody can really complain if we rip out all the
SHM_HUGE_ stuff.

Thanks,
Davidlohr

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/4] mm,hugetlb: compute page_size_log properly
  2017-02-22 16:03           ` Davidlohr Bueso
@ 2017-02-22 16:11             ` Michal Hocko
  -1 siblings, 0 replies; 18+ messages in thread
From: Michal Hocko @ 2017-02-22 16:11 UTC (permalink / raw)
  To: Davidlohr Bueso; +Cc: akpm, manfred, linux-kernel, linux-mm, Davidlohr Bueso

On Wed 22-02-17 08:03:19, Davidlohr Bueso wrote:
> On Mon, 20 Feb 2017, Michal Hocko wrote:
> 
> > I am not sure I understand.
> > $ git grep SHM_HUGE_ include/uapi/
> > $
> > 
> > So there doesn't seem to be any user visible constant. The man page
> > mentiones is but I do not really see how is the userspace supposed to
> > use it.
> 
> Yeah, userspace is not supposed to use it, it's just there because
> the manpage describes kernel internals.

Which is wrong!

> I'm not really a big fan
> of touching manpages (and ipc is already full of corner cases),
> but I guess nobody can really complain if we rip out all the
> SHM_HUGE_ stuff.

yeah, let's just get rid of it.
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/4] mm,hugetlb: compute page_size_log properly
@ 2017-02-22 16:11             ` Michal Hocko
  0 siblings, 0 replies; 18+ messages in thread
From: Michal Hocko @ 2017-02-22 16:11 UTC (permalink / raw)
  To: Davidlohr Bueso; +Cc: akpm, manfred, linux-kernel, linux-mm, Davidlohr Bueso

On Wed 22-02-17 08:03:19, Davidlohr Bueso wrote:
> On Mon, 20 Feb 2017, Michal Hocko wrote:
> 
> > I am not sure I understand.
> > $ git grep SHM_HUGE_ include/uapi/
> > $
> > 
> > So there doesn't seem to be any user visible constant. The man page
> > mentiones is but I do not really see how is the userspace supposed to
> > use it.
> 
> Yeah, userspace is not supposed to use it, it's just there because
> the manpage describes kernel internals.

Which is wrong!

> I'm not really a big fan
> of touching manpages (and ipc is already full of corner cases),
> but I guess nobody can really complain if we rip out all the
> SHM_HUGE_ stuff.

yeah, let's just get rid of it.
-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2017-02-22 16:23 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-09 20:52 [PATCH -tip 0/4] ipc/shm: moar updates for v4.11 Davidlohr Bueso
2017-02-09 20:52 ` [PATCH 1/4] ipc/shm: do not check for MAP_POPULATE Davidlohr Bueso
2017-02-10  2:15   ` Hugh Dickins
2017-02-13 18:14     ` Davidlohr Bueso
2017-02-09 20:53 ` [PATCH 2/4] ipc/shm: some shmat cleanups Davidlohr Bueso
2017-02-09 20:53 ` [PATCH 3/4] sysv,ipc: cacheline align kern_ipc_perm Davidlohr Bueso
2017-02-09 20:53 ` [PATCH 4/4] mm,hugetlb: compute page_size_log properly Davidlohr Bueso
2017-02-09 20:53   ` Davidlohr Bueso
2017-02-10 10:20   ` Michal Hocko
2017-02-10 10:20     ` Michal Hocko
2017-02-10 16:51     ` Davidlohr Bueso
2017-02-10 16:51       ` Davidlohr Bueso
2017-02-20 16:11       ` Michal Hocko
2017-02-20 16:11         ` Michal Hocko
2017-02-22 16:03         ` Davidlohr Bueso
2017-02-22 16:03           ` Davidlohr Bueso
2017-02-22 16:11           ` Michal Hocko
2017-02-22 16:11             ` Michal Hocko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.