Subject: Re: SysV SHM locking and reallocating support
To: None <tech-kern@netbsd.org>
From: Christos Zoulas <christos@astron.com>
List: tech-kern
Date: 09/24/2006 17:07:39
In article <20060924013154.368a6b12.unex@linija.org>,
Mindaugas  <unex@linija.org> wrote:
>-=-=-=-=-=-
>
>Hello,
>
>here is an attached patch to support SysV IPC shared memory locking to
>physical memory by shmctl(2) SHM_LOCK/SHM_UNLOCK options. These options
>is not defined in standard, but exists in systems like Solaris, Linux,
>HP-UX and probably others. Also there is an sysctl's
>kern.ipc.shm_use.phys parameter, insipired from FreeBSD.
>For memory locking I have used uvm_map_pageable(), but I am not sure is
>this OK. It needs testing. Please review the patch and comment it.

I am not sure either... Someone with vm knowledge can help.

>
>There is also an implementation of shmmni, shmseg and shmmaxpgs changing
>via sysctl nodes. At least I have tested - it works fine.
>There is an XXX comment in shmrealloc(), where, I think, should be an
>interrupt locking. I'm not sure about  lockings on sysctl parameters
>changing?

I think that you should do the realloc from the sysctl handler, and
yes it needs locking. A simple mutex should be enough. You should not
use bcopy, but memcpy in this case. The realloc should return an error
if there is an invalid argument or could not allocate memory. This
would be an indication to the user that something failed at sysctl
time (and the sysctl value should not be changed on failure).
The sysctls should be limited to the super-user (or elad can
chime in on what kauth privilege is appropriate).

>Another question - should we need to set up some upper limits
>(calculated by RAM?) for shmmaxpgs and shmni parameters (marked with
>XXX comments in sysctl's functions)?

I guess so. But since the calls are limited to the superuser it is
not that critical. You should think what the appropriate limits are
and enforce them.

>In attached patch, there is a new kern.ipc node for mentioned SHM
>parameters. In my opinion, structurically better to move all SysV IPC
>parameters to kern.ipc node, including the kern.sysvmsg, kern.sysvsem
>and kern.sysvshm parameters. In patch I experimentally moved
>kern.sysvipc_info to kern.ipc.sysvipc_info (this needs only few changes
>in ipcs and emulation). And shouldn't it better to move an
>initialization of these parameters form init_sysctl.c to sysv_ipc.c?
>What is your thoughts of these structural sysctl's changes?

Yes, I think that they should be moved. I am not worried much about
compatibility issues because we've moved sysctls in the past. If you
are going to do this, please submit the relevant documentation changes.

>Finally, is there something essentially wrong in this patch and these
>changes? If no, I would finish it.

I don't see anything fundamentally wrong.

>Thanks.

>P.S. Sorry if my questions is from poor knowlege of NetBSD internals.

Thank you for your work! Some of your questions are above my level of
knowledge :-)

christos