On 2/23/24 11:15, Martin KaFai Lau wrote:
On 2/23/24 11:05 AM, Kui-Feng Lee wrote:
On 2/23/24 10:42, Martin KaFai Lau wrote:
On 2/23/24 10:29 AM, Kui-Feng Lee wrote:
One thing I forgot to mention is that bpf_dummy_ops has to call
bpf_jit_uncharge_modmem(PAGE_SIZE) as well. The other option is to move
bpf_jit_charge_modmem() out of bpf_struct_ops_prepare_trampoline(),
meaning bpf_struct_ops_map_update_elem() should handle the case that
the
allocation in bpf_struct_ops_prepare_trampoline() successes, but
bpf_jit_charge_modmem() fails.
Keep the charge/uncharge in bpf_struct_ops_prepare_trampoline().
It is fine to have bpf_dummy_ops charge and then uncharge a
PAGE_SIZE. There is no need to optimize for bpf_dummy_ops. Use
bpf_struct_ops_free_trampoline() in bpf_dummy_ops to uncharge and free.
Then, I don't get the point here.
I agree with moving the allocation into
bpf_struct_ops_prepare_trampoline() to avoid duplication of the code
about flags and tlinks. It really simplifies the code with the fact
that bpf_dummy_ops is still there. So, I tried to pass a st_map to
bpf_struct_ops_prepare_trampoline() to keep page managements code
together. But, you said to simplify the code of bpf_dummy_ops by
allocating pages in bpf_struct_ops_prepare_trampoline(), do bookkeeping
in bpf_struct_ops_map_update_elem(), so bpf_dummy_ops doesn't have to
I don't think I ever mentioned to do book keeping in
bpf_struct_ops_map_update_elem(). Have you looked at my earlier code in
bpf_struct_ops_prepare_trampoline() which also does the memory charging
also?
The bookkeeping that I am saying is about maintaining image_pages and
image_pages_cnt.
allocate memory. But, we have to move a bpf_jit_uncharge_modmem() to
bpf_dummy_ops. For me, this trade-off that include removing an
allocation and adding a bpf_jit_uncharge_modmem() make no sense.
Which part make no sense? Having bpf_dummy_ops charge/uncharge memory also?
Simplifying bpf_dummy_ops by removing the duty of allocation but adding
the duty of uncharge memory doesn't make sense to me in terms of
simplification. Although the lines of code would be similar, it actually
makes it more complicated than before.
The bpf_dummy_ops() uses the below bpf_struct_ops_free_trampoline()
which does uncharge and free. bpf_struct_ops_prepare_trampoline() does
charge and alloc.
charge/alloc matches with uncharge/free.
void bpf_struct_ops_free_trampoline(void *image)
{
bpf_jit_uncharge_modmem(PAGE_SIZE);
arch_free_bpf_trampoline(image, PAGE_SIZE);
}
