> Am 14.11.2019 um 19:14 schrieb Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx>: > > On Thu, Nov 14, 2019 at 9:40 AM Song Liu <liu.song.a23@xxxxxxxxx> wrote: >> >> On Wed, Nov 13, 2019 at 9:20 AM Ilya Leoshkevich <iii@xxxxxxxxxxxxx> wrote: >>> >>> Currently passing alignment greater than 4 to bpf_jit_binary_alloc does >>> not work: in such cases it aligns only to 4 bytes. >>> >>> However, this is required on s390, where in order to load a constant >>> from memory in a large (>512k) BPF program, one must use lgrl >>> instruction, whose memory operand must be aligned on an 8-byte boundary. >>> >>> This patch makes it possible to request an arbitrary power-of-2 >>> alignment from bpf_jit_binary_alloc by allocating extra padding bytes >>> and aligning the resulting pointer rather than the start offset. >>> >>> An alternative would be to simply increase the alignment of >>> bpf_binary_header.image to 8, but this would increase the risk of >>> wasting a page on arches that don't need it, and would also be >>> insufficient in case someone needs e.g. 16-byte alignment in the >>> future. > > why not 8 or 16? I don't follow why that would waste a page. It might waste a page because bpf_jit_binary_alloc rounds up allocation size to PAGE_SIZE, and unnecessary padding might be the last straw that would cause another page to be allocated. But that would apply only to a tiny amount of programs, whose JITed size is slightly smaller than a multiple of PAGE_SIZE. Sorry, I didn't fully get the 8 vs 16 question. At the moment, for s390-specific purpose 8 would be enough. I used 16 just to demonstrate that this solution wouldn't be future-proof. AFAIK some Intel instructions might want 16 (VMOVDQA?). But maybe it's better to think about it when someone actually needs to use them. >>> >>> Signed-off-by: Ilya Leoshkevich <iii@xxxxxxxxxxxxx> >> >> Maybe we can just make it 8 byte aligned for all architectures? >> >> #define BPF_IMAGE_ALIGNMENT 8 Seems like I'm overthinking this. If just bumping the alignment to 8 is OK, then I'll send a simpler patch. Best regards, Ilya
