| Message ID | baf4788b6ac529a2132ace735fc1caf4989307da.1459827115.git.lucien.xin@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On 2016/04/05 12:32, Xin Long wrote: > There are some repetitive codes in stp_state_store, we can remove > them by calling store_bridge_parm. > > Signed-off-by: Xin Long <lucien.xin@gmail.com> > --- > net/bridge/br_sysfs_br.c | 24 +++++++----------------- > 1 file changed, 7 insertions(+), 17 deletions(-) > > diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c > index 137cd3b..9918763 100644 > --- a/net/bridge/br_sysfs_br.c > +++ b/net/bridge/br_sysfs_br.c > @@ -128,27 +128,17 @@ static ssize_t stp_state_show(struct device *d, > } > > > +static int set_stp_state(struct net_bridge *br, unsigned long val) > +{ You forgot to add rtnl lock here? The missing lock is restored in patch 4, but at this point bisect could break.. > + br_stp_set_enabled(br, val); > + return 0; > +} Toshiaki Makita
From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> Date: Tue, 5 Apr 2016 14:08:13 +0900 > On 2016/04/05 12:32, Xin Long wrote: >> There are some repetitive codes in stp_state_store, we can remove >> them by calling store_bridge_parm. >> >> Signed-off-by: Xin Long <lucien.xin@gmail.com> >> --- >> net/bridge/br_sysfs_br.c | 24 +++++++----------------- >> 1 file changed, 7 insertions(+), 17 deletions(-) >> >> diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c >> index 137cd3b..9918763 100644 >> --- a/net/bridge/br_sysfs_br.c >> +++ b/net/bridge/br_sysfs_br.c >> @@ -128,27 +128,17 @@ static ssize_t stp_state_show(struct device *d, >> } >> >> >> +static int set_stp_state(struct net_bridge *br, unsigned long val) >> +{ > > You forgot to add rtnl lock here? > The missing lock is restored in patch 4, but at this point bisect could > break.. Agreed, this has to be fixed.
On Thu, Apr 7, 2016 at 4:10 AM, David Miller <davem@davemloft.net> wrote: > From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> > Date: Tue, 5 Apr 2016 14:08:13 +0900 > >> On 2016/04/05 12:32, Xin Long wrote: >>> There are some repetitive codes in stp_state_store, we can remove >>> them by calling store_bridge_parm. >>> >>> Signed-off-by: Xin Long <lucien.xin@gmail.com> >>> --- >>> net/bridge/br_sysfs_br.c | 24 +++++++----------------- >>> 1 file changed, 7 insertions(+), 17 deletions(-) >>> >>> diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c >>> index 137cd3b..9918763 100644 >>> --- a/net/bridge/br_sysfs_br.c >>> +++ b/net/bridge/br_sysfs_br.c >>> @@ -128,27 +128,17 @@ static ssize_t stp_state_show(struct device *d, >>> } >>> >>> >>> +static int set_stp_state(struct net_bridge *br, unsigned long val) >>> +{ >> >> You forgot to add rtnl lock here? >> The missing lock is restored in patch 4, but at this point bisect could >> break.. OK, if I will fix this, if there's no other issues on this patchset, I will post v3 later. Thanks. > > Agreed, this has to be fixed.
diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c index 137cd3b..9918763 100644 --- a/net/bridge/br_sysfs_br.c +++ b/net/bridge/br_sysfs_br.c @@ -128,27 +128,17 @@ static ssize_t stp_state_show(struct device *d, } +static int set_stp_state(struct net_bridge *br, unsigned long val) +{ + br_stp_set_enabled(br, val); + return 0; +} + static ssize_t stp_state_store(struct device *d, struct device_attribute *attr, const char *buf, size_t len) { - struct net_bridge *br = to_bridge(d); - char *endp; - unsigned long val; - - if (!ns_capable(dev_net(br->dev)->user_ns, CAP_NET_ADMIN)) - return -EPERM; - - val = simple_strtoul(buf, &endp, 0); - if (endp == buf) - return -EINVAL; - - if (!rtnl_trylock()) - return restart_syscall(); - br_stp_set_enabled(br, val); - rtnl_unlock(); - - return len; + return store_bridge_parm(d, buf, len, set_stp_state); } static DEVICE_ATTR_RW(stp_state);
There are some repetitive codes in stp_state_store, we can remove them by calling store_bridge_parm. Signed-off-by: Xin Long <lucien.xin@gmail.com> --- net/bridge/br_sysfs_br.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-)