| Message ID | 20190614082015.23336-3-toshiaki.makita1@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | Devmap fixes around memory and RCU | expand |
On Fri, 14 Jun 2019 17:20:14 +0900 Toshiaki Makita <toshiaki.makita1@gmail.com> wrote: > dev_map_free() forgot to free bulk queue when freeing its entries. > > Fixes: 5d053f9da431 ("bpf: devmap prepare xdp frames for bulking") > Signed-off-by: Toshiaki Makita <toshiaki.makita1@gmail.com> > --- > kernel/bpf/devmap.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c > index e001fb1..a126d95 100644 > --- a/kernel/bpf/devmap.c > +++ b/kernel/bpf/devmap.c > @@ -186,6 +186,7 @@ static void dev_map_free(struct bpf_map *map) > if (!dev) > continue; > > + free_percpu(dev->bulkq); > dev_put(dev->dev); > kfree(dev); > } Do we need to call need to call dev_map_flush_old() before free_percpu(dev->bulkq) ? Looking the code, I guess this is not needed as, above we are ensuring all pending flush operations have completed. Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
On 19/06/14 (金) 20:58:06, Jesper Dangaard Brouer wrote: > On Fri, 14 Jun 2019 17:20:14 +0900 > Toshiaki Makita <toshiaki.makita1@gmail.com> wrote: > >> dev_map_free() forgot to free bulk queue when freeing its entries. >> >> Fixes: 5d053f9da431 ("bpf: devmap prepare xdp frames for bulking") >> Signed-off-by: Toshiaki Makita <toshiaki.makita1@gmail.com> >> --- >> kernel/bpf/devmap.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c >> index e001fb1..a126d95 100644 >> --- a/kernel/bpf/devmap.c >> +++ b/kernel/bpf/devmap.c >> @@ -186,6 +186,7 @@ static void dev_map_free(struct bpf_map *map) >> if (!dev) >> continue; >> >> + free_percpu(dev->bulkq); >> dev_put(dev->dev); >> kfree(dev); >> } > > Do we need to call need to call dev_map_flush_old() before > free_percpu(dev->bulkq) ? > > Looking the code, I guess this is not needed as, above we are ensuring > all pending flush operations have completed. My understanding is the same. The code waits for NAPI to flush the queue, so no need for dev_map_flush_old(). > > Acked-by: Jesper Dangaard Brouer <brouer@redhat.com> Thanks! Toshiaki Makita
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c index e001fb1..a126d95 100644 --- a/kernel/bpf/devmap.c +++ b/kernel/bpf/devmap.c @@ -186,6 +186,7 @@ static void dev_map_free(struct bpf_map *map) if (!dev) continue; + free_percpu(dev->bulkq); dev_put(dev->dev); kfree(dev); }
dev_map_free() forgot to free bulk queue when freeing its entries. Fixes: 5d053f9da431 ("bpf: devmap prepare xdp frames for bulking") Signed-off-by: Toshiaki Makita <toshiaki.makita1@gmail.com> --- kernel/bpf/devmap.c | 1 + 1 file changed, 1 insertion(+)