On 2023/3/2 09:12, Luis Chamberlain wrot
I've taken the time to rebase this but I'm not a big fan of how fragile
it is, you can easily forget to do the proper accounting or bailing out.
Upon looking at all this it reminded me tons of times Eric has
said a few calls are just compatibility wrappers, and otherwise they are
deprecated. Ie, they exist just to old users but we should have new
users move on to the new helpers. When / if we can move the older ones
When a user registers sysctl, the entry is register_sysctl. In order to
be compatible with the previous method, I added the following statement:
+#define register_sysctl(path, table) register_sysctl_with_num(path,
table, ARRAY_SIZE(table))
On this basis, we can provide both register_sysctl and
register_sysctl_with_num.
away that'd be great. Knowing that simplifies the use-cases we have to
address for this case too.
We need to modify the helper description information, but this does not
affect the compatible use of the current old method and the new method now.
So I phased out completely register_sysctl_paths() and then started to
work on register_sysctl_table(). I didn't complete phasing out
register_sysctl_table() but with a bit of patience and looking at the
few last examples I did I think we can quickly phase it out with coccinelle.
Here's where I'm at:
https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git/log/?h=sysctl-testing
On top of that I've rebased your patches but I'm not confident in them
so I just put this out here in case others want to work on it:
https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git/log/?h=sysctl-testing-opt
What I think we should do first instead is do a non-functional change
which transforms all loops to list_for_each_table_entry() and then
we can consider using the bail out *within* the list_for_each_table_entry()
macro itself.
That would solve the first part -- the fragile odd checks to bail out
early. But not the odd accounting we have to do at times. So it begs
the question if we can instead deprecate register_sysctl_table() and
then have a counter for us at all times. Also maybe an even simpler
alternative may just be to see to have the nr_entries be inferred with
ARRAY_SIZE() if count_subheaders() == 1? I haven't looked into that yet.
Do you want to know here is whether it is possible to accurately
calculate nr_entries if entry->child is established?
This is a problem. In the current patch, count_subheaders() still needs
to get the ARRAY_SIZE() of the table. If there is a child, I still use
list_for_each_table_entry(entry, child_table) to deal with it.
That is, when calling count_subheaders():
1)Specify the table ARRAY_SIZE(), then count_subheaders(table,
ARRAY_SIZE(table));
2)Unknown table ARRAY_SIZE(), then count_subheaders(table, 0).
Use list_for_each_table_entry(entry, child_table), end traversal until
the entry->procname is NULL.
This results in the child_table still needing to end with a “[]”. But I
haven't thought of a better way to handle this in this case.
Luis
