Re: [PATCH] ipc,shm: disable shmmax and shmall by default

[Manfred Spraul]

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?
> 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.

a) Have you double checked that 0-sized shm segments work properly?
 Does the swap code handle it properly, ...?

b) It's that yet another risk for user space incompatibility?

c) The patch summary is misleading, the impact on SHMMIN is not mentioned.

--
    Manfred
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/