Messages in this thread Patch in this message | | | Subject | Re: [PATCH] ipc,shm: disable shmmax and shmall by default | From | Davidlohr Bueso <> | Date | Fri, 11 Apr 2014 13:27:15 -0700 |
| |
On Fri, 2014-04-11 at 20:28 +0200, Manfred Spraul wrote: > Hi Davidlohr, > > On 04/03/2014 02:20 AM, Davidlohr Bueso wrote: > > The default size for shmmax is, and always has been, 32Mb. > > Today, in the XXI century, it seems that this value is rather small, > > making users have to increase it via sysctl, which can cause > > unnecessary work and userspace application workarounds[1]. > > > > [snip] > > Running this patch through LTP, everything passes, except the following, > > which, due to the nature of this change, is quite expected: > > > > shmget02 1 TFAIL : call succeeded unexpectedly > Why is this TFAIL expected?
So looking at shmget02.c, this is the case that fails:
for (i = 0; i < TST_TOTAL; i++) { /* * Look for a failure ... */
TEST(shmget(*(TC[i].skey), TC[i].size, TC[i].flags));
if (TEST_RETURN != -1) { tst_resm(TFAIL, "call succeeded unexpectedly"); continue; }
Where TC[0] is: struct test_case_t { int *skey; int size; int flags; int error; } TC[] = { /* EINVAL - size is 0 */ { &shmkey2, 0, IPC_CREAT | IPC_EXCL | SHM_RW, EINVAL},
So it's expected because now 0 is actually valid. And before:
EINVAL A new segment was to be created and size < SHMMIN or size > SHMMAX
> > > > diff --git a/ipc/shm.c b/ipc/shm.c > > index 7645961..ae01ffa 100644 > > --- a/ipc/shm.c > > +++ b/ipc/shm.c > > @@ -490,10 +490,12 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params) > > int id; > > vm_flags_t acctflag = 0; > > > > - if (size < SHMMIN || size > ns->shm_ctlmax) > > + if (ns->shm_ctlmax && > > + (size < SHMMIN || size > ns->shm_ctlmax)) > > return -EINVAL; > > > > - if (ns->shm_tot + numpages > ns->shm_ctlall) > > + if (ns->shm_ctlall && > > + ns->shm_tot + numpages > ns->shm_ctlall) > > return -ENOSPC; > > > > shp = ipc_rcu_alloc(sizeof(*shp)); > Ok, I understand it: > Your patch disables checking shmmax, shmall *AND* checking for SHMMIN.
Right, if shmmax is 0, then there's no point checking for shmmin, otherwise we'd always end up returning EINVAL.
> > a) Have you double checked that 0-sized shm segments work properly? > Does the swap code handle it properly, ...? EINVAL A new segment was to be created and size < SHMMIN or size > SHMMAX
Hmm so I've been using this patch just fine on my laptop since I sent it. So far I haven't seen any issues. Are you refering to something in particular? I'd be happy to run any cases you're concerned with.
> b) It's that yet another risk for user space incompatibility?
Sorry, I don't follow here.
> c) The patch summary is misleading, the impact on SHMMIN is not mentioned.
Sure, I can explicitly add it to the changelog.
Thanks, Davidlohr
| |