On Mon, Aug 21, 2023 at 3:13 PM Yonghong Song <yonghong.song@xxxxxxxxx> wrote: > > > > On 8/21/23 12:44 PM, Alexei Starovoitov wrote: > > On Mon, Aug 21, 2023 at 9:03 AM Kumar Kartikeya Dwivedi > > <memxor@xxxxxxxxx> wrote: > >>>> > >>>> The fix on its own looks ok to me, but any reason you'd not like to > >>>> delegate to map_kptr_match_type? > >>>> Just to collect kptr related type matching logic in its own place. It > >>>> doesn't matter too much though. > >>> > >>> From comments from Alexei in > >>> > >>> https://lore.kernel.org/bpf/20230819002907.io3iphmnuk43xblu@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/#t > >>> > >>> ===== > >>> The map_kptr_match_type() should have been used for kptrs pointing to > >>> kernel objects only. > >>> But you're calling it for MEM_ALLOC object with prog's BTF... > >>> ===== > >>> > >>> So looks like map_kptr_match_type() is for kptrs pointing to > >>> kernel objects only. So that is why I didn't use it. > >>> > >> > >> That function was added by me. Back then I added this check as we were > >> discussing possibly supporting such local kptr and more thought would > >> be needed about the design before just doing type matching. Also it > >> was using kernel_type_name which was later renamed as btf_type_name, > >> so as a precaution I added the btf_is_kernel check. Apart from that I > >> remember no other reason, so I think it should be ok to drop it now > >> and use it. > > > > Agree with Kumar. > > When I said map_kptr_match_type() is only used with kernel BTF I meant > > that as code stands it was the intent of that helper. > > With MEM_ALLOC being kptr_xchg-ed it's better to share the code and > > map_kptr_match_type() should probably be adopted to work with both kernel > > and prog's BTFs. > > Sounds good to me. Will use map_kptr_match_type() in v2. btw it's a bit risky for bpf tree this late into rc-s. Pls target bpf-next.
