Messages in this thread | | | From | Jamal Hadi Salim <> | Date | Mon, 21 Aug 2023 10:35:29 -0400 | Subject | Re: [PATCH][next] net: sched: cls_u32: Fix allocation in u32_init() |
| |
On Fri, Aug 18, 2023 at 10:38 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Thu, 17 Aug 2023 09:58:53 -0600 Gustavo A. R. Silva wrote: > > Subject: [PATCH][next] net: sched: cls_u32: Fix allocation in u32_init() > > Date: Thu, 17 Aug 2023 09:58:53 -0600 > > > > Replace struct_size() with sizeof(), and avoid allocating 8 too many > > bytes. > > What are you fixing? > > > The following difference in binary output is expected and reflects the > > desired change: > > > > | net/sched/cls_u32.o > > | @@ -6148,7 +6148,7 @@ > > | include/linux/slab.h:599 > > | 2cf5: mov 0x0(%rip),%rdi # 2cfc <u32_init+0xfc> > > | 2cf8: R_X86_64_PC32 kmalloc_caches+0xc > > |- 2cfc: mov $0x98,%edx > > |+ 2cfc: mov $0x90,%edx > > Sure, but why are you doing this? And how do you know the change is > correct? > > There are 2 other instances where we allocate 1 entry or +1 entry. > Are they not all wrong? > > Also some walking code seems to walk <= divisor, divisor IIUC being > the array bound - 1? > > Jamal acked so changes are this is right, but I'd really like to > understand what's going on, and I shouldn't have to ask you all > these questions :S
This is a "bug fix" given that the structure had no zero array construct as was implied by d61491a51f7e . I didnt want to call it out as a bug fix (for -net) because existing code was not harmful but allocated extra memory which this patch gives back. The other instances have a legit need for "flexible array".
cheers, jamal > -- > pw-bot: cr
| |