From: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> Date: Wed, 15 Feb 2023 09:48:21 +0800 > On Tue, 14 Feb 2023 15:45:12 +0100, Alexander Lobakin <alexandr.lobakin@xxxxxxxxx> wrote: >> From: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> >> Date: Tue, 14 Feb 2023 09:51:12 +0800 >> >>> When we try to start AF_XDP on some machines with long running time, due >>> to the machine's memory fragmentation problem, there is no sufficient >>> contiguous physical memory that will cause the start failure. >> >> [...] >> >>> @@ -1319,13 +1317,10 @@ static int xsk_mmap(struct file *file, struct socket *sock, >>> >>> /* Matches the smp_wmb() in xsk_init_queue */ >>> smp_rmb(); >>> - qpg = virt_to_head_page(q->ring); >>> - if (size > page_size(qpg)) >>> + if (size > PAGE_ALIGN(q->ring_size)) >> >> You can set q->ring_size as PAGE_ALIGN(size) already at the allocation >> to simplify this. I don't see any other places where you use it. > > That's it, but I think it is not particularly appropriate to change the > the semantics of ring_size just for simplify this code. This may make > people feel strange. You can name it 'vmalloc_size' then. By "ring_size" I first of all assume the number of elements, not the allocation size. Also, wait, shouldn't you do this PAGE_ALIGN() *before* you actually vmalloc() it? Can't here be out-of-bounds with the current approach? > > I agree with you other opinions. > > Thanks. Thanks, Olek
