Re: [PATCH 00/69] faster tree-based sysctl implementation

[Lucian Adrian Grijincu]

On Mon, May 2, 2011 at 12:49 AM, Eric W. Biederman
<ebiederm@xxxxxxxxxxxx> wrote:
>> Patch 60 makes sure that if a directory is not found a
>> ctl_table_header will be created for it. That directory can be freed
>> by anyone else when the reference count becomes 0.
>> E.g. register /newdir/file1, register /newdir/file2, unregister
>> /newdir/file1, unregister /newdir/file1
>> - 1st registration created 'newdir' and takes a reference to it.
>> - 2nd regisitration incs the reference count.
>> - 1st unregister decs the reference count.
>> - 2nd unregister decs the reference count which becomes 0 and frees 'newdir'.
>>
>> I may have misunderstood your comment.
>
> While you can make directory lifetimes work by ref counting. ÂMaking it
> work by ref counting removes a concept of ownership and makes it hard to
> see when a sysctl directory should exist.


I can do what you want but I see no good reason to this restriction.
Entities that register sysctls are interested in their files showing
up there.
In my previous example if two modules want to add /newdir/file1 and
/newdir/file2 they must either:
- find a third party that will register /newdir for them
- make sure they have a strict dependency relation (only registere
/newdir/file2 if the first module is loaded and it registered
/newdir/file1 or at least /newdir)

This is why we register an empty directory for "/proc/sys/dev/" (a
third party that registers the 'dev' dir and all others use it).


Again, I can do what you ask, but it either means rethinking some of
my patches, or adding the restriction on top of them.

> If we are reforming this we should go all of the way and make strict
> lifetime rules of directories. ÂSo that it is required to do:
>
> register newdir
> register newdir/file1
> unregister newdir/file1
> unregister newdir
>
> Today there is a WARN_ON in unregister_sysctl_table enforcing the
> ordering that directories must come first and be removed after
> the files in those directories. ÂIf we change the table registrations
> to only include leaves (as your removal of .child patches do) I
> believe that rule becomes absolute.


> That WARN_ON came in with Al Viro's last major sweep through the code,
> and it started moving sysctl in the direction of a normal filesystem.


Regarding that: I'm now trying to reproduce a NULL access in
sysctl_is_seen that I got only once. It's coupled with a
"rcu_sched_state stall detected" warning from RCU.


> For long term maintainability and comprehensibility I think moving
> sysctl in the direction of a normal filesystem makes a lot of sense.


Ok, I agree that a later redesign of sysctl would suffer if there
would be a mess regarding registration/unregistration of directories
and no ownership information/enforcement. I'll see how this can be
addressed in the next respin of this series.


-- 
Â.
..: Lucian
--
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/