| Message ID | 20240507132000.614591-1-edumazet@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 3b09b2bd0d62de1f359b0c130570711c99c5e80b |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: dst_cache: annotate data-races around dst_cache->reset_ts | expand |
On Tue, 2024-05-07 at 13:20 +0000, Eric Dumazet wrote: > dst_cache->reset_ts is read or written locklessly, > add READ_ONCE() and WRITE_ONCE() annotations. > > Signed-off-by: Eric Dumazet <edumazet@google.com> I was wondering if we need the annotation when the RD/WR ops all happen on the same CPU, but dst_cache_reset() can happen on a different core, so LGTM, thanks! Acked-by: Paolo Abeni <pabeni@redhat.com>
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Tue, 7 May 2024 13:20:00 +0000 you wrote: > dst_cache->reset_ts is read or written locklessly, > add READ_ONCE() and WRITE_ONCE() annotations. > > Signed-off-by: Eric Dumazet <edumazet@google.com> > --- > include/net/dst_cache.h | 2 +- > net/core/dst_cache.c | 5 +++-- > 2 files changed, 4 insertions(+), 3 deletions(-) Here is the summary with links: - [net-next] net: dst_cache: annotate data-races around dst_cache->reset_ts https://git.kernel.org/netdev/net-next/c/3b09b2bd0d62 You are awesome, thank you!
diff --git a/include/net/dst_cache.h b/include/net/dst_cache.h index df6622a5fe98f0a9732617bb2a757ef9c9611797..b4a55d2d5e71b974b0012bc7ec9a5970a6c0dffe 100644 --- a/include/net/dst_cache.h +++ b/include/net/dst_cache.h @@ -76,7 +76,7 @@ struct dst_entry *dst_cache_get_ip6(struct dst_cache *dst_cache, */ static inline void dst_cache_reset(struct dst_cache *dst_cache) { - dst_cache->reset_ts = jiffies; + WRITE_ONCE(dst_cache->reset_ts, jiffies); } /** diff --git a/net/core/dst_cache.c b/net/core/dst_cache.c index 0c0bdb058c5b1ab81e14aa48d8612a6253a7c852..f9df84a6c4b2dbe63c6f61fb431e179f92e072e0 100644 --- a/net/core/dst_cache.c +++ b/net/core/dst_cache.c @@ -47,7 +47,8 @@ static struct dst_entry *dst_cache_per_cpu_get(struct dst_cache *dst_cache, /* the cache already hold a dst reference; it can't go away */ dst_hold(dst); - if (unlikely(!time_after(idst->refresh_ts, dst_cache->reset_ts) || + if (unlikely(!time_after(idst->refresh_ts, + READ_ONCE(dst_cache->reset_ts)) || (dst->obsolete && !dst->ops->check(dst, idst->cookie)))) { dst_cache_per_cpu_dst_set(idst, NULL, 0); dst_release(dst); @@ -170,7 +171,7 @@ void dst_cache_reset_now(struct dst_cache *dst_cache) if (!dst_cache->cache) return; - dst_cache->reset_ts = jiffies; + dst_cache_reset(dst_cache); for_each_possible_cpu(i) { struct dst_cache_pcpu *idst = per_cpu_ptr(dst_cache->cache, i); struct dst_entry *dst = idst->dst;
dst_cache->reset_ts is read or written locklessly, add READ_ONCE() and WRITE_ONCE() annotations. Signed-off-by: Eric Dumazet <edumazet@google.com> --- include/net/dst_cache.h | 2 +- net/core/dst_cache.c | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-)