- Linuxarm - mailweb.openeuler.org

[PATCH] nvme: use octal permissions '0444' and '0644'
by yumeng 25 Mar '21

25 Mar '21

-------- 转发的消息 -------- 主题: [PATCH] nvme: use octal permissions '0444' and '0644' 日期: Thu, 25 Mar 2021 11:45:45 +0800 From: Meng Yu <yumeng18(a)huawei.com> 收件人: xuzaibo(a)huawei.com, wangzhou1(a)hisilicon.com, fanghao11(a)huawei.com, yekai13(a)huawei.com, qianweili(a)huawei.com, shenyang39(a)huawei.com, tanghui20(a)huawei.com, liulongfang(a)huawei.com, chensihang1(a)hisilicon.com, yumeng18(a)huawei.com, songzhiqi1(a)huawei.com Fixed following checkpatch warnings: 1. Symbolic permissions 'S_IRUGO | S_IWUSR' are not preferred. Consider using octal permissions '0644'; 2. Symbolic permissions 'S_IRUGO | S_IWUSR' are not preferred. Consider using octal permissions '0644'. Signed-off-by: Meng Yu <yumeng18(a)huawei.com> --- drivers/nvme/host/core.c | 30 +++++++++++++++--------------- drivers/nvme/host/lightnvm.c | 6 +++--- drivers/nvme/host/multipath.c | 6 +++--- drivers/nvme/host/pci.c | 4 ++-- 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index e68a8c4..aec94d2 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -2867,7 +2867,7 @@ static ssize_t nvme_subsys_show_nqn(struct device *dev, return sysfs_emit(buf, "%s\n", subsys->subnqn); } -static SUBSYS_ATTR_RO(subsysnqn, S_IRUGO, nvme_subsys_show_nqn); +static SUBSYS_ATTR_RO(subsysnqn, 0444, nvme_subsys_show_nqn); #define nvme_subsys_show_str_function(field) \ static ssize_t subsys_##field##_show(struct device *dev, \ @@ -2878,7 +2878,7 @@ static ssize_t subsys_##field##_show(struct device *dev, \ return sprintf(buf, "%.*s\n", \ (int)sizeof(subsys->field), subsys->field); \ } \ -static SUBSYS_ATTR_RO(field, S_IRUGO, subsys_##field##_show); +static SUBSYS_ATTR_RO(field, 0444, subsys_##field##_show); nvme_subsys_show_str_function(model); nvme_subsys_show_str_function(serial); @@ -3224,7 +3224,7 @@ int nvme_init_identify(struct nvme_ctrl *ctrl) ret = nvme_configure_apst(ctrl); if (ret < 0) return ret; - + ret = nvme_configure_timestamp(ctrl); if (ret < 0) return ret; @@ -3520,7 +3520,7 @@ static ssize_t field##_show(struct device *dev, \ return sprintf(buf, "%.*s\n", \ (int)sizeof(ctrl->subsys->field), ctrl->subsys->field); \ } \ -static DEVICE_ATTR(field, S_IRUGO, field##_show, NULL); +static DEVICE_ATTR(field, 0444, field##_show, NULL); nvme_show_str_function(model); nvme_show_str_function(serial); @@ -3533,7 +3533,7 @@ static ssize_t field##_show(struct device *dev, \ struct nvme_ctrl *ctrl = dev_get_drvdata(dev); \ return sprintf(buf, "%d\n", ctrl->field); \ } \ -static DEVICE_ATTR(field, S_IRUGO, field##_show, NULL); +static DEVICE_ATTR(field, 0444, field##_show, NULL); nvme_show_int_function(cntlid); nvme_show_int_function(numa_node); @@ -3560,7 +3560,7 @@ static ssize_t nvme_sysfs_show_transport(struct device *dev, return sysfs_emit(buf, "%s\n", ctrl->ops->name); } -static DEVICE_ATTR(transport, S_IRUGO, nvme_sysfs_show_transport, NULL); +static DEVICE_ATTR(transport, 0444, nvme_sysfs_show_transport, NULL); static ssize_t nvme_sysfs_show_state(struct device *dev, struct device_attribute *attr, @@ -3584,7 +3584,7 @@ static ssize_t nvme_sysfs_show_state(struct device *dev, return sprintf(buf, "unknown state\n"); } -static DEVICE_ATTR(state, S_IRUGO, nvme_sysfs_show_state, NULL); +static DEVICE_ATTR(state, 0444, nvme_sysfs_show_state, NULL); static ssize_t nvme_sysfs_show_subsysnqn(struct device *dev, struct device_attribute *attr, @@ -3594,7 +3594,7 @@ static ssize_t nvme_sysfs_show_subsysnqn(struct device *dev, return sysfs_emit(buf, "%s\n", ctrl->subsys->subnqn); } -static DEVICE_ATTR(subsysnqn, S_IRUGO, nvme_sysfs_show_subsysnqn, NULL); +static DEVICE_ATTR(subsysnqn, 0444, nvme_sysfs_show_subsysnqn, NULL); static ssize_t nvme_sysfs_show_hostnqn(struct device *dev, struct device_attribute *attr, @@ -3604,7 +3604,7 @@ static ssize_t nvme_sysfs_show_hostnqn(struct device *dev, return sysfs_emit(buf, "%s\n", ctrl->opts->host->nqn); } -static DEVICE_ATTR(hostnqn, S_IRUGO, nvme_sysfs_show_hostnqn, NULL); +static DEVICE_ATTR(hostnqn, 0444, nvme_sysfs_show_hostnqn, NULL); static ssize_t nvme_sysfs_show_hostid(struct device *dev, struct device_attribute *attr, @@ -3614,7 +3614,7 @@ static ssize_t nvme_sysfs_show_hostid(struct device *dev, return sysfs_emit(buf, "%pU\n", &ctrl->opts->host->id); } -static DEVICE_ATTR(hostid, S_IRUGO, nvme_sysfs_show_hostid, NULL); +static DEVICE_ATTR(hostid, 0444, nvme_sysfs_show_hostid, NULL); static ssize_t nvme_sysfs_show_address(struct device *dev, struct device_attribute *attr, @@ -3624,7 +3624,7 @@ static ssize_t nvme_sysfs_show_address(struct device *dev, return ctrl->ops->get_address(ctrl, buf, PAGE_SIZE); } -static DEVICE_ATTR(address, S_IRUGO, nvme_sysfs_show_address, NULL); +static DEVICE_ATTR(address, 0444, nvme_sysfs_show_address, NULL); static ssize_t nvme_ctrl_loss_tmo_show(struct device *dev, struct device_attribute *attr, char *buf) @@ -3656,8 +3656,8 @@ static ssize_t nvme_ctrl_loss_tmo_store(struct device *dev, opts->reconnect_delay); return count; } -static DEVICE_ATTR(ctrl_loss_tmo, S_IRUGO | S_IWUSR, - nvme_ctrl_loss_tmo_show, nvme_ctrl_loss_tmo_store); +static DEVICE_ATTR(ctrl_loss_tmo, 0644, nvme_ctrl_loss_tmo_show, + nvme_ctrl_loss_tmo_store); static ssize_t nvme_ctrl_reconnect_delay_show(struct device *dev, struct device_attribute *attr, char *buf) @@ -3683,8 +3683,8 @@ static ssize_t nvme_ctrl_reconnect_delay_store(struct device *dev, ctrl->opts->reconnect_delay = v; return count; } -static DEVICE_ATTR(reconnect_delay, S_IRUGO | S_IWUSR, - nvme_ctrl_reconnect_delay_show, nvme_ctrl_reconnect_delay_store); +static DEVICE_ATTR(reconnect_delay, 0644, nvme_ctrl_reconnect_delay_show, + nvme_ctrl_reconnect_delay_store); static struct attribute *nvme_dev_attrs[] = { &dev_attr_reset_controller.attr, diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c index b705988..ec25565 100644 --- a/drivers/nvme/host/lightnvm.c +++ b/drivers/nvme/host/lightnvm.c @@ -1137,11 +1137,11 @@ static ssize_t nvm_dev_attr_show_20(struct device *dev, } #define NVM_DEV_ATTR_RO(_name) \ - DEVICE_ATTR(_name, S_IRUGO, nvm_dev_attr_show, NULL) + DEVICE_ATTR(_name, 0444, nvm_dev_attr_show, NULL) #define NVM_DEV_ATTR_12_RO(_name) \ - DEVICE_ATTR(_name, S_IRUGO, nvm_dev_attr_show_12, NULL) + DEVICE_ATTR(_name, 0444, nvm_dev_attr_show_12, NULL) #define NVM_DEV_ATTR_20_RO(_name) \ - DEVICE_ATTR(_name, S_IRUGO, nvm_dev_attr_show_20, NULL) + DEVICE_ATTR(_name, 0444, nvm_dev_attr_show_20, NULL) /* general attributes */ static NVM_DEV_ATTR_RO(version); diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c index a1d476e..216604f 100644 --- a/drivers/nvme/host/multipath.c +++ b/drivers/nvme/host/multipath.c @@ -622,8 +622,8 @@ static ssize_t nvme_subsys_iopolicy_store(struct device *dev, return -EINVAL; } -SUBSYS_ATTR_RW(iopolicy, S_IRUGO | S_IWUSR, - nvme_subsys_iopolicy_show, nvme_subsys_iopolicy_store); +SUBSYS_ATTR_RW(iopolicy, 0644, nvme_subsys_iopolicy_show, + nvme_subsys_iopolicy_store); static ssize_t ana_grpid_show(struct device *dev, struct device_attribute *attr, char *buf) @@ -670,7 +670,7 @@ void nvme_mpath_add_disk(struct nvme_ns *ns, struct nvme_id_ns *id) nvme_update_ns_ana_state(&desc, ns); } } else { - ns->ana_state = NVME_ANA_OPTIMIZED; + ns->ana_state = NVME_ANA_OPTIMIZED; nvme_mpath_set_live(ns); } diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 17ab332..678eeba 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -1801,7 +1801,7 @@ static ssize_t nvme_cmb_show(struct device *dev, return scnprintf(buf, PAGE_SIZE, "cmbloc : x%08x\ncmbsz : x%08x\n", ndev->cmbloc, ndev->cmbsz); } -static DEVICE_ATTR(cmb, S_IRUGO, nvme_cmb_show, NULL); +static DEVICE_ATTR(cmb, 0444, nvme_cmb_show, NULL); static u64 nvme_cmb_size_unit(struct nvme_dev *dev) { @@ -2178,7 +2178,7 @@ static int nvme_setup_io_queues(struct nvme_dev *dev) if (nr_io_queues == 0) return 0; - + clear_bit(NVMEQ_ENABLED, &adminq->flags); if (dev->cmb_use_sqes) { -- 2.8.1 .

1 0

[PATCH v2] sched/topology: remove redundant cpumask_and in init_overlap_sched_group
by Barry Song 25 Mar '21

25 Mar '21

mask is built in build_balance_mask() by for_each_cpu(i, sg_span), so it must be a subset of sched_group_span(sg). Though cpumask_first_and doesn't lead to a wrong result of balance cpu, it is pointless to do cpumask_and again. Signed-off-by: Barry Song <song.bao.hua(a)hisilicon.com> Reviewed-by: Valentin Schneider <Valentin.Schneider(a)arm.com> --- -v2: add reviewed-by of Valentin, thanks! kernel/sched/topology.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c index f2066d682cd8..d1aec244c027 100644 --- a/kernel/sched/topology.c +++ b/kernel/sched/topology.c @@ -934,7 +934,7 @@ static void init_overlap_sched_group(struct sched_domain *sd, int cpu; build_balance_mask(sd, sg, mask); - cpu = cpumask_first_and(sched_group_span(sg), mask); + cpu = cpumask_first(mask); sg->sgc = *per_cpu_ptr(sdd->sgc, cpu); if (atomic_inc_return(&sg->sgc->ref) == 1) -- 2.25.1

1 0

[PATCH net v2] net: sched: fix packet stuck problem for lockless qdisc
by Yunsheng Lin 25 Mar '21

25 Mar '21

Lockless qdisc has below concurrent problem: cpu0 cpu1 . . q->enqueue . . . qdisc_run_begin() . . . dequeue_skb() . . . sch_direct_xmit() . . . . q->enqueue . qdisc_run_begin() . return and do nothing . . qdisc_run_end() . cpu1 enqueue a skb without calling __qdisc_run() because cpu0 has not released the lock yet and spin_trylock() return false for cpu1 in qdisc_run_begin(), and cpu0 do not see the skb enqueued by cpu1 when calling dequeue_skb() because cpu1 may enqueue the skb after cpu0 calling dequeue_skb() and before cpu0 calling qdisc_run_end(). Lockless qdisc has below another concurrent problem when tx_action is involved: cpu0(serving tx_action) cpu1 cpu2 . . . . q->enqueue . . qdisc_run_begin() . . dequeue_skb() . . . q->enqueue . . . . sch_direct_xmit() . . . qdisc_run_begin() . . return and do nothing . . . clear __QDISC_STATE_SCHED . . qdisc_run_begin() . . return and do nothing . . . . . . qdisc_run_end() . This patch fixes the above data race by: 1. Get the flag before doing spin_trylock(). 2. If the first spin_trylock() return false and the flag is not set before the first spin_trylock(), Set the flag and retry another spin_trylock() in case other CPU may not see the new flag after it releases the lock. 3. reschedule if the flags is set after the lock is released at the end of qdisc_run_end(). For tx_action case, the flags is also set when cpu1 is at the end if qdisc_run_end(), so tx_action will be rescheduled again to dequeue the skb enqueued by cpu2. Only clear the flag before retrying a dequeuing when dequeuing returns NULL in order to reduce the overhead of the above double spin_trylock() and __netif_schedule() calling. The performance impact of this patch, tested using pktgen and dummy netdev with pfifo_fast qdisc attached: threads without+this_patch with+this_patch delta 1 2.6Mpps 2.6Mpps +0.0% 2 3.9Mpps 3.8Mpps -2.5% 4 5.6Mpps 5.6Mpps -0.0% 8 2.7Mpps 2.8Mpps +3.7% 16 2.2Mpps 2.2Mpps +0.0% Fixes: 6b3ba9146fe6 ("net: sched: allow qdiscs to handle locking") Signed-off-by: Yunsheng Lin <linyunsheng(a)huawei.com> --- V2: Avoid the overhead of fixing the data race as much as possible. --- include/net/sch_generic.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++- net/sched/sch_generic.c | 12 ++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index f7a6e14..09a755d 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -36,6 +36,7 @@ struct qdisc_rate_table { enum qdisc_state_t { __QDISC_STATE_SCHED, __QDISC_STATE_DEACTIVATED, + __QDISC_STATE_NEED_RESCHEDULE, }; struct qdisc_size_table { @@ -159,12 +160,42 @@ static inline bool qdisc_is_empty(const struct Qdisc *qdisc) static inline bool qdisc_run_begin(struct Qdisc *qdisc) { if (qdisc->flags & TCQ_F_NOLOCK) { + bool dont_retry = test_bit(__QDISC_STATE_NEED_RESCHEDULE, + &qdisc->state); + + if (spin_trylock(&qdisc->seqlock)) + goto out; + + /* If the flag is set before doing the spin_trylock() and + * the above spin_trylock() return false, it means other cpu + * holding the lock will do dequeuing for us, or it wil see + * the flag set after releasing lock and reschedule the + * net_tx_action() to do the dequeuing. + */ + if (dont_retry) + return false; + + /* We could do set_bit() before the first spin_trylock(), + * and avoid doing secord spin_trylock() completely, then + * we could have multi cpus doing the test_bit(). Here use + * dont_retry to avoiding the test_bit() and the second + * spin_trylock(), which has 5% performance improvement than + * doing the set_bit() before the first spin_trylock(). + */ + set_bit(__QDISC_STATE_NEED_RESCHEDULE, + &qdisc->state); + + /* Retry again in case other CPU may not see the new flag + * after it releases the lock at the end of qdisc_run_end(). + */ if (!spin_trylock(&qdisc->seqlock)) return false; WRITE_ONCE(qdisc->empty, false); } else if (qdisc_is_running(qdisc)) { return false; } + +out: /* Variant of write_seqcount_begin() telling lockdep a trylock * was attempted. */ @@ -176,8 +207,23 @@ static inline bool qdisc_run_begin(struct Qdisc *qdisc) static inline void qdisc_run_end(struct Qdisc *qdisc) { write_seqcount_end(&qdisc->running); - if (qdisc->flags & TCQ_F_NOLOCK) + if (qdisc->flags & TCQ_F_NOLOCK) { spin_unlock(&qdisc->seqlock); + + /* qdisc_run_end() is protected by RCU lock, and + * qdisc reset will do a synchronize_net() after + * setting __QDISC_STATE_DEACTIVATED, so testing + * the below two bits separately should be fine. + * For qdisc_run() in net_tx_action() case, we + * really should provide rcu protection explicitly + * for document purposes or PREEMPT_RCU. + */ + if (unlikely(test_bit(__QDISC_STATE_NEED_RESCHEDULE, + &qdisc->state) && + !test_bit(__QDISC_STATE_DEACTIVATED, + &qdisc->state))) + __netif_schedule(qdisc); + } } static inline bool qdisc_may_bulk(const struct Qdisc *qdisc) diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c index 44991ea..7e3426b 100644 --- a/net/sched/sch_generic.c +++ b/net/sched/sch_generic.c @@ -640,8 +640,10 @@ static struct sk_buff *pfifo_fast_dequeue(struct Qdisc *qdisc) { struct pfifo_fast_priv *priv = qdisc_priv(qdisc); struct sk_buff *skb = NULL; + bool need_retry = true; int band; +retry: for (band = 0; band < PFIFO_FAST_BANDS && !skb; band++) { struct skb_array *q = band2list(priv, band); @@ -652,6 +654,16 @@ static struct sk_buff *pfifo_fast_dequeue(struct Qdisc *qdisc) } if (likely(skb)) { qdisc_update_stats_at_dequeue(qdisc, skb); + } else if (need_retry && + test_and_clear_bit(__QDISC_STATE_NEED_RESCHEDULE, + &qdisc->stat)) { + /* do another enqueuing after clearing the flag to + * avoid calling __netif_schedule(). + */ + smp_mb__after_atomic(); + need_retry = false; + + goto retry; } else { WRITE_ONCE(qdisc->empty, true); } -- 2.7.4

2 2

[PATCH] ACPI: AC: fix some errors and warnings reported by checkpatch.pl
by Xiaofei Tan 25 Mar '21

25 Mar '21

Fix some errors and warnings reported by checkpatch.pl, including following five types: ERROR: "foo * bar" should be "foo *bar" ERROR: code indent should use tabs where possible WARNING: Block comments use a trailing */ on a separate line WARNING: braces {} are not necessary for single statement blocks WARNING: void function return statements are not generally useful Signed-off-by: Xiaofei Tan <tanxiaofei(a)huawei.com> --- drivers/acpi/ac.c | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c index b86ee6e..07987854 100644 --- a/drivers/acpi/ac.c +++ b/drivers/acpi/ac.c @@ -78,17 +78,16 @@ static struct acpi_driver acpi_ac_driver = { struct acpi_ac { struct power_supply *charger; struct power_supply_desc charger_desc; - struct acpi_device * device; + struct acpi_device *device; unsigned long long state; struct notifier_block battery_nb; }; #define to_acpi_ac(x) power_supply_get_drvdata(x) -/* -------------------------------------------------------------------------- - AC Adapter Management - -------------------------------------------------------------------------- */ - +/* + * AC Adapter Management + */ static int acpi_ac_get_state(struct acpi_ac *ac) { acpi_status status = AE_OK; @@ -109,9 +108,9 @@ static int acpi_ac_get_state(struct acpi_ac *ac) return 0; } -/* -------------------------------------------------------------------------- - sysfs I/F - -------------------------------------------------------------------------- */ +/* + * sysfs I/F + */ static int get_ac_property(struct power_supply *psy, enum power_supply_property psp, union power_supply_propval *val) @@ -138,10 +137,9 @@ static enum power_supply_property ac_props[] = { POWER_SUPPLY_PROP_ONLINE, }; -/* -------------------------------------------------------------------------- - Driver Model - -------------------------------------------------------------------------- */ - +/* + * Driver Model + */ static void acpi_ac_notify(struct acpi_device *device, u32 event) { struct acpi_ac *ac = acpi_driver_data(device); @@ -174,8 +172,6 @@ static void acpi_ac_notify(struct acpi_device *device, u32 event) acpi_notifier_call_chain(device, event, (u32) ac->state); kobject_uevent(&ac->charger->dev.kobj, KOBJ_CHANGE); } - - return; } static int acpi_ac_battery_notify(struct notifier_block *nb, @@ -282,9 +278,8 @@ static int acpi_ac_add(struct acpi_device *device) ac->battery_nb.notifier_call = acpi_ac_battery_notify; register_acpi_notifier(&ac->battery_nb); end: - if (result) { + if (result) kfree(ac); - } return result; } @@ -293,7 +288,7 @@ static int acpi_ac_add(struct acpi_device *device) static int acpi_ac_resume(struct device *dev) { struct acpi_ac *ac; - unsigned old_state; + unsigned int old_state; if (!dev) return -EINVAL; @@ -352,9 +347,8 @@ static int __init acpi_ac_init(void) } result = acpi_bus_register_driver(&acpi_ac_driver); - if (result < 0) { + if (result < 0) return -ENODEV; - } return 0; } -- 2.8.1

2 3

[PATCH for-next 0/2] RDMA/hns: Support to select congestion control algorithm
by Weihang Li 24 Mar '21

24 Mar '21

The configuration of congestion control algorithm is recorded in the firmware, the driver queries it and then sets it to the hardware. Wei Xu (1): RDMA/hns: Support query information of functions from FW Yangyang Li (1): RDMA/hns: Support congestion control type selection according to the FW drivers/infiniband/hw/hns/hns_roce_device.h | 11 ++ drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 192 ++++++++++++++++++++++++++++ drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 34 ++++- drivers/infiniband/hw/hns/hns_roce_main.c | 2 + 4 files changed, 236 insertions(+), 3 deletions(-) -- 2.8.1

3 6

Re: [bug report] net: hns3: vf indexing in hclge_add_fd_entry()
by shenjian (K) 24 Mar '21

24 Mar '21

Hello Dan, Thanks for your report! 在 2021/3/24 15:01, Dan Carpenter 写道: > Hello Jian Shen, > > The patch 5f2b1238b33c: "net: hns3: refactor out > hclge_add_fd_entry()" from Mar 22, 2021, leads to the following > static checker warning: > > drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c:6512 hclge_fd_parse_ring_cookie() > warn: array off by one? 'hdev->vport[vf]' > > drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c > 6493 static int hclge_fd_parse_ring_cookie(struct hclge_dev *hdev, u64 ring_cookie, > 6494 u16 *vport_id, u8 *action, u16 *queue_id) > 6495 { > 6496 struct hclge_vport *vport = hdev->vport; > 6497 > 6498 if (ring_cookie == RX_CLS_FLOW_DISC) { > 6499 *action = HCLGE_FD_ACTION_DROP_PACKET; > 6500 } else { > 6501 u32 ring = ethtool_get_flow_spec_ring(ring_cookie); > 6502 u8 vf = ethtool_get_flow_spec_ring_vf(ring_cookie); > 6503 u16 tqps; > 6504 > 6505 if (vf > hdev->num_req_vfs) { > ^^^^^^^^^^^^^^^^^^^^^^ > > The is off by one but checking hdev->num_req_vfs in this context doesn't > make sense. Should it instead be check hdev->num_alloc_vport? Also > should we add HCLGE_VF_VPORT_START_NUM? > > vf = ethtool_get_flow_spec_ring_vf(ring_cookie); > vf += HCLGE_VF_VPORT_START_NUM; > if (vf >= hdev->num_alloc_vport) > return -EINVAL; > > 6506 dev_err(&hdev->pdev->dev, > 6507 "Error: vf id (%u) > max vf num (%u)\n", > ^^ > Use >= > > 6508 vf, hdev->num_req_vfs); > 6509 return -EINVAL; > 6510 } > 6511 > 6512 *vport_id = vf ? hdev->vport[vf].vport_id : vport->vport_id; > ^^^^^^^^^ > 6513 tqps = hdev->vport[vf].nic.kinfo.num_tqps; > ^^^ > The vport array has hdev->num_vmdq_vport + hdev->num_req_vfs + 1; > elements. ->vport[0] is tqp_main_vport. The next elements are > hdev->num_vmdq_vport and the last part of the array is hdev->num_req_vfs. > > Another possibility is that perhaps this is what was intended? > > idx = vf + 1 + hdev->num_vmdq_vport; > > *vport_id = vf ? vport[idx].vport_id : vport[0].vport_id; > tqps = vport[idx].nic.kinfo.num_tqps; > > There is related code that offers clues but I'm not sure what to do. > > 6514 > 6515 if (ring >= tqps) { > 6516 dev_err(&hdev->pdev->dev, > 6517 "Error: queue id (%u) > max tqp num (%u)\n", > 6518 ring, tqps - 1); > 6519 return -EINVAL; > 6520 } > 6521 > 6522 *action = HCLGE_FD_ACTION_SELECT_QUEUE; > 6523 *queue_id = ring; > 6524 } > 6525 > 6526 return 0; > 6527 } > > [ snip ] > > 9111 static bool hclge_check_vf_mac_exist(struct hclge_vport *vport, int vf_idx, > 9112 u8 *mac_addr) > 9113 { > 9114 struct hclge_mac_vlan_tbl_entry_cmd req; > 9115 struct hclge_dev *hdev = vport->back; > 9116 struct hclge_desc desc; > 9117 u16 egress_port = 0; > 9118 int i; > 9119 > 9120 if (is_zero_ether_addr(mac_addr)) > 9121 return false; > 9122 > 9123 memset(&req, 0, sizeof(req)); > 9124 hnae3_set_field(egress_port, HCLGE_MAC_EPORT_VFID_M, > 9125 HCLGE_MAC_EPORT_VFID_S, vport->vport_id); > 9126 req.egress_port = cpu_to_le16(egress_port); > 9127 hclge_prepare_mac_addr(&req, mac_addr, false); > 9128 > 9129 if (hclge_lookup_mac_vlan_tbl(vport, &req, &desc, false) != -ENOENT) > 9130 return true; > 9131 > 9132 vf_idx += HCLGE_VF_VPORT_START_NUM; > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > We're are skipping the first element. Should it instead be?: > > vf_idx += hdev->num_vmdq_vport + 1; > > 9133 for (i = hdev->num_vmdq_vport + 1; i < hdev->num_alloc_vport; i++) > ^^^^^^^^^^^^^^^^^^^^^^^^ > We're only checking the last part of the array. > > 9134 if (i != vf_idx && > 9135 ether_addr_equal(mac_addr, hdev->vport[i].vf_info.mac)) > 9136 return true; > 9137 > 9138 return false; > 9139 } > > Another thing that's not clear to me is how pci_num_vf() relates to > this. I suspect that it is the same as hdev->num_vmdq_vport, but I > can't be sure. > > regards, > dan carpenter > . The use for num_vmdq_vport is confusing. At the beginning, the HNS3 driver is planed to support VMDQ . Whereafter the hardware supports SR-IOV with better performance. So VMDQ feature is discarded, but the codes of vmdq is remained. For the value of num_vmdq_vport is always 0, so the vport id of VF is actually start from 1. That's why the driver still work. As the num_vmdq_vport is actually useless and confusing, I will send a patch to remove it soon. regards, Jian Shen

1 0

[PATCH RFC] net: sched: implement TCQ_F_CAN_BYPASS for lockless qdisc
by Yunsheng Lin 24 Mar '21

24 Mar '21

Currently pfifo_fast has both TCQ_F_CAN_BYPASS and TCQ_F_NOLOCK flag set, but queue discipline by-pass does not work for lockless qdisc because skb is always enqueued to qdisc even when the qdisc is empty, see __dev_xmit_skb(). This patch calles sch_direct_xmit() to transmit the skb directly to the driver for empty lockless qdisc too, which aviod enqueuing and dequeuing operation. qdisc->empty is set to false whenever a skb is enqueued, and is set to true when skb dequeuing return NULL, see pfifo_fast_dequeue(). Also, qdisc is scheduled at the end of qdisc_run_end() when q->empty is false to avoid packet stuck problem. The performance for ip_forward test increases about 10% with this patch. Signed-off-by: Yunsheng Lin <linyunsheng(a)huawei.com> --- include/net/sch_generic.h | 7 +++++-- net/core/dev.c | 11 +++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index 2d6eb60..6591356 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -161,7 +161,6 @@ static inline bool qdisc_run_begin(struct Qdisc *qdisc) if (qdisc->flags & TCQ_F_NOLOCK) { if (!spin_trylock(&qdisc->seqlock)) return false; - WRITE_ONCE(qdisc->empty, false); } else if (qdisc_is_running(qdisc)) { return false; } @@ -176,8 +175,12 @@ static inline bool qdisc_run_begin(struct Qdisc *qdisc) static inline void qdisc_run_end(struct Qdisc *qdisc) { write_seqcount_end(&qdisc->running); - if (qdisc->flags & TCQ_F_NOLOCK) + if (qdisc->flags & TCQ_F_NOLOCK) { spin_unlock(&qdisc->seqlock); + + if (unlikely(!READ_ONCE(qdisc->empty))) + __netif_schedule(qdisc); + } } static inline bool qdisc_may_bulk(const struct Qdisc *qdisc) diff --git a/net/core/dev.c b/net/core/dev.c index 2bfdd52..fa8504d 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -3791,7 +3791,18 @@ static inline int __dev_xmit_skb(struct sk_buff *skb, struct Qdisc *q, qdisc_calculate_pkt_len(skb, q); if (q->flags & TCQ_F_NOLOCK) { + if (q->flags & TCQ_F_CAN_BYPASS && READ_ONCE(q->empty) && qdisc_run_begin(q)) { + qdisc_bstats_cpu_update(q, skb); + + if (sch_direct_xmit(skb, q, dev, txq, NULL, true) && !READ_ONCE(q->empty)) + __qdisc_run(q); + + qdisc_run_end(q); + return NET_XMIT_SUCCESS; + } + rc = q->enqueue(skb, q, &to_free) & NET_XMIT_MASK; + WRITE_ONCE(q->empty, false); qdisc_run(q); if (unlikely(to_free)) -- 2.7.4

9 25

[PATCH] dma-mapping: make map_benchmark compile into module
by Tian Tao 24 Mar '21

24 Mar '21

under some scenarios, it is necessary to compile map_benchmark into module to test iommu, so this patch changed Kconfig and export_symbol to implement map_benchmark compiled into module. Signed-off-by: Tian Tao <tiantao6(a)hisilicon.com> --- kernel/dma/Kconfig | 2 +- kernel/kthread.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig index 77b4055..0468293 100644 --- a/kernel/dma/Kconfig +++ b/kernel/dma/Kconfig @@ -223,7 +223,7 @@ config DMA_API_DEBUG_SG If unsure, say N. config DMA_MAP_BENCHMARK - bool "Enable benchmarking of streaming DMA mapping" + tristate "Enable benchmarking of streaming DMA mapping" depends on DEBUG_FS help Provides /sys/kernel/debug/dma_map_benchmark that helps with testing diff --git a/kernel/kthread.c b/kernel/kthread.c index 1578973..fa4736f 100644 --- a/kernel/kthread.c +++ b/kernel/kthread.c @@ -455,6 +455,7 @@ void kthread_bind_mask(struct task_struct *p, const struct cpumask *mask) { __kthread_bind_mask(p, mask, TASK_UNINTERRUPTIBLE); } +EXPORT_SYMBOL(kthread_bind_mask); /** * kthread_bind - bind a just-created kthread to a cpu. -- 2.7.4

2 1

[PATCH net] net: sched: fix packet stuck problem for lockless qdisc
by Yunsheng Lin 23 Mar '21

23 Mar '21

Lockless qdisc has below concurrent problem: cpu0 cpu1 . . q->enqueue . . . qdisc_run_begin() . . . dequeue_skb() . . . sch_direct_xmit() . . . . q->enqueue . qdisc_run_begin() . return and do nothing . . qdisc_run_end() . cpu1 enqueue a skb without calling __qdisc_run() because cpu0 has not released the lock yet and spin_trylock() return false for cpu1 in qdisc_run_begin(), and cpu0 do not see the skb enqueued by cpu1 when calling dequeue_skb() because cpu1 may enqueue the skb after cpu0 calling dequeue_skb() and before cpu0 calling qdisc_run_end(). Lockless qdisc has another concurrent problem when tx_action is involved: cpu0(serving tx_action) cpu1 cpu2 . . . . q->enqueue . . qdisc_run_begin() . . dequeue_skb() . . . q->enqueue . . . . sch_direct_xmit() . . . qdisc_run_begin() . . return and do nothing . . . clear __QDISC_STATE_SCHED . . qdisc_run_begin() . . return and do nothing . . . . . . qdisc_run_begin() . This patch fixes the above data race by: 1. Set a flag after spin_trylock() return false. 2. Retry a spin_trylock() in case other CPU may not see the new flag after it releases the lock. 3. reschedule if the flag is set after the lock is released at the end of qdisc_run_end(). For tx_action case, the flags is also set when cpu1 is at the end if qdisc_run_begin(), so tx_action will be rescheduled again to dequeue the skb enqueued by cpu2. Also clear the flag before dequeuing in order to reduce the overhead of the above process, and aviod doing the heavy test_and_clear_bit() at the end of qdisc_run_end(). Fixes: 6b3ba9146fe6 ("net: sched: allow qdiscs to handle locking") Signed-off-by: Yunsheng Lin <linyunsheng(a)huawei.com> --- For those who has not been following the qdsic scheduling discussion, there is packet stuck problem for lockless qdisc, see [1], and I has done some cleanup and added some enhanced features too, see [2] [3]. While I was doing the optimization for lockless qdisc, it accurred to me that these optimization is useless if there is still basic bug in lockless qdisc, even the bug is not easily reproducible. So look through [1] again, I found that the data race for tx action mentioned by Michael, and thought deep about it and came up with this patch trying to fix it. So I am really appreciated some who still has the reproducer can try this patch and report back. 1. https://lore.kernel.org/netdev/d102074f-7489-e35a-98cf-e2cad7efd8a2@netroun… 2. https://patchwork.kernel.org/project/netdevbpf/patch/1615777818-13969-1-git… 3. https://patchwork.kernel.org/project/netdevbpf/patch/1615800610-34700-1-git… --- include/net/sch_generic.h | 23 ++++++++++++++++++++--- net/sched/sch_generic.c | 1 + 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index f7a6e14..4220eab 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -36,6 +36,7 @@ struct qdisc_rate_table { enum qdisc_state_t { __QDISC_STATE_SCHED, __QDISC_STATE_DEACTIVATED, + __QDISC_STATE_NEED_RESCHEDULE, }; struct qdisc_size_table { @@ -159,8 +160,17 @@ static inline bool qdisc_is_empty(const struct Qdisc *qdisc) static inline bool qdisc_run_begin(struct Qdisc *qdisc) { if (qdisc->flags & TCQ_F_NOLOCK) { - if (!spin_trylock(&qdisc->seqlock)) - return false; + if (!spin_trylock(&qdisc->seqlock)) { + set_bit(__QDISC_STATE_NEED_RESCHEDULE, + &qdisc->state); + + /* Retry again in case other CPU may not see the + * new flags after it releases the lock at the + * end of qdisc_run_end(). + */ + if (!spin_trylock(&qdisc->seqlock)) + return false; + } WRITE_ONCE(qdisc->empty, false); } else if (qdisc_is_running(qdisc)) { return false; @@ -176,8 +186,15 @@ static inline bool qdisc_run_begin(struct Qdisc *qdisc) static inline void qdisc_run_end(struct Qdisc *qdisc) { write_seqcount_end(&qdisc->running); - if (qdisc->flags & TCQ_F_NOLOCK) + if (qdisc->flags & TCQ_F_NOLOCK) { spin_unlock(&qdisc->seqlock); + + if (unlikely(test_bit(__QDISC_STATE_NEED_RESCHEDULE, + &qdisc->state) && + !test_bit(__QDISC_STATE_DEACTIVATED, + &qdisc->state))) + __netif_schedule(qdisc); + } } static inline bool qdisc_may_bulk(const struct Qdisc *qdisc) diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c index 44991ea..25d75d8 100644 --- a/net/sched/sch_generic.c +++ b/net/sched/sch_generic.c @@ -205,6 +205,7 @@ static struct sk_buff *dequeue_skb(struct Qdisc *q, bool *validate, const struct netdev_queue *txq = q->dev_queue; struct sk_buff *skb = NULL; + clear_bit(__QDISC_STATE_NEED_RESCHEDULE, &q->state); *packets = 1; if (unlikely(!skb_queue_empty(&q->gso_skb))) { spinlock_t *lock = NULL; -- 2.7.4

4 11

[PATCH net-next 0/8] net: hns: add some cleanups
by Huazhong Tan 23 Mar '21

23 Mar '21

This series includes some cleanups for the HNS ethernet driver. Huazhong Tan (6): net: hns: remove unused get_autoneg() net: hns: remove unused set_autoneg() net: hns: remove unused set_rx_ignore_pause_frames() net: hns: remove unused config_half_duplex() net: hns: remove unused NIC_LB_TEST_RX_PKG_ERR net: hns: remove unused HNS_LED_PC_REG Yonglong Liu (2): net: hns: remove unnecessary !! operation in hns_mac_config_sds_loopback_acpi() net: hns: remove redundant variable initialization drivers/net/ethernet/hisilicon/hns/hnae.h | 6 ------ drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c | 22 +--------------------- drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c | 18 ------------------ drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c | 2 +- drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h | 4 ---- drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c | 16 ++++++++-------- drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c | 2 +- drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c | 4 ++-- .../net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c | 18 +----------------- drivers/net/ethernet/hisilicon/hns/hns_enet.c | 4 ++-- drivers/net/ethernet/hisilicon/hns/hns_ethtool.c | 3 +-- drivers/net/ethernet/hisilicon/hns_mdio.c | 4 ++-- 12 files changed, 19 insertions(+), 84 deletions(-) -- 2.7.4

1 8