[PATCH 07/08]
This patch makes msgmni not recomputed anymore upon ipc namespace creation /
removal or memory add/remove, as soon as it has been set from userland.
As soon as msgmni is explicitely set via procfs or sysctl(), the associated
callback routine is unregistered from the ipc namespace notifier chain.
Signed-off-by: Nadia Derbey <[email protected]>
---
ipc/ipc_sysctl.c | 43 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 41 insertions(+), 2 deletions(-)
Index: linux-2.6.24-mm1/ipc/ipc_sysctl.c
===================================================================
--- linux-2.6.24-mm1.orig/ipc/ipc_sysctl.c 2008-02-08 16:07:15.000000000 +0100
+++ linux-2.6.24-mm1/ipc/ipc_sysctl.c 2008-02-08 16:08:32.000000000 +0100
@@ -35,6 +35,24 @@ static int proc_ipc_dointvec(ctl_table *
return proc_dointvec(&ipc_table, write, filp, buffer, lenp, ppos);
}
+static int proc_ipc_callback_dointvec(ctl_table *table, int write,
+ struct file *filp, void __user *buffer, size_t *lenp, loff_t *ppos)
+{
+ size_t lenp_bef = *lenp;
+ int rc;
+
+ rc = proc_ipc_dointvec(table, write, filp, buffer, lenp, ppos);
+
+ if (write && !rc && lenp_bef == *lenp)
+ /*
+ * Tunable has successfully been changed from userland:
+ * disable its automatic recomputing.
+ */
+ unregister_ipcns_notifier(current->nsproxy->ipc_ns);
+
+ return rc;
+}
+
static int proc_ipc_doulongvec_minmax(ctl_table *table, int write,
struct file *filp, void __user *buffer, size_t *lenp, loff_t *ppos)
{
@@ -49,6 +67,7 @@ static int proc_ipc_doulongvec_minmax(ct
#else
#define proc_ipc_doulongvec_minmax NULL
#define proc_ipc_dointvec NULL
+#define proc_ipc_callback_dointvec NULL
#endif
#ifdef CONFIG_SYSCTL_SYSCALL
@@ -90,8 +109,28 @@ static int sysctl_ipc_data(ctl_table *ta
}
return 1;
}
+
+static int sysctl_ipc_registered_data(ctl_table *table, int __user *name,
+ int nlen, void __user *oldval, size_t __user *oldlenp,
+ void __user *newval, size_t newlen)
+{
+ int rc;
+
+ rc = sysctl_ipc_data(table, name, nlen, oldval, oldlenp, newval,
+ newlen);
+
+ if (newval && newlen && rc > 0)
+ /*
+ * Tunable has successfully been changed from userland:
+ * disable its automatic recomputing.
+ */
+ unregister_ipcns_notifier(current->nsproxy->ipc_ns);
+
+ return rc;
+}
#else
#define sysctl_ipc_data NULL
+#define sysctl_ipc_registered_data NULL
#endif
static struct ctl_table ipc_kern_table[] = {
@@ -137,8 +176,8 @@ static struct ctl_table ipc_kern_table[]
.data = &init_ipc_ns.msg_ctlmni,
.maxlen = sizeof (init_ipc_ns.msg_ctlmni),
.mode = 0644,
- .proc_handler = proc_ipc_dointvec,
- .strategy = sysctl_ipc_data,
+ .proc_handler = proc_ipc_callback_dointvec,
+ .strategy = sysctl_ipc_registered_data,
},
{
.ctl_name = KERN_MSGMNB,
--
On Mon, 11 Feb 2008 15:16:53 +0100
[email protected] wrote:
> [PATCH 07/08]
>
> This patch makes msgmni not recomputed anymore upon ipc namespace creation /
> removal or memory add/remove, as soon as it has been set from userland.
>
> As soon as msgmni is explicitely set via procfs or sysctl(), the associated
> callback routine is unregistered from the ipc namespace notifier chain.
>
The patch series looks pretty good.
> ===================================================================
> --- linux-2.6.24-mm1.orig/ipc/ipc_sysctl.c 2008-02-08 16:07:15.000000000 +0100
> +++ linux-2.6.24-mm1/ipc/ipc_sysctl.c 2008-02-08 16:08:32.000000000 +0100
> @@ -35,6 +35,24 @@ static int proc_ipc_dointvec(ctl_table *
> return proc_dointvec(&ipc_table, write, filp, buffer, lenp, ppos);
> }
>
> +static int proc_ipc_callback_dointvec(ctl_table *table, int write,
> + struct file *filp, void __user *buffer, size_t *lenp, loff_t *ppos)
> +{
> + size_t lenp_bef = *lenp;
> + int rc;
> +
> + rc = proc_ipc_dointvec(table, write, filp, buffer, lenp, ppos);
> +
> + if (write && !rc && lenp_bef == *lenp)
> + /*
> + * Tunable has successfully been changed from userland:
> + * disable its automatic recomputing.
> + */
> + unregister_ipcns_notifier(current->nsproxy->ipc_ns);
> +
> + return rc;
> +}
If you haven't done so, could you please check that it all builds cleanly
with CONFIG_PROCFS=n, and that all code which isn't needed if procfs is
disabled is not present in the final binary?
Andrew Morton wrote:
> On Mon, 11 Feb 2008 15:16:53 +0100
> [email protected] wrote:
>
>
>>[PATCH 07/08]
>>
>>This patch makes msgmni not recomputed anymore upon ipc namespace creation /
>>removal or memory add/remove, as soon as it has been set from userland.
>>
>>As soon as msgmni is explicitely set via procfs or sysctl(), the associated
>>callback routine is unregistered from the ipc namespace notifier chain.
>>
>
>
> The patch series looks pretty good.
>
>
>>===================================================================
>>--- linux-2.6.24-mm1.orig/ipc/ipc_sysctl.c 2008-02-08 16:07:15.000000000 +0100
>>+++ linux-2.6.24-mm1/ipc/ipc_sysctl.c 2008-02-08 16:08:32.000000000 +0100
>>@@ -35,6 +35,24 @@ static int proc_ipc_dointvec(ctl_table *
>> return proc_dointvec(&ipc_table, write, filp, buffer, lenp, ppos);
>> }
>>
>>+static int proc_ipc_callback_dointvec(ctl_table *table, int write,
>>+ struct file *filp, void __user *buffer, size_t *lenp, loff_t *ppos)
>>+{
>>+ size_t lenp_bef = *lenp;
>>+ int rc;
>>+
>>+ rc = proc_ipc_dointvec(table, write, filp, buffer, lenp, ppos);
>>+
>>+ if (write && !rc && lenp_bef == *lenp)
>>+ /*
>>+ * Tunable has successfully been changed from userland:
>>+ * disable its automatic recomputing.
>>+ */
>>+ unregister_ipcns_notifier(current->nsproxy->ipc_ns);
>>+
>>+ return rc;
>>+}
>
>
> If you haven't done so, could you please check that it all builds cleanly
> with CONFIG_PROCFS=n, and that all code which isn't needed if procfs is
> disabled is not present in the final binary?
>
>
>
>
Andrew,
it builds fine, modulo some changes in ipv4 and ipv6 (see attached patch
- didn't find it in the hot fixes).
Regards,
Nadia
On Tue, 12 Feb 2008 10:32:31 +0100 Nadia Derbey <[email protected]> wrote:
> it builds fine, modulo some changes in ipv4 and ipv6 (see attached patch
> - didn't find it in the hot fixes).
OK, thanks for checking. Did you confirm that we don't have unneeded code
in vmlinux when CONFIG_PROCFS=n? I guess before-and-after comparison of
the size(1) output would tell us.
Those networking build problems appear to have already been fixed.
In future, please quote the compiler error output in the changelog when
sending build fixes or warning fixes, thanks.
Nadia Derbey wrote:
> Andrew Morton wrote:
>
>> On Mon, 11 Feb 2008 15:16:53 +0100
>> [email protected] wrote:
>>
>>
>>> [PATCH 07/08]
>>>
>>> This patch makes msgmni not recomputed anymore upon ipc namespace
>>> creation /
>>> removal or memory add/remove, as soon as it has been set from userland.
>>>
>>> As soon as msgmni is explicitely set via procfs or sysctl(), the
>>> associated
>>> callback routine is unregistered from the ipc namespace notifier chain.
>>>
>>
>>
>> The patch series looks pretty good.
>>
>>
>>> ===================================================================
>>> --- linux-2.6.24-mm1.orig/ipc/ipc_sysctl.c 2008-02-08
>>> 16:07:15.000000000 +0100
>>> +++ linux-2.6.24-mm1/ipc/ipc_sysctl.c 2008-02-08
>>> 16:08:32.000000000 +0100
>>> @@ -35,6 +35,24 @@ static int proc_ipc_dointvec(ctl_table *
>>> return proc_dointvec(&ipc_table, write, filp, buffer, lenp, ppos);
>>> }
>>>
>>> +static int proc_ipc_callback_dointvec(ctl_table *table, int write,
>>> + struct file *filp, void __user *buffer, size_t *lenp, loff_t *ppos)
>>> +{
>>> + size_t lenp_bef = *lenp;
>>> + int rc;
>>> +
>>> + rc = proc_ipc_dointvec(table, write, filp, buffer, lenp, ppos);
>>> +
>>> + if (write && !rc && lenp_bef == *lenp)
>>> + /*
>>> + * Tunable has successfully been changed from userland:
>>> + * disable its automatic recomputing.
>>> + */
>>> + unregister_ipcns_notifier(current->nsproxy->ipc_ns);
>>> +
>>> + return rc;
>>> +}
>>
>>
>>
>> If you haven't done so, could you please check that it all builds cleanly
>> with CONFIG_PROCFS=n, and that all code which isn't needed if procfs is
>> disabled is not present in the final binary?
>>
>>
>>
>>
>
> Andrew,
>
> it builds fine, modulo some changes in ipv4 and ipv6 (see attached patch
> - didn't find it in the hot fixes).
>
> Regards,
> Nadia
>
>
Oops, forgot the patch. Thx Benjamin!
Andrew Morton wrote:
> On Tue, 12 Feb 2008 10:32:31 +0100 Nadia Derbey <[email protected]> wrote:
>
>
>>it builds fine, modulo some changes in ipv4 and ipv6 (see attached patch
>>- didn't find it in the hot fixes).
>
>
> OK, thanks for checking. Did you confirm that we don't have unneeded code
> in vmlinux when CONFIG_PROCFS=n? I guess before-and-after comparison of
> the size(1) output would tell us.
>
> Those networking build problems appear to have already been fixed.
>
> In future, please quote the compiler error output in the changelog when
> sending build fixes or warning fixes, thanks.
>
>
>
BEFORE:
lkernel@akt$ size vmlinux
text data bss dec hex filename
4318525 454484 462848 5235857 4fe491 vmlinux
AFTER:
lkernel@akt$ size vmlinux
text data bss dec hex filename
4323161 454484 462848 5240493 4ff6ad vmlinux
which makes it +4636 = +0.11%
I've got the details for */built-in.o if needed.
Regards,
Nadia
On Tue, 12 Feb 2008 16:15:00 +0100
Nadia Derbey <[email protected]> wrote:
> Andrew Morton wrote:
> > On Tue, 12 Feb 2008 10:32:31 +0100 Nadia Derbey <[email protected]> wrote:
> >
> >
> >>it builds fine, modulo some changes in ipv4 and ipv6 (see attached patch
> >>- didn't find it in the hot fixes).
> >
> >
> > OK, thanks for checking. Did you confirm that we don't have unneeded code
> > in vmlinux when CONFIG_PROCFS=n? I guess before-and-after comparison of
> > the size(1) output would tell us.
> >
> > Those networking build problems appear to have already been fixed.
> >
> > In future, please quote the compiler error output in the changelog when
> > sending build fixes or warning fixes, thanks.
> >
> >
> >
>
> BEFORE:
>
> lkernel@akt$ size vmlinux
> text data bss dec hex filename
> 4318525 454484 462848 5235857 4fe491 vmlinux
>
>
> AFTER:
>
> lkernel@akt$ size vmlinux
> text data bss dec hex filename
> 4323161 454484 462848 5240493 4ff6ad vmlinux
>
> which makes it +4636 = +0.11%
>
> I've got the details for */built-in.o if needed.
>
That seems to be a lot of increase. Are you sure you had CONFIG_PROCFS=n
in both cases? If so, the patch must have added a lot of code which will
never be executed?
Andrew Morton wrote:
> On Tue, 12 Feb 2008 16:15:00 +0100
> Nadia Derbey <[email protected]> wrote:
>
>
>>Andrew Morton wrote:
>>
>>>On Tue, 12 Feb 2008 10:32:31 +0100 Nadia Derbey <[email protected]> wrote:
>>>
>>>
>>>
>>>>it builds fine, modulo some changes in ipv4 and ipv6 (see attached patch
>>>>- didn't find it in the hot fixes).
>>>
>>>
>>>OK, thanks for checking. Did you confirm that we don't have unneeded code
>>>in vmlinux when CONFIG_PROCFS=n? I guess before-and-after comparison of
>>>the size(1) output would tell us.
>>>
>>>Those networking build problems appear to have already been fixed.
>>>
>>>In future, please quote the compiler error output in the changelog when
>>>sending build fixes or warning fixes, thanks.
>>>
>>>
>>>
>>
>>BEFORE:
>>
>>lkernel@akt$ size vmlinux
>> text data bss dec hex filename
>>4318525 454484 462848 5235857 4fe491 vmlinux
>>
>>
>>AFTER:
>>
>>lkernel@akt$ size vmlinux
>> text data bss dec hex filename
>>4323161 454484 462848 5240493 4ff6ad vmlinux
>>
>>which makes it +4636 = +0.11%
>>
>>I've got the details for */built-in.o if needed.
>>
>
>
> That seems to be a lot of increase. Are you sure you had CONFIG_PROCFS=n
> in both cases? If so, the patch must have added a lot of code which will
> never be executed?
>
>
>
Well, the patches that are impacted by procfs being configured or not
are #7 and #8. While the sizes I've sent you are before all patches
applied vs after all patches applied :-(
So here are the "interesting sizes" - all with CONFIG_PROC_FS unset:
before patch 7:
text data bss dec hex filename
4318757 454484 462848 5236089 4fe579 vmlinux
before patch 8:
text data bss dec hex filename
4318853 454484 462848 5236185 4fe5d9 vmlinux +96
after patch 8:
4319055 454484 462848 5236387 4fe6a3 vmlinux +202
The higher difference after patch 8 is because I'm adding the new
interface blocking_notifier_chain_cond_register() even if CONFIG_PROC_FS
is not defined. This is to cover the case where msgmni is set through
the sysctl() syscall (CONFIG_SYSCTL_SYSCALL).
Regards,
Nadia