From: Toshiaki Makita <toshiaki.makita1@gmail.com>
To: John Fastabend <john.fastabend@gmail.com>,
bjorn.topel@gmail.com, bpf@vger.kernel.org, toke@redhat.com
Cc: netdev@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net
Subject: Re: [bpf PATCH 2/2] bpf: xdp, remove no longer required rcu_read_{un}lock()
Date: Thu, 9 Jan 2020 15:01:09 +0900 [thread overview]
Message-ID: <a4bb8f06-f960-cdda-f73a-8b87744445af@gmail.com> (raw)
In-Reply-To: <157851930654.21459.7236323146782270917.stgit@john-Precision-5820-Tower>
On 2020/01/09 6:35, John Fastabend wrote:
> Now that we depend on rcu_call() and synchronize_rcu() to also wait
> for preempt_disabled region to complete the rcu read critical section
> in __dev_map_flush() is no longer relevant.
>
> These originally ensured the map reference was safe while a map was
> also being free'd. But flush by new rules can only be called from
> preempt-disabled NAPI context. The synchronize_rcu from the map free
> path and the rcu_call from the delete path will ensure the reference
> here is safe. So lets remove the rcu_read_lock and rcu_read_unlock
> pair to avoid any confusion around how this is being protected.
>
> If the rcu_read_lock was required it would mean errors in the above
> logic and the original patch would also be wrong.
>
> Fixes: 0536b85239b84 ("xdp: Simplify devmap cleanup")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
> kernel/bpf/devmap.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> index f0bf525..0129d4a 100644
> --- a/kernel/bpf/devmap.c
> +++ b/kernel/bpf/devmap.c
> @@ -378,10 +378,8 @@ void __dev_map_flush(void)
> struct list_head *flush_list = this_cpu_ptr(&dev_map_flush_list);
> struct xdp_bulk_queue *bq, *tmp;
>
> - rcu_read_lock();
> list_for_each_entry_safe(bq, tmp, flush_list, flush_node)
> bq_xmit_all(bq, XDP_XMIT_FLUSH);
> - rcu_read_unlock();
I introduced this lock because some drivers have assumption that
.ndo_xdp_xmit() is called under RCU. (commit 86723c864063)
Maybe devmap deletion logic does not need this anymore, but is it
OK to drivers?
Toshiaki Makita
next prev parent reply other threads:[~2020-01-09 6:01 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-08 21:34 [bpf PATCH 0/2] xdp devmap improvements cleanup John Fastabend
2020-01-08 21:34 ` [bpf PATCH 1/2] bpf: xdp, update devmap comments to reflect napi/rcu usage John Fastabend
2020-01-09 1:20 ` Song Liu
2020-01-08 21:35 ` [bpf PATCH 2/2] bpf: xdp, remove no longer required rcu_read_{un}lock() John Fastabend
2020-01-09 1:21 ` Song Liu
2020-01-09 6:01 ` Toshiaki Makita [this message]
2020-01-09 6:34 ` Toshiaki Makita
2020-01-09 6:35 ` John Fastabend
2020-01-09 6:57 ` Toshiaki Makita
2020-01-09 14:58 ` John Fastabend
2020-01-09 6:09 ` [bpf PATCH 0/2] xdp devmap improvements cleanup Björn Töpel
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=a4bb8f06-f960-cdda-f73a-8b87744445af@gmail.com \
--to=toshiaki.makita1@gmail.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 \
/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).