From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752555AbaEMBfT (ORCPT ); Mon, 12 May 2014 21:35:19 -0400 Received: from g4t3426.houston.hp.com ([15.201.208.54]:55354 "EHLO g4t3426.houston.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751068AbaEMBfR (ORCPT ); Mon, 12 May 2014 21:35:17 -0400 Message-ID: <1399944913.2648.56.camel@buesod1.americas.hpqcorp.net> Subject: Re: [PATCH v2] ipc,shm: document new limits in the uapi header From: Davidlohr Bueso To: mtk.manpages@gmail.com Cc: Manfred Spraul , Davidlohr Bueso , Martin Schwidefsky , LKML , Andrew Morton , KAMEZAWA Hiroyuki , KOSAKI Motohiro , Greg Thelen , aswin@hp.com, "linux-mm@kvack.org" Date: Mon, 12 May 2014 18:35:13 -0700 In-Reply-To: References: <1398090397-2397-1-git-send-email-manfred@colorfullife.com> <1399406800.13799.20.camel@buesod1.americas.hpqcorp.net> <1399414081.30629.2.camel@buesod1.americas.hpqcorp.net> <5369C43D.1000206@gmail.com> <1399486965.4567.9.camel@buesod1.americas.hpqcorp.net> <1399490251.4567.24.camel@buesod1.americas.hpqcorp.net> <1399841186.8629.6.camel@buesod1.americas.hpqcorp.net> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.6.4 (3.6.4-3.fc18) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2014-05-12 at 09:44 +0200, Michael Kerrisk (man-pages) wrote: > Hi Davidlohr, > > On Sun, May 11, 2014 at 10:46 PM, Davidlohr Bueso wrote: > > On Fri, 2014-05-09 at 10:44 +0200, Michael Kerrisk (man-pages) wrote: > >> On Wed, May 7, 2014 at 9:17 PM, Davidlohr Bueso wrote: > >> > This is useful in the future and allows users to > >> > better understand the reasoning behind the changes. > >> > > >> > Signed-off-by: Davidlohr Bueso > >> > --- > >> > include/uapi/linux/shm.h | 15 +++++++++------ > >> > 1 file changed, 9 insertions(+), 6 deletions(-) > >> > > >> > diff --git a/include/uapi/linux/shm.h b/include/uapi/linux/shm.h > >> > index 74e786d..3400b6e 100644 > >> > --- a/include/uapi/linux/shm.h > >> > +++ b/include/uapi/linux/shm.h > >> > @@ -8,17 +8,20 @@ > >> > #endif > >> > > >> > /* > >> > - * SHMMAX, SHMMNI and SHMALL are upper limits are defaults which can > >> > - * be modified by sysctl. > >> > + * SHMMNI, SHMMAX and SHMALL are the default upper limits which can be > >> > + * modified by sysctl. Both SHMMAX and SHMALL have their default values > >> > + * to the maximum limit which is as large as it can be without helping > >> > + * userspace overflow the values. There is really nothing the kernel > >> > + * can do to avoid this any further. It is therefore not advised to > >> > + * make them any larger. These limits are suitable for both 32 and > >> > + * 64-bit systems. > >> > >> I somehow find that text still rather impenetrable. What about this: > >> > >> SHMMNI, SHMMAX and SHMALL are default upper limits which can be > >> modified by sysctl. The SHMMAX and SHMALL values have been chosen to > >> be as large possible without facilitating scenarios where userspace > >> causes overflows when adjusting the limits via operations of the form > >> "retrieve current limit; add X; update limit". It is therefore not > >> advised to make SHMMAX and SHMALL any larger. These limits are > >> suitable for both 32 and 64-bit systems. > > > > I don't really have much preference, imho both read pretty much the > > same, specially considering this is still code after all. If you guys > > really prefer updating it, let me know and I'll send a v3. But perhaps > > your text is a bit more suitable in the svipc manpage? > > The problem is that part of your text is still broken grammatically In > particular, the piece "Both SHMMAX and SHMALL have their default > values to the maximum limit" at the very least lacks a word. That's > what prompted me to propose the alternative, rather than just say > "this is wrong"--and I thought that I might as well make a more > thoroughgoing attempt at helping improve the text. > > I agree that text something like this should land in the man page at > some point, but as long as we're going to the trouble to improve the > comments in the code, let's make them as good and helpful as we can. Fair enough, and I trust your grammar corrections over mine ;) Thanks for taking a closer look. I've added your text below. 8<------------------------------------------ From: Davidlohr Bueso Subject: [PATCH v3] ipc,shm: document new limits in the uapi header This is useful in the future and allows users to better understand the reasoning behind the changes. Signed-off-by: Davidlohr Bueso --- include/uapi/linux/shm.h | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/include/uapi/linux/shm.h b/include/uapi/linux/shm.h index 74e786d..1fbf24e 100644 --- a/include/uapi/linux/shm.h +++ b/include/uapi/linux/shm.h @@ -8,17 +8,20 @@ #endif /* - * SHMMAX, SHMMNI and SHMALL are upper limits are defaults which can - * be modified by sysctl. + * SHMMNI, SHMMAX and SHMALL are default upper limits which can be + * modified by sysctl. The SHMMAX and SHMALL values have been chosen to + * be as large possible without facilitating scenarios where userspace + * causes overflows when adjusting the limits via operations of the form + * "retrieve current limit; add X; update limit". It is therefore not + * advised to make SHMMAX and SHMALL any larger. These limits are + * suitable for both 32 and 64-bit systems. */ - #define SHMMIN 1 /* min shared seg size (bytes) */ #define SHMMNI 4096 /* max num of segs system wide */ -#define SHMMAX (ULONG_MAX - (1L<<24)) /* max shared seg size (bytes) */ -#define SHMALL (ULONG_MAX - (1L<<24)) /* max shm system wide (pages) */ +#define SHMMAX (ULONG_MAX - (1UL << 24)) /* max shared seg size (bytes) */ +#define SHMALL (ULONG_MAX - (1UL << 24)) /* max shm system wide (pages) */ #define SHMSEG SHMMNI /* max shared segs per process */ - /* Obsolete, used only for backwards compatibility and libc5 compiles */ struct shmid_ds { struct ipc_perm shm_perm; /* operation perms */ -- 1.8.1.4