| Message ID | 20201209005444.1949356-4-weiwan@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | implement kthread based napi poll | expand |
On Tue, 8 Dec 2020 16:54:44 -0800 Wei Wang wrote: > +static void dev_disable_threaded_all(struct net_device *dev) > +{ > + struct napi_struct *napi; > + > + list_for_each_entry(napi, &dev->napi_list, dev_list) > + napi_set_threaded(napi, false); > +} This is an implementation detail which should be hidden in dev.c IMHO. Since the sysfs knob is just a device global on/off the iteration over napis and all is best hidden away. (sorry about the delayed review BTW, hope we can do a minor revision and still hit 5.12) > +static int modify_napi_threaded(struct net_device *dev, unsigned long val) > +{ > + struct napi_struct *napi; > + int ret; > + > + if (list_empty(&dev->napi_list)) > + return -EOPNOTSUPP; > + > + list_for_each_entry(napi, &dev->napi_list, dev_list) { > + ret = napi_set_threaded(napi, !!val); > + if (ret) { > + /* Error occurred on one of the napi, > + * reset threaded mode on all napi. > + */ > + dev_disable_threaded_all(dev); > + break; > + } > + } > + > + return ret; > +}
On Sat, Dec 12, 2020 at 2:59 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Tue, 8 Dec 2020 16:54:44 -0800 Wei Wang wrote: > > +static void dev_disable_threaded_all(struct net_device *dev) > > +{ > > + struct napi_struct *napi; > > + > > + list_for_each_entry(napi, &dev->napi_list, dev_list) > > + napi_set_threaded(napi, false); > > +} > > This is an implementation detail which should be hidden in dev.c IMHO. > Since the sysfs knob is just a device global on/off the iteration over > napis and all is best hidden away. OK. Will move it. > > (sorry about the delayed review BTW, hope we can do a minor revision > and still hit 5.12) > > > +static int modify_napi_threaded(struct net_device *dev, unsigned long val) > > +{ > > + struct napi_struct *napi; > > + int ret; > > + > > + if (list_empty(&dev->napi_list)) > > + return -EOPNOTSUPP; > > + > > + list_for_each_entry(napi, &dev->napi_list, dev_list) { > > + ret = napi_set_threaded(napi, !!val); > > + if (ret) { > > + /* Error occurred on one of the napi, > > + * reset threaded mode on all napi. > > + */ > > + dev_disable_threaded_all(dev); > > + break; > > + } > > + } > > + > > + return ret; > > +}
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index 94fff0700bdd..d14dc1da4c97 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -538,6 +538,75 @@ static ssize_t phys_switch_id_show(struct device *dev, } static DEVICE_ATTR_RO(phys_switch_id); +static ssize_t threaded_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct net_device *netdev = to_net_dev(dev); + struct napi_struct *n; + bool enabled; + int ret; + + if (!rtnl_trylock()) + return restart_syscall(); + + if (!dev_isalive(netdev)) { + ret = -EINVAL; + goto unlock; + } + + if (list_empty(&netdev->napi_list)) { + ret = -EOPNOTSUPP; + goto unlock; + } + + n = list_first_entry(&netdev->napi_list, struct napi_struct, dev_list); + enabled = !!test_bit(NAPI_STATE_THREADED, &n->state); + + ret = sprintf(buf, fmt_dec, enabled); + +unlock: + rtnl_unlock(); + return ret; +} + +static void dev_disable_threaded_all(struct net_device *dev) +{ + struct napi_struct *napi; + + list_for_each_entry(napi, &dev->napi_list, dev_list) + napi_set_threaded(napi, false); +} + +static int modify_napi_threaded(struct net_device *dev, unsigned long val) +{ + struct napi_struct *napi; + int ret; + + if (list_empty(&dev->napi_list)) + return -EOPNOTSUPP; + + list_for_each_entry(napi, &dev->napi_list, dev_list) { + ret = napi_set_threaded(napi, !!val); + if (ret) { + /* Error occurred on one of the napi, + * reset threaded mode on all napi. + */ + dev_disable_threaded_all(dev); + break; + } + } + + return ret; +} + +static ssize_t threaded_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t len) +{ + return netdev_store(dev, attr, buf, len, modify_napi_threaded); +} +static DEVICE_ATTR_RW(threaded); + static struct attribute *net_class_attrs[] __ro_after_init = { &dev_attr_netdev_group.attr, &dev_attr_type.attr, @@ -570,6 +639,7 @@ static struct attribute *net_class_attrs[] __ro_after_init = { &dev_attr_proto_down.attr, &dev_attr_carrier_up_count.attr, &dev_attr_carrier_down_count.attr, + &dev_attr_threaded.attr, NULL, }; ATTRIBUTE_GROUPS(net_class);