- Linuxarm - mailweb.openeuler.org

[PATCH v2] sched/topology: fix the issue groups don't span domain->span for NUMA diameter > 2
by Barry Song 19 Feb '21

19 Feb '21

As long as NUMA diameter > 2, building sched_domain by sibling's child domain will definitely create a sched_domain with sched_group which will span out of the sched_domain: +------+ +------+ +-------+ +------+ | node | 12 |node | 20 | node | 12 |node | | 0 +---------+1 +--------+ 2 +-------+3 | +------+ +------+ +-------+ +------+ domain0 node0 node1 node2 node3 domain1 node0+1 node0+1 node2+3 node2+3 + domain2 node0+1+2 | group: node0+1 | group:node2+3 <-------------------+ when node2 is added into the domain2 of node0, kernel is using the child domain of node2's domain2, which is domain1(node2+3). Node 3 is outside the span of the domain including node0+1+2. This will make load_balance() run based on screwed avg_load and group_type in the sched_group spanning out of the sched_domain, and it also makes select_task_rq_fair() pick an idle CPU out of the sched_domain. Real servers which suffer from this problem include Kunpeng920 and 8-node Sun Fire X4600-M2, at least. Here we move to use the *child* domain of the *child* domain of node2's domain2 as the new added sched_group. At the same time, we re-use the lower level sgc directly. +------+ +------+ +-------+ +------+ | node | 12 |node | 20 | node | 12 |node | | 0 +---------+1 +--------+ 2 +-------+3 | +------+ +------+ +-------+ +------+ domain0 node0 node1 +- node2 node3 | domain1 node0+1 node0+1 | node2+3 node2+3 | domain2 node0+1+2 | group: node0+1 | group:node2 <-------------------+ Tested by the below topology: qemu-system-aarch64 -M virt -nographic \ -smp cpus=8 \ -numa node,cpus=0-1,nodeid=0 \ -numa node,cpus=2-3,nodeid=1 \ -numa node,cpus=4-5,nodeid=2 \ -numa node,cpus=6-7,nodeid=3 \ -numa dist,src=0,dst=1,val=12 \ -numa dist,src=0,dst=2,val=20 \ -numa dist,src=0,dst=3,val=22 \ -numa dist,src=1,dst=2,val=22 \ -numa dist,src=2,dst=3,val=12 \ -numa dist,src=1,dst=3,val=24 \ -m 4G -cpu cortex-a57 -kernel arch/arm64/boot/Image w/o patch, we get lots of "groups don't span domain->span": [ 0.802139] CPU0 attaching sched-domain(s): [ 0.802193] domain-0: span=0-1 level=MC [ 0.802443] groups: 0:{ span=0 cap=1013 }, 1:{ span=1 cap=979 } [ 0.802693] domain-1: span=0-3 level=NUMA [ 0.802731] groups: 0:{ span=0-1 cap=1992 }, 2:{ span=2-3 cap=1943 } [ 0.802811] domain-2: span=0-5 level=NUMA [ 0.802829] groups: 0:{ span=0-3 cap=3935 }, 4:{ span=4-7 cap=3937 } [ 0.802881] ERROR: groups don't span domain->span [ 0.803058] domain-3: span=0-7 level=NUMA [ 0.803080] groups: 0:{ span=0-5 mask=0-1 cap=5843 }, 6:{ span=4-7 mask=6-7 cap=4077 } [ 0.804055] CPU1 attaching sched-domain(s): [ 0.804072] domain-0: span=0-1 level=MC [ 0.804096] groups: 1:{ span=1 cap=979 }, 0:{ span=0 cap=1013 } [ 0.804152] domain-1: span=0-3 level=NUMA [ 0.804170] groups: 0:{ span=0-1 cap=1992 }, 2:{ span=2-3 cap=1943 } [ 0.804219] domain-2: span=0-5 level=NUMA [ 0.804236] groups: 0:{ span=0-3 cap=3935 }, 4:{ span=4-7 cap=3937 } [ 0.804302] ERROR: groups don't span domain->span [ 0.804520] domain-3: span=0-7 level=NUMA [ 0.804546] groups: 0:{ span=0-5 mask=0-1 cap=5843 }, 6:{ span=4-7 mask=6-7 cap=4077 } [ 0.804677] CPU2 attaching sched-domain(s): [ 0.804687] domain-0: span=2-3 level=MC [ 0.804705] groups: 2:{ span=2 cap=934 }, 3:{ span=3 cap=1009 } [ 0.804754] domain-1: span=0-3 level=NUMA [ 0.804772] groups: 2:{ span=2-3 cap=1943 }, 0:{ span=0-1 cap=1992 } [ 0.804820] domain-2: span=0-5 level=NUMA [ 0.804836] groups: 2:{ span=0-3 mask=2-3 cap=3991 }, 4:{ span=0-1,4-7 mask=4-5 cap=5985 } [ 0.804944] ERROR: groups don't span domain->span [ 0.805108] domain-3: span=0-7 level=NUMA [ 0.805134] groups: 2:{ span=0-5 mask=2-3 cap=5899 }, 6:{ span=0-1,4-7 mask=6-7 cap=6125 } [ 0.805223] CPU3 attaching sched-domain(s): [ 0.805232] domain-0: span=2-3 level=MC [ 0.805249] groups: 3:{ span=3 cap=1009 }, 2:{ span=2 cap=934 } [ 0.805319] domain-1: span=0-3 level=NUMA [ 0.805336] groups: 2:{ span=2-3 cap=1943 }, 0:{ span=0-1 cap=1992 } [ 0.805383] domain-2: span=0-5 level=NUMA [ 0.805399] groups: 2:{ span=0-3 mask=2-3 cap=3991 }, 4:{ span=0-1,4-7 mask=4-5 cap=5985 } [ 0.805458] ERROR: groups don't span domain->span [ 0.805605] domain-3: span=0-7 level=NUMA [ 0.805626] groups: 2:{ span=0-5 mask=2-3 cap=5899 }, 6:{ span=0-1,4-7 mask=6-7 cap=6125 } [ 0.805712] CPU4 attaching sched-domain(s): [ 0.805721] domain-0: span=4-5 level=MC [ 0.805738] groups: 4:{ span=4 cap=984 }, 5:{ span=5 cap=924 } [ 0.805787] domain-1: span=4-7 level=NUMA [ 0.805803] groups: 4:{ span=4-5 cap=1908 }, 6:{ span=6-7 cap=2029 } [ 0.805851] domain-2: span=0-1,4-7 level=NUMA [ 0.805867] groups: 4:{ span=4-7 cap=3937 }, 0:{ span=0-3 cap=3935 } [ 0.805915] ERROR: groups don't span domain->span [ 0.806108] domain-3: span=0-7 level=NUMA [ 0.806130] groups: 4:{ span=0-1,4-7 mask=4-5 cap=5985 }, 2:{ span=0-3 mask=2-3 cap=3991 } [ 0.806214] CPU5 attaching sched-domain(s): [ 0.806222] domain-0: span=4-5 level=MC [ 0.806240] groups: 5:{ span=5 cap=924 }, 4:{ span=4 cap=984 } [ 0.806841] domain-1: span=4-7 level=NUMA [ 0.806866] groups: 4:{ span=4-5 cap=1908 }, 6:{ span=6-7 cap=2029 } [ 0.806934] domain-2: span=0-1,4-7 level=NUMA [ 0.806953] groups: 4:{ span=4-7 cap=3937 }, 0:{ span=0-3 cap=3935 } [ 0.807004] ERROR: groups don't span domain->span [ 0.807312] domain-3: span=0-7 level=NUMA [ 0.807386] groups: 4:{ span=0-1,4-7 mask=4-5 cap=5985 }, 2:{ span=0-3 mask=2-3 cap=3991 } [ 0.807686] CPU6 attaching sched-domain(s): [ 0.807710] domain-0: span=6-7 level=MC [ 0.807750] groups: 6:{ span=6 cap=1017 }, 7:{ span=7 cap=1012 } [ 0.807840] domain-1: span=4-7 level=NUMA [ 0.807870] groups: 6:{ span=6-7 cap=2029 }, 4:{ span=4-5 cap=1908 } [ 0.807952] domain-2: span=0-1,4-7 level=NUMA [ 0.807985] groups: 6:{ span=4-7 mask=6-7 cap=4077 }, 0:{ span=0-5 mask=0-1 cap=5843 } [ 0.808045] ERROR: groups don't span domain->span [ 0.808257] domain-3: span=0-7 level=NUMA [ 0.808571] groups: 6:{ span=0-1,4-7 mask=6-7 cap=6125 }, 2:{ span=0-5 mask=2-3 cap=5899 } [ 0.808848] CPU7 attaching sched-domain(s): [ 0.808860] domain-0: span=6-7 level=MC [ 0.808880] groups: 7:{ span=7 cap=1012 }, 6:{ span=6 cap=1017 } [ 0.808953] domain-1: span=4-7 level=NUMA [ 0.808974] groups: 6:{ span=6-7 cap=2029 }, 4:{ span=4-5 cap=1908 } [ 0.809034] domain-2: span=0-1,4-7 level=NUMA [ 0.809055] groups: 6:{ span=4-7 mask=6-7 cap=4077 }, 0:{ span=0-5 mask=0-1 cap=5843 } [ 0.809128] ERROR: groups don't span domain->span [ 0.810361] domain-3: span=0-7 level=NUMA [ 0.810400] groups: 6:{ span=0-1,4-7 mask=6-7 cap=5961 }, 2:{ span=0-5 mask=2-3 cap=5903 } w/ patch, we don't get "groups don't span domain->span" any more: [ 1.486271] CPU0 attaching sched-domain(s): [ 1.486820] domain-0: span=0-1 level=MC [ 1.500924] groups: 0:{ span=0 cap=980 }, 1:{ span=1 cap=994 } [ 1.515717] domain-1: span=0-3 level=NUMA [ 1.515903] groups: 0:{ span=0-1 cap=1974 }, 2:{ span=2-3 cap=1989 } [ 1.516989] domain-2: span=0-5 level=NUMA [ 1.517124] groups: 0:{ span=0-3 cap=3963 }, 4:{ span=4-5 cap=1949 } [ 1.517369] domain-3: span=0-7 level=NUMA [ 1.517423] groups: 0:{ span=0-5 mask=0-1 cap=5912 }, 6:{ span=4-7 mask=6-7 cap=4054 } [ 1.520027] CPU1 attaching sched-domain(s): [ 1.520097] domain-0: span=0-1 level=MC [ 1.520184] groups: 1:{ span=1 cap=994 }, 0:{ span=0 cap=980 } [ 1.520429] domain-1: span=0-3 level=NUMA [ 1.520487] groups: 0:{ span=0-1 cap=1974 }, 2:{ span=2-3 cap=1989 } [ 1.520687] domain-2: span=0-5 level=NUMA [ 1.520744] groups: 0:{ span=0-3 cap=3963 }, 4:{ span=4-5 cap=1949 } [ 1.520948] domain-3: span=0-7 level=NUMA [ 1.521038] groups: 0:{ span=0-5 mask=0-1 cap=5912 }, 6:{ span=4-7 mask=6-7 cap=4054 } [ 1.522068] CPU2 attaching sched-domain(s): [ 1.522348] domain-0: span=2-3 level=MC [ 1.522606] groups: 2:{ span=2 cap=1003 }, 3:{ span=3 cap=986 } [ 1.522832] domain-1: span=0-3 level=NUMA [ 1.522885] groups: 2:{ span=2-3 cap=1989 }, 0:{ span=0-1 cap=1974 } [ 1.523043] domain-2: span=0-5 level=NUMA [ 1.523092] groups: 2:{ span=0-3 mask=2-3 cap=4037 }, 4:{ span=4-5 cap=1949 } [ 1.523302] domain-3: span=0-7 level=NUMA [ 1.523352] groups: 2:{ span=0-5 mask=2-3 cap=5986 }, 6:{ span=0-1,4-7 mask=6-7 cap=6102 } [ 1.523748] CPU3 attaching sched-domain(s): [ 1.523774] domain-0: span=2-3 level=MC [ 1.523825] groups: 3:{ span=3 cap=986 }, 2:{ span=2 cap=1003 } [ 1.524009] domain-1: span=0-3 level=NUMA [ 1.524086] groups: 2:{ span=2-3 cap=1989 }, 0:{ span=0-1 cap=1974 } [ 1.524281] domain-2: span=0-5 level=NUMA [ 1.524331] groups: 2:{ span=0-3 mask=2-3 cap=4037 }, 4:{ span=4-5 cap=1949 } [ 1.524534] domain-3: span=0-7 level=NUMA [ 1.524586] groups: 2:{ span=0-5 mask=2-3 cap=5986 }, 6:{ span=0-1,4-7 mask=6-7 cap=6102 } [ 1.524847] CPU4 attaching sched-domain(s): [ 1.524873] domain-0: span=4-5 level=MC [ 1.524954] groups: 4:{ span=4 cap=958 }, 5:{ span=5 cap=991 } [ 1.525105] domain-1: span=4-7 level=NUMA [ 1.525153] groups: 4:{ span=4-5 cap=1949 }, 6:{ span=6-7 cap=2006 } [ 1.525368] domain-2: span=0-1,4-7 level=NUMA [ 1.525428] groups: 4:{ span=4-7 cap=3955 }, 0:{ span=0-1 cap=1974 } [ 1.532726] domain-3: span=0-7 level=NUMA [ 1.532811] groups: 4:{ span=0-1,4-7 mask=4-5 cap=6003 }, 2:{ span=0-3 mask=2-3 cap=4037 } [ 1.534125] CPU5 attaching sched-domain(s): [ 1.534159] domain-0: span=4-5 level=MC [ 1.534303] groups: 5:{ span=5 cap=991 }, 4:{ span=4 cap=958 } [ 1.534490] domain-1: span=4-7 level=NUMA [ 1.534572] groups: 4:{ span=4-5 cap=1949 }, 6:{ span=6-7 cap=2006 } [ 1.534734] domain-2: span=0-1,4-7 level=NUMA [ 1.534783] groups: 4:{ span=4-7 cap=3955 }, 0:{ span=0-1 cap=1974 } [ 1.536057] domain-3: span=0-7 level=NUMA [ 1.536430] groups: 4:{ span=0-1,4-7 mask=4-5 cap=6003 }, 2:{ span=0-3 mask=2-3 cap=3896 } [ 1.536815] CPU6 attaching sched-domain(s): [ 1.536846] domain-0: span=6-7 level=MC [ 1.536934] groups: 6:{ span=6 cap=1005 }, 7:{ span=7 cap=1001 } [ 1.537144] domain-1: span=4-7 level=NUMA [ 1.537262] groups: 6:{ span=6-7 cap=2006 }, 4:{ span=4-5 cap=1949 } [ 1.537553] domain-2: span=0-1,4-7 level=NUMA [ 1.537613] groups: 6:{ span=4-7 mask=6-7 cap=4054 }, 0:{ span=0-1 cap=1805 } [ 1.537872] domain-3: span=0-7 level=NUMA [ 1.537998] groups: 6:{ span=0-1,4-7 mask=6-7 cap=6102 }, 2:{ span=0-5 mask=2-3 cap=5845 } [ 1.538448] CPU7 attaching sched-domain(s): [ 1.538505] domain-0: span=6-7 level=MC [ 1.538586] groups: 7:{ span=7 cap=1001 }, 6:{ span=6 cap=1005 } [ 1.538746] domain-1: span=4-7 level=NUMA [ 1.538798] groups: 6:{ span=6-7 cap=2006 }, 4:{ span=4-5 cap=1949 } [ 1.539048] domain-2: span=0-1,4-7 level=NUMA [ 1.539111] groups: 6:{ span=4-7 mask=6-7 cap=4054 }, 0:{ span=0-1 cap=1805 } [ 1.539571] domain-3: span=0-7 level=NUMA [ 1.539610] groups: 6:{ span=0-1,4-7 mask=6-7 cap=6102 }, 2:{ span=0-5 mask=2-3 cap=5845 } Reported-by: Valentin Schneider <valentin.schneider(a)arm.com> Tested-by: Meelis Roos <mroos(a)linux.ee> Signed-off-by: Barry Song <song.bao.hua(a)hisilicon.com> --- -v2: merged Valentin's edit on v1, thus fixed two issues * "one cpu in one numa" was not supported * the capacity of groups from grandchild couldn't be updated Thanks for Valentin's nice edit! And also added some comments in build_balance_mask kernel/sched/topology.c | 85 +++++++++++++++++++++++++---------------- 1 file changed, 53 insertions(+), 32 deletions(-) diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c index 5d3675c7a76b..964ed89001fe 100644 --- a/kernel/sched/topology.c +++ b/kernel/sched/topology.c @@ -723,35 +723,6 @@ cpu_attach_domain(struct sched_domain *sd, struct root_domain *rd, int cpu) for (tmp = sd; tmp; tmp = tmp->parent) numa_distance += !!(tmp->flags & SD_NUMA); - /* - * FIXME: Diameter >=3 is misrepresented. - * - * Smallest diameter=3 topology is: - * - * node 0 1 2 3 - * 0: 10 20 30 40 - * 1: 20 10 20 30 - * 2: 30 20 10 20 - * 3: 40 30 20 10 - * - * 0 --- 1 --- 2 --- 3 - * - * NUMA-3 0-3 N/A N/A 0-3 - * groups: {0-2},{1-3} {1-3},{0-2} - * - * NUMA-2 0-2 0-3 0-3 1-3 - * groups: {0-1},{1-3} {0-2},{2-3} {1-3},{0-1} {2-3},{0-2} - * - * NUMA-1 0-1 0-2 1-3 2-3 - * groups: {0},{1} {1},{2},{0} {2},{3},{1} {3},{2} - * - * NUMA-0 0 1 2 3 - * - * The NUMA-2 groups for nodes 0 and 3 are obviously buggered, as the - * group span isn't a subset of the domain span. - */ - WARN_ONCE(numa_distance > 2, "Shortest NUMA path spans too many nodes\n"); - sched_domain_debug(sd, cpu); rq_attach_root(rq, rd); @@ -908,6 +879,17 @@ build_balance_mask(struct sched_domain *sd, struct sched_group *sg, struct cpuma for_each_cpu(i, sg_span) { sibling = *per_cpu_ptr(sdd->sd, i); + /* + * for NUMA diameter >= 3, while sibling->child spans out of + * the domain being built, we are using the grandchild; + * build_overlap_sched_groups includes words on how this could + * happen + */ + while (sibling->child && + !cpumask_subset(sched_domain_span(sibling->child), + sched_domain_span(sd))) + sibling = sibling->child; + /* * Can happen in the asymmetric case, where these siblings are * unused. The mask will not be empty because those CPUs that @@ -955,10 +937,11 @@ build_group_from_child_sched_domain(struct sched_domain *sd, int cpu) } static void init_overlap_sched_group(struct sched_domain *sd, - struct sched_group *sg) + struct sched_group *sg, + struct sched_domain *grandchild) { struct cpumask *mask = sched_domains_tmpmask2; - struct sd_data *sdd = sd->private; + struct sd_data *sdd = grandchild ? grandchild->private : sd->private; struct cpumask *sg_span; int cpu; @@ -996,6 +979,7 @@ build_overlap_sched_groups(struct sched_domain *sd, int cpu) for_each_cpu_wrap(i, span, cpu) { struct cpumask *sg_span; + bool from_grandchild = false; if (cpumask_test_cpu(i, covered)) continue; @@ -1015,6 +999,43 @@ build_overlap_sched_groups(struct sched_domain *sd, int cpu) if (!cpumask_test_cpu(i, sched_domain_span(sibling))) continue; + /* + * for NUMA diameter >= 3, building sched_domain by sibling's + * child's child domain to prevent sched_group from spanning + * out of sched_domain + * if we don't do this, Diameter >=3 is misrepresented: + * + * Smallest diameter=3 topology is: + * + * node 0 1 2 3 + * 0: 10 20 30 40 + * 1: 20 10 20 30 + * 2: 30 20 10 20 + * 3: 40 30 20 10 + * + * 0 --- 1 --- 2 --- 3 + * + * NUMA-3 0-3 N/A N/A 0-3 + * groups: {0-2},{1-3} {1-3},{0-2} + * + * NUMA-2 0-2 0-3 0-3 1-3 + * groups: {0-1},{1-3} {0-2},{2-3} {1-3},{0-1} {2-3},{0-2} + * + * NUMA-1 0-1 0-2 1-3 2-3 + * groups: {0},{1} {1},{2},{0} {2},{3},{1} {3},{2} + * + * NUMA-0 0 1 2 3 + * + * The NUMA-2 groups for nodes 0 and 3 are obviously buggered, as the + * group span isn't a subset of the domain span. + */ + while (sibling->child && + !cpumask_subset(sched_domain_span(sibling->child), + span)) { + sibling = sibling->child; + from_grandchild = true; + } + sg = build_group_from_child_sched_domain(sibling, cpu); if (!sg) goto fail; @@ -1022,7 +1043,7 @@ build_overlap_sched_groups(struct sched_domain *sd, int cpu) sg_span = sched_group_span(sg); cpumask_or(covered, covered, sg_span); - init_overlap_sched_group(sd, sg); + init_overlap_sched_group(sd, sg, from_grandchild ? sibling : NULL); if (!first) first = sg; -- 2.25.1

5 10

Re: [PATCH v11 12/13] vfio/pci: Register a DMA fault response region
by Shameerali Kolothum Thodi 18 Feb '21

18 Feb '21

Hi Eric, > > -----Original Message----- > > From: Eric Auger [mailto:eric.auger@redhat.com] > > Sent: 16 November 2020 11:00 > > To: eric.auger.pro(a)gmail.com; eric.auger(a)redhat.com; > > iommu(a)lists.linux-foundation.org; linux-kernel(a)vger.kernel.org; > > kvm(a)vger.kernel.org; kvmarm(a)lists.cs.columbia.edu; will(a)kernel.org; > > joro(a)8bytes.org; maz(a)kernel.org; robin.murphy(a)arm.com; > > alex.williamson(a)redhat.com > > Cc: jean-philippe(a)linaro.org; zhangfei.gao(a)linaro.org; > > zhangfei.gao(a)gmail.com; vivek.gautam(a)arm.com; Shameerali Kolothum > > Thodi <shameerali.kolothum.thodi(a)huawei.com>; > > jacob.jun.pan(a)linux.intel.com; yi.l.liu(a)intel.com; tn(a)semihalf.com; > > nicoleotsuka(a)gmail.com; yuzenghui <yuzenghui(a)huawei.com> > > Subject: [PATCH v11 12/13] vfio/pci: Register a DMA fault response > > region > > > > In preparation for vSVA, let's register a DMA fault response region, > > where the userspace will push the page responses and increment the > > head of the buffer. The kernel will pop those responses and inject > > them on iommu side. > > > > Signed-off-by: Eric Auger <eric.auger(a)redhat.com> > > --- > > drivers/vfio/pci/vfio_pci.c | 114 +++++++++++++++++++++++++--- > > drivers/vfio/pci/vfio_pci_private.h | 5 ++ > > drivers/vfio/pci/vfio_pci_rdwr.c | 39 ++++++++++ > > include/uapi/linux/vfio.h | 32 ++++++++ > > 4 files changed, 181 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c > > index 65a83fd0e8c0..e9a904ce3f0d 100644 > > --- a/drivers/vfio/pci/vfio_pci.c > > +++ b/drivers/vfio/pci/vfio_pci.c > > @@ -318,9 +318,20 @@ static void vfio_pci_dma_fault_release(struct > > vfio_pci_device *vdev, > > kfree(vdev->fault_pages); > > } > > > > -static int vfio_pci_dma_fault_mmap(struct vfio_pci_device *vdev, > > - struct vfio_pci_region *region, > > - struct vm_area_struct *vma) > > +static void > > +vfio_pci_dma_fault_response_release(struct vfio_pci_device *vdev, > > + struct vfio_pci_region *region) { > > + if (vdev->dma_fault_response_wq) > > + destroy_workqueue(vdev->dma_fault_response_wq); > > + kfree(vdev->fault_response_pages); > > + vdev->fault_response_pages = NULL; > > +} > > + > > +static int __vfio_pci_dma_fault_mmap(struct vfio_pci_device *vdev, > > + struct vfio_pci_region *region, > > + struct vm_area_struct *vma, > > + u8 *pages) > > { > > u64 phys_len, req_len, pgoff, req_start; > > unsigned long long addr; > > @@ -333,14 +344,14 @@ static int vfio_pci_dma_fault_mmap(struct > > vfio_pci_device *vdev, > > ((1U << (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT)) - 1); > > req_start = pgoff << PAGE_SHIFT; > > > > - /* only the second page of the producer fault region is mmappable */ > > + /* only the second page of the fault region is mmappable */ > > if (req_start < PAGE_SIZE) > > return -EINVAL; > > > > if (req_start + req_len > phys_len) > > return -EINVAL; > > > > - addr = virt_to_phys(vdev->fault_pages); > > + addr = virt_to_phys(pages); > > vma->vm_private_data = vdev; > > vma->vm_pgoff = (addr >> PAGE_SHIFT) + pgoff; > > > > @@ -349,13 +360,29 @@ static int vfio_pci_dma_fault_mmap(struct > > vfio_pci_device *vdev, > > return ret; > > } > > > > -static int vfio_pci_dma_fault_add_capability(struct vfio_pci_device *vdev, > > - struct vfio_pci_region *region, > > - struct vfio_info_cap *caps) > > +static int vfio_pci_dma_fault_mmap(struct vfio_pci_device *vdev, > > + struct vfio_pci_region *region, > > + struct vm_area_struct *vma) > > +{ > > + return __vfio_pci_dma_fault_mmap(vdev, region, vma, > > vdev->fault_pages); > > +} > > + > > +static int > > +vfio_pci_dma_fault_response_mmap(struct vfio_pci_device *vdev, > > + struct vfio_pci_region *region, > > + struct vm_area_struct *vma) > > +{ > > + return __vfio_pci_dma_fault_mmap(vdev, region, vma, > > vdev->fault_response_pages); > > +} > > + > > +static int __vfio_pci_dma_fault_add_capability(struct vfio_pci_device *vdev, > > + struct vfio_pci_region *region, > > + struct vfio_info_cap *caps, > > + u32 cap_id) > > { > > struct vfio_region_info_cap_sparse_mmap *sparse = NULL; > > struct vfio_region_info_cap_fault cap = { > > - .header.id = VFIO_REGION_INFO_CAP_DMA_FAULT, > > + .header.id = cap_id, > > .header.version = 1, > > .version = 1, > > }; > > @@ -383,6 +410,14 @@ static int > > vfio_pci_dma_fault_add_capability(struct > > vfio_pci_device *vdev, > > return ret; > > } > > > > +static int vfio_pci_dma_fault_add_capability(struct vfio_pci_device *vdev, > > + struct vfio_pci_region *region, > > + struct vfio_info_cap *caps) { > > + return __vfio_pci_dma_fault_add_capability(vdev, region, caps, > > + VFIO_REGION_INFO_CAP_DMA_FAULT); } > > + > > static const struct vfio_pci_regops vfio_pci_dma_fault_regops = { > > .rw = vfio_pci_dma_fault_rw, > > .release = vfio_pci_dma_fault_release, > > @@ -390,6 +425,13 @@ static const struct vfio_pci_regops > > vfio_pci_dma_fault_regops = { > > .add_capability = vfio_pci_dma_fault_add_capability, > > }; > > > > +static const struct vfio_pci_regops vfio_pci_dma_fault_response_regops = { > > + .rw = vfio_pci_dma_fault_response_rw, > > + .release = vfio_pci_dma_fault_response_release, > > + .mmap = vfio_pci_dma_fault_response_mmap, > > + .add_capability = vfio_pci_dma_fault_add_capability, As I mentioned in the Qemu patch ([RFC v7 26/26] vfio/pci: Implement return_page_response page response callback), it looks like we are using the VFIO_REGION_INFO_CAP_DMA_FAULT cap id for the dma_fault_response here as well. Is that intentional? (Was wondering how it worked in the first place and noted this). Please check. Thanks, Shameer

2 1

Re: [PATCH v2] iommu: Check dev->iommu in iommu_dev_xxx functions
by Shameerali Kolothum Thodi 16 Feb '21

16 Feb '21

Hi Robin/Joerg, > -----Original Message----- > From: Shameer Kolothum [mailto:shameerali.kolothum.thodi@huawei.com] > Sent: 01 February 2021 12:41 > To: linux-kernel(a)vger.kernel.org; iommu(a)lists.linux-foundation.org > Cc: joro(a)8bytes.org; robin.murphy(a)arm.com; jean-philippe(a)linaro.org; > will(a)kernel.org; Zengtao (B) <prime.zeng(a)hisilicon.com>; > linuxarm(a)openeuler.org > Subject: [Linuxarm] [PATCH v2] iommu: Check dev->iommu in iommu_dev_xxx > functions > > The device iommu probe/attach might have failed leaving dev->iommu > to NULL and device drivers may still invoke these functions resulting > in a crash in iommu vendor driver code. Hence make sure we check that. > > Also added iommu_ops to the "struct dev_iommu" and set it if the dev > is successfully associated with an iommu. > > Fixes: a3a195929d40 ("iommu: Add APIs for multiple domains per device") > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi(a)huawei.com> > --- > v1 --> v2: > -Added iommu_ops to struct dev_iommu based on the discussion with Robin. > -Rebased against iommu-tree core branch. A gentle ping on this... Thanks, Shameer > --- > drivers/iommu/iommu.c | 19 +++++++------------ > include/linux/iommu.h | 2 ++ > 2 files changed, 9 insertions(+), 12 deletions(-) > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index fd76e2f579fe..6023d0b7c542 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -217,6 +217,7 @@ static int __iommu_probe_device(struct device *dev, > struct list_head *group_list > } > > dev->iommu->iommu_dev = iommu_dev; > + dev->iommu->ops = iommu_dev->ops; > > group = iommu_group_get_for_dev(dev); > if (IS_ERR(group)) { > @@ -2865,10 +2866,8 @@ EXPORT_SYMBOL_GPL(iommu_fwspec_add_ids); > */ > int iommu_dev_enable_feature(struct device *dev, enum > iommu_dev_features feat) > { > - const struct iommu_ops *ops = dev->bus->iommu_ops; > - > - if (ops && ops->dev_enable_feat) > - return ops->dev_enable_feat(dev, feat); > + if (dev->iommu && dev->iommu->ops->dev_enable_feat) > + return dev->iommu->ops->dev_enable_feat(dev, feat); > > return -ENODEV; > } > @@ -2881,10 +2880,8 @@ > EXPORT_SYMBOL_GPL(iommu_dev_enable_feature); > */ > int iommu_dev_disable_feature(struct device *dev, enum > iommu_dev_features feat) > { > - const struct iommu_ops *ops = dev->bus->iommu_ops; > - > - if (ops && ops->dev_disable_feat) > - return ops->dev_disable_feat(dev, feat); > + if (dev->iommu && dev->iommu->ops->dev_disable_feat) > + return dev->iommu->ops->dev_disable_feat(dev, feat); > > return -EBUSY; > } > @@ -2892,10 +2889,8 @@ > EXPORT_SYMBOL_GPL(iommu_dev_disable_feature); > > bool iommu_dev_feature_enabled(struct device *dev, enum > iommu_dev_features feat) > { > - const struct iommu_ops *ops = dev->bus->iommu_ops; > - > - if (ops && ops->dev_feat_enabled) > - return ops->dev_feat_enabled(dev, feat); > + if (dev->iommu && dev->iommu->ops->dev_feat_enabled) > + return dev->iommu->ops->dev_feat_enabled(dev, feat); > > return false; > } > diff --git a/include/linux/iommu.h b/include/linux/iommu.h > index 524ffc2ff64f..ff0c76bdfb67 100644 > --- a/include/linux/iommu.h > +++ b/include/linux/iommu.h > @@ -354,6 +354,7 @@ struct iommu_fault_param { > * @fault_param: IOMMU detected device fault reporting data > * @fwspec: IOMMU fwspec data > * @iommu_dev: IOMMU device this device is linked to > + * @ops: iommu-ops for talking to the iommu_dev > * @priv: IOMMU Driver private data > * > * TODO: migrate other per device data pointers under iommu_dev_data, > e.g. > @@ -364,6 +365,7 @@ struct dev_iommu { > struct iommu_fault_param *fault_param; > struct iommu_fwspec *fwspec; > struct iommu_device *iommu_dev; > + const struct iommu_ops *ops; > void *priv; > }; > > -- > 2.17.1 > _______________________________________________ > Linuxarm mailing list -- linuxarm(a)openeuler.org > To unsubscribe send an email to linuxarm-leave(a)openeuler.org

2 4

[PATCH V2 net-next 00/13] net: hns3: some cleanups for -next
by Huazhong Tan 13 Feb '21

13 Feb '21

To improve code readability and maintainability, the series refactor out some bloated functions in the HNS3 ethernet driver. change log: V2: remove an unused variable in #5 previous version: V1: https://patchwork.kernel.org/project/netdevbpf/cover/1612943005-59416-1-git… Hao Chen (1): net: hns3: refactor out hclge_rm_vport_all_mac_table() Huazhong Tan (2): net: hns3: refactor out hclge_set_rss_tuple() net: hns3: refactor out hclgevf_set_rss_tuple() Jian Shen (3): net: hns3: refactor out hclge_get_rss_tuple() net: hns3: refactor out hclgevf_get_rss_tuple() net: hns3: split out hclge_dbg_dump_qos_buf_cfg() Jiaran Zhang (1): net: hns3: use ipv6_addr_any() helper Peng Li (4): net: hns3: refactor out hclge_cmd_convert_err_code() net: hns3: refactor out hclgevf_cmd_convert_err_code() net: hns3: clean up hns3_dbg_cmd_write() net: hns3: refactor out hclge_set_vf_vlan_common() Yufeng Mo (2): net: hns3: split out hclge_cmd_send() net: hns3: split out hclgevf_cmd_send() .../ethernet/hisilicon/hns3/hns3_debugfs.c | 44 +-- .../hisilicon/hns3/hns3pf/hclge_cmd.c | 155 ++++++----- .../hisilicon/hns3/hns3pf/hclge_debugfs.c | 158 ++++++++--- .../hisilicon/hns3/hns3pf/hclge_main.c | 254 +++++++++++------- .../hisilicon/hns3/hns3vf/hclgevf_cmd.c | 194 +++++++------ .../hisilicon/hns3/hns3vf/hclgevf_main.c | 114 +++++--- 6 files changed, 571 insertions(+), 348 deletions(-) -- 2.25.1

2 9

[PATCH for next v1 0/2] gpio: few clean up patches to replace spin_lock_irqsave with spin_lock
by Luo Jiaxing 13 Feb '21

13 Feb '21

There is no need to use API with _irqsave in hard IRQ handler, So replace those with spin_lock. Luo Jiaxing (2): gpio: omap: Replace raw_spin_lock_irqsave with raw_spin_lock in omap_gpio_irq_handler() gpio: grgpio: Replace spin_lock_irqsave with spin_lock in grgpio_irq_handler() drivers/gpio/gpio-grgpio.c | 5 ++--- drivers/gpio/gpio-omap.c | 15 ++++++--------- 2 files changed, 8 insertions(+), 12 deletions(-) -- 2.7.4

6 29

[PATCH] i2c: busses: Replace spin_lock_irqsave with spin_lock in hard IRQ
by Weihang Li 12 Feb '21

12 Feb '21

There is no need to do irqsave and irqrestore in context of hard IRQ. Cc: Baruch Siach <baruch(a)tkos.co.il> Cc: Paul Cercueil <paul(a)crapouillou.net> Signed-off-by: Weihang Li <liweihang(a)huawei.com> --- drivers/i2c/busses/i2c-digicolor.c | 5 ++--- drivers/i2c/busses/i2c-jz4780.c | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/i2c/busses/i2c-digicolor.c b/drivers/i2c/busses/i2c-digicolor.c index f67639d..60c838c 100644 --- a/drivers/i2c/busses/i2c-digicolor.c +++ b/drivers/i2c/busses/i2c-digicolor.c @@ -160,12 +160,11 @@ static irqreturn_t dc_i2c_irq(int irq, void *dev_id) { struct dc_i2c *i2c = dev_id; int cmd_status = dc_i2c_cmd_status(i2c); - unsigned long flags; u8 addr_cmd; writeb_relaxed(1, i2c->regs + II_INTFLAG_CLEAR); - spin_lock_irqsave(&i2c->lock, flags); + spin_lock(&i2c->lock); if (cmd_status == II_CMD_STATUS_ACK_BAD || cmd_status == II_CMD_STATUS_ABORT) { @@ -207,7 +206,7 @@ static irqreturn_t dc_i2c_irq(int irq, void *dev_id) } out: - spin_unlock_irqrestore(&i2c->lock, flags); + spin_unlock(&i2c->lock); return IRQ_HANDLED; } diff --git a/drivers/i2c/busses/i2c-jz4780.c b/drivers/i2c/busses/i2c-jz4780.c index cb4a25e..8509c5f 100644 --- a/drivers/i2c/busses/i2c-jz4780.c +++ b/drivers/i2c/busses/i2c-jz4780.c @@ -437,9 +437,8 @@ static irqreturn_t jz4780_i2c_irq(int irqno, void *dev_id) unsigned short intst; unsigned short intmsk; struct jz4780_i2c *i2c = dev_id; - unsigned long flags; - spin_lock_irqsave(&i2c->lock, flags); + spin_lock(&i2c->lock); intmsk = jz4780_i2c_readw(i2c, JZ4780_I2C_INTM); intst = jz4780_i2c_readw(i2c, JZ4780_I2C_INTST); @@ -551,7 +550,7 @@ static irqreturn_t jz4780_i2c_irq(int irqno, void *dev_id) } done: - spin_unlock_irqrestore(&i2c->lock, flags); + spin_unlock(&i2c->lock); return IRQ_HANDLED; } -- 2.8.1

2 1

[PATCH V2 net-next 09/13] net: hns3: split out hclge_cmd_send()
by Huazhong Tan 12 Feb '21

12 Feb '21

From: Yufeng Mo <moyufeng(a)huawei.com> hclge_cmd_send() is bloated, so split it into separate functions for readability and maintainability. Signed-off-by: Yufeng Mo <moyufeng(a)huawei.com> Signed-off-by: Huazhong Tan <tanhuazhong(a)huawei.com> --- .../hisilicon/hns3/hns3pf/hclge_cmd.c | 100 +++++++++++------- 1 file changed, 59 insertions(+), 41 deletions(-) diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c index cb2c955ce52c..1bd0ddfaec4d 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c @@ -194,6 +194,22 @@ struct errcode { int common_errno; }; +static void hclge_cmd_copy_desc(struct hclge_hw *hw, struct hclge_desc *desc, + int num) +{ + struct hclge_desc *desc_to_use; + int handle = 0; + + while (handle < num) { + desc_to_use = &hw->cmq.csq.desc[hw->cmq.csq.next_to_use]; + *desc_to_use = desc[handle]; + (hw->cmq.csq.next_to_use)++; + if (hw->cmq.csq.next_to_use >= hw->cmq.csq.desc_num) + hw->cmq.csq.next_to_use = 0; + handle++; + } +} + static int hclge_cmd_convert_err_code(u16 desc_ret) { struct errcode hclge_cmd_errcode[] = { @@ -243,6 +259,44 @@ static int hclge_cmd_check_retval(struct hclge_hw *hw, struct hclge_desc *desc, return hclge_cmd_convert_err_code(desc_ret); } +static int hclge_cmd_check_result(struct hclge_hw *hw, struct hclge_desc *desc, + int num, int ntc) +{ + struct hclge_dev *hdev = container_of(hw, struct hclge_dev, hw); + bool is_completed = false; + u32 timeout = 0; + int handle, ret; + + /** + * If the command is sync, wait for the firmware to write back, + * if multi descriptors to be sent, use the first one to check + */ + if (HCLGE_SEND_SYNC(le16_to_cpu(desc->flag))) { + do { + if (hclge_cmd_csq_done(hw)) { + is_completed = true; + break; + } + udelay(1); + timeout++; + } while (timeout < hw->cmq.tx_timeout); + } + + if (!is_completed) + ret = -EBADE; + else + ret = hclge_cmd_check_retval(hw, desc, num, ntc); + + /* Clean the command send queue */ + handle = hclge_cmd_csq_clean(hw); + if (handle < 0) + ret = handle; + else if (handle != num) + dev_warn(&hdev->pdev->dev, + "cleaned %d, need to clean %d\n", handle, num); + return ret; +} + /** * hclge_cmd_send - send command to command queue * @hw: pointer to the hw struct @@ -256,11 +310,7 @@ int hclge_cmd_send(struct hclge_hw *hw, struct hclge_desc *desc, int num) { struct hclge_dev *hdev = container_of(hw, struct hclge_dev, hw); struct hclge_cmq_ring *csq = &hw->cmq.csq; - struct hclge_desc *desc_to_use; - bool complete = false; - u32 timeout = 0; - int handle = 0; - int retval; + int ret; int ntc; spin_lock_bh(&hw->cmq.csq.lock); @@ -284,49 +334,17 @@ int hclge_cmd_send(struct hclge_hw *hw, struct hclge_desc *desc, int num) * which will be use for hardware to write back */ ntc = hw->cmq.csq.next_to_use; - while (handle < num) { - desc_to_use = &hw->cmq.csq.desc[hw->cmq.csq.next_to_use]; - *desc_to_use = desc[handle]; - (hw->cmq.csq.next_to_use)++; - if (hw->cmq.csq.next_to_use >= hw->cmq.csq.desc_num) - hw->cmq.csq.next_to_use = 0; - handle++; - } + + hclge_cmd_copy_desc(hw, desc, num); /* Write to hardware */ hclge_write_dev(hw, HCLGE_NIC_CSQ_TAIL_REG, hw->cmq.csq.next_to_use); - /** - * If the command is sync, wait for the firmware to write back, - * if multi descriptors to be sent, use the first one to check - */ - if (HCLGE_SEND_SYNC(le16_to_cpu(desc->flag))) { - do { - if (hclge_cmd_csq_done(hw)) { - complete = true; - break; - } - udelay(1); - timeout++; - } while (timeout < hw->cmq.tx_timeout); - } - - if (!complete) - retval = -EBADE; - else - retval = hclge_cmd_check_retval(hw, desc, num, ntc); - - /* Clean the command send queue */ - handle = hclge_cmd_csq_clean(hw); - if (handle < 0) - retval = handle; - else if (handle != num) - dev_warn(&hdev->pdev->dev, - "cleaned %d, need to clean %d\n", handle, num); + ret = hclge_cmd_check_result(hw, desc, num, ntc); spin_unlock_bh(&hw->cmq.csq.lock); - return retval; + return ret; } static void hclge_set_default_capability(struct hclge_dev *hdev) -- 2.25.1

1 4

Re: [PATCH for-next 00/32] spin lock usage optimization for SCSI drivers
by Song Bao Hua (Barry Song) 12 Feb '21

12 Feb '21

> > > > On Wed, 10 Feb 2021, Song Bao Hua (Barry Song) wrote: > > > > > > On Wed, 10 Feb 2021, Song Bao Hua (Barry Song) wrote: > > > > > > > > > > On Wed, 10 Feb 2021, Song Bao Hua (Barry Song) wrote: > > > > > > > > > > > > > > There is no warning from m68k builds. That's because > > > > > > > > arch_irqs_disabled() returns true when the IPL is non-zero. > > > > > > > > > > > > > > So for m68k, the case is arch_irqs_disabled() is true, but > > > > > > > interrupts can still come? > > > > > > > > > > > > > > Then it seems it is very confusing. If prioritized interrupts > > > > > > > can still come while arch_irqs_disabled() is true, > > > > > > > > > > > > Yes, on m68k CPUs, an IRQ having a priority level higher than the > > > > > > present priority mask will get serviced. > > > > > > > > > > > > Non-Maskable Interrupt (NMI) is not subject to this rule and gets > > > > > > serviced regardless. > > > > > > > > > > > > > how could spin_lock_irqsave() block the prioritized interrupts? > > > > > > > > > > > > It raises the the mask level to 7. Again, please see > > > > > > arch/m68k/include/asm/irqflags.h > > > > > > > > > > Hi Finn, > > > > > Thanks for your explanation again. > > > > > > > > > > TBH, that is why m68k is so confusing. irqs_disabled() on m68k > > > > > should just reflect the status of all interrupts have been disabled > > > > > except NMI. > > > > > > > > > > irqs_disabled() should be consistent with the calling of APIs such > > > > > as local_irq_disable, local_irq_save, spin_lock_irqsave etc. > > > > > > > > > > > > > When irqs_disabled() returns true, we cannot infer that > > > > arch_local_irq_disable() was called. But I have not yet found driver > > > > code or core kernel code attempting that inference. > > > > > > > > > > > > > > > > > Isn't arch_irqs_disabled() a status reflection of irq disable > > > > > > > API? > > > > > > > > > > > > > > > > > > > Why not? > > > > > > > > > > If so, arch_irqs_disabled() should mean all interrupts have been > > > > > masked except NMI as NMI is unmaskable. > > > > > > > > > > > > > Can you support that claim with a reference to core kernel code or > > > > documentation? (If some arch code agrees with you, that's neither here > > > > nor there.) > > > > > > I think those links I share you have supported this. Just you don't > > > believe :-) > > > > > > > Your links show that the distinction between fast and slow handlers was > > removed. Your links don't support your claim that "arch_irqs_disabled() > > should mean all interrupts have been masked". Where is the code that makes > > that inference? Where is the documentation that supports your claim? > > (1) > https://lwn.net/Articles/380931/ > Looking at all these worries, one might well wonder if a system which *disabled > interrupts for all handlers* would function well at all. So it is interesting > to note one thing: any system which has the lockdep locking checker enabled > has been running all handlers that way for some years now. Many developers > and testers run lockdep-enabled kernels, and they are available for some of > the more adventurous distributions (Rawhide, for example) as well. So we > have quite a bit of test coverage for this mode of operation already. > > (2) > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/ > ?id=b738a50a > > "We run all handlers *with interrupts disabled* and expect them not to > enable them. Warn when we catch one who does." > > (3) > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/ > ?id=e58aa3d2d0cc > genirq: Run irq handlers *with interrupts disabled* > > Running interrupt handlers with interrupts enabled can cause stack > overflows. That has been observed with multiqueue NICs delivering all > their interrupts to a single core. We might band aid that somehow by > checking the interrupt stacks, but the real safe fix is to *run the irq > handlers with interrupts disabled*. > > > All these documents say we are running irq handler with interrupts > disabled. but it seems you think high-prio interrupts don't belong > to "interrupts" in those documents :-) > > that is why we can't get agreement. I think "interrupts" mean > all except NMI in these documents, but you insist high-prio IRQ > is an exception. > > > > > > > > > > > > > > > > > > > Are all interrupts (including NMI) masked whenever > > > > > > arch_irqs_disabled() returns true on your platforms? > > > > > > > > > > On my platform, once irqs_disabled() is true, all interrupts are > > > > > masked except NMI. NMI just ignore spin_lock_irqsave or > > > > > local_irq_disable. > > > > > > > > > > On ARM64, we also have high-priority interrupts, but they are > > > > > running as PESUDO_NMI: > > > > > https://lwn.net/Articles/755906/ > > > > > > > > > > > > > A glance at the ARM GIC specification suggests that your hardware > > > > works much like 68000 hardware. > > > > > > > > When enabled, a CPU interface takes the highest priority pending > > > > interrupt for its connected processor and determines whether the > > > > interrupt has sufficient priority for it to signal the interrupt > > > > request to the processor. [...] > > > > > > > > When the processor acknowledges the interrupt at the CPU interface, > > > > the Distributor changes the status of the interrupt from pending to > > > > either active, or active and pending. At this point the CPU > > > > interface can signal another interrupt to the processor, to preempt > > > > interrupts that are active on the processor. If there is no pending > > > > interrupt with sufficient priority for signaling to the processor, > > > > the interface deasserts the interrupt request signal to the > > > > processor. > > > > > > > > https://developer.arm.com/documentation/ihi0048/b/ > > > > > > > > Have you considered that Linux/arm might benefit if it could fully > > > > exploit hardware features already available, such as the interrupt > > > > priority masking feature in the GIC in existing arm systems? > > > > > > I guess no:-) there are only two levels: IRQ and NMI. Injecting a > > > high-prio IRQ level between them makes no sense. > > > > > > To me, arm64's design is quite clear and has no any confusion. > > > > > > > Are you saying that the ARM64 hardware design is confusing because it > > implements a priority mask, and that's why you had to simplify it with a > > pseudo-nmi scheme in software? > > No, I was not saying this. I think both m68k and arm64 have good hardware > design. Just Linux's implementation is running irq-handlers with interrupts > disabled. So ARM64's pseudo-nmi is adapted to Linux better. > > > > > > > > > > > > On m68k, it seems you mean： > > > > > irq_disabled() is true, but high-priority interrupts can still come; > > > > > local_irq_disable() can disable high-priority interrupts, and at that > > > > > time, irq_disabled() is also true. > > > > > > > > > > TBH, this is wrong and confusing on m68k. > > > > > > > > > > > > > Like you, I was surprised when I learned about it. But that doesn't mean > > > > it's wrong. The fact that it works should tell you something. > > > > > > > > > > The fact is that m68k lets arch_irq_disabled() return true to pretend > > > all IRQs are disabled while high-priority IRQ is still open, thus "pass" > > > all sanitizing check in genirq and kernel core. > > > > > > > The fact is that m68k has arch_irq_disabled() return false when all IRQs > > are enabled. So there is no bug. > > But it has arch_irq_disabled() return true while some interrupts(not NMI) > are still open. > > > > > > > Things could always be made simpler. But discarding features isn't > > > > necessarily an improvement. > > > > > > This feature could be used by calling local_irq_enable_in_hardirq() in > > > those IRQ handlers who hope high-priority interrupts to preempt it for a > > > while. > > > > > > > So, if one handler is sensitive to interrupt latency, all other handlers > > should be modified? I don't think that's workable. > > I think we just enable preempt_rt or force threaded_irq, and then improve > the priority of the irq thread who is sensitive to latency. No need to > touch all threads. > > I also understand your point, we let one high-prio interrupt preempt > low priority interrupt, then we don't need to change the whole system. > But I think Linux prefers the method of threaded_irq or preempt_rt > for this kind of problems. > > > > > In anycase, what you're describing is a completely different nested > > interrupt scheme that would defeat the priority level mechanism that the > > hardware provides us with. > > Yes. Indeed. > > > > > > It shouldn't hide somewhere and make confusion. > > > > > > > The problem is hiding so well that no-one has found it! I say it doesn't > > exist. > > Long long ago(before 2.6.38), we had a kernel supporting IRQF_DISABLED and > nested interrupts were widely supported, but system also ran well in most > cases. That means nested interrupts don't really matter in most cases. > That is why m68k is also running well even though it is still nesting. > > > > > > On the other hand, those who care about realtime should use threaded IRQ > > > and let IRQ threads preempt each other. > > > > > > > Yes. And those threads also have priority levels. > > Finn, I am not a m68k guy, would you help check if this could activate a > warning on m68k. maybe we can discuss this question in genirq maillist from > this warning if you are happy. Thanks very much. > > diff --git a/include/linux/hardirq.h b/include/linux/hardirq.h > index 7c9d6a2d7e90..b8ca27555c76 100644 > --- a/include/linux/hardirq.h > +++ b/include/linux/hardirq.h > @@ -32,6 +32,7 @@ static __always_inline void rcu_irq_enter_check_tick(void) > */ > #define __irq_enter() \ > do { \ > + WARN_ONCE(in_hardirq() && irqs_disabled(), "nested > interrupts\n"); \ > preempt_count_add(HARDIRQ_OFFSET); \ > lockdep_hardirq_enter(); \ > account_hardirq_enter(current); \ > @@ -44,6 +45,7 @@ static __always_inline void rcu_irq_enter_check_tick(void) > */ > #define __irq_enter_raw() \ > do { \ > + WARN_ONCE(in_hardirq() && irqs_disabled(), "nested > interrupts\n"); \ > preempt_count_add(HARDIRQ_OFFSET); \ > lockdep_hardirq_enter(); \ > } while (0) > I am not a m68k guy, here I can show you some code in cortex-m, which supports full interrupt priority preemption in hardware, but its arch code just disables it to satisfy Linux: arch/arm/kernel/entry-header.S .macro v7m_exception_entry ... @ Linux expects to have irqs off. Do it here before taking stack space cpsid i arch/arm/kernel/entry-v7m.S __irq_entry: v7m_exception_entry @ @ Invoke the IRQ handler @ ... @ routine called with r0 = irq number, r1 = struct pt_regs * bl nvic_handle_irq pop {lr} Another example in arch/arc/kernel/entry-arcv2.S: ENTRY(handle_interrupt) INTERRUPT_PROLOGUE # irq control APIs local_irq_save/restore/disable/enable fiddle with # global interrupt enable bits in STATUS32 (.IE for 1 prio, .E[] for 2 prio) # However a taken interrupt doesn't clear these bits. Thus irqs_disabled() # query in hard ISR path would return false (since .IE is set) which would # trips genirq interrupt handling asserts. # # So do a "soft" disable of interrutps here. # # Note this disable is only for consistent book-keeping as further interrupts # will be disabled anyways even w/o this. Hardware tracks active interrupts # seperately in AUX_IRQ_ACT.active and will not take new interrupts # unless this one returns (or higher prio becomes pending in 2-prio scheme) IRQ_DISABLE ; icause is banked: one per priority level ; so a higher prio interrupt taken here won't clobber prev prio icause lr r0, [ICAUSE] mov blink, ret_from_exception b.d arch_do_IRQ mov r1, sp Actually in m68k, I also saw its IRQ entry disabled interrupts by ' move #0x2700,%sr /* disable intrs */' arch/m68k/include/asm/entry.h: .macro SAVE_ALL_SYS move #0x2700,%sr /* disable intrs */ btst #5,%sp@(2) /* from user? */ bnes 6f /* no, skip */ movel %sp,sw_usp /* save user sp */ ... .macro SAVE_ALL_INT SAVE_ALL_SYS moveq #-1,%d0 /* not system call entry */ movel %d0,%sp@(PT_OFF_ORIG_D0) .endm arch/m68k/kernel/entry.S: /* This is the main interrupt handler for autovector interrupts */ ENTRY(auto_inthandler) SAVE_ALL_INT GET_CURRENT(%d0) | put exception # in d0 bfextu %sp@(PT_OFF_FORMATVEC){#4,#10},%d0 subw #VEC_SPUR,%d0 movel %sp,%sp@- movel %d0,%sp@- | put vector # on stack auto_irqhandler_fixup = . + 2 jsr do_IRQ | process the IRQ addql #8,%sp | pop parameters off stack jra ret_from_exception So my question is that " move #0x2700,%sr" is actually disabling all interrupts? And is m68k actually running irq handlers with interrupts disabled? Best Regards Barry

2 4

[PATCH v3 for-next] RDMA/hns: Adjust definition of FRMR fields
by Weihang Li 11 Feb '21

11 Feb '21

From: Yixing Liu <liuyixing1(a)huawei.com> FRMR is not well-supported on HIP08, it is re-designed for HIP09 and the position of related fields is changed. Then the ULPs should be forbidden to use FRMR on older hardwares. Signed-off-by: Yixing Liu <liuyixing1(a)huawei.com> Signed-off-by: Weihang Li <liweihang(a)huawei.com> --- This patch is one of series "RDMA/hns: Updates for 5.12", the others have been applied. Changes since v2: * Aovid setting IB_DEVICE_MEM_MGT_EXTENSIONS for HIP08 or older hardwares. * Link: https://patchwork.kernel.org/project/linux-rdma/patch/1612859923-44041-1-gi… Changes since v1: * Add check for HW version and rewrite the description. * Link: https://patchwork.kernel.org/project/linux-rdma/patch/1612517974-31867-6-gi… drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 45 +++++++++++++++++------------- drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 12 ++++---- drivers/infiniband/hw/hns/hns_roce_main.c | 3 +- 3 files changed, 34 insertions(+), 26 deletions(-) diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c index 012076a..c3934ab 100644 --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c @@ -99,16 +99,16 @@ static void set_frmr_seg(struct hns_roce_v2_rc_send_wqe *rc_sq_wqe, u64 pbl_ba; /* use ib_access_flags */ - roce_set_bit(rc_sq_wqe->byte_4, V2_RC_FRMR_WQE_BYTE_4_BIND_EN_S, - wr->access & IB_ACCESS_MW_BIND ? 1 : 0); - roce_set_bit(rc_sq_wqe->byte_4, V2_RC_FRMR_WQE_BYTE_4_ATOMIC_S, - wr->access & IB_ACCESS_REMOTE_ATOMIC ? 1 : 0); - roce_set_bit(rc_sq_wqe->byte_4, V2_RC_FRMR_WQE_BYTE_4_RR_S, - wr->access & IB_ACCESS_REMOTE_READ ? 1 : 0); - roce_set_bit(rc_sq_wqe->byte_4, V2_RC_FRMR_WQE_BYTE_4_RW_S, - wr->access & IB_ACCESS_REMOTE_WRITE ? 1 : 0); - roce_set_bit(rc_sq_wqe->byte_4, V2_RC_FRMR_WQE_BYTE_4_LW_S, - wr->access & IB_ACCESS_LOCAL_WRITE ? 1 : 0); + roce_set_bit(fseg->byte_40, V2_RC_FRMR_WQE_BYTE_40_BIND_EN_S, + !!(wr->access & IB_ACCESS_MW_BIND)); + roce_set_bit(fseg->byte_40, V2_RC_FRMR_WQE_BYTE_40_ATOMIC_S, + !!(wr->access & IB_ACCESS_REMOTE_ATOMIC)); + roce_set_bit(fseg->byte_40, V2_RC_FRMR_WQE_BYTE_40_RR_S, + !!(wr->access & IB_ACCESS_REMOTE_READ)); + roce_set_bit(fseg->byte_40, V2_RC_FRMR_WQE_BYTE_40_RW_S, + !!(wr->access & IB_ACCESS_REMOTE_WRITE)); + roce_set_bit(fseg->byte_40, V2_RC_FRMR_WQE_BYTE_40_LW_S, + !!(wr->access & IB_ACCESS_LOCAL_WRITE)); /* Data structure reuse may lead to confusion */ pbl_ba = mr->pbl_mtr.hem_cfg.root_ba; @@ -121,12 +121,10 @@ static void set_frmr_seg(struct hns_roce_v2_rc_send_wqe *rc_sq_wqe, rc_sq_wqe->va = cpu_to_le64(wr->mr->iova); fseg->pbl_size = cpu_to_le32(mr->npages); - roce_set_field(fseg->mode_buf_pg_sz, - V2_RC_FRMR_WQE_BYTE_40_PBL_BUF_PG_SZ_M, + roce_set_field(fseg->byte_40, V2_RC_FRMR_WQE_BYTE_40_PBL_BUF_PG_SZ_M, V2_RC_FRMR_WQE_BYTE_40_PBL_BUF_PG_SZ_S, to_hr_hw_page_shift(mr->pbl_mtr.hem_cfg.buf_pg_shift)); - roce_set_bit(fseg->mode_buf_pg_sz, - V2_RC_FRMR_WQE_BYTE_40_BLK_MODE_S, 0); + roce_set_bit(fseg->byte_40, V2_RC_FRMR_WQE_BYTE_40_BLK_MODE_S, 0); } static void set_atomic_seg(const struct ib_send_wr *wr, @@ -522,10 +520,12 @@ static inline int set_ud_wqe(struct hns_roce_qp *qp, return 0; } -static int set_rc_opcode(struct hns_roce_v2_rc_send_wqe *rc_sq_wqe, +static int set_rc_opcode(struct hns_roce_dev *hr_dev, + struct hns_roce_v2_rc_send_wqe *rc_sq_wqe, const struct ib_send_wr *wr) { u32 ib_op = wr->opcode; + int ret = 0; rc_sq_wqe->immtdata = get_immtdata(wr); @@ -545,7 +545,10 @@ static int set_rc_opcode(struct hns_roce_v2_rc_send_wqe *rc_sq_wqe, rc_sq_wqe->va = cpu_to_le64(atomic_wr(wr)->remote_addr); break; case IB_WR_REG_MR: - set_frmr_seg(rc_sq_wqe, reg_wr(wr)); + if (hr_dev->pci_dev->revision >= PCI_REVISION_ID_HIP09) + set_frmr_seg(rc_sq_wqe, reg_wr(wr)); + else + ret = -EOPNOTSUPP; break; case IB_WR_LOCAL_INV: roce_set_bit(rc_sq_wqe->byte_4, V2_RC_SEND_WQE_BYTE_4_SO_S, 1); @@ -554,19 +557,23 @@ static int set_rc_opcode(struct hns_roce_v2_rc_send_wqe *rc_sq_wqe, rc_sq_wqe->inv_key = cpu_to_le32(wr->ex.invalidate_rkey); break; default: - return -EINVAL; + ret = -EINVAL; } + if (unlikely(ret)) + return ret; + roce_set_field(rc_sq_wqe->byte_4, V2_RC_SEND_WQE_BYTE_4_OPCODE_M, V2_RC_SEND_WQE_BYTE_4_OPCODE_S, to_hr_opcode(ib_op)); - return 0; + return ret; } static inline int set_rc_wqe(struct hns_roce_qp *qp, const struct ib_send_wr *wr, void *wqe, unsigned int *sge_idx, unsigned int owner_bit) { + struct hns_roce_dev *hr_dev = to_hr_dev(qp->ibqp.device); struct hns_roce_v2_rc_send_wqe *rc_sq_wqe = wqe; unsigned int curr_idx = *sge_idx; unsigned int valid_num_sge; @@ -577,7 +584,7 @@ static inline int set_rc_wqe(struct hns_roce_qp *qp, rc_sq_wqe->msg_len = cpu_to_le32(msg_len); - ret = set_rc_opcode(rc_sq_wqe, wr); + ret = set_rc_opcode(hr_dev, rc_sq_wqe, wr); if (WARN_ON(ret)) return ret; diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h index 7a0f8ac..39621fb 100644 --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h @@ -1255,15 +1255,15 @@ struct hns_roce_v2_rc_send_wqe { #define V2_RC_SEND_WQE_BYTE_4_INLINE_S 12 -#define V2_RC_FRMR_WQE_BYTE_4_BIND_EN_S 19 +#define V2_RC_FRMR_WQE_BYTE_40_BIND_EN_S 10 -#define V2_RC_FRMR_WQE_BYTE_4_ATOMIC_S 20 +#define V2_RC_FRMR_WQE_BYTE_40_ATOMIC_S 11 -#define V2_RC_FRMR_WQE_BYTE_4_RR_S 21 +#define V2_RC_FRMR_WQE_BYTE_40_RR_S 12 -#define V2_RC_FRMR_WQE_BYTE_4_RW_S 22 +#define V2_RC_FRMR_WQE_BYTE_40_RW_S 13 -#define V2_RC_FRMR_WQE_BYTE_4_LW_S 23 +#define V2_RC_FRMR_WQE_BYTE_40_LW_S 14 #define V2_RC_SEND_WQE_BYTE_4_FLAG_S 31 @@ -1280,7 +1280,7 @@ struct hns_roce_v2_rc_send_wqe { struct hns_roce_wqe_frmr_seg { __le32 pbl_size; - __le32 mode_buf_pg_sz; + __le32 byte_40; }; #define V2_RC_FRMR_WQE_BYTE_40_PBL_BUF_PG_SZ_S 4 diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c b/drivers/infiniband/hw/hns/hns_roce_main.c index c29215a..c9c0836 100644 --- a/drivers/infiniband/hw/hns/hns_roce_main.c +++ b/drivers/infiniband/hw/hns/hns_roce_main.c @@ -201,7 +201,8 @@ static int hns_roce_query_device(struct ib_device *ib_dev, props->max_srq_sge = hr_dev->caps.max_srq_sges; } - if (hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_FRMR) { + if (hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_FRMR && + hr_dev->pci_dev->revision >= PCI_REVISION_ID_HIP09) { props->device_cap_flags |= IB_DEVICE_MEM_MGT_EXTENSIONS; props->max_fast_reg_page_list_len = HNS_ROCE_FRMR_MAX_PA; } -- 2.8.1

2 1

[PATCH for next v1 0/2] gpio: few clean up patches to replace spin_lock_irqsave with spin_lock
by Luo Jiaxing 10 Feb '21

10 Feb '21

There is no need to use API with _irqsave in hard IRQ handler, So replace those with spin_lock. Luo Jiaxing (2): gpio: omap: Replace raw_spin_lock_irqsave with raw_spin_lock in omap_gpio_irq_handler() gpio: grgpio: Replace spin_lock_irqsave with spin_lock in grgpio_irq_handler() drivers/gpio/gpio-grgpio.c | 5 ++--- drivers/gpio/gpio-omap.c | 15 ++++++--------- 2 files changed, 8 insertions(+), 12 deletions(-) -- 2.7.4

2 3

2024

2023

2022

2021

2020

Linuxarm