From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
To: John Fastabend <john.fastabend@gmail.com>
Cc: bjorn.topel@gmail.com, bpf@vger.kernel.org, toke@redhat.com,
toshiaki.makita1@gmail.com, netdev@vger.kernel.org,
ast@kernel.org, daniel@iogearbox.net
Subject: Re: [bpf-next PATCH v2 1/2] bpf: xdp, update devmap comments to reflect napi/rcu usage
Date: Sun, 12 Jan 2020 08:47:22 +0100 [thread overview]
Message-ID: <20200112074722.GA24420@ranger.igk.intel.com> (raw)
In-Reply-To: <157879664156.8200.4955971883120344808.stgit@john-Precision-5820-Tower>
On Sat, Jan 11, 2020 at 06:37:21PM -0800, John Fastabend wrote:
Small nits for typos, can be ignored.
> Now that we rely on synchronize_rcu and call_rcu waiting to
> exit perempt-disable regions (NAPI) lets update the comments
s/perempt/preempt
> to reflect this.
>
> Fixes: 0536b85239b84 ("xdp: Simplify devmap cleanup")
> Acked-by: Björn Töpel <bjorn.topel@intel.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
> kernel/bpf/devmap.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> index da9c832..f0bf525 100644
> --- a/kernel/bpf/devmap.c
> +++ b/kernel/bpf/devmap.c
> @@ -193,10 +193,12 @@ static void dev_map_free(struct bpf_map *map)
>
> /* At this point bpf_prog->aux->refcnt == 0 and this map->refcnt == 0,
> * so the programs (can be more than one that used this map) were
> - * disconnected from events. Wait for outstanding critical sections in
> - * these programs to complete. The rcu critical section only guarantees
> - * no further reads against netdev_map. It does __not__ ensure pending
> - * flush operations (if any) are complete.
> + * disconnected from events. The following synchronize_rcu() guarantees
> + * both rcu read critical sections complete and waits for
> + * preempt-disable regions (NAPI being the relavent context here) so we
s/relavent/relevant
> + * are certain there will be no further reads against the netdev_map and
> + * all flush operations are complete. Flush operations can only be done
> + * from NAPI context for this reason.
> */
>
> spin_lock(&dev_map_lock);
> @@ -498,12 +500,11 @@ static int dev_map_delete_elem(struct bpf_map *map, void *key)
> return -EINVAL;
>
> /* Use call_rcu() here to ensure any rcu critical sections have
> - * completed, but this does not guarantee a flush has happened
> - * yet. Because driver side rcu_read_lock/unlock only protects the
> - * running XDP program. However, for pending flush operations the
> - * dev and ctx are stored in another per cpu map. And additionally,
> - * the driver tear down ensures all soft irqs are complete before
> - * removing the net device in the case of dev_put equals zero.
> + * completed as well as any flush operations because call_rcu
> + * will wait for preempt-disable region to complete, NAPI in this
> + * context. And additionally, the driver tear down ensures all
> + * soft irqs are complete before removing the net device in the
> + * case of dev_put equals zero.
> */
> old_dev = xchg(&dtab->netdev_map[k], NULL);
> if (old_dev)
>
next prev parent reply other threads:[~2020-01-12 14:54 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-12 2:36 [bpf-next PATCH v2 0/2] xdp devmap improvements cleanup John Fastabend
2020-01-12 2:37 ` [bpf-next PATCH v2 1/2] bpf: xdp, update devmap comments to reflect napi/rcu usage John Fastabend
2020-01-12 7:47 ` Maciej Fijalkowski [this message]
2020-01-14 3:27 ` John Fastabend
2020-01-12 11:21 ` Toke Høiland-Jørgensen
2020-01-12 2:37 ` [bpf-next PATCH v2 2/2] bpf: xdp, remove no longer required rcu_read_{un}lock() John Fastabend
2020-01-12 6:49 ` Maciej Fijalkowski
2020-01-14 3:25 ` John Fastabend
2020-01-14 0:31 ` Maciej Fijalkowski
2020-01-12 11:22 ` Toke Høiland-Jørgensen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200112074722.GA24420@ranger.igk.intel.com \
--to=maciej.fijalkowski@intel.com \
--cc=ast@kernel.org \
--cc=bjorn.topel@gmail.com \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=john.fastabend@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=toke@redhat.com \
--cc=toshiaki.makita1@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).