On Wed, 15 Feb 2023 17:50:54 +0100, Alexander Lobakin <aleksander.lobakin@xxxxxxxxx> wrote: > 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. Maybe "ring_vmalloc_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? vmalloc_user() will do PAGE_ALIGN(). Thanks. > > > > > I agree with you other opinions. > > > > Thanks. > > Thanks, > Olek
