* Re: [PATCH v5 01/10] capabilities: introduce CAP_PERFMON to kernel and user space
From: Alexey Budankov @ 2020-02-06 18:03 UTC (permalink / raw)
To: Stephen Smalley, Serge Hallyn, James Morris
Cc: Mark Rutland, Song Liu, Peter Zijlstra,
joonas.lahtinen@linux.intel.com, Will Deacon, Alexei Starovoitov,
Lionel Landwerlin, Paul Mackerras, Jiri Olsa, Alexei Starovoitov,
Andi Kleen, Igor Lubashev, Alexander Shishkin, Ingo Molnar,
oprofile-list, linux-arm-kernel, Robert Richter,
selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
jani.nikula@linux.intel.com, Arnaldo Carvalho de Melo,
rodrigo.vivi@intel.com, Namhyung Kim, Thomas Gleixner,
Stephane Eranian, linux-parisc@vger.kernel.org, linux-kernel,
Andy Lutomirski, linux-perf-users@vger.kernel.org,
linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <ac0dbab7-de47-ee34-bb88-4c43d3538b7d@linux.intel.com>
On 22.01.2020 17:25, Alexey Budankov wrote:
>
> On 22.01.2020 17:07, Stephen Smalley wrote:
>> On 1/22/20 5:45 AM, Alexey Budankov wrote:
>>>
>>> On 21.01.2020 21:27, Alexey Budankov wrote:
>>>>
>>>> On 21.01.2020 20:55, Alexei Starovoitov wrote:
>>>>> On Tue, Jan 21, 2020 at 9:31 AM Alexey Budankov
>>>>> <alexey.budankov@linux.intel.com> wrote:
>>>>>>
>>>>>>
>>>>>> On 21.01.2020 17:43, Stephen Smalley wrote:
>>>>>>> On 1/20/20 6:23 AM, Alexey Budankov wrote:
>>>>>>>>
>>>>>>>> Introduce CAP_PERFMON capability designed to secure system performance
>>>>>>>> monitoring and observability operations so that CAP_PERFMON would assist
>>>>>>>> CAP_SYS_ADMIN capability in its governing role for perf_events, i915_perf
>>>>>>>> and other performance monitoring and observability subsystems.
>>>>>>>>
>>>>>>>> CAP_PERFMON intends to harden system security and integrity during system
>>>>>>>> performance monitoring and observability operations by decreasing attack
>>>>>>>> surface that is available to a CAP_SYS_ADMIN privileged process [1].
>>>>>>>> Providing access to system performance monitoring and observability
>>>>>>>> operations under CAP_PERFMON capability singly, without the rest of
>>>>>>>> CAP_SYS_ADMIN credentials, excludes chances to misuse the credentials and
>>>>>>>> makes operation more secure.
>>>>>>>>
>>>>>>>> CAP_PERFMON intends to take over CAP_SYS_ADMIN credentials related to
>>>>>>>> system performance monitoring and observability operations and balance
>>>>>>>> amount of CAP_SYS_ADMIN credentials following the recommendations in the
>>>>>>>> capabilities man page [1] for CAP_SYS_ADMIN: "Note: this capability is
>>>>>>>> overloaded; see Notes to kernel developers, below."
>>>>>>>>
>>>>>>>> Although the software running under CAP_PERFMON can not ensure avoidance
>>>>>>>> of related hardware issues, the software can still mitigate these issues
>>>>>>>> following the official embargoed hardware issues mitigation procedure [2].
>>>>>>>> The bugs in the software itself could be fixed following the standard
>>>>>>>> kernel development process [3] to maintain and harden security of system
>>>>>>>> performance monitoring and observability operations.
>>>>>>>>
>>>>>>>> [1] http://man7.org/linux/man-pages/man7/capabilities.7.html
>>>>>>>> [2] https://www.kernel.org/doc/html/latest/process/embargoed-hardware-issues.html
>>>>>>>> [3] https://www.kernel.org/doc/html/latest/admin-guide/security-bugs.html
<SNIP>
>>>>>>>>
>>>>>>>> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
>>>>>>>
>>>>>>> Why _noaudit()? Normally only used when a permission failure is non-fatal to the operation. Otherwise, we want the audit message.
>>>
>>> So far so good, I suggest using the simplest version for v6:
>>>
>>> static inline bool perfmon_capable(void)
>>> {
>>> return capable(CAP_PERFMON) || capable(CAP_SYS_ADMIN);
>>> }
>>>
>>> It keeps the implementation simple and readable. The implementation is more
>>> performant in the sense of calling the API - one capable() call for CAP_PERFMON
>>> privileged process.
>>>
>>> Yes, it bloats audit log for CAP_SYS_ADMIN privileged and unprivileged processes,
>>> but this bloating also advertises and leverages using more secure CAP_PERFMON
>>> based approach to use perf_event_open system call.
>>
>> I can live with that. We just need to document that when you see both a CAP_PERFMON and a CAP_SYS_ADMIN audit message for a process, try only allowing CAP_PERFMON first and see if that resolves the issue. We have a similar issue with CAP_DAC_READ_SEARCH versus CAP_DAC_OVERRIDE.
>
> perf security [1] document can be updated, at least, to align and document
> this audit logging specifics.
And I plan to update the document right after this patch set is accepted.
Feel free to let me know of the places in the kernel docs that also
require update w.r.t CAP_PERFMON extension.
~Alexey
>
> ~Alexey
>
> [1] https://www.kernel.org/doc/html/latest/admin-guide/perf-security.html
>
^ permalink raw reply
* [powerpc:merge] BUILD SUCCESS 530a1cfd52af0aba1af4b1c9a7bc66a202a459b1
From: kbuild test robot @ 2020-02-06 16:20 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git merge
branch HEAD: 530a1cfd52af0aba1af4b1c9a7bc66a202a459b1 Automatic merge of branches 'master', 'next' and 'fixes' into merge
elapsed time: 3124m
configs tested: 279
configs skipped: 1
The following configs have been built successfully.
More configs may be tested in the coming days.
arm allmodconfig
arm allnoconfig
arm allyesconfig
arm at91_dt_defconfig
arm efm32_defconfig
arm exynos_defconfig
arm multi_v5_defconfig
arm multi_v7_defconfig
arm shmobile_defconfig
arm sunxi_defconfig
arm64 allmodconfig
arm64 allnoconfig
arm64 allyesconfig
arm64 defconfig
sparc allyesconfig
riscv defconfig
riscv nommu_virt_defconfig
sparc64 defconfig
um defconfig
nds32 defconfig
sh rsk7269_defconfig
sparc64 allmodconfig
i386 allnoconfig
h8300 h8300h-sim_defconfig
sparc defconfig
alpha defconfig
parisc defconfig
h8300 h8s-sim_defconfig
um i386_defconfig
arc defconfig
parisc allnoconfig
um x86_64_defconfig
microblaze nommu_defconfig
microblaze mmu_defconfig
s390 defconfig
riscv allnoconfig
i386 alldefconfig
i386 allyesconfig
i386 defconfig
ia64 alldefconfig
ia64 allmodconfig
ia64 allnoconfig
ia64 allyesconfig
ia64 defconfig
c6x allyesconfig
c6x evmc6678_defconfig
nios2 10m50_defconfig
nios2 3c120_defconfig
openrisc or1ksim_defconfig
openrisc simple_smp_defconfig
xtensa common_defconfig
xtensa iss_defconfig
csky defconfig
nds32 allnoconfig
h8300 edosk2674_defconfig
m68k allmodconfig
m68k m5475evb_defconfig
m68k multi_defconfig
m68k sun3_defconfig
arc allyesconfig
powerpc allnoconfig
powerpc defconfig
powerpc ppc64_defconfig
powerpc rhel-kconfig
mips 32r2_defconfig
mips 64r6el_defconfig
mips allmodconfig
mips allnoconfig
mips allyesconfig
mips fuloong2e_defconfig
mips malta_kvm_defconfig
parisc allyesonfig
parisc b180_defconfig
parisc c3000_defconfig
x86_64 randconfig-a001-20200206
x86_64 randconfig-a002-20200206
x86_64 randconfig-a003-20200206
i386 randconfig-a001-20200206
i386 randconfig-a002-20200206
i386 randconfig-a003-20200206
x86_64 randconfig-a001-20200204
x86_64 randconfig-a002-20200204
x86_64 randconfig-a003-20200204
i386 randconfig-a001-20200204
i386 randconfig-a002-20200204
i386 randconfig-a003-20200204
alpha randconfig-a001-20200206
m68k randconfig-a001-20200206
mips randconfig-a001-20200206
nds32 randconfig-a001-20200206
parisc randconfig-a001-20200206
riscv randconfig-a001-20200206
alpha randconfig-a001-20200204
m68k randconfig-a001-20200204
mips randconfig-a001-20200204
nds32 randconfig-a001-20200204
parisc randconfig-a001-20200204
riscv randconfig-a001-20200204
c6x randconfig-a001-20200206
h8300 randconfig-a001-20200206
microblaze randconfig-a001-20200206
nios2 randconfig-a001-20200206
sparc64 randconfig-a001-20200206
c6x randconfig-a001-20200205
h8300 randconfig-a001-20200205
microblaze randconfig-a001-20200205
nios2 randconfig-a001-20200205
sparc64 randconfig-a001-20200205
c6x randconfig-a001-20200204
h8300 randconfig-a001-20200204
microblaze randconfig-a001-20200204
nios2 randconfig-a001-20200204
sparc64 randconfig-a001-20200204
csky randconfig-a001-20200205
openrisc randconfig-a001-20200205
s390 randconfig-a001-20200205
sh randconfig-a001-20200205
xtensa randconfig-a001-20200205
csky randconfig-a001-20200204
openrisc randconfig-a001-20200204
s390 randconfig-a001-20200204
sh randconfig-a001-20200204
xtensa randconfig-a001-20200204
csky randconfig-a001-20200206
openrisc randconfig-a001-20200206
s390 randconfig-a001-20200206
sh randconfig-a001-20200206
xtensa randconfig-a001-20200206
x86_64 randconfig-b001-20200206
x86_64 randconfig-b002-20200206
x86_64 randconfig-b003-20200206
i386 randconfig-b001-20200206
i386 randconfig-b002-20200206
i386 randconfig-b003-20200206
x86_64 randconfig-b001-20200205
x86_64 randconfig-b002-20200205
x86_64 randconfig-b003-20200205
i386 randconfig-b001-20200205
i386 randconfig-b002-20200205
i386 randconfig-b003-20200205
i386 randconfig-b001-20200204
i386 randconfig-b002-20200204
x86_64 randconfig-b002-20200204
x86_64 randconfig-b001-20200204
i386 randconfig-b003-20200204
x86_64 randconfig-b003-20200204
x86_64 randconfig-c001-20200204
x86_64 randconfig-c002-20200204
x86_64 randconfig-c003-20200204
i386 randconfig-c001-20200204
i386 randconfig-c002-20200204
i386 randconfig-c003-20200204
x86_64 randconfig-c001-20200205
x86_64 randconfig-c002-20200205
x86_64 randconfig-c003-20200205
i386 randconfig-c001-20200205
i386 randconfig-c002-20200205
i386 randconfig-c003-20200205
x86_64 randconfig-d001-20200204
x86_64 randconfig-d002-20200204
x86_64 randconfig-d003-20200204
i386 randconfig-d001-20200204
i386 randconfig-d002-20200204
i386 randconfig-d003-20200204
x86_64 randconfig-d001-20200205
x86_64 randconfig-d002-20200205
x86_64 randconfig-d003-20200205
i386 randconfig-d001-20200205
i386 randconfig-d002-20200205
i386 randconfig-d003-20200205
x86_64 randconfig-d001-20200206
x86_64 randconfig-d002-20200206
x86_64 randconfig-d003-20200206
i386 randconfig-d001-20200206
i386 randconfig-d002-20200206
i386 randconfig-d003-20200206
x86_64 randconfig-e001-20200206
x86_64 randconfig-e002-20200206
x86_64 randconfig-e003-20200206
i386 randconfig-e001-20200206
i386 randconfig-e002-20200206
i386 randconfig-e003-20200206
x86_64 randconfig-e001-20200204
x86_64 randconfig-e002-20200204
x86_64 randconfig-e003-20200204
i386 randconfig-e001-20200204
i386 randconfig-e002-20200204
i386 randconfig-e003-20200204
i386 randconfig-f002-20200204
i386 randconfig-f003-20200204
x86_64 randconfig-f002-20200204
i386 randconfig-f001-20200204
x86_64 randconfig-f001-20200204
x86_64 randconfig-f003-20200204
x86_64 randconfig-f001-20200205
x86_64 randconfig-f002-20200205
x86_64 randconfig-f003-20200205
i386 randconfig-f001-20200205
i386 randconfig-f002-20200205
i386 randconfig-f003-20200205
x86_64 randconfig-f001-20200206
x86_64 randconfig-f002-20200206
x86_64 randconfig-f003-20200206
i386 randconfig-f001-20200206
i386 randconfig-f002-20200206
i386 randconfig-f003-20200206
x86_64 randconfig-g001-20200205
x86_64 randconfig-g002-20200205
x86_64 randconfig-g003-20200205
i386 randconfig-g001-20200205
i386 randconfig-g002-20200205
i386 randconfig-g003-20200205
x86_64 randconfig-g001-20200204
x86_64 randconfig-g002-20200204
x86_64 randconfig-g003-20200204
i386 randconfig-g001-20200204
i386 randconfig-g002-20200204
i386 randconfig-g003-20200204
x86_64 randconfig-g001-20200206
x86_64 randconfig-g002-20200206
x86_64 randconfig-g003-20200206
i386 randconfig-g001-20200206
i386 randconfig-g002-20200206
i386 randconfig-g003-20200206
x86_64 randconfig-h001-20200206
x86_64 randconfig-h002-20200206
x86_64 randconfig-h003-20200206
i386 randconfig-h001-20200206
i386 randconfig-h002-20200206
i386 randconfig-h003-20200206
x86_64 randconfig-h001-20200204
x86_64 randconfig-h002-20200204
x86_64 randconfig-h003-20200204
i386 randconfig-h001-20200204
i386 randconfig-h002-20200204
i386 randconfig-h003-20200204
x86_64 randconfig-h001-20200205
x86_64 randconfig-h002-20200205
x86_64 randconfig-h003-20200205
i386 randconfig-h001-20200205
i386 randconfig-h002-20200205
i386 randconfig-h003-20200205
arc randconfig-a001-20200204
arm randconfig-a001-20200204
arm64 randconfig-a001-20200204
ia64 randconfig-a001-20200204
powerpc randconfig-a001-20200204
sparc randconfig-a001-20200204
arm randconfig-a001-20200205
arm64 randconfig-a001-20200205
ia64 randconfig-a001-20200205
powerpc randconfig-a001-20200205
arc randconfig-a001-20200206
arm randconfig-a001-20200206
arm64 randconfig-a001-20200206
ia64 randconfig-a001-20200206
powerpc randconfig-a001-20200206
sparc randconfig-a001-20200206
riscv allmodconfig
riscv allyesconfig
riscv rv32_defconfig
s390 alldefconfig
s390 allmodconfig
s390 allnoconfig
s390 allyesconfig
s390 debug_defconfig
s390 zfcpdump_defconfig
sh allmodconfig
sh allnoconfig
sh sh7785lcr_32bit_defconfig
sh titan_defconfig
sparc64 allnoconfig
sparc64 allyesconfig
x86_64 fedora-25
x86_64 kexec
x86_64 lkp
x86_64 rhel
x86_64 rhel-7.2-clear
x86_64 rhel-7.6
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Applied "ASoC: fsl_sai: Fix exiting path on probing failure" to the asoc tree
From: Mark Brown @ 2020-02-06 15:54 UTC (permalink / raw)
To: Oleksandr Suvorov
Cc: alsa-devel, Timur Tabi, Philippe Schenker, Marcel Ziswiler,
Daniel Baluta, Shengjiu Wang, linuxppc-dev, Liam Girdwood,
Jaroslav Kysela, Nicolin Chen, Igor Opaniuk, Mark Brown, Xiubo Li,
Takashi Iwai, Fabio Estevam, linux-kernel
In-Reply-To: <20200205160436.3813642-1-oleksandr.suvorov@toradex.com>
The patch
ASoC: fsl_sai: Fix exiting path on probing failure
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.6
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From d1520889782dff58610c0b6b54d4cf3211ceb690 Mon Sep 17 00:00:00 2001
From: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
Date: Wed, 5 Feb 2020 18:04:36 +0200
Subject: [PATCH] ASoC: fsl_sai: Fix exiting path on probing failure
If the imx-sdma driver is built as a module, the fsl-sai device doesn't
disable on probing failure, which causes the warning in the next probing:
==================================================================
fsl-sai 308a0000.sai: Unbalanced pm_runtime_enable!
fsl-sai 308a0000.sai: Unbalanced pm_runtime_enable!
fsl-sai 308a0000.sai: Unbalanced pm_runtime_enable!
fsl-sai 308a0000.sai: Unbalanced pm_runtime_enable!
fsl-sai 308a0000.sai: Unbalanced pm_runtime_enable!
fsl-sai 308a0000.sai: Unbalanced pm_runtime_enable!
==================================================================
Disabling the device properly fixes the issue.
Fixes: 812ad463e089 ("ASoC: fsl_sai: Add support for runtime pm")
Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
Link: https://lore.kernel.org/r/20200205160436.3813642-1-oleksandr.suvorov@toradex.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
sound/soc/fsl/fsl_sai.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 8c3ea7300972..9d436b0c5718 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -1020,12 +1020,24 @@ static int fsl_sai_probe(struct platform_device *pdev)
ret = devm_snd_soc_register_component(&pdev->dev, &fsl_component,
&fsl_sai_dai, 1);
if (ret)
- return ret;
+ goto err_pm_disable;
- if (sai->soc_data->use_imx_pcm)
- return imx_pcm_dma_init(pdev, IMX_SAI_DMABUF_SIZE);
- else
- return devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
+ if (sai->soc_data->use_imx_pcm) {
+ ret = imx_pcm_dma_init(pdev, IMX_SAI_DMABUF_SIZE);
+ if (ret)
+ goto err_pm_disable;
+ } else {
+ ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
+ if (ret)
+ goto err_pm_disable;
+ }
+
+ return ret;
+
+err_pm_disable:
+ pm_runtime_disable(&pdev->dev);
+
+ return ret;
}
static int fsl_sai_remove(struct platform_device *pdev)
--
2.20.1
^ permalink raw reply related
* Re: Latest Git kernel: avahi-daemon[2410]: ioctl(): Inappropriate ioctl for device
From: Christian Zigotzky @ 2020-02-06 14:28 UTC (permalink / raw)
To: Michael Ellerman, Jakub Kicinski
Cc: DTML, Darren Stevens, mad skateman, netdev@vger.kernel.org,
Linux Kernel Mailing List, linuxppc-dev, contact@a-eon.com,
R.T.Dickinson, Christoph Hellwig
In-Reply-To: <87tv441gg1.fsf@mpe.ellerman.id.au>
On 06 February 2020 at 05:35 am, Michael Ellerman wrote:
> Christian Zigotzky <chzigotzky@xenosoft.de> writes:
>> Kernel 5.5 PowerPC is also affected.
> I don't know what you mean by that. What sha are you talking about?
>
> I have a system with avahi running and everything's fine.
>
> # grep use- /etc/avahi/avahi-daemon.conf
> use-ipv4=yes
> use-ipv6=yes
>
> # systemctl status -l --no-pager avahi-daemon
> ● avahi-daemon.service - Avahi mDNS/DNS-SD Stack
> Loaded: loaded (/lib/systemd/system/avahi-daemon.service; enabled; vendor preset: enabled)
> Active: active (running) since Thu 2020-02-06 14:55:34 AEDT; 38min ago
> Main PID: 1884 (avahi-daemon)
> Status: "avahi-daemon 0.7 starting up."
> CGroup: /system.slice/avahi-daemon.service
> ├─1884 avahi-daemon: running [mpe-ubuntu-le.local]
> └─1888 avahi-daemon: chroot helper
>
> Feb 06 14:55:34 mpe-ubuntu-le avahi-daemon[1884]: Registering new address record for fe80::5054:ff:fe66:2a19 on eth0.*.
> Feb 06 14:55:34 mpe-ubuntu-le avahi-daemon[1884]: Registering new address record for 10.61.141.81 on eth0.IPv4.
> Feb 06 14:55:34 mpe-ubuntu-le avahi-daemon[1884]: Registering new address record for ::1 on lo.*.
> Feb 06 14:55:34 mpe-ubuntu-le avahi-daemon[1884]: Registering new address record for 127.0.0.1 on lo.IPv4.
> Feb 06 14:55:34 mpe-ubuntu-le systemd[1]: Started Avahi mDNS/DNS-SD Stack.
> Feb 06 14:55:35 mpe-ubuntu-le avahi-daemon[1884]: Server startup complete. Host name is mpe-ubuntu-le.local. Local service cookie is 3972418141.
> Feb 06 14:55:38 mpe-ubuntu-le avahi-daemon[1884]: Leaving mDNS multicast group on interface eth0.IPv6 with address fe80::5054:ff:fe66:2a19.
> Feb 06 14:55:38 mpe-ubuntu-le avahi-daemon[1884]: Joining mDNS multicast group on interface eth0.IPv6 with address fd69:d75f:b8b5:61:5054:ff:fe66:2a19.
> Feb 06 14:55:38 mpe-ubuntu-le avahi-daemon[1884]: Registering new address record for fd69:d75f:b8b5:61:5054:ff:fe66:2a19 on eth0.*.
> Feb 06 14:55:38 mpe-ubuntu-le avahi-daemon[1884]: Withdrawing address record for fe80::5054:ff:fe66:2a19 on eth0.
>
> # uname -r
> 5.5.0-gcc-8.2.0
>
>
> The key question is what ioctl is it complaining about. You should be
> able to find that via strace.
>
> cheers
>
Hello Michael,
Sorry it isn't true that the kernel 5.5 is also affected. A Power Mac G5
user told me that but this isn't correct. I compiled and tested the
stable kernel 5.5.1 and 5.5.2 today and both kernels don't have the
issue with the avahi daemon.
Could you please also test the latest Git kernel?
strace /usr/sbin/avahi-daemon
...
poll([{fd=4, events=POLLIN}, {fd=16, events=POLLIN}, {fd=15,
events=POLLIN}, {fd=14, events=POLLIN}, {fd=13, events=POLLIN}, {fd=12,
events=POLLIN}, {fd=11, events=POLLIN}, {fd=10, events=POLLIN}, {fd=9,
events=POLLIN}, {fd=8, events=POLLIN}, {fd=6, events=POLLIN}], 11, 65) =
2 ([{fd=12, revents=POLLIN}, {fd=9, revents=POLLIN}])
ioctl(12, FIONREAD, 0xffba6f24) = -1 ENOTTY (Inappropriate ioctl
for device)
write(2, "ioctl(): Inappropriate ioctl for"..., 39ioctl(): Inappropriate
ioctl for device) = 39
write(2, "\n", 1
) = 1
...
Thanks,
Christian
^ permalink raw reply
* [PATCH] powerpc/hugetlb: Fix 8M hugepages on 8xx
From: Christophe Leroy @ 2020-02-06 13:50 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
aneesh.kumar
Cc: linuxppc-dev, linux-kernel
Commit 55c8fc3f4930 ("powerpc/8xx: reintroduce 16K pages with HW
assistance") redefined pte_t as a struct of 4 pte_basic_t, because
in 16K pages mode there are four identical entries in the page table.
But hugepd entries for 8k pages require only one entrie of size
pte_basic_t. So there is no point in creating a cache for 4 entries
page tables.
Also, with HW assistance the entries must be 4k aligned, the 8xx
drops the last 12 bits. Redefine HUGEPD_SHIFT_MASK to mask them out.
Calculate PTE_T_ORDER using the size of pte_basic_t instead of pte_t.
In 16k mode, define a specific set_huge_pte_at() function which writes
the pte in a single entry instead of using set_pte_at() which writes
4 identical entries. Define set_pte_filter() inline otherwise GCC
doesn't inline it anymore because it is now used twice, and that gives
a pretty suboptimal code because of pte_t being a struct of 4 entries.
This function is also used for 512k pages which only require one entry
as well allthough replicating it four times is harmless as 512k pages
entries are spread every 128 bytes in the table.
Fixes: 22569b881d37 ("powerpc/8xx: Enable 8M hugepage support with HW assistance")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/hugetlb.h | 5 +++++
arch/powerpc/include/asm/page.h | 5 +++++
arch/powerpc/mm/hugetlbpage.c | 3 ++-
arch/powerpc/mm/pgtable.c | 19 ++++++++++++++++++-
4 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/hugetlb.h b/arch/powerpc/include/asm/hugetlb.h
index bd6504c28c2f..f43cfbcf014f 100644
--- a/arch/powerpc/include/asm/hugetlb.h
+++ b/arch/powerpc/include/asm/hugetlb.h
@@ -64,6 +64,11 @@ static inline void arch_clear_hugepage_flags(struct page *page)
{
}
+#if defined(CONFIG_PPC_8xx) && defined(CONFIG_PPC_16K_PAGES)
+#define __HAVE_ARCH_HUGE_SET_HUGE_PTE_AT
+void set_huge_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep, pte_t pte);
+#endif
+
#include <asm-generic/hugetlb.h>
#else /* ! CONFIG_HUGETLB_PAGE */
diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index 86332080399a..080a0bf8e54b 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -295,8 +295,13 @@ static inline bool pfn_valid(unsigned long pfn)
/*
* Some number of bits at the level of the page table that points to
* a hugepte are used to encode the size. This masks those bits.
+ * On 8xx, HW assistance requires 4k alignment for the hugepte.
*/
+#ifdef CONFIG_PPC_8xx
+#define HUGEPD_SHIFT_MASK 0xfff
+#else
#define HUGEPD_SHIFT_MASK 0x3f
+#endif
#ifndef __ASSEMBLY__
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 73d4873fc7f8..c61032580185 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -30,7 +30,8 @@ bool hugetlb_disabled = false;
#define hugepd_none(hpd) (hpd_val(hpd) == 0)
-#define PTE_T_ORDER (__builtin_ffs(sizeof(pte_t)) - __builtin_ffs(sizeof(void *)))
+#define PTE_T_ORDER (__builtin_ffs(sizeof(pte_basic_t)) - \
+ __builtin_ffs(sizeof(void *)))
pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr, unsigned long sz)
{
diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
index e3759b69f81b..7a38eaa6ca72 100644
--- a/arch/powerpc/mm/pgtable.c
+++ b/arch/powerpc/mm/pgtable.c
@@ -100,7 +100,7 @@ static pte_t set_pte_filter_hash(pte_t pte) { return pte; }
* as we don't have two bits to spare for _PAGE_EXEC and _PAGE_HWEXEC so
* instead we "filter out" the exec permission for non clean pages.
*/
-static pte_t set_pte_filter(pte_t pte)
+static inline pte_t set_pte_filter(pte_t pte)
{
struct page *pg;
@@ -259,6 +259,23 @@ int huge_ptep_set_access_flags(struct vm_area_struct *vma,
return changed;
#endif
}
+
+#if defined(CONFIG_PPC_8xx) && defined(CONFIG_PPC_16K_PAGES)
+void set_huge_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep, pte_t pte)
+{
+ /*
+ * Make sure hardware valid bit is not set. We don't do
+ * tlb flush for this update.
+ */
+ VM_WARN_ON(pte_hw_valid(*ptep) && !pte_protnone(*ptep));
+
+ pte = pte_mkpte(pte);
+
+ pte = set_pte_filter(pte);
+
+ ptep->pte = pte_val(pte);
+}
+#endif
#endif /* CONFIG_HUGETLB_PAGE */
#ifdef CONFIG_DEBUG_VM
--
2.25.0
^ permalink raw reply related
* [PATCH] powerpc/hugetlb: Fix 512k hugepages on 8xx with 16k page size
From: Christophe Leroy @ 2020-02-06 13:50 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
aneesh.kumar
Cc: linuxppc-dev, linux-kernel
Commit 55c8fc3f4930 ("powerpc/8xx: reintroduce 16K pages with HW
assistance") redefined pte_t as a struct of 4 pte_basic_t, because
in 16K pages mode there are four identical entries in the
page table. But the size of hugepage tables is calculated based
of the size of (void *). Therefore, we end up with page tables
of size 1k instead of 4k for 512k pages.
As 512k hugepage tables are the same size as standard page tables,
ie 4k, use the standard page tables instead of PGT_CACHE tables.
Fixes: 3fb69c6a1a13 ("powerpc/8xx: Enable 512k hugepage support with HW assistance")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/mm/hugetlbpage.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index c61032580185..edf511c2a30a 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -54,20 +54,24 @@ static int __hugepte_alloc(struct mm_struct *mm, hugepd_t *hpdp,
if (pshift >= pdshift) {
cachep = PGT_CACHE(PTE_T_ORDER);
num_hugepd = 1 << (pshift - pdshift);
+ new = NULL;
} else if (IS_ENABLED(CONFIG_PPC_8xx)) {
- cachep = PGT_CACHE(PTE_INDEX_SIZE);
+ cachep = NULL;
num_hugepd = 1;
+ new = pte_alloc_one(mm);
} else {
cachep = PGT_CACHE(pdshift - pshift);
num_hugepd = 1;
+ new = NULL;
}
- if (!cachep) {
+ if (!cachep && !new) {
WARN_ONCE(1, "No page table cache created for hugetlb tables");
return -ENOMEM;
}
- new = kmem_cache_alloc(cachep, pgtable_gfp_flags(mm, GFP_KERNEL));
+ if (cachep)
+ new = kmem_cache_alloc(cachep, pgtable_gfp_flags(mm, GFP_KERNEL));
BUG_ON(pshift > HUGEPD_SHIFT_MASK);
BUG_ON((unsigned long)new & HUGEPD_SHIFT_MASK);
@@ -98,7 +102,10 @@ static int __hugepte_alloc(struct mm_struct *mm, hugepd_t *hpdp,
if (i < num_hugepd) {
for (i = i - 1 ; i >= 0; i--, hpdp--)
*hpdp = __hugepd(0);
- kmem_cache_free(cachep, new);
+ if (cachep)
+ kmem_cache_free(cachep, new);
+ else
+ pte_free(mm, new);
} else {
kmemleak_ignore(new);
}
@@ -325,8 +332,7 @@ static void free_hugepd_range(struct mmu_gather *tlb, hugepd_t *hpdp, int pdshif
if (shift >= pdshift)
hugepd_free(tlb, hugepte);
else if (IS_ENABLED(CONFIG_PPC_8xx))
- pgtable_free_tlb(tlb, hugepte,
- get_hugepd_cache_index(PTE_INDEX_SIZE));
+ pgtable_free_tlb(tlb, hugepte, 0);
else
pgtable_free_tlb(tlb, hugepte,
get_hugepd_cache_index(pdshift - shift));
@@ -640,12 +646,13 @@ static int __init hugetlbpage_init(void)
* if we have pdshift and shift value same, we don't
* use pgt cache for hugepd.
*/
- if (pdshift > shift && IS_ENABLED(CONFIG_PPC_8xx))
- pgtable_cache_add(PTE_INDEX_SIZE);
- else if (pdshift > shift)
- pgtable_cache_add(pdshift - shift);
- else if (IS_ENABLED(CONFIG_PPC_FSL_BOOK3E) || IS_ENABLED(CONFIG_PPC_8xx))
+ if (pdshift > shift) {
+ if (!IS_ENABLED(CONFIG_PPC_8xx))
+ pgtable_cache_add(pdshift - shift);
+ } else if (IS_ENABLED(CONFIG_PPC_FSL_BOOK3E) ||
+ IS_ENABLED(CONFIG_PPC_8xx)) {
pgtable_cache_add(PTE_T_ORDER);
+ }
configured = true;
}
--
2.25.0
^ permalink raw reply related
* Re: [yyu168-linux_cet:cet 55/58] powerpc64le-linux-ld: warning: discarding dynamic section .rela___ksymtab+jiffies_to_timeval
From: H.J. Lu @ 2020-02-06 12:55 UTC (permalink / raw)
To: Michael Ellerman
Cc: Yu-cheng Yu, kbuild-all, Kees Cook, Paul Mackerras, linuxppc-dev
In-Reply-To: <87wo901jm0.fsf@mpe.ellerman.id.au>
On Wed, Feb 5, 2020 at 7:26 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> "H.J. Lu" <hjl.tools@gmail.com> writes:
> > On Tue, Feb 4, 2020 at 3:37 PM kbuild test robot <lkp@intel.com> wrote:
> >>
> >> tree: https://github.com/yyu168/linux_cet.git cet
> >> head: bba707cc4715c1036b6561ab38b16747f9c49cfa
> >> commit: 71bb971dd76eeacd351690f28864ad5c5bec3691 [55/58] Discard .note.gnu.property sections in generic NOTES
> >> config: powerpc-rhel-kconfig (attached as .config)
> >> compiler: powerpc64le-linux-gcc (GCC) 7.5.0
> >> reproduce:
> >> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >> chmod +x ~/bin/make.cross
> >> git checkout 71bb971dd76eeacd351690f28864ad5c5bec3691
> >> # save the attached .config to linux build tree
> >> GCC_VERSION=7.5.0 make.cross ARCH=powerpc
> >>
> >> If you fix the issue, kindly add following tag
> >> Reported-by: kbuild test robot <lkp@intel.com>
> >>
> >> All warnings (new ones prefixed by >>):
> >>
> >> powerpc64le-linux-ld: warning: discarding dynamic section .rela___ksymtab_gpl+__wait_rcu_gp
> >
> > arch/powerpc/kernel/vmlinux.lds.S has
> >
> > .rela.dyn : AT(ADDR(.rela.dyn) - (0xc000000000000000 -0x00000000))
> > {
> > __rela_dyn_start = .;
> > *(.rela*) <<<<<<<< Keep .rela* sections
> > }
>
> The above is inside #ifdef CONFIG_RELOCATABLE
>
> > ...
> > /DISCARD/ : {
> > *(*.EMB.apuinfo)
> > *(.glink .iplt .plt .rela* .comment)
> > ^^^^ Discard .rela* sections. But it is ignored.
> > *(.gnu.version*)
> > *(.gnu.attributes)
> > *(.eh_frame)
> > }
>
> But that is not #ifdef'ed at all.
>
> > With my
> >
> > ommit 71bb971dd76eeacd351690f28864ad5c5bec3691
> > Author: H.J. Lu <hjl.tools@gmail.com>
> > Date: Thu Jan 30 12:39:09 2020 -0800
> >
> > Discard .note.gnu.property sections in generic NOTES
> >
> > With the command-line option, -mx86-used-note=yes, the x86 assembler
> > in binutils 2.32 and above generates a program property note in a note
> > section, .note.gnu.property, to encode used x86 ISAs and features. But
> > kernel linker script only contains a single NOTE segment:
> >
> > /DISCARD/ : { *(.note.gnu.property) }
> >
> > is placed before
> >
> > .rela.dyn : AT(ADDR(.rela.dyn) - (0xc000000000000000 -0x00000000))
> > {
> > __rela_dyn_start = .;
> > *(.rela*) <<<<<<<< Keep .rela* sections
> > }
> >
> > Then .rela* in
> >
> > /DISCARD/ : {
> > *(*.EMB.apuinfo)
> > *(.glink .iplt .plt .rela* .comment)
> > *(.gnu.version*)
> > *(.gnu.attributes)
> > *(.eh_frame)
> > }
> >
> > is honored. Can someone from POWERPC comment on it?
>
> Hmm OK. I'm not really a toolchain person.
>
> The comment on DISCARDS says:
>
> * Some archs want to discard exit text/data at runtime rather than
> * link time due to cross-section references such as alt instructions,
> * bug table, eh_frame, etc. DISCARDS must be the last of output
> * section definitions so that such archs put those in earlier section
> * definitions.
> */
>
> But I guess you're changing those semantics in your series.
>
> This seems to fix the warning for me?
>
> diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
> index b4c89a1acebb..076b3e8a849d 100644
> --- a/arch/powerpc/kernel/vmlinux.lds.S
> +++ b/arch/powerpc/kernel/vmlinux.lds.S
> @@ -365,9 +365,12 @@ SECTIONS
> DISCARDS
> /DISCARD/ : {
> *(*.EMB.apuinfo)
> - *(.glink .iplt .plt .rela* .comment)
> + *(.glink .iplt .plt .comment)
> *(.gnu.version*)
> *(.gnu.attributes)
> *(.eh_frame)
> +#ifndef CONFIG_RELOCATABLE
> + *(.rela*)
> +#endif
> }
> }
>
>
> cheers
This looks correct me.
Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
Thanks.
--
H.J.
^ permalink raw reply
* [powerpc:fixes-test] BUILD SUCCESS 4399efc3f2f04ad8e6948461795de04ed1358402
From: kbuild test robot @ 2020-02-06 10:25 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://github.com/linuxppc/linux fixes-test
branch HEAD: 4399efc3f2f04ad8e6948461795de04ed1358402 selftests/powerpc: Don't rely on segfault to rerun the test
elapsed time: 2768m
configs tested: 280
configs skipped: 5
The following configs have been built successfully.
More configs may be tested in the coming days.
arm allmodconfig
arm allnoconfig
arm allyesconfig
arm at91_dt_defconfig
arm efm32_defconfig
arm exynos_defconfig
arm multi_v5_defconfig
arm multi_v7_defconfig
arm shmobile_defconfig
arm sunxi_defconfig
arm64 allmodconfig
arm64 allnoconfig
arm64 allyesconfig
arm64 defconfig
sparc allyesconfig
um defconfig
xtensa common_defconfig
sh titan_defconfig
h8300 edosk2674_defconfig
riscv defconfig
riscv nommu_virt_defconfig
i386 allyesconfig
microblaze mmu_defconfig
sparc64 defconfig
m68k m5475evb_defconfig
nios2 3c120_defconfig
nds32 defconfig
sh rsk7269_defconfig
sparc64 allmodconfig
i386 allnoconfig
h8300 h8300h-sim_defconfig
sparc defconfig
alpha defconfig
parisc defconfig
h8300 h8s-sim_defconfig
um i386_defconfig
parisc b180_defconfig
s390 alldefconfig
arc defconfig
parisc allnoconfig
um x86_64_defconfig
microblaze nommu_defconfig
ia64 allyesconfig
mips fuloong2e_defconfig
s390 defconfig
ia64 defconfig
m68k allmodconfig
i386 alldefconfig
powerpc allnoconfig
s390 debug_defconfig
riscv allnoconfig
i386 defconfig
ia64 alldefconfig
ia64 allmodconfig
ia64 allnoconfig
c6x allyesconfig
c6x evmc6678_defconfig
nios2 10m50_defconfig
openrisc or1ksim_defconfig
openrisc simple_smp_defconfig
xtensa iss_defconfig
csky defconfig
nds32 allnoconfig
m68k multi_defconfig
m68k sun3_defconfig
arc allyesconfig
powerpc defconfig
powerpc ppc64_defconfig
powerpc rhel-kconfig
mips 32r2_defconfig
mips 64r6el_defconfig
mips allmodconfig
mips allnoconfig
mips allyesconfig
mips malta_kvm_defconfig
parisc allyesonfig
parisc c3000_defconfig
x86_64 randconfig-a001-20200206
x86_64 randconfig-a002-20200206
x86_64 randconfig-a003-20200206
i386 randconfig-a001-20200206
i386 randconfig-a002-20200206
i386 randconfig-a003-20200206
i386 randconfig-a003-20200204
i386 randconfig-a002-20200204
x86_64 randconfig-a003-20200204
x86_64 randconfig-a002-20200204
i386 randconfig-a001-20200204
x86_64 randconfig-a001-20200204
x86_64 randconfig-a001-20200205
x86_64 randconfig-a002-20200205
x86_64 randconfig-a003-20200205
i386 randconfig-a001-20200205
i386 randconfig-a002-20200205
i386 randconfig-a003-20200205
alpha randconfig-a001-20200206
m68k randconfig-a001-20200206
mips randconfig-a001-20200206
nds32 randconfig-a001-20200206
parisc randconfig-a001-20200206
riscv randconfig-a001-20200206
alpha randconfig-a001-20200204
m68k randconfig-a001-20200204
mips randconfig-a001-20200204
nds32 randconfig-a001-20200204
parisc randconfig-a001-20200204
riscv randconfig-a001-20200204
c6x randconfig-a001-20200206
h8300 randconfig-a001-20200206
microblaze randconfig-a001-20200206
nios2 randconfig-a001-20200206
sparc64 randconfig-a001-20200206
c6x randconfig-a001-20200205
h8300 randconfig-a001-20200205
microblaze randconfig-a001-20200205
nios2 randconfig-a001-20200205
sparc64 randconfig-a001-20200205
c6x randconfig-a001-20200204
h8300 randconfig-a001-20200204
microblaze randconfig-a001-20200204
nios2 randconfig-a001-20200204
sparc64 randconfig-a001-20200204
csky randconfig-a001-20200205
openrisc randconfig-a001-20200205
s390 randconfig-a001-20200205
sh randconfig-a001-20200205
xtensa randconfig-a001-20200205
csky randconfig-a001-20200206
openrisc randconfig-a001-20200206
s390 randconfig-a001-20200206
sh randconfig-a001-20200206
xtensa randconfig-a001-20200206
x86_64 randconfig-b001-20200206
x86_64 randconfig-b002-20200206
x86_64 randconfig-b003-20200206
i386 randconfig-b001-20200206
i386 randconfig-b002-20200206
i386 randconfig-b003-20200206
x86_64 randconfig-b001-20200205
x86_64 randconfig-b002-20200205
x86_64 randconfig-b003-20200205
i386 randconfig-b001-20200205
i386 randconfig-b002-20200205
i386 randconfig-b003-20200205
i386 randconfig-b001-20200204
i386 randconfig-b002-20200204
x86_64 randconfig-b002-20200204
x86_64 randconfig-b001-20200204
i386 randconfig-b003-20200204
x86_64 randconfig-b003-20200204
x86_64 randconfig-c001-20200204
x86_64 randconfig-c002-20200204
x86_64 randconfig-c003-20200204
i386 randconfig-c001-20200204
i386 randconfig-c002-20200204
i386 randconfig-c003-20200204
x86_64 randconfig-c001-20200205
x86_64 randconfig-c002-20200205
x86_64 randconfig-c003-20200205
i386 randconfig-c001-20200205
i386 randconfig-c002-20200205
i386 randconfig-c003-20200205
x86_64 randconfig-d001-20200205
x86_64 randconfig-d002-20200205
x86_64 randconfig-d003-20200205
i386 randconfig-d001-20200205
i386 randconfig-d002-20200205
i386 randconfig-d003-20200205
x86_64 randconfig-d001-20200206
x86_64 randconfig-d002-20200206
x86_64 randconfig-d003-20200206
i386 randconfig-d001-20200206
i386 randconfig-d002-20200206
i386 randconfig-d003-20200206
i386 randconfig-e003-20200205
i386 randconfig-e002-20200205
x86_64 randconfig-e001-20200205
x86_64 randconfig-e003-20200205
i386 randconfig-e001-20200205
x86_64 randconfig-e002-20200205
x86_64 randconfig-e001-20200206
x86_64 randconfig-e002-20200206
x86_64 randconfig-e003-20200206
i386 randconfig-e001-20200206
i386 randconfig-e002-20200206
i386 randconfig-e003-20200206
x86_64 randconfig-e001-20200204
x86_64 randconfig-e002-20200204
x86_64 randconfig-e003-20200204
i386 randconfig-e001-20200204
i386 randconfig-e002-20200204
i386 randconfig-e003-20200204
x86_64 randconfig-f001-20200206
x86_64 randconfig-f002-20200206
x86_64 randconfig-f003-20200206
i386 randconfig-f001-20200206
i386 randconfig-f002-20200206
i386 randconfig-f003-20200206
x86_64 randconfig-f001-20200204
x86_64 randconfig-f002-20200204
x86_64 randconfig-f003-20200204
i386 randconfig-f001-20200204
i386 randconfig-f002-20200204
i386 randconfig-f003-20200204
i386 randconfig-f002-20200205
i386 randconfig-f003-20200205
x86_64 randconfig-f002-20200205
i386 randconfig-f001-20200205
x86_64 randconfig-f001-20200205
x86_64 randconfig-f003-20200205
x86_64 randconfig-g001-20200205
x86_64 randconfig-g002-20200205
x86_64 randconfig-g003-20200205
i386 randconfig-g001-20200205
i386 randconfig-g002-20200205
i386 randconfig-g003-20200205
x86_64 randconfig-g001-20200206
x86_64 randconfig-g002-20200206
x86_64 randconfig-g003-20200206
i386 randconfig-g001-20200206
i386 randconfig-g002-20200206
i386 randconfig-g003-20200206
x86_64 randconfig-g001-20200204
x86_64 randconfig-g002-20200204
x86_64 randconfig-g003-20200204
i386 randconfig-g001-20200204
i386 randconfig-g002-20200204
i386 randconfig-g003-20200204
x86_64 randconfig-h001-20200206
x86_64 randconfig-h002-20200206
x86_64 randconfig-h003-20200206
i386 randconfig-h001-20200206
i386 randconfig-h002-20200206
i386 randconfig-h003-20200206
x86_64 randconfig-h001-20200205
x86_64 randconfig-h002-20200205
x86_64 randconfig-h003-20200205
i386 randconfig-h001-20200205
i386 randconfig-h002-20200205
i386 randconfig-h003-20200205
x86_64 randconfig-h001-20200204
x86_64 randconfig-h002-20200204
x86_64 randconfig-h003-20200204
i386 randconfig-h001-20200204
i386 randconfig-h002-20200204
i386 randconfig-h003-20200204
arc randconfig-a001-20200204
arm randconfig-a001-20200204
arm64 randconfig-a001-20200204
ia64 randconfig-a001-20200204
powerpc randconfig-a001-20200204
sparc randconfig-a001-20200204
arc randconfig-a001-20200206
arm randconfig-a001-20200206
arm64 randconfig-a001-20200206
ia64 randconfig-a001-20200206
powerpc randconfig-a001-20200206
sparc randconfig-a001-20200206
arm randconfig-a001-20200205
arm64 randconfig-a001-20200205
ia64 randconfig-a001-20200205
powerpc randconfig-a001-20200205
riscv allmodconfig
riscv allyesconfig
riscv rv32_defconfig
s390 allmodconfig
s390 allnoconfig
s390 allyesconfig
s390 zfcpdump_defconfig
sh allmodconfig
sh allnoconfig
sh sh7785lcr_32bit_defconfig
sparc64 allnoconfig
sparc64 allyesconfig
x86_64 fedora-25
x86_64 kexec
x86_64 lkp
x86_64 rhel
x86_64 rhel-7.2-clear
x86_64 rhel-7.6
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
^ permalink raw reply
* Re: [PATCH v2 2/5] powerpc/perf: Implement a global lock to avoid races between trace, core and thread imc events.
From: maddy @ 2020-02-06 9:04 UTC (permalink / raw)
To: Anju T Sudhakar, mpe; +Cc: nasastry, maddy, linuxppc-dev
In-Reply-To: <20200121101728.14858-3-anju@linux.vnet.ibm.com>
On 1/21/20 3:47 PM, Anju T Sudhakar wrote:
> IMC(In-memory Collection Counters) does performance monitoring in
> two different modes, i.e accumulation mode(core-imc and thread-imc events),
> and trace mode(trace-imc events). A cpu thread can either be in
> accumulation-mode or trace-mode at a time and this is done via the LDBAR
> register in POWER architecture. The current design does not address the
> races between thread-imc and trace-imc events.
>
> Patch implements a global id and lock to avoid the races between
> core, trace and thread imc events. With this global id-lock
> implementation, the system can either run core, thread or trace imc
> events at a time. i.e. to run any core-imc events, thread/trace imc events
> should not be enabled/monitored.
Changes looks fine to me.
Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>
> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> ---
> arch/powerpc/perf/imc-pmu.c | 177 +++++++++++++++++++++++++++++++-----
> 1 file changed, 153 insertions(+), 24 deletions(-)
>
> diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
> index cb50a9e1fd2d..2e220f199530 100644
> --- a/arch/powerpc/perf/imc-pmu.c
> +++ b/arch/powerpc/perf/imc-pmu.c
> @@ -44,6 +44,16 @@ static DEFINE_PER_CPU(u64 *, trace_imc_mem);
> static struct imc_pmu_ref *trace_imc_refc;
> static int trace_imc_mem_size;
>
> +/*
> + * Global data structure used to avoid races between thread,
> + * core and trace-imc
> + */
> +static struct imc_pmu_ref imc_global_refc = {
> + .lock = __MUTEX_INITIALIZER(imc_global_refc.lock),
> + .id = 0,
> + .refc = 0,
> +};
> +
> static struct imc_pmu *imc_event_to_pmu(struct perf_event *event)
> {
> return container_of(event->pmu, struct imc_pmu, pmu);
> @@ -759,6 +769,20 @@ static void core_imc_counters_release(struct perf_event *event)
> ref->refc = 0;
> }
> mutex_unlock(&ref->lock);
> +
> + mutex_lock(&imc_global_refc.lock);
> + if (imc_global_refc.id == IMC_DOMAIN_CORE) {
> + imc_global_refc.refc--;
> + /*
> + * If no other thread is running any core-imc
> + * event, set the global id to zero.
> + */
> + if (imc_global_refc.refc <= 0) {
> + imc_global_refc.refc = 0;
> + imc_global_refc.id = 0;
> + }
> + }
> + mutex_unlock(&imc_global_refc.lock);
> }
>
> static int core_imc_event_init(struct perf_event *event)
> @@ -779,6 +803,22 @@ static int core_imc_event_init(struct perf_event *event)
> if (event->cpu < 0)
> return -EINVAL;
>
> + /*
> + * Take the global lock, and make sure
> + * no other thread is running any trace OR thread imc event
> + */
> + mutex_lock(&imc_global_refc.lock);
> + if (imc_global_refc.id == 0) {
> + imc_global_refc.id = IMC_DOMAIN_CORE;
> + imc_global_refc.refc++;
> + } else if (imc_global_refc.id == IMC_DOMAIN_CORE) {
> + imc_global_refc.refc++;
> + } else {
> + mutex_unlock(&imc_global_refc.lock);
> + return -EBUSY;
> + }
> + mutex_unlock(&imc_global_refc.lock);
> +
> event->hw.idx = -1;
> pmu = imc_event_to_pmu(event);
>
> @@ -877,7 +917,16 @@ static int ppc_thread_imc_cpu_online(unsigned int cpu)
>
> static int ppc_thread_imc_cpu_offline(unsigned int cpu)
> {
> - mtspr(SPRN_LDBAR, 0);
> + /*
> + * Toggle the bit 0 of LDBAR.
> + *
> + * If bit 0 of LDBAR is unset, it will stop posting
> + * the counetr data to memory.
> + * For thread-imc, bit 0 of LDBAR will be set to 1 in the
> + * event_add function. So toggle this bit here, to stop the updates
> + * to memory in the cpu_offline path.
> + */
> + mtspr(SPRN_LDBAR, (mfspr(SPRN_LDBAR) ^ (1UL << 63)));
> return 0;
> }
>
> @@ -889,6 +938,24 @@ static int thread_imc_cpu_init(void)
> ppc_thread_imc_cpu_offline);
> }
>
> +static void thread_imc_counters_release(struct perf_event *event)
> +{
> +
> + mutex_lock(&imc_global_refc.lock);
> + if (imc_global_refc.id == IMC_DOMAIN_THREAD) {
> + imc_global_refc.refc--;
> + /*
> + * If no other thread is running any thread-imc
> + * event, set the global id to zero.
> + */
> + if (imc_global_refc.refc <= 0) {
> + imc_global_refc.refc = 0;
> + imc_global_refc.id = 0;
> + }
> + }
> + mutex_unlock(&imc_global_refc.lock);
> +}
> +
> static int thread_imc_event_init(struct perf_event *event)
> {
> u32 config = event->attr.config;
> @@ -905,6 +972,27 @@ static int thread_imc_event_init(struct perf_event *event)
> if (event->hw.sample_period)
> return -EINVAL;
>
> + mutex_lock(&imc_global_refc.lock);
> + /*
> + * Check if any other thread is running
> + * core-engine, if not set the global id to
> + * thread-imc.
> + */
> + if (imc_global_refc.id == 0) {
> + imc_global_refc.id = IMC_DOMAIN_THREAD;
> + imc_global_refc.refc++;
> + } else if (imc_global_refc.id == IMC_DOMAIN_THREAD) {
> + /*
> + * Increase the ref count if the global id is
> + * set to thread-imc.
> + */
> + imc_global_refc.refc++;
> + } else {
> + mutex_unlock(&imc_global_refc.lock);
> + return -EBUSY;
> + }
> + mutex_unlock(&imc_global_refc.lock);
> +
> event->hw.idx = -1;
> pmu = imc_event_to_pmu(event);
>
> @@ -917,6 +1005,7 @@ static int thread_imc_event_init(struct perf_event *event)
> return -EINVAL;
>
> event->pmu->task_ctx_nr = perf_sw_context;
> + event->destroy = thread_imc_counters_release;
> return 0;
> }
>
> @@ -1063,10 +1152,12 @@ static void thread_imc_event_del(struct perf_event *event, int flags)
> int core_id;
> struct imc_pmu_ref *ref;
>
> - mtspr(SPRN_LDBAR, 0);
> -
> core_id = smp_processor_id() / threads_per_core;
> ref = &core_imc_refc[core_id];
> + if (!ref) {
> + pr_debug("imc: Failed to get event reference count\n");
> + return;
> + }
>
> mutex_lock(&ref->lock);
> ref->refc--;
> @@ -1082,6 +1173,10 @@ static void thread_imc_event_del(struct perf_event *event, int flags)
> ref->refc = 0;
> }
> mutex_unlock(&ref->lock);
> +
> + /* Toggle bit 0 of LDBAR */
> + mtspr(SPRN_LDBAR, (mfspr(SPRN_LDBAR) ^ (1UL << 63)));
> +
> /*
> * Take a snapshot and calculate the delta and update
> * the event counter values.
> @@ -1133,7 +1228,8 @@ static int ppc_trace_imc_cpu_online(unsigned int cpu)
>
> static int ppc_trace_imc_cpu_offline(unsigned int cpu)
> {
> - mtspr(SPRN_LDBAR, 0);
> + /* Toggle bit 0 of LDBAR. */
> + mtspr(SPRN_LDBAR, (mfspr(SPRN_LDBAR) ^ (1UL << 63)));
> return 0;
> }
>
> @@ -1226,15 +1322,14 @@ static int trace_imc_event_add(struct perf_event *event, int flags)
> local_mem = get_trace_imc_event_base_addr();
> ldbar_value = ((u64)local_mem & THREAD_IMC_LDBAR_MASK) | TRACE_IMC_ENABLE;
>
> - if (core_imc_refc)
> - ref = &core_imc_refc[core_id];
> + /* trace-imc reference count */
> + if (trace_imc_refc)
> + ref = &trace_imc_refc[core_id];
> if (!ref) {
> - /* If core-imc is not enabled, use trace-imc reference count */
> - if (trace_imc_refc)
> - ref = &trace_imc_refc[core_id];
> - if (!ref)
> - return -EINVAL;
> + pr_debug("imc: Failed to get the event reference count\n");
> + return -EINVAL;
> }
> +
> mtspr(SPRN_LDBAR, ldbar_value);
> mutex_lock(&ref->lock);
> if (ref->refc == 0) {
> @@ -1242,13 +1337,11 @@ static int trace_imc_event_add(struct perf_event *event, int flags)
> get_hard_smp_processor_id(smp_processor_id()))) {
> mutex_unlock(&ref->lock);
> pr_err("trace-imc: Unable to start the counters for core %d\n", core_id);
> - mtspr(SPRN_LDBAR, 0);
> return -EINVAL;
> }
> }
> ++ref->refc;
> mutex_unlock(&ref->lock);
> -
> return 0;
> }
>
> @@ -1274,16 +1367,13 @@ static void trace_imc_event_del(struct perf_event *event, int flags)
> int core_id = smp_processor_id() / threads_per_core;
> struct imc_pmu_ref *ref = NULL;
>
> - if (core_imc_refc)
> - ref = &core_imc_refc[core_id];
> + if (trace_imc_refc)
> + ref = &trace_imc_refc[core_id];
> if (!ref) {
> - /* If core-imc is not enabled, use trace-imc reference count */
> - if (trace_imc_refc)
> - ref = &trace_imc_refc[core_id];
> - if (!ref)
> - return;
> + pr_debug("imc: Failed to get event reference count\n");
> + return;
> }
> - mtspr(SPRN_LDBAR, 0);
> +
> mutex_lock(&ref->lock);
> ref->refc--;
> if (ref->refc == 0) {
> @@ -1297,9 +1387,30 @@ static void trace_imc_event_del(struct perf_event *event, int flags)
> ref->refc = 0;
> }
> mutex_unlock(&ref->lock);
> +
> + /* Toggle bit 0 of LDBAR */
> + mtspr(SPRN_LDBAR, (mfspr(SPRN_LDBAR) ^ (1UL << 63)));
> +
> trace_imc_event_stop(event, flags);
> }
>
> +static void trace_imc_counters_release(struct perf_event *event)
> +{
> + mutex_lock(&imc_global_refc.lock);
> + if (imc_global_refc.id == IMC_DOMAIN_TRACE) {
> + imc_global_refc.refc--;
> + /*
> + * If no other thread is running any trace-imc
> + * event, set the global id to zero.
> + */
> + if (imc_global_refc.refc <= 0) {
> + imc_global_refc.refc = 0;
> + imc_global_refc.id = 0;
> + }
> + }
> + mutex_unlock(&imc_global_refc.lock);
> +}
> +
> static int trace_imc_event_init(struct perf_event *event)
> {
> struct task_struct *target;
> @@ -1314,10 +1425,28 @@ static int trace_imc_event_init(struct perf_event *event)
> if (event->attr.sample_period == 0)
> return -ENOENT;
>
> + /*
> + * Take the global lock, and make sure
> + * no other thread is running any core/thread imc
> + * event
> + */
> + mutex_lock(&imc_global_refc.lock);
> + if (imc_global_refc.id == 0) {
> + imc_global_refc.id = IMC_DOMAIN_TRACE;
> + imc_global_refc.refc++;
> + } else if (imc_global_refc.id == IMC_DOMAIN_TRACE) {
> + imc_global_refc.refc++;
> + } else {
> + mutex_unlock(&imc_global_refc.lock);
> + return -EBUSY;
> + }
> + mutex_unlock(&imc_global_refc.lock);
> +
> event->hw.idx = -1;
> target = event->hw.target;
>
> event->pmu->task_ctx_nr = perf_hw_context;
> + event->destroy = trace_imc_counters_release;
> return 0;
> }
>
> @@ -1429,10 +1558,10 @@ static void cleanup_all_core_imc_memory(void)
> static void thread_imc_ldbar_disable(void *dummy)
> {
> /*
> - * By Zeroing LDBAR, we disable thread-imc
> - * updates.
> + * By toggling 0th bit of LDBAR, we disable thread-imc
> + * updates to memory.
> */
> - mtspr(SPRN_LDBAR, 0);
> + mtspr(SPRN_LDBAR, (mfspr(SPRN_LDBAR) ^ (1UL << 63)));
> }
>
> void thread_imc_disable(void)
^ permalink raw reply
* Re: [PATCH v2 1/5] powerpc/powernv: Re-enable imc trace-mode in kernel
From: maddy @ 2020-02-06 9:03 UTC (permalink / raw)
To: Anju T Sudhakar, mpe; +Cc: nasastry, maddy, linuxppc-dev
In-Reply-To: <20200121101728.14858-2-anju@linux.vnet.ibm.com>
On 1/21/20 3:47 PM, Anju T Sudhakar wrote:
> commit <249fad734a25> ""powerpc/perf: Disable trace_imc pmu"
> disables IMC(In-Memory Collection) trace-mode in kernel, since frequent
> mode switching between accumulation mode and trace mode via the spr LDBAR
> in the hardware can trigger a checkstop(system crash).
>
> Patch to re-enable imc-trace mode in kernel.
>
> The following patch in this series will address the mode switching issue
> by implementing a global lock, and will restrict the usage of
> accumulation and trace-mode at a time.
Reviewed-by: MAdhavan Srinivasan <maddy@linux.ibm.com>
>
> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> ---
> arch/powerpc/platforms/powernv/opal-imc.c | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
> index 000b350d4060..3b4518f4b643 100644
> --- a/arch/powerpc/platforms/powernv/opal-imc.c
> +++ b/arch/powerpc/platforms/powernv/opal-imc.c
> @@ -278,14 +278,7 @@ static int opal_imc_counters_probe(struct platform_device *pdev)
> domain = IMC_DOMAIN_THREAD;
> break;
> case IMC_TYPE_TRACE:
> - /*
> - * FIXME. Using trace_imc events to monitor application
> - * or KVM thread performance can cause a checkstop
> - * (system crash).
> - * Disable it for now.
> - */
> - pr_info_once("IMC: disabling trace_imc PMU\n");
> - domain = -1;
> + domain = IMC_DOMAIN_TRACE;
> break;
> default:
> pr_warn("IMC Unknown Device type \n");
^ permalink raw reply
* [PATCH] selftests/vm: Fix map_hugetlb length used for testing read and write
From: Christophe Leroy @ 2020-02-06 8:42 UTC (permalink / raw)
To: Michael Ellerman, Shuah Khan
Cc: linux-mm, linuxppc-dev, linux-kernel, linux-kselftest
Commit fa7b9a805c79 ("tools/selftest/vm: allow choosing mem size and
page size in map_hugetlb") added the possibility to change the size
of memory mapped for the test, but left the read and write test using
the default value. This is unnoticed when mapping a length greater
than the default one, but segfaults otherwise.
Fix read_bytes() and write_bytes() by giving them the real length.
Also fix the call to munmap().
Fixes: fa7b9a805c79 ("tools/selftest/vm: allow choosing mem size and page size in map_hugetlb")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
tools/testing/selftests/vm/map_hugetlb.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/vm/map_hugetlb.c b/tools/testing/selftests/vm/map_hugetlb.c
index 5a2d7b8efc40..6af951900aa3 100644
--- a/tools/testing/selftests/vm/map_hugetlb.c
+++ b/tools/testing/selftests/vm/map_hugetlb.c
@@ -45,20 +45,20 @@ static void check_bytes(char *addr)
printf("First hex is %x\n", *((unsigned int *)addr));
}
-static void write_bytes(char *addr)
+static void write_bytes(char *addr, size_t length)
{
unsigned long i;
- for (i = 0; i < LENGTH; i++)
+ for (i = 0; i < length; i++)
*(addr + i) = (char)i;
}
-static int read_bytes(char *addr)
+static int read_bytes(char *addr, size_t length)
{
unsigned long i;
check_bytes(addr);
- for (i = 0; i < LENGTH; i++)
+ for (i = 0; i < length; i++)
if (*(addr + i) != (char)i) {
printf("Mismatch at %lu\n", i);
return 1;
@@ -96,11 +96,11 @@ int main(int argc, char **argv)
printf("Returned address is %p\n", addr);
check_bytes(addr);
- write_bytes(addr);
- ret = read_bytes(addr);
+ write_bytes(addr, length);
+ ret = read_bytes(addr, length);
/* munmap() length of MAP_HUGETLB memory must be hugepage aligned */
- if (munmap(addr, LENGTH)) {
+ if (munmap(addr, length)) {
perror("munmap");
exit(1);
}
--
2.25.0
^ permalink raw reply related
* [PATCH] selftest/lkdtm: Don't pollute 'git status'
From: Christophe Leroy @ 2020-02-06 8:11 UTC (permalink / raw)
To: Kees Cook, Shuah Khan; +Cc: linux-mm, linuxppc-dev, linux-kernel
Commit 46d1a0f03d66 ("selftests/lkdtm: Add tests for LKDTM targets")
added generation of lkdtm test scripts.
Ignore those generated scripts when performing 'git status'
Fixes: 46d1a0f03d66 ("selftests/lkdtm: Add tests for LKDTM targets")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
.gitignore | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/.gitignore b/.gitignore
index b849a72d69d5..bb05dce58f8e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -100,6 +100,10 @@ modules.order
/include/ksym/
/arch/*/include/generated/
+# Generated lkdtm tests
+/tools/testing/selftests/lkdtm/*.sh
+!/tools/testing/selftests/lkdtm/run.sh
+
# stgit generated dirs
patches-*
--
2.25.0
^ permalink raw reply related
* [PATCH 2/2] cpufreq/powernv: Fix unsafe notifiers
From: Oliver O'Halloran @ 2020-02-06 6:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Vaidyanathan Srinivasan, Oliver O'Halloran
In-Reply-To: <20200206062622.28235-1-oohall@gmail.com>
The PowerNV cpufreq driver registers two notifiers: one to catch throttle
messages from the OCC and one to bump the CPU frequency back to normal
before a reboot. Both require the cpufreq driver to be registered in order
to function since the notifier callbacks use various cpufreq_*() functions.
Right now we register both notifiers before we've initialised the driver.
This seems to work, but we should head off any protential problems by
registering the notifiers after the driver is initialised.
Cc: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
drivers/cpufreq/powernv-cpufreq.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
index 1806b1d..03798c4 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -1114,9 +1114,6 @@ static int __init powernv_cpufreq_init(void)
if (rc)
goto out;
- register_reboot_notifier(&powernv_cpufreq_reboot_nb);
- opal_message_notifier_register(OPAL_MSG_OCC, &powernv_cpufreq_opal_nb);
-
if (powernv_pstate_info.wof_enabled)
powernv_cpufreq_driver.boost_enabled = true;
else
@@ -1125,15 +1122,17 @@ static int __init powernv_cpufreq_init(void)
rc = cpufreq_register_driver(&powernv_cpufreq_driver);
if (rc) {
pr_info("Failed to register the cpufreq driver (%d)\n", rc);
- goto cleanup_notifiers;
+ goto cleanup;
}
if (powernv_pstate_info.wof_enabled)
cpufreq_enable_boost_support();
+ register_reboot_notifier(&powernv_cpufreq_reboot_nb);
+ opal_message_notifier_register(OPAL_MSG_OCC, &powernv_cpufreq_opal_nb);
+
return 0;
-cleanup_notifiers:
- unregister_all_notifiers();
+cleanup:
clean_chip_info();
out:
pr_info("Platform driver disabled. System does not support PState control\n");
--
2.9.5
^ permalink raw reply related
* [PATCH 1/2] cpufreq/powernv: Fix use-after-free
From: Oliver O'Halloran @ 2020-02-06 6:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Vaidyanathan Srinivasan, Oliver O'Halloran
The cpufreq driver has a use-after-free that we can hit if:
a) There's an OCC message pending when the notifier is registered, and
b) The cpufreq driver fails to register with the core.
When a) occurs the notifier schedules a workqueue item to handle the
message. The backing work_struct is located on chips[].throttle and when b)
happens we clean up by freeing the array. Once we get to the (now free)
queued item and the kernel crashes.
Cc: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
Fixes: c5e29ea ("cpufreq: powernv: Fix bugs in powernv_cpufreq_{init/exit}")
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
drivers/cpufreq/powernv-cpufreq.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
index 56f4bc0..1806b1d 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -1080,6 +1080,12 @@ static int init_chip_info(void)
static inline void clean_chip_info(void)
{
+ int i;
+
+ /* flush any pending work items */
+ if (chips)
+ for (i = 0; i < nr_chips; i++)
+ cancel_work_sync(&chips[i].throttle);
kfree(chips);
}
--
2.9.5
^ permalink raw reply related
* Re: [PATCH v6 10/11] powerpc/mm: Adds counting method to track lockless pagetable walks
From: Christophe Leroy @ 2020-02-06 6:23 UTC (permalink / raw)
To: Leonardo Bras, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Arnd Bergmann, Andrew Morton, Aneesh Kumar K.V,
Nicholas Piggin, Steven Price, Robin Murphy, Mahesh Salgaonkar,
Balbir Singh, Reza Arbab, Thomas Gleixner, Allison Randal,
Greg Kroah-Hartman, Mike Rapoport, Michal Suchanek
Cc: linux-arch, linux-mm, linuxppc-dev, linux-kernel, kvm-ppc
In-Reply-To: <20200206030900.147032-11-leonardo@linux.ibm.com>
Le 06/02/2020 à 04:08, Leonardo Bras a écrit :
> Implements an additional feature to track lockless pagetable walks,
> using a per-cpu counter: lockless_pgtbl_walk_counter.
>
> Before a lockless pagetable walk, preemption is disabled and the
> current cpu's counter is increased.
> When the lockless pagetable walk finishes, the current cpu counter
> is decreased and the preemption is enabled.
>
> With that, it's possible to know in which cpus are happening lockless
> pagetable walks, and optimize serialize_against_pte_lookup().
>
> Implementation notes:
> - Every counter can be changed only by it's CPU
> - It makes use of the original memory barrier in the functions
> - Any counter can be read by any CPU
>
> Due to not locking nor using atomic variables, the impact on the
> lockless pagetable walk is intended to be minimum.
atomic variables have a lot less impact than preempt_enable/disable.
preemt_disable forces a re-scheduling, it really has impact. Why not use
atomic variables instead ?
Christophe
>
> Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
> ---
> arch/powerpc/mm/book3s64/pgtable.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
> index 535613030363..bb138b628f86 100644
> --- a/arch/powerpc/mm/book3s64/pgtable.c
> +++ b/arch/powerpc/mm/book3s64/pgtable.c
> @@ -83,6 +83,7 @@ static void do_nothing(void *unused)
>
> }
>
> +static DEFINE_PER_CPU(int, lockless_pgtbl_walk_counter);
> /*
> * Serialize against find_current_mm_pte which does lock-less
> * lookup in page tables with local interrupts disabled. For huge pages
> @@ -120,6 +121,15 @@ unsigned long __begin_lockless_pgtbl_walk(bool disable_irq)
> if (disable_irq)
> local_irq_save(irq_mask);
>
> + /*
> + * Counts this instance of lockless pagetable walk for this cpu.
> + * Disables preempt to make sure there is no cpu change between
> + * begin/end lockless pagetable walk, so that percpu counting
> + * works fine.
> + */
> + preempt_disable();
> + (*this_cpu_ptr(&lockless_pgtbl_walk_counter))++;
> +
> /*
> * This memory barrier pairs with any code that is either trying to
> * delete page tables, or split huge pages. Without this barrier,
> @@ -158,6 +168,14 @@ inline void __end_lockless_pgtbl_walk(unsigned long irq_mask, bool enable_irq)
> */
> smp_mb();
>
> + /*
> + * Removes this instance of lockless pagetable walk for this cpu.
> + * Enables preempt only after end lockless pagetable walk,
> + * so that percpu counting works fine.
> + */
> + (*this_cpu_ptr(&lockless_pgtbl_walk_counter))--;
> + preempt_enable();
> +
> /*
> * Interrupts must be disabled during the lockless page table walk.
> * That's because the deleting or splitting involves flushing TLBs,
>
^ permalink raw reply
* Re: [PATCH 2/5] mm/memremap_pages: Introduce memremap_compat_align()
From: Aneesh Kumar K.V @ 2020-02-06 6:21 UTC (permalink / raw)
To: Dan Williams, Michael Ellerman
Cc: linux-nvdimm, Linux Kernel Mailing List, Jeff Moyer,
Paul Mackerras, Vishal L Verma, linuxppc-dev, Christoph Hellwig
In-Reply-To: <CAPcyv4jVHnJbPYp1gqDnuwtEgt1NNHDt72vby7hK5dP43C+s8Q@mail.gmail.com>
On 2/6/20 11:21 AM, Dan Williams wrote:
....
>>>
>>> Link: http://lore.kernel.org/r/CAPcyv4gBGNP95APYaBcsocEa50tQj9b5h__83vgngjq3ouGX_Q@mail.gmail.com
>>> Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>> Reported-by: Jeff Moyer <jmoyer@redhat.com>
>>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>> Cc: Paul Mackerras <paulus@samba.org>
>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>>> ---
>>> arch/powerpc/include/asm/io.h | 10 ++++++++++
>>> drivers/nvdimm/pfn_devs.c | 2 +-
>>> include/linux/io.h | 23 +++++++++++++++++++++++
>>> include/linux/mmzone.h | 1 +
>>> 4 files changed, 35 insertions(+), 1 deletion(-)
>>
>> The powerpc change here looks fine to me.
>>
>> Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
>
> Thanks Michael, unfortunately the kbuild robot just woke up and said
> that mips does not like including mmzone.h from io.h. The
> entanglements look intractable.
>
> Is there a file I can stash a strong definition of
> memremap_compat_align(), maybe arch/powerpc/mm/mem.c? Then I can put a
> generic __weak definition in mm/memremap.c rather than play header
> file include games.
>
arch/powerpc/mm/ioremap.c ?
-aneesh
^ permalink raw reply
* Re: [PATCH v6 07/11] powerpc/kvm/e500: Use functions to track lockless pgtbl walks
From: Christophe Leroy @ 2020-02-06 6:18 UTC (permalink / raw)
To: Leonardo Bras, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Arnd Bergmann, Andrew Morton, Aneesh Kumar K.V,
Nicholas Piggin, Steven Price, Robin Murphy, Mahesh Salgaonkar,
Balbir Singh, Reza Arbab, Thomas Gleixner, Allison Randal,
Greg Kroah-Hartman, Mike Rapoport, Michal Suchanek
Cc: linux-arch, linux-mm, linuxppc-dev, linux-kernel, kvm-ppc
In-Reply-To: <20200206030900.147032-8-leonardo@linux.ibm.com>
Le 06/02/2020 à 04:08, Leonardo Bras a écrit :
> Applies the new functions used for tracking lockless pgtable walks on
> kvmppc_e500_shadow_map().
>
> Fixes the place where local_irq_restore() is called: previously, if ptep
> was NULL, local_irq_restore() would never be called.
>
> local_irq_{save,restore} is already inside {begin,end}_lockless_pgtbl_walk,
> so there is no need to repeat it here.
>
> Variable that saves the irq mask was renamed from flags to irq_mask so it
> doesn't lose meaning now it's not directly passed to local_irq_* functions.
>
> Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
> ---
> arch/powerpc/kvm/e500_mmu_host.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
> index 425d13806645..3dcf11f77256 100644
> --- a/arch/powerpc/kvm/e500_mmu_host.c
> +++ b/arch/powerpc/kvm/e500_mmu_host.c
> @@ -336,7 +336,7 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
> pte_t *ptep;
> unsigned int wimg = 0;
> pgd_t *pgdir;
> - unsigned long flags;
> + unsigned long irq_mask;
>
> /* used to check for invalidations in progress */
> mmu_seq = kvm->mmu_notifier_seq;
> @@ -473,7 +473,7 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
> * We are holding kvm->mmu_lock so a notifier invalidate
> * can't run hence pfn won't change.
> */
> - local_irq_save(flags);
> + irq_mask = begin_lockless_pgtbl_walk();
> ptep = find_linux_pte(pgdir, hva, NULL, NULL);
> if (ptep) {
> pte_t pte = READ_ONCE(*ptep);
> @@ -481,15 +481,16 @@ static inline int kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
> if (pte_present(pte)) {
> wimg = (pte_val(pte) >> PTE_WIMGE_SHIFT) &
> MAS2_WIMGE_MASK;
> - local_irq_restore(flags);
> } else {
> - local_irq_restore(flags);
> + end_lockless_pgtbl_walk(irq_mask);
> pr_err_ratelimited("%s: pte not present: gfn %lx,pfn %lx\n",
> __func__, (long)gfn, pfn);
> ret = -EINVAL;
> goto out;
> }
> }
> + end_lockless_pgtbl_walk(irq_mask);
> +
I don't really like unbalanced begin/end.
Something like the following would be cleaner:
begin_lockless_pgtbl_walk()
ptep = find()
if (ptep) {
pte = READ_ONCE()
if (pte_present(pte))
wing=
else
ret = -EINVAL;
}
end_lockless_pgtbl_walk()
if (ret) {
pr_err_rate...()
goto out;
}
> kvmppc_e500_ref_setup(ref, gtlbe, pfn, wimg);
>
> kvmppc_e500_setup_stlbe(&vcpu_e500->vcpu, gtlbe, tsize,
>
Christophe
^ permalink raw reply
* Re: [PATCH v6 06/11] powerpc/mm/book3s64/hash: Use functions to track lockless pgtbl walks
From: Christophe Leroy @ 2020-02-06 6:06 UTC (permalink / raw)
To: Leonardo Bras, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Arnd Bergmann, Andrew Morton, Aneesh Kumar K.V,
Nicholas Piggin, Steven Price, Robin Murphy, Mahesh Salgaonkar,
Balbir Singh, Reza Arbab, Thomas Gleixner, Allison Randal,
Greg Kroah-Hartman, Mike Rapoport, Michal Suchanek
Cc: linux-arch, linux-mm, linuxppc-dev, linux-kernel, kvm-ppc
In-Reply-To: <20200206030900.147032-7-leonardo@linux.ibm.com>
Le 06/02/2020 à 04:08, Leonardo Bras a écrit :
> Applies the new tracking functions to all hash-related functions that do
> lockless pagetable walks.
>
> hash_page_mm: Adds comment that explain that there is no need to
> local_int_disable/save given that it is only called from DataAccess
> interrupt, so interrupts are already disabled.
>
> local_irq_{save,restore} is already inside {begin,end}_lockless_pgtbl_walk,
> so there is no need to repeat it here.
>
> Variable that saves the irq mask was renamed from flags to irq_mask so it
> doesn't lose meaning now it's not directly passed to local_irq_* functions.
>
> Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
> ---
> arch/powerpc/mm/book3s64/hash_tlb.c | 6 +++---
> arch/powerpc/mm/book3s64/hash_utils.c | 27 +++++++++++++++++----------
> 2 files changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/arch/powerpc/mm/book3s64/hash_tlb.c b/arch/powerpc/mm/book3s64/hash_tlb.c
> index 4a70d8dd39cd..86547c4151f6 100644
> --- a/arch/powerpc/mm/book3s64/hash_tlb.c
> +++ b/arch/powerpc/mm/book3s64/hash_tlb.c
> @@ -194,7 +194,7 @@ void __flush_hash_table_range(struct mm_struct *mm, unsigned long start,
> {
> bool is_thp;
> int hugepage_shift;
> - unsigned long flags;
> + unsigned long irq_mask;
>
> start = _ALIGN_DOWN(start, PAGE_SIZE);
> end = _ALIGN_UP(end, PAGE_SIZE);
> @@ -209,7 +209,7 @@ void __flush_hash_table_range(struct mm_struct *mm, unsigned long start,
> * to being hashed). This is not the most performance oriented
> * way to do things but is fine for our needs here.
> */
> - local_irq_save(flags);
> + irq_mask = begin_lockless_pgtbl_walk();
> arch_enter_lazy_mmu_mode();
> for (; start < end; start += PAGE_SIZE) {
> pte_t *ptep = find_current_mm_pte(mm->pgd, start, &is_thp,
> @@ -229,7 +229,7 @@ void __flush_hash_table_range(struct mm_struct *mm, unsigned long start,
> hpte_need_flush(mm, start, ptep, pte, hugepage_shift);
> }
> arch_leave_lazy_mmu_mode();
> - local_irq_restore(flags);
> + end_lockless_pgtbl_walk(irq_mask);
> }
>
> void flush_tlb_pmd_range(struct mm_struct *mm, pmd_t *pmd, unsigned long addr)
> diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
> index 523d4d39d11e..e6d4ab42173b 100644
> --- a/arch/powerpc/mm/book3s64/hash_utils.c
> +++ b/arch/powerpc/mm/book3s64/hash_utils.c
> @@ -1341,12 +1341,16 @@ int hash_page_mm(struct mm_struct *mm, unsigned long ea,
> ea &= ~((1ul << mmu_psize_defs[psize].shift) - 1);
> #endif /* CONFIG_PPC_64K_PAGES */
>
> - /* Get PTE and page size from page tables */
> + /* Get PTE and page size from page tables :
> + * Called in from DataAccess interrupt (data_access_common: 0x300),
> + * interrupts are disabled here.
> + */
Comments formatting is not in line with Linux kernel rules. Should be no
text on the first /* line.
> + __begin_lockless_pgtbl_walk(false);
I think it would be better to not use __begin_lockless_pgtbl_walk()
directly but keep it in a single place, and define something like
begin_lockless_pgtbl_walk_noirq() similar to begin_lockless_pgtbl_walk()
> ptep = find_linux_pte(pgdir, ea, &is_thp, &hugeshift);
> if (ptep == NULL || !pte_present(*ptep)) {
> DBG_LOW(" no PTE !\n");
> rc = 1;
> - goto bail;
> + goto bail_pgtbl_walk;
What's the point in changing the name of this label ? There is only one
label, why polute the function with so huge name ?
For me, everyone understand what 'bail' means. Unneccessary changes
should be avoided. If you really really want to do it, it should be
another patch.
See kernel codying style, chapter 'naming':
"LOCAL variable names should be short". This also applies to labels.
"C is a Spartan language, and so should your naming be. Unlike Modula-2
and Pascal programmers, C programmers do not use cute names like
ThisVariableIsATemporaryCounter. A C programmer would call that variable
tmp, which is much easier to write, and not the least more difficult to
understand."
> }
>
> /* Add _PAGE_PRESENT to the required access perm */
> @@ -1359,7 +1363,7 @@ int hash_page_mm(struct mm_struct *mm, unsigned long ea,
> if (!check_pte_access(access, pte_val(*ptep))) {
> DBG_LOW(" no access !\n");
> rc = 1;
> - goto bail;
> + goto bail_pgtbl_walk;
> }
>
> if (hugeshift) {
> @@ -1383,7 +1387,7 @@ int hash_page_mm(struct mm_struct *mm, unsigned long ea,
> if (current->mm == mm)
> check_paca_psize(ea, mm, psize, user_region);
>
> - goto bail;
> + goto bail_pgtbl_walk;
> }
>
> #ifndef CONFIG_PPC_64K_PAGES
> @@ -1457,6 +1461,8 @@ int hash_page_mm(struct mm_struct *mm, unsigned long ea,
> #endif
> DBG_LOW(" -> rc=%d\n", rc);
>
> +bail_pgtbl_walk:
> + __end_lockless_pgtbl_walk(0, false);
> bail:
> exception_exit(prev_state);
> return rc;
> @@ -1545,7 +1551,7 @@ static void hash_preload(struct mm_struct *mm, unsigned long ea,
> unsigned long vsid;
> pgd_t *pgdir;
> pte_t *ptep;
> - unsigned long flags;
> + unsigned long irq_mask;
> int rc, ssize, update_flags = 0;
> unsigned long access = _PAGE_PRESENT | _PAGE_READ | (is_exec ? _PAGE_EXEC : 0);
>
> @@ -1567,11 +1573,12 @@ static void hash_preload(struct mm_struct *mm, unsigned long ea,
> vsid = get_user_vsid(&mm->context, ea, ssize);
> if (!vsid)
> return;
> +
Is this new line related to the patch ?
> /*
> * Hash doesn't like irqs. Walking linux page table with irq disabled
> * saves us from holding multiple locks.
> */
> - local_irq_save(flags);
> + irq_mask = begin_lockless_pgtbl_walk();
>
> /*
> * THP pages use update_mmu_cache_pmd. We don't do
> @@ -1616,7 +1623,7 @@ static void hash_preload(struct mm_struct *mm, unsigned long ea,
> mm_ctx_user_psize(&mm->context),
> pte_val(*ptep));
> out_exit:
> - local_irq_restore(flags);
> + end_lockless_pgtbl_walk(irq_mask);
> }
>
> /*
> @@ -1679,16 +1686,16 @@ u16 get_mm_addr_key(struct mm_struct *mm, unsigned long address)
> {
> pte_t *ptep;
> u16 pkey = 0;
> - unsigned long flags;
> + unsigned long irq_mask;
>
> if (!mm || !mm->pgd)
> return 0;
>
> - local_irq_save(flags);
> + irq_mask = begin_lockless_pgtbl_walk();
> ptep = find_linux_pte(mm->pgd, address, NULL, NULL);
> if (ptep)
> pkey = pte_to_pkey_bits(pte_val(READ_ONCE(*ptep)));
> - local_irq_restore(flags);
> + end_lockless_pgtbl_walk(irq_mask);
>
> return pkey;
> }
>
Christophe
^ permalink raw reply
* Re: [PATCH v6 01/11] asm-generic/pgtable: Adds generic functions to track lockless pgtable walks
From: Christophe Leroy @ 2020-02-06 5:54 UTC (permalink / raw)
To: Leonardo Bras, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Arnd Bergmann, Andrew Morton, Aneesh Kumar K.V,
Nicholas Piggin, Steven Price, Robin Murphy, Mahesh Salgaonkar,
Balbir Singh, Reza Arbab, Thomas Gleixner, Allison Randal,
Greg Kroah-Hartman, Mike Rapoport, Michal Suchanek
Cc: linux-arch, linux-mm, linuxppc-dev, linux-kernel, kvm-ppc
In-Reply-To: <20200206030900.147032-2-leonardo@linux.ibm.com>
Le 06/02/2020 à 04:08, Leonardo Bras a écrit :
> It's necessary to track lockless pagetable walks, in order to avoid doing
> THP splitting/collapsing during them.
>
> The default solution is to disable irq before lockless pagetable walks and
> enable it after it's finished.
>
> On code, this means you can find local_irq_disable() and local_irq_enable()
> around some pieces of code, usually without comments on why it is needed.
>
> This patch proposes a set of generic functions to be called before starting
> and after finishing a lockless pagetable walk. It is supposed to make clear
> that a lockless pagetable walk happens there, and also carries information
> on why the irq disable/enable is needed.
>
> begin_lockless_pgtbl_walk()
> Insert before starting any lockless pgtable walk
> end_lockless_pgtbl_walk()
> Insert after the end of any lockless pgtable walk
> (Mostly after the ptep is last used)
>
> A memory barrier was also added just to make sure there is no speculative
> read outside the interrupt disabled area. Other than that, it is not
> supposed to have any change of behavior from current code.
Is that speculative barrier necessary for all architectures ? Does it
impact performance ? Shouldn't this be another patch ?
>
> It is planned to allow arch-specific versions, so that additional steps can
> be added while keeping the code clean.
>
> Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
> ---
> include/asm-generic/pgtable.h | 51 +++++++++++++++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
>
> diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
> index e2e2bef07dd2..8d368d3c0974 100644
> --- a/include/asm-generic/pgtable.h
> +++ b/include/asm-generic/pgtable.h
> @@ -1222,6 +1222,57 @@ static inline bool arch_has_pfn_modify_check(void)
> #endif
> #endif
>
> +#ifndef __HAVE_ARCH_LOCKLESS_PGTBL_WALK_CONTROL
> +/*
> + * begin_lockless_pgtbl_walk: Must be inserted before a function call that does
> + * lockless pagetable walks, such as __find_linux_pte()
> + */
> +static inline
> +unsigned long begin_lockless_pgtbl_walk(void)
What about keeping the same syntax as local_irq_save(), something like:
#define begin_lockless_pgtbl_walk(flags) \
do {
local_irq_save(flags);
smp_mb();
} while (0)
> +{
> + unsigned long irq_mask;
> +
> + /*
> + * Interrupts must be disabled during the lockless page table walk.
> + * That's because the deleting or splitting involves flushing TLBs,
> + * which in turn issues interrupts, that will block when disabled.
> + */
> + local_irq_save(irq_mask);
> +
> + /*
> + * This memory barrier pairs with any code that is either trying to
> + * delete page tables, or split huge pages. Without this barrier,
> + * the page tables could be read speculatively outside of interrupt
> + * disabling.
> + */
> + smp_mb();
> +
> + return irq_mask;
> +}
> +
> +/*
> + * end_lockless_pgtbl_walk: Must be inserted after the last use of a pointer
> + * returned by a lockless pagetable walk, such as __find_linux_pte()
> + */
> +static inline void end_lockless_pgtbl_walk(unsigned long irq_mask)
Same
#define end_lockless_pgtbl_walk(flags) \
do {
smp_mb();
local_irq_restore(flags);
} while (0);
> +{
> + /*
> + * This memory barrier pairs with any code that is either trying to
> + * delete page tables, or split huge pages. Without this barrier,
> + * the page tables could be read speculatively outside of interrupt
> + * disabling.
> + */
> + smp_mb();
> +
> + /*
> + * Interrupts must be disabled during the lockless page table walk.
> + * That's because the deleting or splitting involves flushing TLBs,
> + * which in turn issues interrupts, that will block when disabled.
> + */
> + local_irq_restore(irq_mask);
> +}
> +#endif
> +
> /*
> * On some architectures it depends on the mm if the p4d/pud or pmd
> * layer of the page table hierarchy is folded or not.
>
Christophe
^ permalink raw reply
* Re: [PATCH 2/5] mm/memremap_pages: Introduce memremap_compat_align()
From: Dan Williams @ 2020-02-06 5:51 UTC (permalink / raw)
To: Michael Ellerman
Cc: linux-nvdimm, Aneesh Kumar K.V, Linux Kernel Mailing List,
Jeff Moyer, Paul Mackerras, Vishal L Verma, linuxppc-dev,
Christoph Hellwig
In-Reply-To: <875zgl3fa9.fsf@mpe.ellerman.id.au>
On Tue, Feb 4, 2020 at 7:05 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Dan Williams <dan.j.williams@intel.com> writes:
> > The "sub-section memory hotplug" facility allows memremap_pages() users
> > like libnvdimm to compensate for hardware platforms like x86 that have a
> > section size larger than their hardware memory mapping granularity. The
> > compensation that sub-section support affords is being tolerant of
> > physical memory resources shifting by units smaller (64MiB on x86) than
> > the memory-hotplug section size (128 MiB). Where the platform
> > physical-memory mapping granularity is limited by the number and
> > capability of address-decode-registers in the memory controller.
> >
> > While the sub-section support allows memremap_pages() to operate on
> > sub-section (2MiB) granularity, the Power architecture may still
> > require 16MiB alignment on "!radix_enabled()" platforms.
> >
> > In order for libnvdimm to be able to detect and manage this per-arch
> > limitation, introduce memremap_compat_align() as a common minimum
> > alignment across all driver-facing memory-mapping interfaces, and let
> > Power override it to 16MiB in the "!radix_enabled()" case.
> >
> > The assumption / requirement for 16MiB to be a viable
> > memremap_compat_align() value is that Power does not have platforms
> > where its equivalent of address-decode-registers never hardware remaps a
> > persistent memory resource on smaller than 16MiB boundaries.
> >
> > Based on an initial patch by Aneesh.
> >
> > Link: http://lore.kernel.org/r/CAPcyv4gBGNP95APYaBcsocEa50tQj9b5h__83vgngjq3ouGX_Q@mail.gmail.com
> > Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> > Reported-by: Jeff Moyer <jmoyer@redhat.com>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> > arch/powerpc/include/asm/io.h | 10 ++++++++++
> > drivers/nvdimm/pfn_devs.c | 2 +-
> > include/linux/io.h | 23 +++++++++++++++++++++++
> > include/linux/mmzone.h | 1 +
> > 4 files changed, 35 insertions(+), 1 deletion(-)
>
> The powerpc change here looks fine to me.
>
> Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
Thanks Michael, unfortunately the kbuild robot just woke up and said
that mips does not like including mmzone.h from io.h. The
entanglements look intractable.
Is there a file I can stash a strong definition of
memremap_compat_align(), maybe arch/powerpc/mm/mem.c? Then I can put a
generic __weak definition in mm/memremap.c rather than play header
file include games.
^ permalink raw reply
* Re: [PATCH v6 04/11] powerpc/mce_power: Use functions to track lockless pgtbl walks
From: Christophe Leroy @ 2020-02-06 5:48 UTC (permalink / raw)
To: Leonardo Bras, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Arnd Bergmann, Andrew Morton, Aneesh Kumar K.V,
Nicholas Piggin, Steven Price, Robin Murphy, Mahesh Salgaonkar,
Balbir Singh, Reza Arbab, Thomas Gleixner, Allison Randal,
Greg Kroah-Hartman, Mike Rapoport, Michal Suchanek
Cc: linux-arch, linux-mm, linuxppc-dev, linux-kernel, kvm-ppc
In-Reply-To: <20200206030900.147032-5-leonardo@linux.ibm.com>
Le 06/02/2020 à 04:08, Leonardo Bras a écrit :
> Applies the new functions used for tracking lockless pgtable walks on
> addr_to_pfn().
>
> local_irq_{save,restore} is already inside {begin,end}_lockless_pgtbl_walk,
> so there is no need to repeat it here.
>
> Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
> ---
> arch/powerpc/kernel/mce_power.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
> index 1cbf7f1a4e3d..a9e38ef4e437 100644
> --- a/arch/powerpc/kernel/mce_power.c
> +++ b/arch/powerpc/kernel/mce_power.c
> @@ -29,7 +29,7 @@ unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
> {
> pte_t *ptep;
> unsigned int shift;
> - unsigned long pfn, flags;
> + unsigned long pfn, irq_mask;
Why change the name ? flags is a well known historical name.
> struct mm_struct *mm;
>
> if (user_mode(regs))
> @@ -37,7 +37,7 @@ unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
> else
> mm = &init_mm;
>
> - local_irq_save(flags);
> + irq_mask = begin_lockless_pgtbl_walk();
> ptep = __find_linux_pte(mm->pgd, addr, NULL, &shift);
>
> if (!ptep || pte_special(*ptep)) {
> @@ -53,7 +53,7 @@ unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr)
> }
>
> out:
> - local_irq_restore(flags);
> + end_lockless_pgtbl_walk(irq_mask);
> return pfn;
> }
>
>
Christophe
^ permalink raw reply
* Re: [PATCH v6 03/11] powerpc/mm: Adds arch-specificic functions to track lockless pgtable walks
From: Christophe Leroy @ 2020-02-06 5:46 UTC (permalink / raw)
To: Leonardo Bras, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Arnd Bergmann, Andrew Morton, Aneesh Kumar K.V,
Nicholas Piggin, Steven Price, Robin Murphy, Mahesh Salgaonkar,
Balbir Singh, Reza Arbab, Thomas Gleixner, Allison Randal,
Greg Kroah-Hartman, Mike Rapoport, Michal Suchanek
Cc: linux-arch, linux-mm, linuxppc-dev, linux-kernel, kvm-ppc
In-Reply-To: <20200206030900.147032-4-leonardo@linux.ibm.com>
Le 06/02/2020 à 04:08, Leonardo Bras a écrit :
> On powerpc, we need to do some lockless pagetable walks from functions
> that already have disabled interrupts, specially from real mode with
> MSR[EE=0].
>
> In these contexts, disabling/enabling interrupts can be very troubling.
When interrupts are already disabled, the flag returned when disabling
it will be such that when we restore it later, interrupts remain
disabled, so what's the problem ?
>
> So, this arch-specific implementation features functions with an extra
> argument that allows interrupt enable/disable to be skipped:
> __begin_lockless_pgtbl_walk() and __end_lockless_pgtbl_walk().
>
> Functions similar to the generic ones are also exported, by calling
> the above functions with parameter {en,dis}able_irq = true.
>
> Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
> ---
> arch/powerpc/include/asm/book3s/64/pgtable.h | 6 ++
> arch/powerpc/mm/book3s64/pgtable.c | 86 +++++++++++++++++++-
> 2 files changed, 91 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
> index 201a69e6a355..78f6ffb1bb3e 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -1375,5 +1375,11 @@ static inline bool pgd_is_leaf(pgd_t pgd)
> return !!(pgd_raw(pgd) & cpu_to_be64(_PAGE_PTE));
> }
>
> +#define __HAVE_ARCH_LOCKLESS_PGTBL_WALK_CONTROL
> +unsigned long begin_lockless_pgtbl_walk(void);
> +unsigned long __begin_lockless_pgtbl_walk(bool disable_irq);
> +void end_lockless_pgtbl_walk(unsigned long irq_mask);
> +void __end_lockless_pgtbl_walk(unsigned long irq_mask, bool enable_irq);
> +
Why not make them static inline just like the generic ones ?
> #endif /* __ASSEMBLY__ */
> #endif /* _ASM_POWERPC_BOOK3S_64_PGTABLE_H_ */
> diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
> index 2bf7e1b4fd82..535613030363 100644
> --- a/arch/powerpc/mm/book3s64/pgtable.c
> +++ b/arch/powerpc/mm/book3s64/pgtable.c
> @@ -82,6 +82,7 @@ static void do_nothing(void *unused)
> {
>
> }
> +
Is this blank line related to the patch ?
> /*
> * Serialize against find_current_mm_pte which does lock-less
> * lookup in page tables with local interrupts disabled. For huge pages
> @@ -98,6 +99,89 @@ void serialize_against_pte_lookup(struct mm_struct *mm)
> smp_call_function_many(mm_cpumask(mm), do_nothing, NULL, 1);
> }
>
> +/* begin_lockless_pgtbl_walk: Must be inserted before a function call that does
> + * lockless pagetable walks, such as __find_linux_pte().
> + * This version allows setting disable_irq=false, so irqs are not touched, which
> + * is quite useful for running when ints are already disabled (like real-mode)
> + */
> +inline
> +unsigned long __begin_lockless_pgtbl_walk(bool disable_irq)
> +{
> + unsigned long irq_mask = 0;
> +
> + /*
> + * Interrupts must be disabled during the lockless page table walk.
> + * That's because the deleting or splitting involves flushing TLBs,
> + * which in turn issues interrupts, that will block when disabled.
> + *
> + * When this function is called from realmode with MSR[EE=0],
> + * it's not needed to touch irq, since it's already disabled.
> + */
> + if (disable_irq)
> + local_irq_save(irq_mask);
> +
> + /*
> + * This memory barrier pairs with any code that is either trying to
> + * delete page tables, or split huge pages. Without this barrier,
> + * the page tables could be read speculatively outside of interrupt
> + * disabling or reference counting.
> + */
> + smp_mb();
> +
> + return irq_mask;
> +}
> +EXPORT_SYMBOL(__begin_lockless_pgtbl_walk);
> +
> +/* begin_lockless_pgtbl_walk: Must be inserted before a function call that does
> + * lockless pagetable walks, such as __find_linux_pte().
> + * This version is used by generic code, and always assume irqs will be disabled
> + */
> +unsigned long begin_lockless_pgtbl_walk(void)
> +{
> + return __begin_lockless_pgtbl_walk(true);
> +}
> +EXPORT_SYMBOL(begin_lockless_pgtbl_walk);
Even more than begin_lockless_pgtbl_walk(), this one is worth being
static inline in the H file.
> +
> +/*
> + * __end_lockless_pgtbl_walk: Must be inserted after the last use of a pointer
> + * returned by a lockless pagetable walk, such as __find_linux_pte()
> + * This version allows setting enable_irq=false, so irqs are not touched, which
> + * is quite useful for running when ints are already disabled (like real-mode)
> + */
> +inline void __end_lockless_pgtbl_walk(unsigned long irq_mask, bool enable_irq)
> +{
> + /*
> + * This memory barrier pairs with any code that is either trying to
> + * delete page tables, or split huge pages. Without this barrier,
> + * the page tables could be read speculatively outside of interrupt
> + * disabling or reference counting.
> + */
> + smp_mb();
> +
> + /*
> + * Interrupts must be disabled during the lockless page table walk.
> + * That's because the deleting or splitting involves flushing TLBs,
> + * which in turn issues interrupts, that will block when disabled.
> + *
> + * When this function is called from realmode with MSR[EE=0],
> + * it's not needed to touch irq, since it's already disabled.
> + */
> + if (enable_irq)
> + local_irq_restore(irq_mask);
> +}
> +EXPORT_SYMBOL(__end_lockless_pgtbl_walk);
> +
> +/*
> + * end_lockless_pgtbl_walk: Must be inserted after the last use of a pointer
> + * returned by a lockless pagetable walk, such as __find_linux_pte()
> + * This version is used by generic code, and always assume irqs will be enabled
> + */
> +void end_lockless_pgtbl_walk(unsigned long irq_mask)
> +{
> + __end_lockless_pgtbl_walk(irq_mask, true);
> +}
> +EXPORT_SYMBOL(end_lockless_pgtbl_walk);
> +
> /*
> * We use this to invalidate a pmdp entry before switching from a
> * hugepte to regular pmd entry.
> @@ -487,7 +571,7 @@ static int __init setup_disable_tlbie(char *str)
> tlbie_capable = false;
> tlbie_enabled = false;
>
> - return 1;
> + return 1;
Is that related to this patch at all ?
> }
> __setup("disable_tlbie", setup_disable_tlbie);
>
>
Christophe
^ permalink raw reply
* Re: [PATCH 4/6] powerpc/eeh: Remove PHB check in probe
From: Sam Bobroff @ 2020-02-06 5:30 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: tyreld, linuxppc-dev
In-Reply-To: <20200203083521.16549-5-oohall@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1579 bytes --]
On Mon, Feb 03, 2020 at 07:35:19PM +1100, Oliver O'Halloran wrote:
> This check for a missing PHB has existing in various forms since the
> initial PPC64 port was upstreamed in 2002. The idea seems to be that we
> need to guard against creating pci-specific data structures for the non-pci
> children of a PCI device tree node (e.g. USB devices). However, we only
> create pci_dn structures for DT nodes that correspond to PCI devices so
> there's not much point in doing this check in the eeh_probe path.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
I always wondered how to test that block... and it's just dead code.
Reviewed-by: Sam Bobroff <sbobroff@linux.ibm.com>
> ---
> arch/powerpc/kernel/eeh.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 9cb3370..a9e4ca7 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -1120,7 +1120,6 @@ core_initcall_sync(eeh_init);
> */
> void eeh_add_device_early(struct pci_dn *pdn)
> {
> - struct pci_controller *phb = pdn ? pdn->phb : NULL;
> struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
>
> if (!edev)
> @@ -1129,11 +1128,6 @@ void eeh_add_device_early(struct pci_dn *pdn)
> if (!eeh_has_flag(EEH_PROBE_MODE_DEVTREE))
> return;
>
> - /* USB Bus children of PCI devices will not have BUID's */
> - if (NULL == phb ||
> - (eeh_has_flag(EEH_PROBE_MODE_DEVTREE) && 0 == phb->buid))
> - return;
> -
> eeh_ops->probe(pdn, NULL);
> }
>
> --
> 2.9.5
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 3/6] powerpc/eeh: Do early EEH init only when required
From: Sam Bobroff @ 2020-02-06 5:22 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: tyreld, linuxppc-dev
In-Reply-To: <20200203083521.16549-4-oohall@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3515 bytes --]
On Mon, Feb 03, 2020 at 07:35:18PM +1100, Oliver O'Halloran wrote:
> The pci hotplug helper (pci_hp_add_devices()) calls
> eeh_add_device_tree_early() to scan the device-tree for new PCI devices and
> do the early EEH probe before the device is scanned. This early probe is a
> no-op in a lot of cases because:
>
> a) The early init is only required to satisfy a PAPR requirement that EEH
> be configured before we start doing config accesses. On PowerNV it is
> a no-op.
>
> b) It's a no-op for devices that have already had their eeh_dev
> initialised.
>
> There are four callers of pci_hp_add_devices():
>
> 1. arch/powerpc/kernel/eeh_driver.c
> Here the hotplug helper is called when re-scanning pci_devs that
> were removed during an EEH recovery pass. The EEH stat for each
> removed device (the eeh_dev) is retained across a recovery pass
> so the early init is a no-op in this case.
>
> 2. drivers/pci/hotplug/pnv_php.c
> This is also a no-op since the PowerNV hotplug driver is, suprisingly,
> PowerNV specific.
>
> 3. drivers/pci/hotplug/rpaphp_core.c
> 4. drivers/pci/hotplug/rpaphp_pci.c
> In these two cases new devices have been hotplugged and FW has
> provided new DT nodes for each. These are the only two cases where
> the EEH we might have new PCI device nodes in the DT so these are
> the only two cases where the early EEH probe needs to be done.
>
> We can move the calls to eeh_add_device_tree_early() to the locations where
> it's needed and remove it from the generic path. This is preparation for
> making the early EEH probe pseries specific.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
Makes sense to me.
Reviewed-by: Sam Bobroff <sbobroff@linux.ibm.com>
> ---
> arch/powerpc/kernel/pci-hotplug.c | 2 --
> drivers/pci/hotplug/rpaphp_core.c | 2 ++
> drivers/pci/hotplug/rpaphp_pci.c | 4 +++-
> 3 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
> index d6a67f8..bf83f76 100644
> --- a/arch/powerpc/kernel/pci-hotplug.c
> +++ b/arch/powerpc/kernel/pci-hotplug.c
> @@ -112,8 +112,6 @@ void pci_hp_add_devices(struct pci_bus *bus)
> struct pci_controller *phb;
> struct device_node *dn = pci_bus_to_OF_node(bus);
>
> - eeh_add_device_tree_early(PCI_DN(dn));
> -
> phb = pci_bus_to_host(bus);
>
> mode = PCI_PROBE_NORMAL;
> diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
> index e408e40..9c1e43e 100644
> --- a/drivers/pci/hotplug/rpaphp_core.c
> +++ b/drivers/pci/hotplug/rpaphp_core.c
> @@ -494,6 +494,8 @@ static int enable_slot(struct hotplug_slot *hotplug_slot)
> return retval;
>
> if (state == PRESENT) {
> + eeh_add_device_tree_early(PCI_DN(slot->dn));
> +
> pci_lock_rescan_remove();
> pci_hp_add_devices(slot->bus);
> pci_unlock_rescan_remove();
> diff --git a/drivers/pci/hotplug/rpaphp_pci.c b/drivers/pci/hotplug/rpaphp_pci.c
> index beca61b..61ebbd8 100644
> --- a/drivers/pci/hotplug/rpaphp_pci.c
> +++ b/drivers/pci/hotplug/rpaphp_pci.c
> @@ -95,8 +95,10 @@ int rpaphp_enable_slot(struct slot *slot)
> return -EINVAL;
> }
>
> - if (list_empty(&bus->devices))
> + if (list_empty(&bus->devices)) {
> + eeh_add_device_tree_early(PCI_DN(slot->dn));
> pci_hp_add_devices(bus);
> + }
>
> if (!list_empty(&bus->devices)) {
> slot->state = CONFIGURED;
> --
> 2.9.5
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: Latest Git kernel: avahi-daemon[2410]: ioctl(): Inappropriate ioctl for device
From: Michael Ellerman @ 2020-02-06 4:35 UTC (permalink / raw)
To: Christian Zigotzky, Jakub Kicinski
Cc: DTML, Darren Stevens, mad skateman, netdev@vger.kernel.org,
Linux Kernel Mailing List, linuxppc-dev, contact@a-eon.com,
R.T.Dickinson, Christoph Hellwig
In-Reply-To: <C11859E1-BE71-494F-81E2-9B27E27E60EE@xenosoft.de>
Christian Zigotzky <chzigotzky@xenosoft.de> writes:
> Kernel 5.5 PowerPC is also affected.
I don't know what you mean by that. What sha are you talking about?
I have a system with avahi running and everything's fine.
# grep use- /etc/avahi/avahi-daemon.conf
use-ipv4=yes
use-ipv6=yes
# systemctl status -l --no-pager avahi-daemon
● avahi-daemon.service - Avahi mDNS/DNS-SD Stack
Loaded: loaded (/lib/systemd/system/avahi-daemon.service; enabled; vendor preset: enabled)
Active: active (running) since Thu 2020-02-06 14:55:34 AEDT; 38min ago
Main PID: 1884 (avahi-daemon)
Status: "avahi-daemon 0.7 starting up."
CGroup: /system.slice/avahi-daemon.service
├─1884 avahi-daemon: running [mpe-ubuntu-le.local]
└─1888 avahi-daemon: chroot helper
Feb 06 14:55:34 mpe-ubuntu-le avahi-daemon[1884]: Registering new address record for fe80::5054:ff:fe66:2a19 on eth0.*.
Feb 06 14:55:34 mpe-ubuntu-le avahi-daemon[1884]: Registering new address record for 10.61.141.81 on eth0.IPv4.
Feb 06 14:55:34 mpe-ubuntu-le avahi-daemon[1884]: Registering new address record for ::1 on lo.*.
Feb 06 14:55:34 mpe-ubuntu-le avahi-daemon[1884]: Registering new address record for 127.0.0.1 on lo.IPv4.
Feb 06 14:55:34 mpe-ubuntu-le systemd[1]: Started Avahi mDNS/DNS-SD Stack.
Feb 06 14:55:35 mpe-ubuntu-le avahi-daemon[1884]: Server startup complete. Host name is mpe-ubuntu-le.local. Local service cookie is 3972418141.
Feb 06 14:55:38 mpe-ubuntu-le avahi-daemon[1884]: Leaving mDNS multicast group on interface eth0.IPv6 with address fe80::5054:ff:fe66:2a19.
Feb 06 14:55:38 mpe-ubuntu-le avahi-daemon[1884]: Joining mDNS multicast group on interface eth0.IPv6 with address fd69:d75f:b8b5:61:5054:ff:fe66:2a19.
Feb 06 14:55:38 mpe-ubuntu-le avahi-daemon[1884]: Registering new address record for fd69:d75f:b8b5:61:5054:ff:fe66:2a19 on eth0.*.
Feb 06 14:55:38 mpe-ubuntu-le avahi-daemon[1884]: Withdrawing address record for fe80::5054:ff:fe66:2a19 on eth0.
# uname -r
5.5.0-gcc-8.2.0
The key question is what ioctl is it complaining about. You should be
able to find that via strace.
cheers
> Christian Zigotzky wrote:
>
> Hi All,
>
> The issue with the avahi-daemon still exist in the latest Git kernel. It's a PowerPC issue. I compiled the latest Git kernel on a PC today and there aren't any issues with the avahi daemon. Another Power Mac user reported the same issue on his G5. I tested with the AmigaOne X1000 and X5000 in the last days.
>
> I bisected today but I think the result isn't correct because it found the other problem with ordering of PCSCSI definition in esp_rev enum. I don't know how to bisect if there is another issue at the same time. Maybe "git bisect skip"?
>
> 2086faae3c55a652cfbd369e18ecdb703aacc493 is the first bad commit
> commit 2086faae3c55a652cfbd369e18ecdb703aacc493
> Author: Kars de Jong <jongk@linux-m68k.org>
> Date: Tue Nov 19 21:20:20 2019 +0100
>
> scsi: esp_scsi: Correct ordering of PCSCSI definition in esp_rev enum
>
> The order of the definitions in the esp_rev enum is important. The values
> are used in comparisons for chip features.
>
> Add a comment to the enum explaining this.
>
> Also, the actual values for the enum fields are irrelevant, so remove the
> explicit values (suggested by Geert Uytterhoeven). This makes adding a new
> field in the middle of the enum easier.
>
> Finally, move the PCSCSI definition to the right place in the enum. In its
> previous location, at the end of the enum, the wrong values are written to
> the CONFIG3 register when used with FAST-SCSI targets.
>
> Link: https://lore.kernel.org/r/20191119202021.28720-2-jongk@linux-m68k.org
> Signed-off-by: Kars de Jong <jongk@linux-m68k.org>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
>
> :040000 040000 cdc128596e33fb60406b5de9b17b79623c187c1a 48ceab06439f95285e8b30181e75f9a68c25fcb5 M drivers
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox