Messages in this thread | | | From | Soheil Hassas Yeganeh <> | Date | Tue, 5 Jul 2022 18:26:42 -0400 | Subject | Re: [PATCH] net: Shrink sock.sk_err sk_err_soft to u16 from int |
| |
On Tue, Jul 5, 2022 at 9:01 AM Leonard Crestez <cdleonard@gmail.com> wrote: > > On 7/5/22 13:31, Paolo Abeni wrote: > > On Sun, 2022-07-03 at 23:06 +0300, Leonard Crestez wrote: > >> These fields hold positive errno values which are limited by > >> ERRNO_MAX=4095 so 16 bits is more than enough. > >> > >> They are also always positive; setting them to a negative errno value > >> can result in falsely reporting a successful read/write of incorrect > >> size. > >> > >> Signed-off-by: Leonard Crestez <cdleonard@gmail.com> > >> --- > >> include/net/sock.h | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> I ran some relatively complex tests without noticing issues but some corner > >> case where this breaks might exist. > > > > Could you please explain in length the rationale behind this change? > > > > Note that this additionally changes the struct sock binary layout, > > which in turn in quite relevant for high speed data transfer. > > The rationale is that shrinking structs is almost always better. I know > that due to various roundings it likely won't actually impact memory > consumption unless accumulated with other size reductions. > > These sk_err fields don't seem to be in a particularly "hot" area so I > don't think it will impact performance. > > My expectation is that after a socket error is reported the socket will > likely be closed so that there will be very few writes to this field.
Since you're packing sk_err and sk_err_soft into a DWORD, I'd suggest adding another patch on top to move both fields right before sk_filter where we have a 4-byte hole. As far as I can tell, this should save one QWORD from "struct sock".
Eric, I believe these fields are read-mostly and that wouldn't infer with your previous layout optimizations. Is my understanding correct?
Thanks, Soheil
> -- > Regards, > Leonard
| |