On 64bit arches, struct u64_stats_sync is empty and provides no help
against load/store tearing. struct copying should not be considered
tear-free. Use u64_stats_reads() instead.
Signed-off-by: David Yang <mmyangfl@gmail.com>
---
net/openvswitch/datapath.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index d5b6e2002bc1..8ba94df7f942 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -770,7 +770,8 @@ static void get_dp_stats(const struct datapath *dp, struct ovs_dp_stats *stats,
do {
start = u64_stats_fetch_begin(&percpu_stats->syncp);
- local_stats = *percpu_stats;
+ u64_stats_reads(&local_stats, percpu_stats,
+ sizeof(local_stats));
} while (u64_stats_fetch_retry(&percpu_stats->syncp, start));
stats->n_hit += local_stats.n_hit;
--
2.51.0On Sat, 24 Jan 2026 00:21:36 +0800
David Yang <mmyangfl@gmail.com> wrote:
> On 64bit arches, struct u64_stats_sync is empty and provides no help
> against load/store tearing. struct copying should not be considered
> tear-free. Use u64_stats_reads() instead.
Except that the compiler doesn't ever generate 'tearing accesses' for
aligned 64bit accesses on any 64bit architecture.
Similarly memcpy() won't generate problematic accesses.
The problem is purely theoretical - the C language lets the compiler
split accesses, but it doesn't.
David
>
> Signed-off-by: David Yang <mmyangfl@gmail.com>
> ---
> net/openvswitch/datapath.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index d5b6e2002bc1..8ba94df7f942 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -770,7 +770,8 @@ static void get_dp_stats(const struct datapath *dp, struct ovs_dp_stats *stats,
>
> do {
> start = u64_stats_fetch_begin(&percpu_stats->syncp);
> - local_stats = *percpu_stats;
> + u64_stats_reads(&local_stats, percpu_stats,
> + sizeof(local_stats));
> } while (u64_stats_fetch_retry(&percpu_stats->syncp, start));
>
> stats->n_hit += local_stats.n_hit;
On Mon, Jan 26, 2026 at 7:29 PM David Laight <david.laight.linux@gmail.com> wrote: > > On Sat, 24 Jan 2026 00:21:36 +0800 > David Yang <mmyangfl@gmail.com> wrote: > > > On 64bit arches, struct u64_stats_sync is empty and provides no help > > against load/store tearing. struct copying should not be considered > > tear-free. Use u64_stats_reads() instead. > > Except that the compiler doesn't ever generate 'tearing accesses' for > aligned 64bit accesses on any 64bit architecture. > Similarly memcpy() won't generate problematic accesses. > > The problem is purely theoretical - the C language lets the compiler > split accesses, but it doesn't. Yeah, although we still have races that KCSAN can detect. data_race() or READ_ONCE() would be necessary to avoid noisy KCSAN reports. While many LCSAN reports are boring, some of them point to real bugs.
On Mon, 26 Jan 2026 19:35:38 +0100 Eric Dumazet <edumazet@google.com> wrote: > On Mon, Jan 26, 2026 at 7:29 PM David Laight > <david.laight.linux@gmail.com> wrote: > > > > On Sat, 24 Jan 2026 00:21:36 +0800 > > David Yang <mmyangfl@gmail.com> wrote: > > > > > On 64bit arches, struct u64_stats_sync is empty and provides no help > > > against load/store tearing. struct copying should not be considered > > > tear-free. Use u64_stats_reads() instead. > > > > Except that the compiler doesn't ever generate 'tearing accesses' for > > aligned 64bit accesses on any 64bit architecture. > > Similarly memcpy() won't generate problematic accesses. > > > > The problem is purely theoretical - the C language lets the compiler > > split accesses, but it doesn't. > > Yeah, although we still have races that KCSAN can detect. > > data_race() or READ_ONCE() would be necessary to avoid noisy KCSAN reports. > > While many KCSAN reports are boring, some of them point to real bugs. > Could something be put in u64_stats_fetch_begin() to stop KCSAN bleating? With the way READ_ONCE and WRITE_ONCE now get enforced I do wonder if some data shouldn't just be marked 'volatile'? That would give 'non-tearing' accesses, but without the inter-cpu ordering that (I think) READ/WRITE_ONCE also give. For stats counters that is enough. David
On 1/23/26 5:21 PM, David Yang wrote:
> On 64bit arches, struct u64_stats_sync is empty and provides no help
> against load/store tearing. struct copying should not be considered
> tear-free. Use u64_stats_reads() instead.
>
> Signed-off-by: David Yang <mmyangfl@gmail.com>
> ---
> net/openvswitch/datapath.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index d5b6e2002bc1..8ba94df7f942 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -770,7 +770,8 @@ static void get_dp_stats(const struct datapath *dp, struct ovs_dp_stats *stats,
>
> do {
> start = u64_stats_fetch_begin(&percpu_stats->syncp);
> - local_stats = *percpu_stats;
> + u64_stats_reads(&local_stats, percpu_stats,
> + sizeof(local_stats));
I suppose, you'll either need to use offset of 'syncp' field here,
or copy fields one by one, which is not great.
Best regards, Ilya Maximets.
> } while (u64_stats_fetch_retry(&percpu_stats->syncp, start));
>
> stats->n_hit += local_stats.n_hit;
Hi David,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/David-Yang/u64_stats-Doc-incorrect-usage-with-plain-variables/20260124-012240
base: net-next/main
patch link: https://lore.kernel.org/r/20260123162159.2877941-5-mmyangfl%40gmail.com
patch subject: [PATCH net-next v2 4/7] net: openvswitch: fix load tearing with u64_stats
config: m68k-randconfig-r073-20260124 (https://download.01.org/0day-ci/archive/20260124/202601240814.75efeFdo-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 8.5.0
smatch version: v0.5.0-8994-gd50c5a4c
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260124/202601240814.75efeFdo-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601240814.75efeFdo-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from <command-line>:
In function 'u64_stats_reads.constprop',
inlined from 'get_dp_stats' at net/openvswitch/datapath.c:773:4:
>> include/linux/compiler_types.h:631:38: error: call to '__compiletime_assert_333' declared with attribute error: BUILD_BUG_ON failed: len % sizeof(u64_stats_t)
_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
^
include/linux/compiler_types.h:612:4: note: in definition of macro '__compiletime_assert'
prefix ## suffix(); \
^~~~~~
include/linux/compiler_types.h:631:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:50:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
^~~~~~~~~~~~~~~~
include/linux/u64_stats_sync.h:157:2: note: in expansion of macro 'BUILD_BUG_ON'
BUILD_BUG_ON(len % sizeof(u64_stats_t));
^~~~~~~~~~~~
vim +/__compiletime_assert_333 +631 include/linux/compiler_types.h
eb5c2d4b45e3d2 Will Deacon 2020-07-21 617
eb5c2d4b45e3d2 Will Deacon 2020-07-21 618 #define _compiletime_assert(condition, msg, prefix, suffix) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 619 __compiletime_assert(condition, msg, prefix, suffix)
eb5c2d4b45e3d2 Will Deacon 2020-07-21 620
eb5c2d4b45e3d2 Will Deacon 2020-07-21 621 /**
eb5c2d4b45e3d2 Will Deacon 2020-07-21 622 * compiletime_assert - break build and emit msg if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21 623 * @condition: a compile-time constant condition to check
eb5c2d4b45e3d2 Will Deacon 2020-07-21 624 * @msg: a message to emit if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21 625 *
eb5c2d4b45e3d2 Will Deacon 2020-07-21 626 * In tradition of POSIX assert, this macro will break the build if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21 627 * supplied condition is *false*, emitting the supplied error message if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21 628 * compiler has support to do so.
eb5c2d4b45e3d2 Will Deacon 2020-07-21 629 */
eb5c2d4b45e3d2 Will Deacon 2020-07-21 630 #define compiletime_assert(condition, msg) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 @631 _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
eb5c2d4b45e3d2 Will Deacon 2020-07-21 632
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
© 2016 - 2026 Red Hat, Inc.