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

[Davidlohr Bueso]

On Sun, 2014-04-06 at 08:42 +0200, Manfred Spraul wrote:
> Hi,
> 
> On 04/05/2014 08:24 PM, KOSAKI Motohiro wrote:
> > On Fri, Apr 4, 2014 at 1:00 AM, Davidlohr Bueso <davidlohr@xxxxxx> wrote:
> >> I don't think it makes much sense to set unlimited for both 0 and
> >> ULONG_MAX, that would probably just create even more confusion.
> I agree.
> Unlimited was INT_MAX since 0.99.10 and ULONG_MAX since 2.3.39 (with 
> proper backward compatibility for user space).
> 
> Adding a second value for unlimited just creates confusion.
> >> But then again, we shouldn't even care about breaking things with shmmax
> >> or shmall with 0 value, it just makes no sense from a user PoV. shmmax
> >> cannot be 0 unless there's an overflow, which voids any valid cases, and
> >> thus shmall cannot be 0 either as it would go against any values set for
> >> shmmax. I think it's safe to ignore this.
> > Agreed.
> > IMHO, until you find out any incompatibility issue of this, we don't
> > need the switch
> > because we can't make good workaround for that. I'd suggest to merge your patch
> > and see what happen.
> I disagree:
> - "shmctl(,IPC_INFO,&buf); if (my_memory_size > buf.shmmax) 
> perror("change shmmax");" worked correctly since 0.99.10. I don't think 
> that merging the patch and seeing what happens is the right approach.

I agree, we *must* get this right the first time. So no rushing into
things that might later come and bite us in the future.

That said, if users are doing that kind of check, then they must also
check against shmmin, which has _always_ been 1. So shmmax == 0 is a no
no. Otherwise it's not the kernel's fault that they're misusing the API,
which IMO is pretty straightforward for such things. And if shmmax
cannot be 0, shmall cannot be 0.

> - setting shmmax by default to ULONG_MAX is the perfect workaround.
> 
> What reasons are there against the one-line patch?

There's really nothing wrong with it, it's just that 0 is a much nicer
value to have for 'unlimited'. And if we can get away with it, then
lets, otherwise yes, we should go with this path.



--
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/