[PATCH v2] powerpc/papr_scm: Update the NUMA distance table for the target node
platform device helper routines won't update the NUMA distance table
while creating a platform device, even if the device is present on
a NUMA node that doesn't have memory or CPU. This is especially true
for pmem devices. If the target node of the pmem device is not online, we
find the nearest online node to the device and associate the pmem
device with that online node. To find the nearest online node, we should
have the numa distance table updated correctly. Update the distance
information during the device probe.
For a papr scm device on NUMA node 3 distance_lookup_table value for
distance_ref_points_depth = 2 before and after fix is below:
Before fix:
node 3 distance depth 0 - 0
node 3 distance depth 1 - 0
node 4 distance depth 0 - 4
node 4 distance depth 1 - 2
node 5 distance depth 0 - 5
node 5 distance depth 1 - 1
After fix
node 3 distance depth 0 - 3
node 3 distance depth 1 - 1
node 4 distance depth 0 - 4
node 4 distance depth 1 - 2
node 5 distance depth 0 - 5
node 5 distance depth 1 - 1
Without the fix, the nearest numa node to the pmem device(NUMA node 3) will be
picked as 4. After the fix, we get the correct numa node which is 5.
Fixes: da1115fdbd6e ("powerpc/nvdimm: Pick nearby online node if the device
node is not online")
Signed-off-by: Aneesh Kumar K.V
---
Changes from v1:
* Update commit message
* Update code comment
arch/powerpc/mm/numa.c| 1 +
arch/powerpc/platforms/pseries/papr_scm.c | 7 +++
2 files changed, 8 insertions(+)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index b44ce71917d7..16cfe56be05b 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -366,6 +366,7 @@ void update_numa_distance(struct device_node *node)
WARN(numa_distance_table[nid][nid] == -1,
"NUMA distance details for node %d not provided\n", nid);
}
+EXPORT_SYMBOL_GPL(update_numa_distance);
/*
* ibm,numa-lookup-index-table= {N, domainid1, domainid2, . domainidN}
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c
b/arch/powerpc/platforms/pseries/papr_scm.c
index 2f8385523a13..500b2bd5417c 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -1428,6 +1428,13 @@ static int papr_scm_probe(struct platform_device *pdev)
return -ENODEV;
}
+ /*
+* open firmware platform device create won't update the
+* numa distance table. For papr scm device we use
numa_map_to_online_node
+* to find the nearest online numa node and that requires corrrect
+* distance table information.
+*/
+ update_numa_distance(dn);
p = kzalloc(sizeof(*p), GFP_KERNEL);
if (!p)
--
2.39.2
[linux-next:master] BUILD REGRESSION 31bd35b66249699343d2416658f57e97314a433a
tree/branch:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
branch HEAD: 31bd35b66249699343d2416658f57e97314a433a Add linux-next specific
files for 20230403
Error/Warning reports:
https://lore.kernel.org/oe-kbuild-all/202303082135.njdx1bij-...@intel.com
https://lore.kernel.org/oe-kbuild-all/202303161521.jbgbafjj-...@intel.com
https://lore.kernel.org/oe-kbuild-all/202304040401.imxt7ubi-...@intel.com
Error/Warning: (recently discovered and may have been fixed)
drivers/gpu/drm/amd/amdgpu/../display/dc/link/link_validation.c:351:13:
warning: variable 'bw_needed' set but not used [-Wunused-but-set-variable]
drivers/gpu/drm/amd/amdgpu/../display/dc/link/link_validation.c:352:25:
warning: variable 'link' set but not used [-Wunused-but-set-variable]
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu13/smu_v13_0_6_ppt.c:309:17: sparse:
int
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu13/smu_v13_0_6_ppt.c:309:17: sparse:
void
drivers/net/wireless/legacy/ray_cs.c:628:17: warning: 'strncpy' specified bound
32 equals destination size [-Wstringop-truncation]
Unverified Error/Warning (likely false positive, please contact us if
interested):
drivers/acpi/property.c:985 acpi_data_prop_read_single() error: potentially
dereferencing uninitialized 'obj'.
drivers/cdx/cdx.c:393:20: error: initialization of 'ssize_t (*)(const struct
bus_type *, const char *, size_t)' {aka 'long int (*)(const struct bus_type *,
const char *, long unsigned int)'} from incompatible pointer type 'ssize_t
(*)(struct bus_type *, const char *, size_t)' {aka 'long int (*)(struct
bus_type *, const char *, long unsigned int)'}
[-Werror=incompatible-pointer-types]
drivers/pinctrl/pinctrl-mlxbf3.c:162:20: sparse: sparse: symbol
'mlxbf3_pmx_funcs' was not declared. Should it be static?
drivers/soc/fsl/qe/tsa.c:140:26: sparse: sparse: incorrect type in argument 2
(different address spaces)
drivers/soc/fsl/qe/tsa.c:150:27: sparse: sparse: incorrect type in argument 1
(different address spaces)
drivers/soc/fsl/qe/tsa.c:189:26: sparse: sparse: dereference of noderef
expression
drivers/soc/fsl/qe/tsa.c:663:22: sparse: sparse: incorrect type in assignment
(different address spaces)
drivers/soc/fsl/qe/tsa.c:673:21: sparse: sparse: incorrect type in assignment
(different address spaces)
drivers/usb/typec/ucsi/ucsi_glink.c:248:20: sparse: sparse: restricted __le32
degrades to integer
drivers/usb/typec/ucsi/ucsi_glink.c:81:23: sparse: sparse: incorrect type in
assignment (different base types)
drivers/usb/typec/ucsi/ucsi_glink.c:82:22: sparse: sparse: incorrect type in
assignment (different base types)
drivers/usb/typec/ucsi/ucsi_glink.c:83:24: sparse: sparse: incorrect type in
assignment (different base types)
include/linux/gpio/consumer.h: linux/err.h is included more than once.
include/linux/gpio/driver.h: asm/bug.h is included more than once.
io_uring/io_uring.c:432 io_prep_async_work() error: we previously assumed
'req->file' could be null (see line 425)
io_uring/kbuf.c:221 __io_remove_buffers() warn: variable dereferenced before
check 'bl->buf_ring' (see line 219)
Error/Warning ids grouped by kconfigs:
gcc_recent_errors
|-- alpha-allyesconfig
| |--
drivers-gpu-drm-amd-amdgpu-..-display-dc-link-link_validation.c:warning:variable-bw_needed-set-but-not-used
| |--
drivers-gpu-drm-amd-amdgpu-..-display-dc-link-link_validation.c:warning:variable-link-set-but-not-used
| `--
drivers-net-wireless-legacy-ray_cs.c:warning:strncpy-specified-bound-equals-destination-size
|-- alpha-randconfig-s051-20230403
| |--
drivers-gpu-drm-amd-amdgpu-..-pm-swsmu-smu13-smu_v13_0_6_ppt.c:sparse:int
| |--
drivers-gpu-drm-amd-amdgpu-..-pm-swsmu-smu13-smu_v13_0_6_ppt.c:sparse:sparse:incompatible-types-in-conditional-expression-(different-base-types):
| |--
drivers-gpu-drm-amd-amdgpu-..-pm-swsmu-smu13-smu_v13_0_6_ppt.c:sparse:void
| `--
drivers-pinctrl-pinctrl-mlxbf3.c:sparse:sparse:symbol-mlxbf3_pmx_funcs-was-not-declared.-Should-it-be-static
|-- arc-allyesconfig
| |--
drivers-gpu-drm-amd-amdgpu-..-display-dc-link-link_validation.c:warning:variable-bw_needed-set-but-not-used
| `--
drivers-gpu-drm-amd-amdgpu-..-display-dc-link-link_validation.c:warning:variable-link-set-but-not-used
|-- arm-allmodconfig
| |--
drivers-gpu-drm-amd-amdgpu-..-display-dc-link-link_validation.c:warning:variable-bw_needed-set-but-not-used
| `--
drivers-gpu-drm-amd-amdgpu-..-display-dc-link-link_validation.c:warning:variable-link-set-but-not-used
|-- arm-allyesconfig
| |--
drivers-gpu-drm-amd-amdgpu-..-display-dc-link-link_validation.c:warning:variable-bw_needed-set-but-not-used
| `--
drivers-gpu-drm-amd-amdgpu-..-display-dc-link-link_validation.c:warning:variable-link-set-but-not-used
|-- arm64-allyesconfig
| |--
drivers-cdx-cdx.c:error:initialization-of-ssize_t-(-)(const-struct-bus_type-const-char-size_t)-aka-long-int-(-)(const-struct-bus_type-const-char-long-unsigned-int)-from-incompatible-pointer-type
[PATCH 2/2] ASoC: fsl_mqs: call pm_runtime_disable() on error path
pm_runtime_disable was missed in cleanup operation, which corresponds to
the earlier call to pm_runtime_enable.
To fix this, add pm_runtime_disable() on error path.
Fixes: 9e28f6532c61 ("ASoC: fsl_mqs: Add MQS component driver")
Signed-off-by: Liliang Ye
Reviewed-by: Dan Carpenter
---
sound/soc/fsl/fsl_mqs.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_mqs.c b/sound/soc/fsl/fsl_mqs.c
index 32d20d351bbf..129d426c60c4 100644
--- a/sound/soc/fsl/fsl_mqs.c
+++ b/sound/soc/fsl/fsl_mqs.c
@@ -251,9 +251,13 @@ static int fsl_mqs_probe(struct platform_device *pdev)
ret = devm_snd_soc_register_component(>dev, _codec_fsl_mqs,
_mqs_dai, 1);
if (ret)
- return ret;
+ goto err_pm_disable;
return 0;
+
+err_pm_disable:
+ pm_runtime_disable(>dev);
+ return ret;
}
static int fsl_mqs_remove(struct platform_device *pdev)
--
2.34.1
[PATCH 1/2] ASoC: fsl_mqs: move of_node_put() to the correct location
of_node_put() should have been done directly after
mqs_priv->regmap = syscon_node_to_regmap(gpr_np);
otherwise it creates a reference leak on the success path.
To fix this, of_node_put() is moved to the correct location, and change
all the gotos to direct returns.
Fixes: a9d273671440 ("ASoC: fsl_mqs: Fix error handling in probe")
Signed-off-by: Liliang Ye
Reviewed-by: Dan Carpenter
---
sound/soc/fsl/fsl_mqs.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/sound/soc/fsl/fsl_mqs.c b/sound/soc/fsl/fsl_mqs.c
index 4922e6795b73..32d20d351bbf 100644
--- a/sound/soc/fsl/fsl_mqs.c
+++ b/sound/soc/fsl/fsl_mqs.c
@@ -210,10 +210,10 @@ static int fsl_mqs_probe(struct platform_device *pdev)
}
mqs_priv->regmap = syscon_node_to_regmap(gpr_np);
+ of_node_put(gpr_np);
if (IS_ERR(mqs_priv->regmap)) {
dev_err(>dev, "failed to get gpr regmap\n");
- ret = PTR_ERR(mqs_priv->regmap);
- goto err_free_gpr_np;
+ return PTR_ERR(mqs_priv->regmap);
}
} else {
regs = devm_platform_ioremap_resource(pdev, 0);
@@ -242,8 +242,7 @@ static int fsl_mqs_probe(struct platform_device *pdev)
if (IS_ERR(mqs_priv->mclk)) {
dev_err(>dev, "failed to get the clock: %ld\n",
PTR_ERR(mqs_priv->mclk));
- ret = PTR_ERR(mqs_priv->mclk);
- goto err_free_gpr_np;
+ return PTR_ERR(mqs_priv->mclk);
}
dev_set_drvdata(>dev, mqs_priv);
@@ -252,13 +251,9 @@ static int fsl_mqs_probe(struct platform_device *pdev)
ret = devm_snd_soc_register_component(>dev, _codec_fsl_mqs,
_mqs_dai, 1);
if (ret)
- goto err_free_gpr_np;
- return 0;
-
-err_free_gpr_np:
- of_node_put(gpr_np);
+ return ret;
- return ret;
+ return 0;
}
static int fsl_mqs_remove(struct platform_device *pdev)
--
2.34.1
Re: [PATCH v1] dt-bindings: move cache controller bindings to a cache directory
On Thu, 30 Mar 2023 18:32:56 +0100, Conor Dooley wrote:
> From: Conor Dooley
>
> There's a bunch of bindings for (mostly l2) cache controllers
> scattered to the four winds, move them to a common directory.
> I renamed the freescale l2cache.txt file, as while that might make sense
> when the parent dir is fsl, it's confusing after the move.
> The two Marvell bindings have had a "marvell," prefix added to match
> their compatibles.
>
> Signed-off-by: Conor Dooley
> ---
> .../{memory-controllers => cache}/baikal,bt1-l2-ctl.yaml| 2 +-
> .../{powerpc/fsl/l2cache.txt => cache/freescale-l2cache.txt}| 0
> Documentation/devicetree/bindings/{arm => cache}/l2c2x0.yaml| 2 +-
> .../{arm/mrvl/feroceon.txt => cache/marvell,feroceon-cache.txt} | 0
> .../{arm/mrvl/tauros2.txt => cache/marvell,tauros2-cache.txt} | 0
> .../devicetree/bindings/{arm/msm => cache}/qcom,llcc.yaml | 2 +-
> .../devicetree/bindings/{riscv => cache}/sifive,ccache0.yaml| 2 +-
> .../socionext => cache}/socionext,uniphier-system-cache.yaml| 2 +-
> MAINTAINERS | 2 ++
> 9 files changed, 7 insertions(+), 5 deletions(-)
> rename Documentation/devicetree/bindings/{memory-controllers =>
> cache}/baikal,bt1-l2-ctl.yaml (95%)
> rename Documentation/devicetree/bindings/{powerpc/fsl/l2cache.txt =>
> cache/freescale-l2cache.txt} (100%)
> rename Documentation/devicetree/bindings/{arm => cache}/l2c2x0.yaml (99%)
> rename Documentation/devicetree/bindings/{arm/mrvl/feroceon.txt =>
> cache/marvell,feroceon-cache.txt} (100%)
> rename Documentation/devicetree/bindings/{arm/mrvl/tauros2.txt =>
> cache/marvell,tauros2-cache.txt} (100%)
> rename Documentation/devicetree/bindings/{arm/msm => cache}/qcom,llcc.yaml
> (96%)
> rename Documentation/devicetree/bindings/{riscv =>
> cache}/sifive,ccache0.yaml (98%)
> rename Documentation/devicetree/bindings/{arm/socionext =>
> cache}/socionext,uniphier-system-cache.yaml (96%)
>
Applied, thanks!
Re: [PATCH v3 26/35] mm: fall back to mmap_lock if vma->anon_vma is not yet set
On Fri, Feb 17, 2023 at 08:10:35AM -0800, Suren Baghdasaryan wrote: > On Fri, Feb 17, 2023 at 8:05 AM Matthew Wilcox wrote: > > > > On Thu, Feb 16, 2023 at 06:14:59PM -0800, Suren Baghdasaryan wrote: > > > On Thu, Feb 16, 2023 at 11:43 AM Suren Baghdasaryan > > > wrote: > > > > > > > > On Thu, Feb 16, 2023 at 7:44 AM Matthew Wilcox > > > > wrote: > > > > > > > > > > On Wed, Feb 15, 2023 at 09:17:41PM -0800, Suren Baghdasaryan wrote: > > > > > > When vma->anon_vma is not set, page fault handler will set it by > > > > > > either > > > > > > reusing anon_vma of an adjacent VMA if VMAs are compatible or by > > > > > > allocating a new one. find_mergeable_anon_vma() walks VMA tree to > > > > > > find > > > > > > a compatible adjacent VMA and that requires not only the faulting > > > > > > VMA > > > > > > to be stable but also the tree structure and other VMAs inside that > > > > > > tree. > > > > > > Therefore locking just the faulting VMA is not enough for this > > > > > > search. > > > > > > Fall back to taking mmap_lock when vma->anon_vma is not set. This > > > > > > situation happens only on the first page fault and should not affect > > > > > > overall performance. > > > > > > > > > > I think I asked this before, but don't remember getting an aswer. > > > > > Why do we defer setting anon_vma to the first fault? Why don't we > > > > > set it up at mmap time? > > > > > > > > Yeah, I remember that conversation Matthew and I could not find the > > > > definitive answer at the time. I'll look into that again or maybe > > > > someone can answer it here. > > > > > > After looking into it again I'm still under the impression that > > > vma->anon_vma is populated lazily (during the first page fault rather > > > than at mmap time) to avoid doing extra work for areas which are never > > > faulted. Though I might be missing some important detail here. > > > > How often does userspace call mmap() and then _never_ fault on it? > > I appreciate that userspace might mmap() gigabytes of address space and > > then only end up using a small amount of it, so populating it lazily > > makes sense. But creating a region and never faulting on it? The only > > use-case I can think of is loading shared libraries: > > > > openat(AT_FDCWD, "/lib/x86_64-linux-gnu/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 > > (...) > > mmap(NULL, 197, PROT_READ, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = > > 0x7f0ce612e000 > > mmap(0x7f0ce6154000, 1396736, PROT_READ|PROT_EXEC, > > MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x26000) = 0x7f0ce6154000 > > mmap(0x7f0ce62a9000, 339968, PROT_READ, > > MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x17b000) = 0x7f0ce62a9000 > > mmap(0x7f0ce62fc000, 24576, PROT_READ|PROT_WRITE, > > MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1ce000) = 0x7f0ce62fc000 > > mmap(0x7f0ce6302000, 53072, PROT_READ|PROT_WRITE, > > MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f0ce6302000 > > > > but that's a file-backed VMA, not an anon VMA. > > Might the case of dup_mmap() while forking be the reason why a VMA in > the child process might be never used while parent uses it (or visa > versa)? Again, I'm not sure this is the reason but I can find no other > good explanation. I found an explanation! Well, a partial one. If we MAP_PRIVATE a file mapping (like, er those ones up there) and only take read faults on it, we can postpone allocation of the anon_vma indefinitely. But once we take a write fault in that VMA, we need to allocate an anon_vma for it so that we can track the anonymous pages that have been allocated to satisfy the copy-on-write (see do_cow_fault()). However, I think in that caase, we could probably skip the find_mergeable_anon_vma() step. We don't today; we check whether a->vm_file == b->vm_file in anon_vma_compatible, but I wonder if that triggers often.
Re: [PATCH 1/2] ASoC: fsl_mqs: move of_node_put() to the correct location
On Mon, Apr 03, 2023 at 11:26:47PM +0800, Liliang Ye wrote:
> of_node_put() should have been done directly after
> mqs_priv->regmap = syscon_node_to_regmap(gpr_np);
> otherwise it creates a reference leak on the success path.
>
> To fix this, of_node_put() is moved to the correct location, and change
> all the gotos to direct returns.
>
> Fixes: a9d273671440 ("ASoC: fsl_mqs: Fix error handling in probe")
> Signed-off-by: Liliang Ye
> Reviewed-by: Dan Carpenter
> ---
These patches are from a university. I think this patch was based on
manual review rather than static analysis. They have not been tested
and this one affects the success path so that's always extra risky.
However I do believe the patch is correct and reviewed it before it was
sent publically.
regards,
dan carpenter
Re: [PATCH v2 1/2] soc: fsl: qbman: Always disable interrupts when taking cgr_lock
On 4/3/23 10:02, Vladimir Oltean wrote:
> On Fri, Mar 31, 2023 at 11:14:12AM -0400, Sean Anderson wrote:
>> smp_call_function_single disables IRQs when executing the callback. To
>> prevent deadlocks, we must disable IRQs when taking cgr_lock elsewhere.
>> This is already done by qman_update_cgr and qman_delete_cgr; fix the
>> other lockers.
>>
>> Fixes: c535e923bb97 ("soc/fsl: Introduce DPAA 1.x QMan device driver")
>
> If you've identified smp_call_function_single() as the problem, then the
> true issue seems to lie in commit 96f413f47677 ("soc/fsl/qbman: fix
> issue in qman_delete_cgr_safe()") and not in the initial commit, no?
Yes, that seems better. I did a blame and saw that qman_delete_cgr_safe
had been around since the initial driver, but I didn't realize it worked
in a different way back then.
--Sean
> Anyway,
>
> Tested-by: Vladimir Oltean
Re: [PATCH v2 2/2] soc: fsl: qbman: Use raw spinlock for cgr_lock
On Fri, Mar 31, 2023 at 11:14:13AM -0400, Sean Anderson wrote:
> cgr_lock may be locked with interrupts already disabled by
> smp_call_function_single. As such, we must use a raw spinlock to avoid
> problems on PREEMPT_RT kernels. Although this bug has existed for a
> while, it was not apparent until commit ef2a8d5478b9 ("net: dpaa: Adjust
> queue depth on rate change") which invokes smp_call_function_single via
> qman_update_cgr_safe every time a link goes up or down.
>
> Fixes: c535e923bb97 ("soc/fsl: Introduce DPAA 1.x QMan device driver")
Same comment about the Fixes tag.
git tag --contains c535e923bb97
v4.9
git tag --contains 96f413f47677
v4.16
Looking at https://www.kernel.org/, I see that kernel 4.14 is still
maintained but should not have this patch backported, do you agree?
> Reported-by: Vladimir Oltean
> Link: https://lore.kernel.org/all/20230323153935.nofnjucqjqnz34ej@skbuf/
> Signed-off-by: Sean Anderson
> ---
Anyway,
Tested-by: Vladimir Oltean
Re: [PATCH v2 1/2] soc: fsl: qbman: Always disable interrupts when taking cgr_lock
On Fri, Mar 31, 2023 at 11:14:12AM -0400, Sean Anderson wrote:
> smp_call_function_single disables IRQs when executing the callback. To
> prevent deadlocks, we must disable IRQs when taking cgr_lock elsewhere.
> This is already done by qman_update_cgr and qman_delete_cgr; fix the
> other lockers.
>
> Fixes: c535e923bb97 ("soc/fsl: Introduce DPAA 1.x QMan device driver")
If you've identified smp_call_function_single() as the problem, then the
true issue seems to lie in commit 96f413f47677 ("soc/fsl/qbman: fix
issue in qman_delete_cgr_safe()") and not in the initial commit, no?
Anyway,
Tested-by: Vladimir Oltean
Re: [PATCH 01/10] locking/atomic: Add missing cast to try_cmpxchg() fallbacks
On Sun, Mar 26, 2023 at 09:28:38PM +0200, Uros Bizjak wrote:
> On Fri, Mar 24, 2023 at 5:33 PM Mark Rutland wrote:
> >
> > On Fri, Mar 24, 2023 at 04:14:22PM +, Mark Rutland wrote:
> > > On Fri, Mar 24, 2023 at 04:43:32PM +0100, Uros Bizjak wrote:
> > > > On Fri, Mar 24, 2023 at 3:13 PM Mark Rutland
> > > > wrote:
> > > > >
> > > > > On Sun, Mar 05, 2023 at 09:56:19PM +0100, Uros Bizjak wrote:
> > > > > > Cast _oldp to the type of _ptr to avoid incompatible-pointer-types
> > > > > > warning.
> > > > >
> > > > > Can you give an example of where we are passing an incompatible
> > > > > pointer?
> > > >
> > > > An example is patch 10/10 from the series, which will fail without
> > > > this fix when fallback code is used. We have:
> > > >
> > > > - } while (local_cmpxchg(>head, offset, head) != offset);
> > > > + } while (!local_try_cmpxchg(>head, , head));
> > > >
> > > > where rb->head is defined as:
> > > >
> > > > typedef struct {
> > > >atomic_long_t a;
> > > > } local_t;
> > > >
> > > > while offset is defined as 'unsigned long'.
> > >
> > > Ok, but that's because we're doing the wrong thing to start with.
> > >
> > > Since local_t is defined in terms of atomic_long_t, we should define the
> > > generic local_try_cmpxchg() in terms of atomic_long_try_cmpxchg(). We'll
> > > still
> > > have a mismatch between 'long *' and 'unsigned long *', but then we can
> > > fix
> > > that in the callsite:
> > >
> > > while (!local_try_cmpxchg(>head, &(long *)offset, head))
> >
> > Sorry, that should be:
> >
> > while (!local_try_cmpxchg(>head, (long *), head))
>
> The fallbacks are a bit more complicated than above, and are different
> from atomic_try_cmpxchg.
>
> Please note in patch 2/10, the falbacks when arch_try_cmpxchg_local
> are not defined call arch_cmpxchg_local. Also in patch 2/10,
> try_cmpxchg_local is introduced, where it calls
> arch_try_cmpxchg_local. Targets (and generic code) simply define (e.g.
> :
>
> #define local_cmpxchg(l, o, n) \
>(cmpxchg_local(&((l)->a.counter), (o), (n)))
> +#define local_try_cmpxchg(l, po, n) \
> + (try_cmpxchg_local(&((l)->a.counter), (po), (n)))
>
> which is part of the local_t API. Targets should either define all
> these #defines, or none. There are no partial fallbacks as is the case
> with atomic_t.
Whether or not there are fallbacks is immaterial.
In those cases, architectures can just as easily write C wrappers, e.g.
long local_cmpxchg(local_t *l, long old, long new)
{
return cmpxchg_local(>a.counter, old, new);
}
long local_try_cmpxchg(local_t *l, long *old, long new)
{
return try_cmpxchg_local(>a.counter, old, new);
}
> The core of the local_h API is in the local.h header. If the target
> doesn't define its own local.h header, then asm-generic/local.h is
> used that does exactly what you propose above regarding the usage of
> atomic functions.
>
> OTOH, when the target defines its own local.h, then the above
> target-dependent #define path applies. The target should define its
> own arch_try_cmpxchg_local, otherwise a "generic" target-dependent
> fallback that calls target arch_cmpxchg_local applies. In the case of
> x86, patch 9/10 enables new instruction by defining
> arch_try_cmpxchg_local.
>
> FYI, the patch sequence is carefully chosen so that x86 also exercises
> fallback code between different patches in the series.
>
> Targets are free to define local_t to whatever they like, but for some
> reason they all define it to:
>
> typedef struct {
> atomic_long_t a;
> } local_t;
Yes, which is why I used atomic_long() above.
> so they have to dig the variable out of the struct like:
>
> #define local_cmpxchg(l, o, n) \
> (cmpxchg_local(&((l)->a.counter), (o), (n)))
>
> Regarding the mismatch of 'long *' vs 'unsigned long *': x86
> target-specific code does for try_cmpxchg:
>
> #define __raw_try_cmpxchg(_ptr, _pold, _new, size, lock) \
> ({ \
> bool success; \
> __typeof__(_ptr) _old = (__typeof__(_ptr))(_pold); \
> __typeof__(*(_ptr)) __old = *_old; \
> __typeof__(*(_ptr)) __new = (_new); \
>
> so, it *does* cast the "old" pointer to the type of "ptr". The generic
> code does *not*. This difference is dangerous, since the compilation
> of some code involving try_cmpxchg will compile OK for x86 but will
> break for other targets that use try_cmpxchg fallback templates (I was
> the unlucky one that tripped on this in the past). Please note that
> this problem is not specific to the proposed local_try_cmpxchg series,
> but affects the existing try_cmpxchg API.
I understand the problem of arch code differing from generic code, and that we
want to have *a* consistent behaviour for hte API.
What I'm saying is that the behaviour we should aim for is where the 'old'
pointer has a specific type (long), and we always require that, as we do for
the various atomic_*() APIs of which local_*() is a cousin.
> Also, I don't think that "fixing" callsites is the right thing to
Re: [PATCH 1/2] pseries/smp: export the smt level in the SYS FS.
On 31/03/2023 18:05:27, Michal Suchánek wrote:
> Hello,
>
> On Fri, Mar 31, 2023 at 05:39:04PM +0200, Laurent Dufour wrote:
>> There is no SMT level recorded in the kernel neither in user space.
>> Indeed there is no real constraint about that and mixed SMT levels are
>> allowed and system is working fine this way.
>>
>> However when new CPU are added, the kernel is onlining all the threads
>> which is leading to mixed SMT levels and confuse end user a bit.
>>
>> To prevent this exports a SMT level from the kernel so user space
>> application like the energy daemon, could read it to adjust their settings.
>> There is no action unless recording the value when a SMT value is written
>> into the new sysfs entry. User space applications like ppc64_cpu should
>> update the sysfs when changing the SMT level to keep the system consistent.
>>
>> Suggested-by: Srikar Dronamraju
>> Signed-off-by: Laurent Dufour
>> ---
>> arch/powerpc/platforms/pseries/pseries.h | 3 ++
>> arch/powerpc/platforms/pseries/smp.c | 39
>> 2 files changed, 42 insertions(+)
>>
>> diff --git a/arch/powerpc/platforms/pseries/pseries.h
>> b/arch/powerpc/platforms/pseries/pseries.h
>> index f8bce40ebd0c..af0a145af98f 100644
>> --- a/arch/powerpc/platforms/pseries/pseries.h
>> +++ b/arch/powerpc/platforms/pseries/pseries.h
>> @@ -23,7 +23,9 @@ extern int pSeries_machine_check_exception(struct pt_regs
>> *regs);
>> extern long pseries_machine_check_realmode(struct pt_regs *regs);
>> void pSeries_machine_check_log_err(void);
>>
>> +
>> #ifdef CONFIG_SMP
>> +extern int pseries_smt;
>> extern void smp_init_pseries(void);
>>
>> /* Get state of physical CPU from query_cpu_stopped */
>> @@ -34,6 +36,7 @@ int smp_query_cpu_stopped(unsigned int pcpu);
>> #define QCSS_HARDWARE_ERROR -1
>> #define QCSS_HARDWARE_BUSY -2
>> #else
>> +#define pseries_smt 1
>
> Is this really needed for anything?
>
> The code using pseries_smt would not compile with a define, and would be
> only compiled with SMP enabled anyway so we should not need this.
>
Hi Michal,
I do agree, the pseries code is implying SMP.
When writing that code, I found that SMP conditional block and just add
this define to be sure the code will compile in the case SMP is not
defined, but that's probably useless.
Instead of resending a new series, Michael, could you please remove that
line when applying the patch to your tree?
Thanks,
Laurent.
> Thanks
>
> Michal
>
>> static inline void smp_init_pseries(void) { }
>> #endif
>>
>> diff --git a/arch/powerpc/platforms/pseries/smp.c
>> b/arch/powerpc/platforms/pseries/smp.c
>> index c597711ef20a..6c382922f8f3 100644
>> --- a/arch/powerpc/platforms/pseries/smp.c
>> +++ b/arch/powerpc/platforms/pseries/smp.c
>> @@ -21,6 +21,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>> #include
>> @@ -45,6 +46,8 @@
>>
>> #include "pseries.h"
>>
>> +int pseries_smt;
>> +
>> /*
>> * The Primary thread of each non-boot processor was started from the OF
>> client
>> * interface by prom_hold_cpus and is spinning on secondary_hold_spinloop.
>> @@ -280,3 +283,39 @@ void __init smp_init_pseries(void)
>>
>> pr_debug(" <- smp_init_pSeries()\n");
>> }
>> +
>> +static ssize_t pseries_smt_store(struct class *class,
>> + struct class_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> +int smt;
>> +
>> +if (kstrtou32(buf, 0, ) || !smt || smt > (u32) threads_per_core) {
>> +pr_err("Invalid pseries_smt specified.\n");
>> +return -EINVAL;
>> +}
>> +
>> +pseries_smt = smt;
>> +
>> +return count;
>> +}
>> +
>> +static ssize_t pseries_smt_show(struct class *class, struct class_attribute
>> *attr,
>> + char *buf)
>> +{
>> +return sysfs_emit(buf, "%d\n", pseries_smt);
>> +}
>> +
>> +static CLASS_ATTR_RW(pseries_smt);
>> +
>> +static int __init pseries_smt_init(void)
>> +{
>> +int rc;
>> +
>> +pseries_smt = smt_enabled_at_boot;
>> +rc = sysfs_create_file(kernel_kobj, _attr_pseries_smt.attr);
>> +if (rc)
>> +pr_err("Can't create pseries_smt sysfs/kernel entry.\n");
>> +return rc;
>> +}
>> +machine_device_initcall(pseries, pseries_smt_init);
>> --
>> 2.40.0
>>
Re: [PATCH] ASoC: fsl_sai: Use physical format width
On 3/31/23 16:34, Mark Brown wrote: > On Fri, Mar 31, 2023 at 02:26:33PM +, Emil Abildgaard Svendsen wrote: >> On 3/31/23 04:55, Shengjiu Wang wrote: > >>> There are different requirements for this slot width. Some need physical >>> width, >>> Some need format width. We need to be careful about change here. > >> I might be wrong but shouldn't physical width always correspond to what >> can be physically measured. If you need the slot width, the same as >> format width you use a format with matching widths like >> SNDRV_PCM_FORMAT_S24_3LE? > > The point is more that things are likely to be relying on the > current behaviour, for example CODECs that don't actually support > 24 bit audio due to a power of two requirement but cope fine when > you play 24 bit audio in a 32 bit timeslot. This creates issues > changing things even if the new behaviour is in some sense > better. This is one of the unfortunate consequences of DT. I guess if you want to do it runtime you have to create an audio card that will do it for you. I would have preferred it to be handled as close to hardware as possible. But that's a good enough compromise for not breaking current behavior. Thanks! Kind regards, Emil Svendsen
