Messages in this thread | | | Date | Tue, 16 Apr 2024 21:32:24 +0200 | From | Thomas Weißschuh <> | Subject | Re: [PATCH v2 00/11] sysctl: treewide: constify ctl_table argument of sysctl handlers |
| |
(+Cc LKML to at least get the conversation into the archives)
Hi Joel,
On 2024-04-16 18:18:19+0200, Joel Granados wrote: > On Mon, Apr 08, 2024 at 04:21:58PM +0200, Thomas Weißschuh wrote: > > On 2024-04-08 10:59:28+0200, Joel Granados wrote: > > > On Tue, Apr 02, 2024 at 07:15:37PM +0200, Thomas Weißschuh wrote: > > > > On 2024-03-28 22:00:16+0100, Joel Granados wrote: > > > > > On Sat, Mar 23, 2024 at 04:44:08PM +0100, Thomas Weißschuh wrote: > > > > > > * Patch 1 is a bugfix for the stack_erasing sysctl handler > > > > > > * Patches 2-10 change various helper functions throughout the kernel to > > > > > > be able to handle 'const ctl_table'. > > > > > > * Patch 11 changes the signatures of all proc handlers through the tree. > > > > > > Some other signatures are also adapted, for details see the commit > > > > > > message. > > > > > > > > > > > > Only patch 1 changes any code at all. > > > > > > > > > > > > The series was compile-tested on top of next-20230322 for > > > > > > i386, x86_64, arm, arm64, riscv, loongarch and s390. > > > > > > > > > > > > This series was split from my larger series sysctl-const series [0]. > > > > > > It only focusses on the proc_handlers but is an important step to be > > > > > > able to move all static definitions of ctl_table into .rodata. > > > > > > > > > > > > [0] https://lore.kernel.org/lkml/20231204-const-sysctl-v2-0-7a5060b11447@weissschuh.net/ > > > > > > > > > > > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > > > > > > --- > > > > > > Changes in v2: > > > > > > - Reduce recipient list > > > > > How did you reduce it? and why did you reduce it? This is quite an > > > > > extensive change; if anything we should have lots of eyes on it. > > > > > > > > I used get_maintainer.pl for v1 and received negative feedback about > > > > sending the series to too many people. > > > I completely missed that. Was that a private mail to you? Do you have a > > > link of the message? I checked my history and I do not have it. > > > > Yes that message was private. From a fairly prominent maintainer. > > > > > > Advice I got was to only send it to the people whose tree it will be > > > > going through. > > > > > > > > The only change affecting actual emitted object code in the series is > > > > "stackleak: don't modify ctl_table argument" and that got acked by Kees > > > > already quite some time ago. > > > > If Kees wants to pick this up for one of his own PRs during this cycle > > > > that would be nice, too. > > > > > > > > I'm open for suggestions to increase the circle of recipients, but > > > > blindly using get_maintainer.pl again doesn't seem the right way. > > > > > IMO, this can go either way: You can get feedback that tells you that it > > > is too many people that you are "bothering" and you can also get > > > maintainers pinging you to be included in what they consider to be part > > > of their job. You can't make everyone happy :) > > > > Absolutely, this is my expectation, too. > > > > But maintainers can nicely opt-in to all patches touching their > > subsystems using lore and lei, while it's harder to opt-out of broad > > recipient lists. > > (Except putting me in their killfile...)
> Not sure how possible it is to be individually black listed in a > maintainers inbox. But it is more of a reason to make it public; because > at least you have done your due diligence and made it available so every > one can see what is going on (even if you are black listed > individually).
The reference to the killfile was more meant as a joke.
> > > > The only real feedback I got was from Dave and that I addressed. > > > yes. I saw this. > > > > > > > If somebody would have had fundamental issues with the aproach they > > > > could have spoken up on v1. > > > This seems reasonable. I'll try to get to it this week and add it to > > > constfy branch if I do not see anything glaring. > > I Just realized that you did not include any kernel lists. I thought > that you had just removed individuals and left the lists. This reduces > the number of eyes on the code which is particularly important in this > specific case where there are so many changes touching so many > subsystems.
This is absolutely an oversight, I intended to at least keep LKML. I have to assume it was me being inattentive.
> Not only that, but it also breaks tools like lei and b4. I have configured b4 to > look at https://lore.kernel.org/all to handle patches coming from contributors. > If the change is not public it breaks my command (`b4 am -o - MESSAGE_ID | git > am -3`).
Understood, as mentioned above the trimming went to far.
FYI: b4 can do the `git am` itself with `b4 shazam MESSAGE_ID`. Use the config `b4.shazam-am-flags` for the `-3` flag.
> I do not know who spooked you but I suggest you just remove this person > from the to:/cc: of your patches and leave the rest as it is. Like I > did with Mathew Wilcox after he asked me to do so here > https://lore.kernel.org/all/ZZbJRiN8ENV%2FFoTV@casper.infradead.org/. > Please resend the patchset including the relevant kernel mailing lists and > maintainers but excepting the person that sent you the private e-mail.
In addition to the complaint I also got guidance from Thomas Gleixner to reduce the scope of recipients.
What do you think about the following:
You do a review of v2 and give feedback on that and I'll incorporate that feedback and afterwards send a v3. In addition to the recipients of v2 I'll add LKML, Greg and Andrew Morton.
> This also goes for your "[PATCH] sysctl: treewide: constify ctl_table_header::ctl_table_arg" > which is also not public.
Thanks for this pointer, too. I'd like to handle it the same way as proposed above.
Sorry for all the back-and-forth, Thomas
| |