Re: [PATCH 58/59] sysctl: Reimplement the sysctl proc support

[Eric W. Biederman]

Ingo Molnar <mingo@xxxxxxx> writes:

> * Alexey Dobriyan <adobriyan@xxxxxxxxx> wrote:
>
>> On 3/14/07, Ingo Molnar <mingo@xxxxxxx> wrote:
>> > #define PROCNAME_PML    "sys/kernel/preempt_max_latency"
>> >
>> > static __init int latency_fs_init(void)
>> > {
>> >         struct proc_dir_entry *entry;
>> >
>> >         if (!(entry = create_proc_entry(PROCNAME_PML, 0644, NULL)))
>> >                 printk("latency_fs_init(): can't create %s\n",
>> >PROCNAME_PML);
>> >
>> > with your change that broke because beyond /proc/sys/ there are no 
>> > real proc entries anymore, there's no de->subdir directory for 
>> > xlate_proc_name() to find. While the latency tracer isnt upstream, 
>> > this change in semantics does not seem to be intended (the changelog 
>> > is certainly silent about it).
>> 
>> Use register_sysctl_table() for sysctls.
>
> yes - i just wanted to point out the incompatibility and subtle breakage 
> that this change caused. I'll now have to convert the current code over 
> to sysctl_table, which isnt that hard but not trivial either, and i 
> certainly could make use that time for other purposes.

Sorry, for the inconvenience.

However it has always been a bug for anything under /proc/sys to not
be a sysctl.   It's not subtle breakage but subtle enforcement of the
existing rules.

The fact that code that now tries to break the rules fails I see as an
unanticipated bonus, because it means someone won't have to  go back
through and fix it after the code has been merged.

As far as I can tell it was really only out of tree code that suffered
from the misconception that /proc/sys was for something other than
sysctls.  The only case I recall fixing in the kernel was the mount
point for binfmt_misc and that was just directories, and easy to
convert.

This is the second time someone with out of tree code has asked me
about it though.   It may be worth adding a test to create_proc_entry
that says "you silly person you need to use sysctls to create an entry
under /proc/sys"

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