Messages in this thread | | | From | "Jonathan Lemon" <> | Subject | Re: [PATCH bpf-next v2 2/3] xdp: xdp_umem: replace kmap on vmap for umem map | Date | Thu, 15 Aug 2019 11:23:16 -0700 |
| |
On 15 Aug 2019, at 5:13, Ivan Khoronzhuk wrote:
> For 64-bit there is no reason to use vmap/vunmap, so use page_address > as it was initially. For 32 bits, in some apps, like in samples > xdpsock_user.c when number of pgs in use is quite big, the kmap > memory can be not enough, despite on this, kmap looks like is > deprecated in such cases as it can block and should be used rather > for dynamic mm. > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > --- > net/xdp/xdp_umem.c | 36 ++++++++++++++++++++++++++++++------ > 1 file changed, 30 insertions(+), 6 deletions(-) > > diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c > index a0607969f8c0..d740c4f8810c 100644 > --- a/net/xdp/xdp_umem.c > +++ b/net/xdp/xdp_umem.c > @@ -14,7 +14,7 @@ > #include <linux/netdevice.h> > #include <linux/rtnetlink.h> > #include <linux/idr.h> > -#include <linux/highmem.h> > +#include <linux/vmalloc.h> > > #include "xdp_umem.h" > #include "xsk_queue.h" > @@ -170,7 +170,30 @@ static void xdp_umem_unmap_pages(struct xdp_umem > *umem) > unsigned int i; > > for (i = 0; i < umem->npgs; i++) > - kunmap(umem->pgs[i]); > + if (PageHighMem(umem->pgs[i])) > + vunmap(umem->pages[i].addr); > +} > + > +static int xdp_umem_map_pages(struct xdp_umem *umem) > +{ > + unsigned int i; > + void *addr; > + > + for (i = 0; i < umem->npgs; i++) { > + if (PageHighMem(umem->pgs[i])) > + addr = vmap(&umem->pgs[i], 1, VM_MAP, PAGE_KERNEL); > + else > + addr = page_address(umem->pgs[i]); > + > + if (!addr) { > + xdp_umem_unmap_pages(umem); > + return -ENOMEM; > + } > + > + umem->pages[i].addr = addr; > + } > + > + return 0; > }
You'll want a __xdp_umem_unmap_pages() helper here that takes an count of the number of pages to unmap, so it can be called from xdp_umem_unmap_pages() in the normal case, and xdp_umem_map_pages() in the error case. Otherwise the error case ends up calling PageHighMem on a null page. -- Jonathan
> static void xdp_umem_unpin_pages(struct xdp_umem *umem) > @@ -312,7 +335,7 @@ static int xdp_umem_reg(struct xdp_umem *umem, > struct xdp_umem_reg *mr) > u32 chunk_size = mr->chunk_size, headroom = mr->headroom; > unsigned int chunks, chunks_per_page; > u64 addr = mr->addr, size = mr->len; > - int size_chk, err, i; > + int size_chk, err; > > if (chunk_size < XDP_UMEM_MIN_CHUNK_SIZE || chunk_size > PAGE_SIZE) > { > /* Strictly speaking we could support this, if: > @@ -378,10 +401,11 @@ static int xdp_umem_reg(struct xdp_umem *umem, > struct xdp_umem_reg *mr) > goto out_account; > } > > - for (i = 0; i < umem->npgs; i++) > - umem->pages[i].addr = kmap(umem->pgs[i]); > + err = xdp_umem_map_pages(umem); > + if (!err) > + return 0; > > - return 0; > + kfree(umem->pages); > > out_account: > xdp_umem_unaccount_pages(umem); > -- > 2.17.1
| |