diff mbox series

[net-next,10/13] net: add netdev_set_operstate() helper

Message ID 20240207142629.3456570-11-edumazet@google.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: complete dev_base_lock removal | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 4542 this patch: 4542
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 9 maintainers not CCed: r-gunasekaran@ti.com johannes.berg@intel.com pctammela@mojatatu.com jhs@mojatatu.com victor@mojatatu.com horms@kernel.org m-karicheri2@ti.com jiri@resnulli.us idosch@nvidia.com
netdev/build_clang success Errors and warnings before: 1164 this patch: 1164
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 4770 this patch: 4770
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 93 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest fail net-next-2024-02-08--03-00 (tests: 774)

Commit Message

Eric Dumazet Feb. 7, 2024, 2:26 p.m. UTC
dev_base_lock is going away, add netdev_set_operstate() helper
so that hsr does not have to know core internals.

Remove dev_base_lock acquisition from rfc2863_policy()

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/rtnetlink.h |  2 ++
 net/core/link_watch.c     |  4 ----
 net/core/rtnetlink.c      | 22 +++++++++++++++-------
 net/hsr/hsr_device.c      | 22 ++++++----------------
 4 files changed, 23 insertions(+), 27 deletions(-)

Comments

kernel test robot Feb. 8, 2024, 11:44 a.m. UTC | #1
Hi Eric,

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/Eric-Dumazet/net-annotate-data-races-around-dev-name_assign_type/20240207-222903
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240207142629.3456570-11-edumazet%40google.com
patch subject: [PATCH net-next 10/13] net: add netdev_set_operstate() helper
config: sh-defconfig (https://download.01.org/0day-ci/archive/20240208/202402081918.OLyGaea3-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240208/202402081918.OLyGaea3-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/202402081918.OLyGaea3-lkp@intel.com/

All errors (new ones prefixed by >>):

   sh4-linux-ld: net/core/rtnetlink.o: in function `set_operstate':
>> rtnetlink.c:(.text+0x1220): undefined reference to `__cmpxchg_called_with_bad_pointer'
   sh4-linux-ld: net/core/rtnetlink.o: in function `netdev_set_operstate':
   rtnetlink.c:(.text+0x17a0): undefined reference to `__cmpxchg_called_with_bad_pointer'
kernel test robot Feb. 8, 2024, 1:20 p.m. UTC | #2
Hi Eric,

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/Eric-Dumazet/net-annotate-data-races-around-dev-name_assign_type/20240207-222903
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240207142629.3456570-11-edumazet%40google.com
patch subject: [PATCH net-next 10/13] net: add netdev_set_operstate() helper
config: riscv-defconfig (https://download.01.org/0day-ci/archive/20240208/202402082151.O18ZoLSK-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 7dd790db8b77c4a833c06632e903dc4f13877a64)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240208/202402082151.O18ZoLSK-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/202402082151.O18ZoLSK-lkp@intel.com/

All errors (new ones prefixed by >>):

>> net/core/rtnetlink.c:852:12: error: call to '__compiletime_assert_784' declared with 'error' attribute: BUILD_BUG failed
     852 |         } while (!try_cmpxchg(&dev->operstate, &old, newstate));
         |                   ^
   include/linux/atomic/atomic-instrumented.h:4838:2: note: expanded from macro 'try_cmpxchg'
    4838 |         raw_try_cmpxchg(__ai_ptr, __ai_oldp, __VA_ARGS__); \
         |         ^
   include/linux/atomic/atomic-arch-fallback.h:192:9: note: expanded from macro 'raw_try_cmpxchg'
     192 |         ___r = raw_cmpxchg((_ptr), ___o, (_new)); \
         |                ^
   include/linux/atomic/atomic-arch-fallback.h:55:21: note: expanded from macro 'raw_cmpxchg'
      55 | #define raw_cmpxchg arch_cmpxchg
         |                     ^
   note: (skipping 5 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:423:2: note: expanded from macro '_compiletime_assert'
     423 |         __compiletime_assert(condition, msg, prefix, suffix)
         |         ^
   include/linux/compiler_types.h:416:4: note: expanded from macro '__compiletime_assert'
     416 |                         prefix ## suffix();                             \
         |                         ^
   <scratch space>:50:1: note: expanded from here
      50 | __compiletime_assert_784
         | ^
>> net/core/rtnetlink.c:852:12: error: call to '__compiletime_assert_784' declared with 'error' attribute: BUILD_BUG failed
   include/linux/atomic/atomic-instrumented.h:4838:2: note: expanded from macro 'try_cmpxchg'
    4838 |         raw_try_cmpxchg(__ai_ptr, __ai_oldp, __VA_ARGS__); \
         |         ^
   include/linux/atomic/atomic-arch-fallback.h:192:9: note: expanded from macro 'raw_try_cmpxchg'
     192 |         ___r = raw_cmpxchg((_ptr), ___o, (_new)); \
         |                ^
   include/linux/atomic/atomic-arch-fallback.h:55:21: note: expanded from macro 'raw_cmpxchg'
      55 | #define raw_cmpxchg arch_cmpxchg
         |                     ^
   note: (skipping 5 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:423:2: note: expanded from macro '_compiletime_assert'
     423 |         __compiletime_assert(condition, msg, prefix, suffix)
         |         ^
   include/linux/compiler_types.h:416:4: note: expanded from macro '__compiletime_assert'
     416 |                         prefix ## suffix();                             \
         |                         ^
   <scratch space>:50:1: note: expanded from here
      50 | __compiletime_assert_784
         | ^
>> net/core/rtnetlink.c:852:12: error: call to '__compiletime_assert_784' declared with 'error' attribute: BUILD_BUG failed
   include/linux/atomic/atomic-instrumented.h:4838:2: note: expanded from macro 'try_cmpxchg'
    4838 |         raw_try_cmpxchg(__ai_ptr, __ai_oldp, __VA_ARGS__); \
         |         ^
   include/linux/atomic/atomic-arch-fallback.h:192:9: note: expanded from macro 'raw_try_cmpxchg'
     192 |         ___r = raw_cmpxchg((_ptr), ___o, (_new)); \
         |                ^
   include/linux/atomic/atomic-arch-fallback.h:55:21: note: expanded from macro 'raw_cmpxchg'
      55 | #define raw_cmpxchg arch_cmpxchg
         |                     ^
   note: (skipping 5 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:423:2: note: expanded from macro '_compiletime_assert'
     423 |         __compiletime_assert(condition, msg, prefix, suffix)
         |         ^
   include/linux/compiler_types.h:416:4: note: expanded from macro '__compiletime_assert'
     416 |                         prefix ## suffix();                             \
         |                         ^
   <scratch space>:50:1: note: expanded from here
      50 | __compiletime_assert_784
         | ^
   3 errors generated.


vim +852 net/core/rtnetlink.c

   844	
   845	void netdev_set_operstate(struct net_device *dev, int newstate)
   846	{
   847		unsigned char old = READ_ONCE(dev->operstate);
   848	
   849		do {
   850			if (old == newstate)
   851				return;
 > 852		} while (!try_cmpxchg(&dev->operstate, &old, newstate));
   853	
   854		netdev_state_change(dev);
   855	}
   856	EXPORT_SYMBOL(netdev_set_operstate);
   857
Eric Dumazet Feb. 8, 2024, 1:35 p.m. UTC | #3
On Thu, Feb 8, 2024 at 2:20 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Eric,
>
> 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/Eric-Dumazet/net-annotate-data-races-around-dev-name_assign_type/20240207-222903
> base:   net-next/main
> patch link:    https://lore.kernel.org/r/20240207142629.3456570-11-edumazet%40google.com
> patch subject: [PATCH net-next 10/13] net: add netdev_set_operstate() helper
> config: riscv-defconfig (https://download.01.org/0day-ci/archive/20240208/202402082151.O18ZoLSK-lkp@intel.com/config)
> compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 7dd790db8b77c4a833c06632e903dc4f13877a64)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240208/202402082151.O18ZoLSK-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/202402082151.O18ZoLSK-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
> >> net/core/rtnetlink.c:852:12: error: call to '__compiletime_assert_784' declared with 'error' attribute: BUILD_BUG failed
>      852 |         } while (!try_cmpxchg(&dev->operstate, &old, newstate));
>          |                   ^
>    include/linux/atomic/atomic-instrumented.h:4838:2: note: expanded from macro 'try_cmpxchg'
>     4838 |         raw_try_cmpxchg(__ai_ptr, __ai_oldp, __VA_ARGS__); \
>          |         ^
>    include/linux/atomic/atomic-arch-fallback.h:192:9: note: expanded from macro 'raw_try_cmpxchg'
>      192 |         ___r = raw_cmpxchg((_ptr), ___o, (_new)); \
>          |                ^
>    include/linux/atomic/atomic-arch-fallback.h:55:21: note: expanded from macro 'raw_cmpxchg'
>       55 | #define raw_cmpxchg arch_cmpxchg
>          |                     ^
>    note: (skipping 5 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
>    include/linux/compiler_types.h:423:2: note: expanded from macro '_compiletime_assert'
>      423 |         __compiletime_assert(condition, msg, prefix, suffix)
>          |         ^
>    include/linux/compiler_types.h:416:4: note: expanded from macro '__compiletime_assert'
>      416 |                         prefix ## suffix();                             \
>          |                         ^
>    <scratch space>:50:1: note: expanded from here
>       50 | __compiletime_assert_784
>          | ^
> >> net/core/rtnetlink.c:852:12: error: call to '__compiletime_assert_784' declared with 'error' attribute: BUILD_BUG failed
>    include/linux/atomic/atomic-instrumented.h:4838:2: note: expanded from macro 'try_cmpxchg'
>     4838 |         raw_try_cmpxchg(__ai_ptr, __ai_oldp, __VA_ARGS__); \
>          |         ^
>    include/linux/atomic/atomic-arch-fallback.h:192:9: note: expanded from macro 'raw_try_cmpxchg'
>      192 |         ___r = raw_cmpxchg((_ptr), ___o, (_new)); \
>          |                ^
>    include/linux/atomic/atomic-arch-fallback.h:55:21: note: expanded from macro 'raw_cmpxchg'
>       55 | #define raw_cmpxchg arch_cmpxchg
>          |                     ^
>    note: (skipping 5 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
>    include/linux/compiler_types.h:423:2: note: expanded from macro '_compiletime_assert'
>      423 |         __compiletime_assert(condition, msg, prefix, suffix)
>          |         ^
>    include/linux/compiler_types.h:416:4: note: expanded from macro '__compiletime_assert'
>      416 |                         prefix ## suffix();                             \
>          |                         ^
>    <scratch space>:50:1: note: expanded from here
>       50 | __compiletime_assert_784
>          | ^
> >> net/core/rtnetlink.c:852:12: error: call to '__compiletime_assert_784' declared with 'error' attribute: BUILD_BUG failed
>    include/linux/atomic/atomic-instrumented.h:4838:2: note: expanded from macro 'try_cmpxchg'
>     4838 |         raw_try_cmpxchg(__ai_ptr, __ai_oldp, __VA_ARGS__); \
>          |         ^
>    include/linux/atomic/atomic-arch-fallback.h:192:9: note: expanded from macro 'raw_try_cmpxchg'
>      192 |         ___r = raw_cmpxchg((_ptr), ___o, (_new)); \
>          |                ^
>    include/linux/atomic/atomic-arch-fallback.h:55:21: note: expanded from macro 'raw_cmpxchg'
>       55 | #define raw_cmpxchg arch_cmpxchg
>          |                     ^
>    note: (skipping 5 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
>    include/linux/compiler_types.h:423:2: note: expanded from macro '_compiletime_assert'
>      423 |         __compiletime_assert(condition, msg, prefix, suffix)
>          |         ^
>    include/linux/compiler_types.h:416:4: note: expanded from macro '__compiletime_assert'
>      416 |                         prefix ## suffix();                             \
>          |                         ^
>    <scratch space>:50:1: note: expanded from here
>       50 | __compiletime_assert_784
>          | ^
>    3 errors generated.
>
>
> vim +852 net/core/rtnetlink.c
>
>    844
>    845  void netdev_set_operstate(struct net_device *dev, int newstate)
>    846  {
>    847          unsigned char old = READ_ONCE(dev->operstate);
>    848
>    849          do {
>    850                  if (old == newstate)
>    851                          return;
>  > 852          } while (!try_cmpxchg(&dev->operstate, &old, newstate));
>    853
>    854          netdev_state_change(dev);
>    855  }
>    856  EXPORT_SYMBOL(netdev_set_operstate);
>    857
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki

Ok, some arches are unable to perform cmpxchg() on u8, only plain
32bit integers.

I will send a v2 then.
diff mbox series

Patch

diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 21780608cf47ca0687dbaaf0d07b561e8631412c..cdfc897f1e3c683940a0958bc8a790c07ae819b0 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -172,4 +172,6 @@  rtnl_notify_needed(const struct net *net, u16 nlflags, u32 group)
 	return (nlflags & NLM_F_ECHO) || rtnl_has_listeners(net, group);
 }
 
+void netdev_set_operstate(struct net_device *dev, int newstate);
+
 #endif	/* __LINUX_RTNETLINK_H */
diff --git a/net/core/link_watch.c b/net/core/link_watch.c
index 1b93e054c9a3cfcdd5d1251a9982d88a071abbaa..83fdeb60dbd21169ab7a52def3674615b2ddedbd 100644
--- a/net/core/link_watch.c
+++ b/net/core/link_watch.c
@@ -70,8 +70,6 @@  static void rfc2863_policy(struct net_device *dev)
 	if (operstate == READ_ONCE(dev->operstate))
 		return;
 
-	write_lock(&dev_base_lock);
-
 	switch(dev->link_mode) {
 	case IF_LINK_MODE_TESTING:
 		if (operstate == IF_OPER_UP)
@@ -88,8 +86,6 @@  static void rfc2863_policy(struct net_device *dev)
 	}
 
 	WRITE_ONCE(dev->operstate, operstate);
-
-	write_unlock(&dev_base_lock);
 }
 
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 4e797326c88fe1e23ca66e82103176767fe5c32e..46710e5f9bd19298403cdb8c179f33f155a4c9ad 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -842,9 +842,22 @@  int rtnl_put_cacheinfo(struct sk_buff *skb, struct dst_entry *dst, u32 id,
 }
 EXPORT_SYMBOL_GPL(rtnl_put_cacheinfo);
 
+void netdev_set_operstate(struct net_device *dev, int newstate)
+{
+	unsigned char old = READ_ONCE(dev->operstate);
+
+	do {
+		if (old == newstate)
+			return;
+	} while (!try_cmpxchg(&dev->operstate, &old, newstate));
+
+	netdev_state_change(dev);
+}
+EXPORT_SYMBOL(netdev_set_operstate);
+
 static void set_operstate(struct net_device *dev, unsigned char transition)
 {
-	unsigned char operstate = dev->operstate;
+	unsigned char operstate = READ_ONCE(dev->operstate);
 
 	switch (transition) {
 	case IF_OPER_UP:
@@ -866,12 +879,7 @@  static void set_operstate(struct net_device *dev, unsigned char transition)
 		break;
 	}
 
-	if (READ_ONCE(dev->operstate) != operstate) {
-		write_lock(&dev_base_lock);
-		WRITE_ONCE(dev->operstate, operstate);
-		write_unlock(&dev_base_lock);
-		netdev_state_change(dev);
-	}
+	netdev_set_operstate(dev, operstate);
 }
 
 static unsigned int rtnl_dev_get_flags(const struct net_device *dev)
diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
index be0e43f46556e028e675147e63c6b787aa72e894..5ef6d437db727e60bfd8cf68f010f0151d0db98b 100644
--- a/net/hsr/hsr_device.c
+++ b/net/hsr/hsr_device.c
@@ -28,29 +28,19 @@  static bool is_slave_up(struct net_device *dev)
 	return dev && is_admin_up(dev) && netif_oper_up(dev);
 }
 
-static void __hsr_set_operstate(struct net_device *dev, int transition)
-{
-	write_lock(&dev_base_lock);
-	if (READ_ONCE(dev->operstate) != transition) {
-		WRITE_ONCE(dev->operstate, transition);
-		write_unlock(&dev_base_lock);
-		netdev_state_change(dev);
-	} else {
-		write_unlock(&dev_base_lock);
-	}
-}
-
 static void hsr_set_operstate(struct hsr_port *master, bool has_carrier)
 {
-	if (!is_admin_up(master->dev)) {
-		__hsr_set_operstate(master->dev, IF_OPER_DOWN);
+	struct net_device *dev = master->dev;
+
+	if (!is_admin_up(dev)) {
+		netdev_set_operstate(dev, IF_OPER_DOWN);
 		return;
 	}
 
 	if (has_carrier)
-		__hsr_set_operstate(master->dev, IF_OPER_UP);
+		netdev_set_operstate(dev, IF_OPER_UP);
 	else
-		__hsr_set_operstate(master->dev, IF_OPER_LOWERLAYERDOWN);
+		netdev_set_operstate(dev, IF_OPER_LOWERLAYERDOWN);
 }
 
 static bool hsr_check_carrier(struct hsr_port *master)