Re: [PATCH v2 1/9] sysctl: support encoding values directly in the table entry

[Joel Granados]

On Fri, Mar 22, 2024 at 11:43:16PM +0800, Wen Yang wrote:
> 
> 
<--- snip --->
> > information that the NULL pointer gave you. This is effectively adding 2
> > bits per ctl_table element for all ctl_table types; event the ones that
> > do not need min max. So how much will we actually save with all this?
> > once you have added these 2 bits and removed the static variables from
> > the files that are not using the pointers? Is saving read only memory
> > the only reason for this? If that is the case, please add some
> > calculations of how much we save to see if it actually make sense. To
> > calculate the memory gains/losses you can use the bloat-o-meter script
> > under the scripts directory (something similar to what we did here
> > https://lore.kernel.org/all/20240314-jag-sysctl_remset_net-v1-0-aa26b44d29d9@xxxxxxxxxxx)
> > 
> > I'll hold off on reviewing the other patches in this set until this is a
> > bit more clear.
> > 
> 
> Thank you for your comments.
> 
> When we started this work, we had not yet seen Thomas's patch, so by 
> borrowing the existing enum type‘s field, we can achieve directly 
> encoding values without increasing the size of the ctl_table.
> 
> We really appreciate that you pointed out this issue. It will take some 
> time for rework and the v3 will be sent out within a few weeks.
> 
> In addition, the patch below is not related to "kill sysctl_vals". It is 
> just a regular optimization and was sent over 10 days ago. We also hope 
> to receive your kind advice:
> 
> https://lkml.org/lkml/2024/3/8/871
> 
> [RESEND PATCH v2] sysctl: move the extra1/2 boundary check of u8 to 
> sysctl_check_table_array

Thx for pointing this out, I had missed it. On my todo list :)

> 
> --
> Best wishes,
> Wen
> 
> 
> >> +	union {
> >> +		struct {
> >> +			void *extra1;
> >> +			void *extra2;
> >> +		};
> >> +		struct {
> >> +			long min;
> >> +			long max;
> >> +		};
> >> +	};
> >> +
> >>   	proc_handler *proc_handler;	/* Callback for text formatting */
> >>   	struct ctl_table_poll *poll;
> >> -	void *extra1;
> >> -	void *extra2;
> >>   } __randomize_layout;
> > ...
> > 
> > Best
> 

-- 

Joel Granados

Attachment: signature.asc
Description: PGP signature