| Message ID | 20230726140635.2059334-7-j.granados@samsung.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | None | expand |
On Wed, Jul 26, 2023 at 04:06:26PM +0200, Joel Granados wrote: > In order to remove the end element from the ctl_table struct arrays, we > replace the register_syctl function with a macro that will add the > ARRAY_SIZE to the new register_sysctl_sz function. In this way the > callers that are already using an array of ctl_table structs do not have > to change. We *do* change the callers that pass the ctl_table array as a > pointer. Thanks for doing this and this series! > Signed-off-by: Joel Granados <j.granados@samsung.com> > --- > diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h > index 0495c858989f..b1168ae281c9 100644 > --- a/include/linux/sysctl.h > +++ b/include/linux/sysctl.h > @@ -215,6 +215,9 @@ struct ctl_path { > const char *procname; > }; > > +#define register_sysctl(path, table) \ > + register_sysctl_sz(path, table, ARRAY_SIZE(table)) > + > #ifdef CONFIG_SYSCTL Wasn't it Greg who had suggested this? Maybe add Suggested-by with him on it. Also, your cover letter and first few patches are not CC'd to the netdev list or others. What you want to do is collect all the email addresses for this small patch series and add them to who you email for your entire series, otherwise at times they won't be able to properly review or understand the exact context of the changes. You want folks to do less work to review, not more. So please resend and add others to the other patches. Luis
On Wed, Jul 26, 2023 at 10:58:30AM -0700, Luis Chamberlain wrote: > On Wed, Jul 26, 2023 at 04:06:26PM +0200, Joel Granados wrote: > > In order to remove the end element from the ctl_table struct arrays, we > > replace the register_syctl function with a macro that will add the > > ARRAY_SIZE to the new register_sysctl_sz function. In this way the > > callers that are already using an array of ctl_table structs do not have > > to change. We *do* change the callers that pass the ctl_table array as a > > pointer. > > Thanks for doing this and this series! > > > Signed-off-by: Joel Granados <j.granados@samsung.com> > > --- > > diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h > > index 0495c858989f..b1168ae281c9 100644 > > --- a/include/linux/sysctl.h > > +++ b/include/linux/sysctl.h > > @@ -215,6 +215,9 @@ struct ctl_path { > > const char *procname; > > }; > > > > +#define register_sysctl(path, table) \ > > + register_sysctl_sz(path, table, ARRAY_SIZE(table)) > > + > > #ifdef CONFIG_SYSCTL > > Wasn't it Greg who had suggested this? Maybe add Suggested-by with him > on it. Yes. I mentioned him in the cover letter and did not add the tag because I had not asked for permission to use it. I'll drop him a mail and include the suggested-by if he agrees. > > Also, your cover letter and first few patches are not CC'd to the netdev > list or others. What you want to do is collect all the email addresses > for this small patch series and add them to who you email for your > entire series, otherwise at times they won't be able to properly review > or understand the exact context of the changes. You want folks to do less > work to review, not more. Here I wanted to avoid very big e-mail headers as I have received rejections from lists in the past. But I for this set, the number of e-mails is ok to just include everyone. I'll do that for V2. thx for your feedback best > > So please resend and add others to the other patches. > > Luis
On Thu, Jul 27, 2023 at 02:22:00PM +0200, Joel Granados wrote: > On Wed, Jul 26, 2023 at 10:58:30AM -0700, Luis Chamberlain wrote: > > On Wed, Jul 26, 2023 at 04:06:26PM +0200, Joel Granados wrote: > > > In order to remove the end element from the ctl_table struct arrays, we > > > replace the register_syctl function with a macro that will add the > > > ARRAY_SIZE to the new register_sysctl_sz function. In this way the > > > callers that are already using an array of ctl_table structs do not have > > > to change. We *do* change the callers that pass the ctl_table array as a > > > pointer. > > > > Thanks for doing this and this series! > > > > > Signed-off-by: Joel Granados <j.granados@samsung.com> > > > --- > > > diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h > > > index 0495c858989f..b1168ae281c9 100644 > > > --- a/include/linux/sysctl.h > > > +++ b/include/linux/sysctl.h > > > @@ -215,6 +215,9 @@ struct ctl_path { > > > const char *procname; > > > }; > > > > > > +#define register_sysctl(path, table) \ > > > + register_sysctl_sz(path, table, ARRAY_SIZE(table)) > > > + > > > #ifdef CONFIG_SYSCTL > > > > Wasn't it Greg who had suggested this? Maybe add Suggested-by with him > > on it. > Yes. I mentioned him in the cover letter and did not add the tag because > I had not asked for permission to use it. I'll drop him a mail and > include the suggested-by if he agrees. FWIW, I never ask, if they ask for it, clearly they suggested it. > > Also, your cover letter and first few patches are not CC'd to the netdev > > list or others. What you want to do is collect all the email addresses > > for this small patch series and add them to who you email for your > > entire series, otherwise at times they won't be able to properly review > > or understand the exact context of the changes. You want folks to do less > > work to review, not more. > Here I wanted to avoid very big e-mail headers as I have received > rejections from lists in the past. But I for this set, the number of > e-mails is ok to just include everyone. I hear that from time to time, if you have issues with adding folks on the To address it may be an SMTP server issue, ie, corp email SMTP server issues. To fix that I avoid corp email SMTP servers. Luis
On Thu, Jul 27, 2023 at 08:42:02AM -0700, Luis Chamberlain wrote: > On Thu, Jul 27, 2023 at 02:22:00PM +0200, Joel Granados wrote: > > On Wed, Jul 26, 2023 at 10:58:30AM -0700, Luis Chamberlain wrote: > > > On Wed, Jul 26, 2023 at 04:06:26PM +0200, Joel Granados wrote: > > > > In order to remove the end element from the ctl_table struct arrays, we > > > > replace the register_syctl function with a macro that will add the > > > > ARRAY_SIZE to the new register_sysctl_sz function. In this way the > > > > callers that are already using an array of ctl_table structs do not have > > > > to change. We *do* change the callers that pass the ctl_table array as a > > > > pointer. > > > > > > Thanks for doing this and this series! > > > > > > > Signed-off-by: Joel Granados <j.granados@samsung.com> > > > > --- > > > > diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h > > > > index 0495c858989f..b1168ae281c9 100644 > > > > --- a/include/linux/sysctl.h > > > > +++ b/include/linux/sysctl.h > > > > @@ -215,6 +215,9 @@ struct ctl_path { > > > > const char *procname; > > > > }; > > > > > > > > +#define register_sysctl(path, table) \ > > > > + register_sysctl_sz(path, table, ARRAY_SIZE(table)) > > > > + > > > > #ifdef CONFIG_SYSCTL > > > > > > Wasn't it Greg who had suggested this? Maybe add Suggested-by with him > > > on it. > > Yes. I mentioned him in the cover letter and did not add the tag because > > I had not asked for permission to use it. I'll drop him a mail and > > include the suggested-by if he agrees. > > FWIW, I never ask, if they ask for it, clearly they suggested it. I was following Documentation/process/submitting-patches.rst: "... Please note that this tag should not be added without the reporter's permission... ". In any case, Greg has already said yes :) > > > > Also, your cover letter and first few patches are not CC'd to the netdev > > > list or others. What you want to do is collect all the email addresses > > > for this small patch series and add them to who you email for your > > > entire series, otherwise at times they won't be able to properly review > > > or understand the exact context of the changes. You want folks to do less > > > work to review, not more. > > Here I wanted to avoid very big e-mail headers as I have received > > rejections from lists in the past. But I for this set, the number of > > e-mails is ok to just include everyone. > > I hear that from time to time, if you have issues with adding folks on > the To address it may be an SMTP server issue, ie, corp email SMTP > server issues. To fix that I avoid corp email SMTP servers. My experience was more from the lists rejecting the e-mail because the header was too big. With that said, I'll look into SMTP alternatives to reduce possible errors > > Luis
diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c index 1febd412b4d2..e459cfd33711 100644 --- a/arch/arm64/kernel/armv8_deprecated.c +++ b/arch/arm64/kernel/armv8_deprecated.c @@ -569,7 +569,7 @@ static void __init register_insn_emulation(struct insn_emulation *insn) sysctl->extra2 = &insn->max; sysctl->proc_handler = emulation_proc_handler; - register_sysctl("abi", sysctl); + register_sysctl_sz("abi", sysctl, 1); } } diff --git a/arch/s390/appldata/appldata_base.c b/arch/s390/appldata/appldata_base.c index bbefe5e86bdf..3b0994625652 100644 --- a/arch/s390/appldata/appldata_base.c +++ b/arch/s390/appldata/appldata_base.c @@ -365,7 +365,7 @@ int appldata_register_ops(struct appldata_ops *ops) ops->ctl_table[0].proc_handler = appldata_generic_handler; ops->ctl_table[0].data = ops; - ops->sysctl_header = register_sysctl(appldata_proc_name, ops->ctl_table); + ops->sysctl_header = register_sysctl_sz(appldata_proc_name, ops->ctl_table, 1); if (!ops->sysctl_header) goto out; return 0; diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c index 8d04f01a89c1..c04293911e7e 100644 --- a/fs/proc/proc_sysctl.c +++ b/fs/proc/proc_sysctl.c @@ -43,7 +43,7 @@ static struct ctl_table sysctl_mount_point[] = { */ struct ctl_table_header *register_sysctl_mount_point(const char *path) { - return register_sysctl(path, sysctl_mount_point); + return register_sysctl_sz(path, sysctl_mount_point, 0); } EXPORT_SYMBOL(register_sysctl_mount_point); @@ -1398,7 +1398,7 @@ struct ctl_table_header *__register_sysctl_table( } /** - * register_sysctl - register a sysctl table + * register_sysctl_sz - register a sysctl table * @path: The path to the directory the sysctl table is in. If the path * doesn't exist we will create it for you. * @table: the table structure. The calller must ensure the life of the @table @@ -1408,25 +1408,20 @@ struct ctl_table_header *__register_sysctl_table( * to call unregister_sysctl_table() and can instead use something like * register_sysctl_init() which does not care for the result of the syctl * registration. + * @table_size: The number of elements in table. * * Register a sysctl table. @table should be a filled in ctl_table * array. A completely 0 filled entry terminates the table. * * See __register_sysctl_table for more details. */ -struct ctl_table_header *register_sysctl(const char *path, struct ctl_table *table) +struct ctl_table_header *register_sysctl_sz(const char *path, struct ctl_table *table, + size_t table_size) { - int count = 0; - struct ctl_table *entry; - struct ctl_table_header t_hdr; - - t_hdr.ctl_table = table; - list_for_each_table_entry(entry, (&t_hdr)) - count++; return __register_sysctl_table(&sysctl_table_root.default_set, - path, table, count); + path, table, table_size); } -EXPORT_SYMBOL(register_sysctl); +EXPORT_SYMBOL(register_sysctl_sz); /** * __register_sysctl_init() - register sysctl table to path @@ -1451,10 +1446,17 @@ EXPORT_SYMBOL(register_sysctl); void __init __register_sysctl_init(const char *path, struct ctl_table *table, const char *table_name) { - struct ctl_table_header *hdr = register_sysctl(path, table); + int count = 0; + struct ctl_table *entry; + struct ctl_table_header t_hdr, *hdr; + + t_hdr.ctl_table = table; + list_for_each_table_entry(entry, (&t_hdr)) + count++; + hdr = register_sysctl_sz(path, table, count); if (unlikely(!hdr)) { - pr_err("failed when register_sysctl %s to %s\n", table_name, path); + pr_err("failed when register_sysctl_sz %s to %s\n", table_name, path); return; } kmemleak_not_leak(hdr); diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h index 0495c858989f..b1168ae281c9 100644 --- a/include/linux/sysctl.h +++ b/include/linux/sysctl.h @@ -215,6 +215,9 @@ struct ctl_path { const char *procname; }; +#define register_sysctl(path, table) \ + register_sysctl_sz(path, table, ARRAY_SIZE(table)) + #ifdef CONFIG_SYSCTL void proc_sys_poll_notify(struct ctl_table_poll *poll); @@ -227,7 +230,8 @@ extern void retire_sysctl_set(struct ctl_table_set *set); struct ctl_table_header *__register_sysctl_table( struct ctl_table_set *set, const char *path, struct ctl_table *table, size_t table_size); -struct ctl_table_header *register_sysctl(const char *path, struct ctl_table *table); +struct ctl_table_header *register_sysctl_sz(const char *path, struct ctl_table *table, + size_t table_size); void unregister_sysctl_table(struct ctl_table_header * table); extern int sysctl_init_bases(void); @@ -262,7 +266,9 @@ static inline struct ctl_table_header *register_sysctl_mount_point(const char *p return NULL; } -static inline struct ctl_table_header *register_sysctl(const char *path, struct ctl_table *table) +static inline struct ctl_table_header *register_sysctl_sz(const char *path, + struct ctl_table *table, + size_t table_size) { return NULL; } diff --git a/kernel/ucount.c b/kernel/ucount.c index 2b80264bb79f..4aa6166cb856 100644 --- a/kernel/ucount.c +++ b/kernel/ucount.c @@ -365,7 +365,7 @@ static __init int user_namespace_sysctl_init(void) * default set so that registrations in the child sets work * properly. */ - user_header = register_sysctl("user", empty); + user_header = register_sysctl_sz("user", empty, 0); kmemleak_ignore(user_header); BUG_ON(!user_header); BUG_ON(!setup_userns_sysctls(&init_user_ns)); diff --git a/net/sysctl_net.c b/net/sysctl_net.c index 8ee4b74bc009..d9cbbb51b143 100644 --- a/net/sysctl_net.c +++ b/net/sysctl_net.c @@ -101,7 +101,7 @@ __init int net_sysctl_init(void) * registering "/proc/sys/net" as an empty directory not in a * network namespace. */ - net_header = register_sysctl("net", empty); + net_header = register_sysctl_sz("net", empty, 0); if (!net_header) goto out; ret = register_pernet_subsys(&sysctl_pernet_ops);
In order to remove the end element from the ctl_table struct arrays, we replace the register_syctl function with a macro that will add the ARRAY_SIZE to the new register_sysctl_sz function. In this way the callers that are already using an array of ctl_table structs do not have to change. We *do* change the callers that pass the ctl_table array as a pointer. Signed-off-by: Joel Granados <j.granados@samsung.com> --- arch/arm64/kernel/armv8_deprecated.c | 2 +- arch/s390/appldata/appldata_base.c | 2 +- fs/proc/proc_sysctl.c | 30 +++++++++++++++------------- include/linux/sysctl.h | 10 ++++++++-- kernel/ucount.c | 2 +- net/sysctl_net.c | 2 +- 6 files changed, 28 insertions(+), 20 deletions(-)