Messages in this thread | | | From | Alexei Starovoitov <> | Date | Fri, 21 Jan 2022 10:29:55 -0800 | Subject | Re: [PATCH v5 bpf-next 7/7] bpf, x86_64: use bpf_prog_pack allocator |
| |
On Fri, Jan 21, 2022 at 9:53 AM Song Liu <songliubraving@fb.com> wrote: > > > > the header->size could be just below 2MB. > > I don't think kzalloc() can handle that. > > Technically, kzalloc can handle 2MB allocation via: > kzalloc() => kmalloc() => kmalloc_large() => kmalloc_order() > > But this would fail when the memory is fragmented. I guess we should use > kvmalloc() instead?
Contiguous 2MB allocation?
> > > >> + if (!tmp_header) { > >> + bpf_jit_binary_free_pack(header); > >> + header = NULL; > >> + prog = orig_prog; > >> + goto out_addrs; > >> + } > >> + tmp_header->size = header->size; > >> + tmp_image = (void *)tmp_header + ((void *)image - (void *)header); > > > > Why is 'tmp_image' needed at all? > > The above math can be done where necessary. > > We pass both image and tmp_image to do_jit(), as it needs both of them. > I think maintaining a tmp_image variable makes the logic cleaner. We can > remove it from x64_jit_data, I guess.
I'd remove from x64_jit_data. The recompute is cheap.
Speaking of tmp_header name... would be great to come up with something more descriptive. Here both tmp_header/tmp_image and header/image are used at the same time. My initial confusion with the patch was due to the name 'tmp'. The "tmp" prefix implies that the tmp_image will be used first and then it will become an image. But it's not the case. Maybe call it 'rw_header' and add a comment that 'header/image' are not writeable directly ? Or call it 'poke_header' ? Other ideas?
| |