drivers/net/ethernet/mediatek/mtk_ppe_offload.c - suspicious code?
While doing some code auditing for -Woverride_init, I spotted some questionable
code
commit 502e84e2382d92654a2ecbc52cdbdb5a11cdcec7
Author: Felix Fietkau
Date: Wed Mar 24 02:30:54 2021 +0100
net: ethernet: mtk_eth_soc: add flow offloading support
In drivers/net/ethernet/mediatek/mtk_ppe_offload.c:
+static const struct rhashtable_params mtk_flow_ht_params = {
+ .head_offset = offsetof(struct mtk_flow_entry, node),
+ .head_offset = offsetof(struct mtk_flow_entry, cookie),
+ .key_len = sizeof(unsigned long),
+ .automatic_shrinking = true,
+};
What's intended for head_offset here?
Christoph: This was the only actual questionable code caught by override-init
across allmodconfig builds of arm, arm64, and x86_64. The other 218 warnings
not already silenced by Makefile twiddling of CFLAGS are all legitimate. I
admit not being sure what that means for its use in W=1 builds in general.
pgpoMkJk7nerj.pgp
Description: PGP signature
next-20210409 build breakage in drivers/iommu/mtk_iommu_v1.c
This commit: commit 8de000cf0265eaa4f63aff9f2c7a3876d2dda9b6 Author: Yong Wu Date: Fri Mar 26 11:23:36 2021 +0800 iommu/mediatek-v1: Allow building as module changes drivers/iommu/Kcconfig config MTK_IOMMU_V1 - bool "MTK IOMMU Version 1 (M4U gen1) Support" + tristate "MediaTek IOMMU Version 1 (M4U gen1) Support" Unfortunately, this doesn't actually build properly as a module, because: MODPOST modules-only.symvers ERROR: modpost: "of_phandle_iterator_args" [drivers/iommu/mtk_iommu_v1.ko] undefined! make[2]: *** [/usr/src/linux-next/scripts/Makefile.modpost:150: modules-only.symvers] Error 1 There's no EXPORT_SYMBOL and drivers/of/base.o is a built-in, so the symbol isn't available to modules. pgp8Me2bJSfuJ.pgp Description: PGP signature
Re: [PATCH RESEND] gcc-plugins: avoid errors with -std=gnu++11 on old gcc
On Thu, 18 Mar 2021 11:41:29 -, David Laight said: > That gcc bug just implies you need a space after "xxx". > That is easily fixable in the sources. It's not quite that simple. In file included from /usr/lib/gcc/x86_64-linux-gnu/5/plugin/include/tm.h:27, from /usr/lib/gcc/x86_64-linux-gnu/5/plugin/include/gcc-plugin.h:31, from /usr/lib/gcc/x86_64-linux-gnu/5/plugin/include/plugin.h:23, from scripts/gcc-plugins/gcc-common.h:9, from scripts/gcc-plugins/latent_entropy_plugin.c:78: >> /usr/lib/gcc/x86_64-linux-gnu/5/plugin/include/config/elfos.h:102:21: >> warning: invalid suffix on literal; C++11 requires a space between literal >> and string macro [-Wliteral-suffix] fprintf ((FILE), "%s"HOST_WIDE_INT_PRINT_UNSIGNED"\n",\ The problem isn't in a kernel source file... To quote an earlier message of mine: > It looks like it's not a kernel source tree issue, it's a g++ issue fixed in > g++ 6 and later. > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69959 > And it looks like there was an intent to backport it to 4.9 and 5.4: > https://gcc.gnu.org/legacy-ml/gcc-patches/2016-02/msg01409.html > The bugtracker doesn't show an equivalent for 69959 being closed against > 4.9.x or 5.[56], > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63254 has a patch for one of the > gcc-supplied files that tosses the warning, but that way lies madness... pgpKAMpPEyXho.pgp Description: PGP signature
Re: [PATCH RESEND] gcc-plugins: avoid errors with -std=gnu++11 on old gcc
On Thu, 18 Mar 2021 18:07:28 +0900, Masahiro Yamada said: > We can require GCC 6+ for building GCC plugins. > + depends on CC_IS_GCC && GCC_VERSION >= 6 I'd be OK with that personally, but the question is whether any gcc 4.9 or 5.x users are using plugins. That's a bit above my pay grade. Kees? Do we have any data on that? (All I know is that there is at least one, because they tripped over the GCC bug that prompted the second patch) > BTW, the commit message mentions that > the issues only happen on GCC 4 and 5, > but the added code was: > > GCC_FLAVOR = $(call cc-ifversion, -ge, 1100, 11, 98) > > instead of > > GCC_FLAVOR = $(call cc-ifversion, -ge, 600, 11, 98) > > So, this patch is also requiring to cover two standards: > > GCC_VERSION >= 11 : -std=gnu++11 > GCC_VERSION < 11 : -std=gnu++98 I chose 1100 so that everything from 4.9 to 10 would keep getting handed gnu++98 the way they had been, and only change it for gcc11. pgpWrthr3a1BK.pgp Description: PGP signature
Re: [PATCH RESEND] gcc-plugins: avoid errors with -std=gnu++11 on old gcc
On Wed, 17 Mar 2021 22:52:56 -0700, Kees Cook said: > On Mon, Mar 08, 2021 at 03:40:21AM -0500, Valdis KlÄtnieks wrote: > > It turns out that older gcc (4.9 and 5.4) have gnu++11 support, but > > due to a gcc bug fixed in gcc6, throw errors during the build. > > The relevant gcc bug is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69959 > > > > Version the option based on what gcc we're using. > > Is there a better way to detect this than with version checking? Not really. gcc 11 needs --std=gnu++11 to build it. And although gcc4 and gcc5 *claim* to support it, there's a known bug, so we *can't* feed gnu++11 to them. We can check versions.. Or heave gcc-plugins over the side entirely.. Or declare that gcc6 is the minimum for building the kernel. But if we support gcc4/5 *and* gcc11 to build gcc-plugins, we have to version-check. (Unrelated - the patch has grown a merge conflict since I sent it, let me know if you want an updated one, or if it's OK as is pgpP_B8IN3TEc.pgp Description: PGP signature
Re: arm64: kernel/sys.c - silence initialization warnings.
On Mon, 15 Mar 2021 19:23:00 -, Christoph Hellwig said:
> On Mon, Mar 15, 2021 at 11:14:34AM +, Catalin Marinas wrote:
> > We do similar initialisation in arch/arm64/kernel/sys32.c and
> > arch/arm64/kernel/traps.c for example. It's a pretty common pattern
> > throughout the kernel.
> >
> > So we either treat W=1 output as diff against the vanilla kernel when
> > checking new patches or we remove override-init altogether from W=1.
> > Mark Rutland pointed me to an older thread:
>
> Please just remove the override-init warning, it is not helpful at all.
The tl;dr: Christoph is *probably* correct that it's not flagging any actual
bugs. And since *my* interest is "get the kernel tree to a point where W=1
or sparse throwing a warning is something worth looking at", I'm not opposed
to a patch to remove it from W=1 tree-wide if it has essentially zero chance of
flagging an actual bug.
The longer version:
So I did a quick analysis...
For an x86_64 allmodconfig, there's not that many left:
16 drivers/ata/ahci.h
1 drivers/ata/pata_atiixp.c
1 drivers/ata/pata_cs5520.c
1 drivers/ata/pata_cs5530.c
1 drivers/ata/pata_sc1200.c
1 drivers/ata/pata_serverworks.c
1 drivers/ata/sata_mv.c
4 drivers/ata/sata_nv.c
1 drivers/ata/sata_sil24.c
1 drivers/block/drbd/drbd_main.c
6 drivers/gpu/drm/amd/amdgpu/../include/asic_reg/dce/dce_6_0_d.h
2 drivers/gpu/drm/amd/amdgpu/../include/asic_reg/dce/dce_6_0_sh_mask.h
1 drivers/input/serio/i8042-x86ia64io.h
1 include/linux/blkdev.h
1 kernel/bpf/btf.c
4 kernel/time/hrtimer.c
1 lib/errname.c
The drivers/ata *.c warnings all appear to be the same type of thing:
static struct scsi_host_template serverworks_osb4_sht = {
ATA_BMDMA_SHT(DRV_NAME),
.sg_tablesize = LIBATA_DUMB_MAX_PRD,
};
The preprocessor macro defining the struct contents, and then
overriding one predefined value. So that's half of x64_64 done right there.
There's a few corners still need looking at, like why drivers/ata/ahci.h
throws 16 warnings on x64, but 30 on arm and 28 on arm64, and why
there's 4 warnings on include/linux/stddef.h on arm64 but not arm or x86.
But the number is certainly small enough that it's only a day or two's work
at most to check every single one. If I go through the rest of x86 and arm
and they're all legit, I'll send a patch to nuke it kernel-wide rather than
piecemeal.
arm64: kernel/sys.c - silence initialization warnings.
Building arch/arm64/kernel/sys.o with W=1 throws over 300 warnings: /usr/src/linux-next/arch/arm64/kernel/sys.c:56:40: warning: initialized field overwritten [-Woverride-init] 56 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym, |^~~~ /usr/src/linux-next/include/uapi/asm-generic/unistd.h:29:37: note: in expansion of macro '__SYSCALL' 29 | #define __SC_COMP(_nr, _sys, _comp) __SYSCALL(_nr, _sys) | ^ /usr/src/linux-next/include/uapi/asm-generic/unistd.h:34:1: note: in expansion of macro '__SC_COMP' 34 | __SC_COMP(__NR_io_setup, sys_io_setup, compat_sys_io_setup) | ^ We know that's pretty much the file's purpose in life, so tell the build system to not remind us. This makes the 1 other warning a lot more noticeable. Signed-off-by: Valdis Kletnieks diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index ed65576ce710..916b21d2b35b 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -8,6 +8,7 @@ CFLAGS_armv8_deprecated.o := -I$(src) CFLAGS_REMOVE_ftrace.o = $(CC_FLAGS_FTRACE) CFLAGS_REMOVE_insn.o = $(CC_FLAGS_FTRACE) CFLAGS_REMOVE_return_address.o = $(CC_FLAGS_FTRACE) +CFLAGS_sys.o += $(call cc-disable-warning, override-init) # Object file lists. obj-y := debug-monitors.o entry.o irq.o fpsimd.o \
[PATCH] drm/msm: Fix sparse warnings in adreno-smmu-priv.h
sparse throws 27 complaints of the form:
CHECK /usr/src/linux-next/drivers/gpu/drm/msm/msm_perf.c
/usr/src/linux-next/drivers/gpu/drm/msm/msm_perf.c: note: in included file
(through /usr/src/linux-next/drivers/gpu/drm/msm/msm_gpu.h):
/usr/src/linux-next/include/linux/adreno-smmu-priv.h:36:33: warning: no newline
at end of file
Give it the missing newline...
Signed-off-by: Valdis Kletnieks
diff --git a/include/linux/adreno-smmu-priv.h b/include/linux/adreno-smmu-priv.h
index a889f28afb42..977e7c3a21e6 100644
--- a/include/linux/adreno-smmu-priv.h
+++ b/include/linux/adreno-smmu-priv.h
@@ -33,4 +33,4 @@ struct adreno_smmu_priv {
int (*set_ttbr0_cfg)(const void *cookie, const struct io_pgtable_cfg *cfg);
};
-#endif /* __ADRENO_SMMU_PRIV_H */
\ No newline at end of file
+#endif /* __ADRENO_SMMU_PRIV_H */
Re: 'make O=' indigestion with module signing
On Fri, 12 Mar 2021 09:01:36 +, David Howells said: > Possibly I can add something like: > > clean-files := signing_key.pem x509.genkey > > inside the > > ifeq ($(CONFIG_MODULE_SIG_KEY),"certs/signing_key.pem") > ... > endif Would that remove them on a 'make clean', or only a 'make mrproper'? The latter sounds like the correct solution to me, as the signing key should have (roughly) the same lifetime rules as the .config file. pgpd6ZUuDEADr.pgp Description: PGP signature
Re: 'make O=' indigestion with module signing
On Thu, 11 Mar 2021 12:04:19 +, David Howells said: > EXTRACT_CERTS /usr/src/linux-next/"certs/signing_key.pem" > > but I don't know why. There are some odd quotes in your line also which may > be related to the problem. The relevant config line looks the same: > > CONFIG_MODULE_SIG_KEY="certs/signing_key.pem" Aha. I figured it out. If you have a *totally* clean source tree, 'make -O' works for all users. If you have in the past done a build in the tree, and then done a 'make mrproper' to clean it out so 'make -O' doesn't complain, it fails because it finds an *old* certs/signing_key.pem in /usr/src/linux-next and tries to put the new generated files in the same directory. So the root cause was: 'make mrproper doesn't clean certs/' out enough, and this chunk of certs/Makefile # If CONFIG_MODULE_SIG_KEY isn't a PKCS#11 URI, depend on it ifeq ($(patsubst pkcs11:%,%,$(firstword $(MODULE_SIG_KEY_FILENAME))),$(firstword $(MODULE_SIG_KEY_FILENAME))) X509_DEP := $(MODULE_SIG_KEY_SRCPREFIX)$(MODULE_SIG_KEY_FILENAME) endif MODULE_SIG_KEY_SRCPREFIX was where my /usr/src/linux-next was coming from... I admit not being sure how (or if) this should be fixed pgpIg5SThq6db.pgp Description: PGP signature
Re: 'make O=' indigestion with module signing
On Thu, 11 Mar 2021 10:49:14 +, David Howells said: > I wonder... Can you grab branch keys-cve-2020-26541-branch from: > > https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/ > > and try that? If that breaks, can you try dropping the top four commits? [/usr/src/linux-next] git checkout keys-cve-2020-26541-branch Checking out files: 100% (13062/13062), done. Previous HEAD position was b01d57bfdc41 Add linux-next specific files for 20210310 Branch 'keys-cve-2020-26541-branch' set up to track remote branch 'keys-cve-2020-26541-branch' from 'linux-fs'. Switched to a new branch 'keys-cve-2020-26541-branch' That still didn't work, and dropping off the 4 commits from Eric Snowberg didn't change things. I checked out next-20210310, did a 'make mrproper', and tested as the owner of the source tree rather than as a different user... LANG=C make O=/tmp/test-as-owner V=1 ARCH=arm64 ASFLAGS='-mcpu=all' CROSS_COMPILE=/opt/aarch64/bin/aarch64-linux-gnu- certs/ make -f /usr/src/linux-next/scripts/Makefile.build obj=certs \ single-build= \ need-builtin=1 need-modorder=1 scripts/extract-cert /usr/src/linux-next/"certs/signing_key.pem" certs/signing_key.x509 Extracted cert: /CN=Build time autogenerated kernel key /opt/aarch64/bin/aarch64-linux-gnu-gcc (...) -o certs/system_keyring.o /usr/src/linux-next/certs/system_keyring.c And the files ended up where they belonged: ls -l /tmp/test-as-owner/certs/ total 72 -rw-r--r-- 1 source source 1288 Mar 11 06:33 blacklist_nohashes.o -rw-r--r-- 1 source source 18496 Mar 11 06:33 blacklist.o -rw-r--r-- 1 source source 542 Mar 11 06:33 built-in.a -rw-r--r-- 1 source source 5856 Mar 11 06:33 common.o -rw-r--r-- 1 source source 0 Mar 11 06:33 modules.order -rw-r--r-- 1 source source 1184 Mar 11 06:33 revocation_certificates.o -rw-r--r-- 1 source source 1357 Mar 11 06:33 signing_key.x509 -rw-r--r-- 1 source source 6888 Mar 11 06:33 system_certificates.o -rw-r--r-- 1 source source 17504 Mar 11 06:33 system_keyring.o -rw-r--r-- 1 source source 0 Mar 11 06:33 x509_certificate_list -rw-r--r-- 1 source source 0 Mar 11 06:33 x509_revocation_list So there's something weird going on with scripts/extract-cert when running as a userid other than the owner of the source tree.. I wonder if it's actually an OpenSSL issue... I'll look at it some more later today... pgpJUBduaedOr.pgp Description: PGP signature
Re: 'make O=' indigestion with module signing
On Thu, 11 Mar 2021 09:34:01 +, David Howells said: > Valdis KlÄtnieks wrote: > > > What i *expected* was that multiple builds with different O= would each > > generate themselves a unique signing key and put it in their own O= > > directory > > and stay out of each other's way. > > Hmmm... Works for me. I use separate build dirs all the time. > > What version of the kernel are you using and what's the build command line - > in particular the full O= option? This is linux-next as of yesterday. For testing, I've been using: LANG=C make O=/tmp/arm64 V=1 ARCH=arm64 ASFLAGS='-mcpu=all' CROSS_COMPILE=/opt/aarch64/bin/aarch64-linux-gnu- certs/ and it insists on trying to make the certs in /usr/src/linux-next rather than /tmp/arm64: make -f /usr/src/linux-next/scripts/Makefile.build obj=certs \ single-build= \ need-builtin=1 need-modorder=1 scripts/extract-cert /usr/src/linux-next/"certs/signing_key.pem" certs/signing_key.x509 At main.c:142: - SSL error:0200100D:system library:fopen:Permission denied: ../crypto/bio/bss_file.c:69 - SSL error:2006D002:BIO routines:BIO_new_file:system lib: ../crypto/bio/bss_file.c:78 extract-cert: /usr/src/linux-next/certs/signing_key.pem: Permission denied make[2]: *** [/usr/src/linux-next/certs/Makefile:106: certs/signing_key.x509] Error 1 make[1]: *** [/usr/src/linux-next/Makefile:1847: certs] Error 2 make[1]: Leaving directory '/tmp/arm64' make: *** [Makefile:215: __sub-make] Error 2 Is it possible that it works for you because although you have separate build dirs, it's still able to write to the source tree? pgpFJPnPb5k0G.pgp Description: PGP signature
'make O=' indigestion with module signing
So, I tried doing a 'make O=... allmodconfig', with a setup where the uid of the build process had write permission to the O= directory, but intentionally did *not* have write permission to the source tree (so they couldn't mess up the tree - I got tired of having to repeatedly do 'make mrproper' because of pilot error) allmodconfig gave me a .config that had: CONFIG_MODULE_SIG_FORMAT=y CONFIG_MODULE_SIG=y CONFIG_MODULE_SIG_FORCE=y CONFIG_MODULE_SIG_ALL=y CONFIG_MODULE_SIG_SHA1=y # CONFIG_MODULE_SIG_SHA224 is not set # CONFIG_MODULE_SIG_SHA256 is not set # CONFIG_MODULE_SIG_SHA384 is not set # CONFIG_MODULE_SIG_SHA512 is not set CONFIG_MODULE_SIG_HASH="sha1" CONFIG_IMA_APPRAISE_REQUIRE_MODULE_SIGS=y CONFIG_MODULE_SIG_KEY="certs/signing_key.pem" What i *expected* was that multiple builds with different O= would each generate themselves a unique signing key and put it in their own O= directory and stay out of each other's way. What actually happened: EXTRACT_CERTS /usr/src/linux-next/"certs/signing_key.pem" At main.c:142: - SSL error:0200100D:system library:fopen:Permission denied: ../crypto/bio/bss_file.c:69 - SSL error:2006D002:BIO routines:BIO_new_file:system lib: ../crypto/bio/bss_file.c:78 extract-cert: /usr/src/linux-next/certs/signing_key.pem: Permission denied make[2]: *** [/usr/src/linux-next/certs/Makefile:106: certs/signing_key.x509] Error 1 make[1]: *** [/usr/src/linux-next/Makefile:1847: certs] Error 2 make[1]: Leaving directory '/usr/src/linux-next/out/arm64' make: *** [Makefile:215: __sub-make] Error 2 It tried to put the key into the source tree rather than the build tree. Before I try to code up a fix for this, is this intentionally designed behavior, or have I just managed to trip over a rarely-tested corner case? pgpL1B7YjVYls.pgp Description: PGP signature
[PATCH RESEND] gcc-plugins: avoid errors with -std=gnu++11 on old gcc
It turns out that older gcc (4.9 and 5.4) have gnu++11 support, but
due to a gcc bug fixed in gcc6, throw errors during the build.
The relevant gcc bug is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69959
Version the option based on what gcc we're using.
Signed-off-by: Valdis Kletnieks
Fixes: 27c287b41659 ("gcc-plugins: fix gcc 11 indigestion with plugins...")
---
diff --git a/scripts/gcc-plugins/Makefile b/scripts/gcc-plugins/Makefile
index b5487cce69e8..cc779973724a 100644
--- a/scripts/gcc-plugins/Makefile
+++ b/scripts/gcc-plugins/Makefile
@@ -21,8 +21,11 @@ always-y += $(GCC_PLUGIN)
GCC_PLUGINS_DIR = $(shell $(CC) -print-file-name=plugin)
+# need gnu++11 for gcc 11, but 4.9 and 5.4 need gnu++98
+GCC_FLAVOR = $(call cc-ifversion, -ge, 1100, 11, 98)
+
plugin_cxxflags= -Wp,-MMD,$(depfile) $(KBUILD_HOSTCXXFLAGS) -fPIC \
- -I $(GCC_PLUGINS_DIR)/include -I $(obj) -std=gnu++11 \
+ -I $(GCC_PLUGINS_DIR)/include -I $(obj)
-std=gnu++$(GCC_FLAVOR) \
-fno-rtti -fno-exceptions -fasynchronous-unwind-tables \
-ggdb -Wno-narrowing -Wno-unused-variable \
-Wno-format-diag
Re: next-20210302 - build issue with linux-firmware and rtl_nic/ firmware.
On Wed, 03 Mar 2021 07:51:06 +0100, Heiner Kallweit said: > > # Firmware loader > > CONFIG_EXTRA_FIRMWARE="rtl_nic/rtl8106e-1.fw" > > CONFIG_EXTRA_FIRMWARE_DIR="/lib/firmware" > > > This is wrong, simply remove it. > > But then I take a closer look at drivers/net/ethernet/realtek/r8169_main.c > > #define FIRMWARE_8168D_1"rtl_nic/rtl8168d-1.fw" > > #define FIRMWARE_8168D_2"rtl_nic/rtl8168d-2.fw" > > #define FIRMWARE_8168E_1"rtl_nic/rtl8168e-1.fw" Yes, but then how are *these* filenames supposed to work? Is a userspace helper supposed to be smart enough to append the .xz, and the EXTRA_FIRMWARE variant for embedding out-of-tree firmware has to point at an uncompressed version?
next-20210302 - build issue with linux-firmware and rtl_nic/ firmware.
So my kernel build died.. UPD drivers/base/firmware_loader/builtin/rtl_nic/rtl8106e-1.fw.gen.S make[4]: *** No rule to make target '/lib/firmware/rtl_nic/rtl8106e-1.fw', needed by 'drivers/base/firmware_loader/builtin/rtl_nic/rtl8106e-1.fw.gen.o'. Stop. make[3]: *** [scripts/Makefile.build:514: drivers/base/firmware_loader/builtin] Error 2 I tracked it down to a linux-firmware update that shipped everything with .xz compression: % rpm2cpio linux-firmware-20201218-116.fc34.noarch.rpm | cpio -itv | grep 8106e-1 -rw-r--r-- 1 root root 1856 Dec 19 04:43 ./usr/lib/firmware/rtl_nic/rtl8106e-1.fw 631034 blocks % rpm2cpio linux-firmware-20210208-117.fc34.noarch.rpm | cpio -itv| grep 8106e-1 -rw-r--r-- 1 root root 848 Feb 8 16:38 ./usr/lib/firmware/rtl_nic/rtl8106e-1.fw.xz 340217 blocks and my .config shows it's self-inflicted (no, I don't remember why it's in there): # Firmware loader CONFIG_EXTRA_FIRMWARE="rtl_nic/rtl8106e-1.fw" CONFIG_EXTRA_FIRMWARE_DIR="/lib/firmware" But then I take a closer look at drivers/net/ethernet/realtek/r8169_main.c #define FIRMWARE_8168D_1"rtl_nic/rtl8168d-1.fw" #define FIRMWARE_8168D_2"rtl_nic/rtl8168d-2.fw" #define FIRMWARE_8168E_1"rtl_nic/rtl8168e-1.fw" So now I'm mystified how this compressing all the firmware files is supposed to work...
Re: [regression -next0117] What is kcompactd and why is he eating 100% of my cpu?
On Tue, 16 Feb 2021 13:36:22 +0100, "Jason A. Donenfeld" said: > Another anecdote: 5.11.0, 64 gigs of ram. If I run QEMU/KVM for a VM > with 16 gigs at the same time as a VMware VM with 16 gigs of ram, > kcompact goes wild and both VMs get really slow. The key here is running > KVM at the same time as VMware. Do things operated as expected if there are 2 KVM instances, or 2 VMware instances? pgp8TQVYliT2k.pgp Description: PGP signature
Re: /usr/lib/gcc/x86_64-linux-gnu/5/plugin/include/config/elfos.h:102:21: warning: invalid suffix on literal; C++11 requires a space between literal and string macro
On Sun, 14 Feb 2021 04:00:31 +0800, kernel test robot said: > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > master > head: dcc0b49040c70ad827a7f3d58a21b01fdb14e749 > commit: 67a5a68013056cbcf0a647e36cb6f4622fb6a470 gcc-plugins: fix gcc 11 > indigestion with plugins... > date: 5 weeks ago > config: x86_64-randconfig-a001-20200622 (attached as .config) > compiler: gcc-5 (Ubuntu 5.5.0-12ubuntu1) 5.5.0 20171010 > reproduce (this is a W=1 build): > # > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=67a5a68013056cbcf0a647e36cb6f4622fb6a470 > git remote add linus > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > git fetch --no-tags linus master > git checkout 67a5a68013056cbcf0a647e36cb6f4622fb6a470 > # save the attached .config to linux build tree > make W=1 ARCH=x86_64 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot > > All warnings (new ones prefixed by >>): > >In file included from > /usr/lib/gcc/x86_64-linux-gnu/5/plugin/include/tm.h:27, > from > /usr/lib/gcc/x86_64-linux-gnu/5/plugin/include/gcc-pugin.h:31, > from > /usr/lib/gcc/x86_64-linux-gnu/5/plugin/include/plugin.h:23, > from scripts/gcc-plugins/gcc-common.h:9, > from scripts/gcc-plugins/latent_entropy_plugin.c:78: > >> /usr/lib/gcc/x86_64-linux-gnu/5/plugin/include/config/elfos.h:102:21: > >> warning: invalid suffix on literal; C++11 requires a space between literal > >> and string macro [-Wliteral-suffix] >fprintf ((FILE), "%s"HOST_WIDE_INT_PRINT_UNSIGNED"\n",\ > ^ Kees: I already sent a patch for this on Jan 13, but apparently it didn't get pushed out.. pgpq5WHgo8LCc.pgp Description: PGP signature
Re: [regression -next0117] What is kcompactd and why is he eating 100% of my cpu?
On Mon, 25 Jan 2021 19:54:38 +0100, Tibor Bana said: > I don't know if it still actual, but I am strugling with this problem right > now and searching the internet for solutions. I read the thread and saw that > you are strugling to reproduce the problem, and I can reproduce it almost > every > day. I'm pretty sure that you have a real bug on your hands. Even if your box is very low on memory, kcompactd should eventually figure out it's not making any progress and wait for the situation to change before trying again. However, I'm also pretty sure that it's a different one than the one we were chasing, because that one never showed up again once all the patches landed in linux-next, some 18 months before 5.9 was released.
[PATCH] scsi: target: iscsi: Fix typo in comment
Correct the spelling of Nagle's name in a comment. Signed-off-by: Valdis Kletnieks diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c index 893d1b406c29..1a9c50401bdb 100644 --- a/drivers/target/iscsi/iscsi_target_login.c +++ b/drivers/target/iscsi/iscsi_target_login.c @@ -896,7 +896,7 @@ int iscsit_setup_np( else len = sizeof(struct sockaddr_in); /* -* Set SO_REUSEADDR, and disable Nagel Algorithm with TCP_NODELAY. +* Set SO_REUSEADDR, and disable Nagle Algorithm with TCP_NODELAY. */ if (np->np_network_transport == ISCSI_TCP) tcp_sock_set_nodelay(sock->sk);
[PATCH] gcc-plugins: avoid errors with -std=gnu++11 on old gcc
It turns out that older gcc (4.9 and 5.4) have gnu++11 support, but
due to a gcc bug fixed in gcc6, throw errors during the build.
The relevant gcc bug is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69959
Version the option based on what gcc we're using.
Signed-off-by: Valdis Kletnieks
Fixes: 27c287b41659 ("gcc-plugins: fix gcc 11 indigestion with plugins...")
---
diff --git a/scripts/gcc-plugins/Makefile b/scripts/gcc-plugins/Makefile
index b5487cce69e8..cc779973724a 100644
--- a/scripts/gcc-plugins/Makefile
+++ b/scripts/gcc-plugins/Makefile
@@ -21,8 +21,11 @@ always-y += $(GCC_PLUGIN)
GCC_PLUGINS_DIR = $(shell $(CC) -print-file-name=plugin)
+# need gnu++11 for gcc 11, but 4.9 and 5.4 need gnu++98
+GCC_FLAVOR = $(call cc-ifversion, -ge, 1100, 11, 98)
+
plugin_cxxflags= -Wp,-MMD,$(depfile) $(KBUILD_HOSTCXXFLAGS) -fPIC \
- -I $(GCC_PLUGINS_DIR)/include -I $(obj) -std=gnu++11 \
+ -I $(GCC_PLUGINS_DIR)/include -I $(obj)
-std=gnu++$(GCC_FLAVOR) \
-fno-rtti -fno-exceptions -fasynchronous-unwind-tables \
-ggdb -Wno-narrowing -Wno-unused-variable \
-Wno-format-diag
Re: [PATCH v2] gcc-plugins: fix gcc 11 indigestion with plugins...
On Wed, 13 Jan 2021 11:08:36 -0600, Josh Poimboeuf said: > The first patch has already been merged into Linus' tree, so this > probably should be an incremental fix on top, with a Fixes: tag. Gaah. v3 in a moment. :) pgpsvMPHiYHXD.pgp Description: PGP signature
[PATCH v2] gcc-plugins: fix gcc 11 indigestion with plugins...
Fedora Rawhide has started including gcc 11,and the g++ compiler throws a wobbly when it hits scripts/gcc-plugins: HOSTCXX scripts/gcc-plugins/latent_entropy_plugin.so In file included from /usr/include/c++/11/type_traits:35, from /usr/lib/gcc/x86_64-redhat-linux/11/plugin/include/system.h:244, from /usr/lib/gcc/x86_64-redhat-linux/11/plugin/include/gcc-plugin.h:28, from scripts/gcc-plugins/gcc-common.h:7, from scripts/gcc-plugins/latent_entropy_plugin.c:78: /usr/include/c++/11/bits/c++0x_warning.h:32:2: error: #error This file requires compiler and library support for the ISO C++ 2011 standard. This support must be enabled with the -std=c++11 or -std=gnu++11 compiler options. 32 | #error This file requires compiler and library support \ Patch is more complicated than would otherwise be needed, because older gcc (4.9, 5.4) have gnu++11 but throw an error due to a gcc bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69959 Signed-off-by: Valdis Kletnieks --- diff --git a/scripts/gcc-plugins/Makefile b/scripts/gcc-plugins/Makefile index d66949bfeba4..cc779973724a 100644 --- a/scripts/gcc-plugins/Makefile +++ b/scripts/gcc-plugins/Makefile @@ -21,10 +21,13 @@ always-y += $(GCC_PLUGIN) GCC_PLUGINS_DIR = $(shell $(CC) -print-file-name=plugin) +# need gnu++11 for gcc 11, but 4.9 and 5.4 need gnu++98 +GCC_FLAVOR = $(call cc-ifversion, -ge, 1100, 11, 98) + plugin_cxxflags= -Wp,-MMD,$(depfile) $(KBUILD_HOSTCXXFLAGS) -fPIC \ - -I $(GCC_PLUGINS_DIR)/include -I $(obj) -std=gnu++98 \ + -I $(GCC_PLUGINS_DIR)/include -I $(obj) -std=gnu++$(GCC_FLAVOR) \ -fno-rtti -fno-exceptions -fasynchronous-unwind-tables \ - -ggdb -Wno-narrowing -Wno-unused-variable -Wno-c++11-compat \ + -ggdb -Wno-narrowing -Wno-unused-variable \ -Wno-format-diag plugin_ldflags = -shared
Re: [PATCH] gcc-plugins: fix gcc 11 indigestion with plugins...
On Mon, 11 Jan 2021 11:48:32 -0800, Kees Cook said: > On Mon, Jan 11, 2021 at 07:37:19AM -0600, Josh Poimboeuf wrote: > > I think putting the flag in a variable (based on call cc-ifversion) > > should be easy enough, then we can put this little saga behind us and > > pretend it never happened :-) > > Yeah, that seems best. Valdis, can you send a patch for this? Yep, I'll send one shortly... pgpWGMS1_pGgc.pgp Description: PGP signature
Re: [PATCH] gcc-plugins: fix gcc 11 indigestion with plugins...
On Mon, 11 Jan 2021 05:56:59 -0500, I said: > > It's probably related. I'm just having a hard time understanding why 4.9 > > and 5.4 > > whine about the lack of a space, while 8.3 and 11 didn't complain... So after more digging, at least some clarity has surfaced. It looks like it's not a kernel source tree issue, it's a g++ issue fixed in g++ 6 and later. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69959 And it looks like there was an intent to backport it to 4.9 and 5.4: https://gcc.gnu.org/legacy-ml/gcc-patches/2016-02/msg01409.html The bugtracker doesn't show an equivalent for 69959 being closed against 4.9.x or 5.[56], https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63254 has a patch for one of the gcc-supplied files that tosses the warning, but that way lies madness... Not sure what we want to do here - the main alternatives I see are: Tell people still using 4.9/5.4 to either live with the warning or upgrade to 6 or later Make the flag a variable and pass either -std=gnu++98 or -std=gnu++11 depending on the output of 'g++ --version' What say the peanut gallery? pgphgTxwfyL4y.pgp Description: PGP signature
Re: [PATCH] gcc-plugins: fix gcc 11 indigestion with plugins...
On Mon, 11 Jan 2021 10:47:23 +0100, Geert Uytterhoeven said: > I guess this is the cause of the new "warning: invalid suffix on > literal; C++11 requires a space between literal and string macro > [-Wliteral-suffix]" with gcc 4.9 or 5.4? Well, we fixed a #error, and picked up a warning. That's progress. ;) It's probably related. I'm just having a hard time understanding why 4.9 and 5.4 whine about the lack of a space, while 8.3 and 11 didn't complain... I'll see if I can cook up a patch that newer gcc are still happy with. You able to easily test with 4.9 or 5.4? pgpOo7zcYqRYQ.pgp Description: PGP signature
Re: Kconfig, DEFAULT_NETSCH, and shooting yourself in the foot..
On Sun, 03 Jan 2021 10:20:11 -0800, Stephen Hemminger said: > You can use a qdisc that is a module, it just has to be available when device > is loaded. Typically that means putting it in initramfs. Apparently, that's not *quite* true regarding the default qdisc, because I hit this situation (copying from another email): --- [/proc/sys/net] cat core/default_qdisc fq_codel [/proc/sys/net] tc -s qdisc show qdisc noqueue 0: dev lo root refcnt 2 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) backlog 0b 0p requeues 0 qdisc pfifo_fast 0: dev eth0 root refcnt 2 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) backlog 0b 0p requeues 0 qdisc noqueue 0: dev wlan0 root refcnt 2 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0) backlog 0b 0p requeues 0 But if I give it a kick in the head... [/proc/sys/net] tc qdisc add dev wlan0 root fq_codel [/proc/sys/net] tc -s qdisc show dev wlan0 qdisc fq_codel 8001: root refcnt 2 limit 10240p flows 1024 quantum 1514 target 5ms interval 100ms memory_limit 32Mb ecn drop_batch 64 Sent 812 bytes 12 pkt (dropped 0, overlimits 0 requeues 0) backlog 0b 0p requeues 0 maxpacket 0 drop_overlimit 0 new_flow_count 0 ecn_mark 0 new_flows_len 0 old_flows_len 0 --- Note that wlan0 doesn't actually get plumbed up until after the initramfs has done its thing and we're quite late in the boot process and the /lib/modules on the production system is accessible, so it doesn't look like an implicit modprobe gets done in that case - fq_codel.ko was very much available when the device was brought up. pgpMx3RBL7IYi.pgp Description: PGP signature
Kconfig, DEFAULT_NETSCH, and shooting yourself in the foot..
Consider the following own goal I just discovered I scored: [~] zgrep -i fq_codel /proc/config.gz CONFIG_NET_SCH_FQ_CODEL=m CONFIG_DEFAULT_FQ_CODEL=y CONFIG_DEFAULT_NET_SCH="fq_codel" Obviously, fq_codel didn't get set as the default, because that happens before the module gets loaded (which may never happen if the sysadmin thinks the DEFAULT_NET_SCH already made it happen) Whoops. My bad, probably - but The deeper question, part 1: There's this chunk in net/sched/Kconfig: config DEFAULT_NET_SCH string default "pfifo_fast" if DEFAULT_PFIFO_FAST default "fq" if DEFAULT_FQ default "fq_codel" if DEFAULT_FQ_CODEL default "fq_pie" if DEFAULT_FQ_PIE default "sfq" if DEFAULT_SFQ default "pfifo_fast" endif (And a similar chunk right above it with a similar issue) Should those be "if (foo=y)" so =m can't be chosen? (I'll be happy to write the patch if that's what we want) Deeper question, part 2: Should there be a way in the Kconfig language to ensure that these two chunks can't accidentally get out of sync? There's other places in the kernel where similar issues arise - a few days ago I was chasing a CPU governor issue where it looked like it was possible to set a default that was a module and thus possibly not actually loaded. pgpcPFS8oQxKT.pgp Description: PGP signature
[PATCH] gcc-plugins: fix gcc 11 indigestion with plugins...
Fedora Rawhide has started including gcc 11,and the g++ compiler throws a wobbly when it hits scripts/gcc-plugins: HOSTCXX scripts/gcc-plugins/latent_entropy_plugin.so In file included from /usr/include/c++/11/type_traits:35, from /usr/lib/gcc/x86_64-redhat-linux/11/plugin/include/system.h:244, from /usr/lib/gcc/x86_64-redhat-linux/11/plugin/include/gcc-plugin.h:28, from scripts/gcc-plugins/gcc-common.h:7, from scripts/gcc-plugins/latent_entropy_plugin.c:78: /usr/include/c++/11/bits/c++0x_warning.h:32:2: error: #error This file requires compiler and library support for the ISO C++ 2011 standard. This support must be enabled with the -std=c++11 or -std=gnu++11 compiler options. 32 | #error This file requires compiler and library support \ In fact, it works just fine with c++11, which has been in gcc since 4.8, and we now require 4.9 as a minimum. Signed-off-by: Valdis Kletnieks diff --git a/scripts/gcc-plugins/Makefile b/scripts/gcc-plugins/Makefile index d66949bfeba4..b5487cce69e8 100644 --- a/scripts/gcc-plugins/Makefile +++ b/scripts/gcc-plugins/Makefile @@ -22,9 +22,9 @@ always-y += $(GCC_PLUGIN) GCC_PLUGINS_DIR = $(shell $(CC) -print-file-name=plugin) plugin_cxxflags= -Wp,-MMD,$(depfile) $(KBUILD_HOSTCXXFLAGS) -fPIC \ - -I $(GCC_PLUGINS_DIR)/include -I $(obj) -std=gnu++98 \ + -I $(GCC_PLUGINS_DIR)/include -I $(obj) -std=gnu++11 \ -fno-rtti -fno-exceptions -fasynchronous-unwind-tables \ - -ggdb -Wno-narrowing -Wno-unused-variable -Wno-c++11-compat \ + -ggdb -Wno-narrowing -Wno-unused-variable \ -Wno-format-diag plugin_ldflags = -shared pgp893phGjNU0.pgp Description: PGP signature
[PATCH] kasan, mm: fix build issue with asmlinkage
commit 2df573d2ca4c1ce6ea33cb7849222f771e759211
Author: Andrey Konovalov
Date: Tue Nov 24 16:45:08 2020 +1100
kasan: shadow declarations only for software modes
introduces a build failure when it removed an include for linux/pgtable.h
It actually only needs linux/linkage.h
Test builds on both x86_64 and arm build cleanly
Fixes: 2df573d2ca4c ("kasan: shadow declarations only for software modes")
Signed-off-by: Valdis Kletnieks
---
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index 83860aa4e89c..5e0655fb2a6f 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -12,6 +12,7 @@ struct task_struct;
#ifdef CONFIG_KASAN
+#include
#include
/* kasan_data struct is used in KUnit tests for KASAN expected failures */
Re: linux-next 20201126 - build error on arm allmodconfig
On Thu, 26 Nov 2020 14:14:29 +, Russell King - ARM Linux admin said: > The real answer is for asm/kasan.h to include linux/linkage.h Looking deeper, there's 7 different arch/../asm/kasan.h - are we better off patching all 7, or having include/linux/kasan.h include it just before the include of asm/kasan.h? pgpbgKVIU3VHi.pgp Description: PGP signature
Re: linux-next 20201126 - build error on arm allmodconfig
On Thu, 26 Nov 2020 14:14:29 +, Russell King - ARM Linux admin said: > The real answer is for asm/kasan.h to include linux/linkage.h OK... I'll cook up the patch. pgpDijjojCGIl.pgp Description: PGP signature
linux-next 20201126 - build error on arm allmodconfig
Seems something is giving it indigestion regarding asmlinkage... CC arch/arm/mm/kasan_init.o In file included from ./include/linux/kasan.h:15, from arch/arm/mm/kasan_init.c:11: ./arch/arm/include/asm/kasan.h:26:11: error: expected ';' before 'void' asmlinkage void kasan_early_init(void); ^ ; make[2]: *** [scripts/Makefile.build:283: arch/arm/mm/kasan_init.o] Error 1 make[1]: *** [scripts/Makefile.build:500: arch/arm/mm] Error 2 make: *** [Makefile:1803: arch/arm] Error 2 Git bisect points at: commit 2df573d2ca4c1ce6ea33cb7849222f771e759211 Author: Andrey Konovalov Date: Tue Nov 24 16:45:08 2020 +1100 kasan: shadow declarations only for software modes Looks like it's this chunk: diff --git a/include/linux/kasan.h b/include/linux/kasan.h index 59538e795df4..26f2ab92e7ca 100644 --- a/include/linux/kasan.h +++ b/include/linux/kasan.h @@ -11,7 +11,6 @@ struct task_struct; #ifdef CONFIG_KASAN -#include #include Testing shows putting that #include back in makes it compile correctly, but it's not obvious why putting that back makes 'asmlinkage' recognized. "You are in a twisty little maze of #includes, all different"... :) pgptwKBXv0U6f.pgp Description: PGP signature
next-20201105 - build issue with KASAN on ARM
commit d6d51a96c7d63b7450860a3037f2d62388286a52 Author: Linus Walleij Date: Sun Oct 25 23:52:08 2020 +0100 ARM: 9014/2: Replace string mem* functions for KASan I'm trying to figure out why this has 3 Tested-By: tags but blows up for fairly obvious reasons on ARM. CC arch/arm/boot/compressed/string.o arch/arm/boot/compressed/string.c:24:1: error: attribute 'alias' argument not a string void *__memcpy(void *__dest, __const void *__src, size_t __n) __alias(memcpy); ^~~~ arch/arm/boot/compressed/string.c:25:1: error: attribute 'alias' argument not a string void *__memmove(void *__dest, __const void *__src, size_t count) __alias(memmove); ^~~~ arch/arm/boot/compressed/string.c:26:1: error: attribute 'alias' argument not a string void *__memset(void *s, int c, size_t count) __alias(memset); ^~~~ make[2]: *** [scripts/Makefile.build:283: arch/arm/boot/compressed/string.o] Error 1 make[1]: *** [arch/arm/boot/Makefile:64: arch/arm/boot/compressed/vmlinux] Error 2 pgpkjzaMmZeLW.pgp Description: PGP signature
Re: [PATCH] clk: imx: scu: Fix compile error with module build of clk-scu.o
On Tue, 03 Nov 2020 07:52:19 +0800, Shawn Guo said: > It's a driver problem which is being addressed by Dong's patch[1]. > > Shawn > > [1] > https://patchwork.kernel.org/project/linux-arm-kernel/patch/20201030153733.30160-1-aisheng.d...@nxp.com/ OK, We'll fix it that way then.
Re: [PATCH] clk: imx: scu: Fix compile error with module build of clk-scu.o
On Mon, 02 Nov 2020 09:15:20 -0800, Randy Dunlap said: > also > Reported-by: kernel test robot > > However, this driver does not directly use . Just my luck - I looked at 3 or 4 other things that include of_platform.h and they all *did* include module.h. > platform_device.h #includes , which is where the > problem lies: > > uses macros that are provided by > so should #include . > > and that fixes this commit: > > commit 4c002c978b7f2f2306d53de051c054504af920a9 > Author: Greg Kroah-Hartman > Date: Mon Dec 9 20:33:03 2019 +0100 > > device.h: move 'struct driver' stuff out to device/driver.h OK.. who's going to do that? Me, or Randy, or Greg? pgp_sHGGC03QP.pgp Description: PGP signature
[PATCH] clk: imx: scu: Fix compile error with module build of clk-scu.o
commit 77d8f3068c63ee0983f0b5ba3207d3f7cce11be4 (HEAD) Author: Dong Aisheng Date: Wed Jul 29 16:00:10 2020 +0800 clk: imx: scu: add two cells binding support This missed a #include, which results in some nasty errors when built as a module CC [M] drivers/clk/imx/clk-scu.o In file included from ./include/linux/device.h:32, from ./include/linux/of_platform.h:9, from drivers/clk/imx/clk-scu.c:11: ./include/linux/device/driver.h:290:1: warning: data definition has no type or storage class device_initcall(__driver##_init); ^~~ ./include/linux/platform_device.h:258:2: note: in expansion of macro 'builtin_driver' builtin_driver(__platform_driver, platform_driver_register) ^~ drivers/clk/imx/clk-scu.c:545:1: note: in expansion of macro 'builtin_platform_driver' builtin_platform_driver(imx_clk_scu_driver); ^~~ ./include/linux/device/driver.h:290:1: error: type defaults to 'int' in declaration of 'device_initcall' [-Werror=implicit-int] device_initcall(__driver##_init); ^~~ ./include/linux/platform_device.h:258:2: note: in expansion of macro 'builtin_driver' builtin_driver(__platform_driver, platform_driver_register) ^~ drivers/clk/imx/clk-scu.c:545:1: note: in expansion of macro 'builtin_platform_driver' builtin_platform_driver(imx_clk_scu_driver); ^~~ drivers/clk/imx/clk-scu.c:545:1: warning: parameter names (without types) in function declaration In file included from ./include/linux/device.h:32, from ./include/linux/of_platform.h:9, from drivers/clk/imx/clk-scu.c:11: drivers/clk/imx/clk-scu.c:545:25: warning: 'imx_clk_scu_driver_init' defined but not used [-Wunused-function] builtin_platform_driver(imx_clk_scu_driver); ^~ ./include/linux/device/driver.h:286:19: note: in definition of macro 'builtin_driver' static int __init __driver##_init(void) \ ^~~~ drivers/clk/imx/clk-scu.c:545:1: note: in expansion of macro 'builtin_platform_driver' builtin_platform_driver(imx_clk_scu_driver); ^~~ cc1: some warnings being treated as errors make[3]: *** [scripts/Makefile.build:283: drivers/clk/imx/clk-scu.o] Error 1 Fix by providing the include. Signed-off-by: Valdis Kletnieks diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c index 229a290ca5b6..15d382f6f9f8 100644 --- a/drivers/clk/imx/clk-scu.c +++ b/drivers/clk/imx/clk-scu.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include pgpAVIKVK9Mph.pgp Description: PGP signature
Re: [PATCH 04/22] KVM: mmu: extract spte.h and spte.c
On Fri, 23 Oct 2020 12:30:06 -0400, Paolo Bonzini said: > The SPTE format will be common to both the shadow and the TDP MMU. > > Extract code that implements the format to a separate module, as a > first step towards adding the TDP MMU and putting mmu.c on a diet. > > Signed-off-by: Paolo Bonzini This died a horrid death on my laptop when building with W=1. CC arch/x86/kvm/mmu/tdp_iter.o In file included from arch/x86/kvm/mmu/tdp_iter.c:5: arch/x86/kvm/mmu/spte.h:120:18: error: 'shadow_nonpresent_or_rsvd_mask_len' defined but not used [-Werror=unused-const-variable=] 120 | static const u64 shadow_nonpresent_or_rsvd_mask_len = 5; | ^~ arch/x86/kvm/mmu/spte.h:115:18: error: 'shadow_acc_track_saved_bits_shift' defined but not used [-Werror=unused-const-variable=] 115 | static const u64 shadow_acc_track_saved_bits_shift = PT64_SECOND_AVAIL_BITS_SHIFT; | ^ arch/x86/kvm/mmu/spte.h:113:18: error: 'shadow_acc_track_saved_bits_mask' defined but not used [-Werror=unused-const-variable=] 113 | static const u64 shadow_acc_track_saved_bits_mask = PT64_EPT_READABLE_MASK | | ^~~~ cc1: all warnings being treated as errors make[2]: *** [scripts/Makefile.build:283: arch/x86/kvm/mmu/tdp_iter.o] Error 1 make[1]: *** [scripts/Makefile.build:500: arch/x86/kvm] Error 2 make: *** [Makefile:1799: arch/x86] Error 2 Do we really need to define 3 static variables in every .c file that includes this .h file, directly or not? pgpgFWDleu1vn.pgp Description: PGP signature
Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma use that should be semicolon)
On Fri, 25 Sep 2020 10:26:27 -0700, Joe Perches said: > And the generic individual maintainer apply rate for > each specific patch is always less than 50%. > > For instance the patches that converted the comma uses > in if/do/while statements to use braces and semicolons > from a month ago: > 29 patches, 13 applied. To be fair, it's *always* been hard to get pure style patches applied, because they usually hit one of two types of code, with different results: Some of them hit code that's been stable for a long time - and those patches don't get applied because of the (admittedly small) risk that a "style" patch may actually break something - yes, that *does* happen often enough to worry a risk-adverse subtree maintainer. Some of them hit code that's actively being worked on - and those patches don't get applied because they can cause merge conflicts. This is a hard problem to fix, because it's difficult to say that either of those viewpoints is *totally* wrong. At best, you can make the case that some maintainers are a tad over-zealous on their attitude. And since its *hard* to find good maintainers, it's not possible to fix the problem by just putting somebody else in charge of a subtree. It's theoretically possible to bypass a problematic maintainer by sending the patch to the person one level up, or directly to Linus - but although that usually works if you have an urgent patch and the maintainer is on vacation or stubborn or whatever, that's got essentially zero chance of succeeding for a mere style patch. Unfortunately, although I understand the problem, I don't have a solution. It's easy to tactfully say "this code is wrong, and here is the fix". It's a lot harder to find a tactful way to say "This person is wrong and should do it this way", because code doesn't fight back when you offer constructive criticism pgpYA0r_IIhxF.pgp Description: PGP signature
Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma use that should be semicolon)
On Fri, 21 Aug 2020 18:08:08 -0700, Joe Perches said:
> (forwarding on to kernel-janitors/mentees and kernelnewbies)
>
> Just fyi for anyone that cares:
>
> A janitorial task for someone might be to use Julia's coccinelle
> script below to convert the existing instances of commas that
> separate statements into semicolons.
Note that you need to *really* check for possible changes in semantics.
It's *usually* OK to do that, but sometimes it's not...
for (i=0; i++, last++; !last) {
changing that comma to a ; will break the compile. In other cases, it can
introduce subtle bugs.
> > I do appreciate that coccinelle adds braces for multiple
> > expression comma use after an if.
> >
> > i.e.:
> > if (foo)
> > a = 1, b = 2;
> > becomes
> > if (foo) {
> > a = 1; b = 2;
> > }
Yeah. Like there, if you forget to add the {}.
pgpCS8NgDtnIU.pgp
Description: PGP signature
Re: Scheduler benchmarks
On Wed, 19 Aug 2020 12:42:54 +0200, Greg KH said: > Look up Spectre and Meltdown for many many examples of what happened and > what went wrong with chip designs and how we had to fix these things in > the kernel a few years ago. And I'm sure that nobody sane thinks we're done with security holes caused by speculative execution... :) pgpPCdkeeT9Y9.pgp Description: PGP signature
Re: [PATCH] Revert "seqlock: lockdep assert non-preemptibility on seqcount_t write"
On Wed, 19 Aug 2020 09:00:22 +0200, Sebastian Andrzej Siewior said: > On 2020-08-18 17:56:49 [-0700], Guenter Roeck wrote: > > Nice catch. FWIW, there is no obvious reason why this would need to be > > atomic. > > The calling code does not set a lock, meaning there can be two (or more) > > callers entering this code. Weird, especially since the code looks like it > > would actually need a mutex to work correctly. It might be interesting to > > see what happens if there are, say, half a dozen scripts/processes trying > > to read the hwmon attribute introduced by this patch at the same time. > > => https://lkml.kernel.org/r/20200818161439.3dkf6jzp3vuwm...@linutronix.de Looks reasonable to me, though I've not verified that it's preemptible at that point... pgpg3xRrZ5oCZ.pgp Description: PGP signature
Re: [PATCH] Revert "seqlock: lockdep assert non-preemptibility on seqcount_t write"
On Mon, 10 Aug 2020 07:10:32 -0700, Guenter Roeck said: > > ERROR: modpost: "__bad_udelay" > > [drivers/net/ethernet/aquantia/atlantic/atlantic.ko] undefined! > > > > I don't think that is new. If anything, it is surprising that builds don't > fail more > widely because of it. AFAICS it was introduced back in 2018 (a hot 50-ms > delay loop > really isn't such a good idea). Well...it wasn't broken in next-20200720. A bit of poking with nm, and building hw_atl/hw_atl_b0.s, it looks like the culprit is this commit: commit 8dcf2ad39fdb2d183b7bd4307c837713e3150b9a Author: Mark Starovoytov Date: Mon Jul 20 21:32:44 2020 +0300 net: atlantic: add hwmon getter for MAC temperature specifically this chunk around line 1634 of hw_atl/hw_atl_b0.c: + err = readx_poll_timeout_atomic(hw_atl_b0_ts_ready_and_latch_high_get, + self, val, val == 1, 1U, 50U); And doing a 'git revert' makes the build work
next-20200807 - arm allmodconfig dies in asm-offsets.s
commit 859247d39fb008ea812e8f0c398a58a20c12899e Author: Ahmed S. Darwish Date: Mon Jul 20 17:55:14 2020 +0200 seqlock: lockdep assert non-preemptibility on seqcount_t write breaks an arm allmodconfig build. 'git revert -n' and the build continues on. CC arch/arm/kernel/asm-offsets.s In file included from ./arch/arm/include/asm/bug.h:60, from ./include/linux/bug.h:5, from ./include/linux/thread_info.h:12, from ./include/asm-generic/current.h:5, from ./arch/arm/include/generated/asm/current.h:1, from ./include/linux/sched.h:12, from arch/arm/kernel/asm-offsets.c:11: ./include/linux/seqlock.h: In function 'write_seqcount_begin_nested': ./include/asm-generic/percpu.h:31:40: error: implicit declaration of function 'raw_smp_processor_id' [-Werror=implicit-function-declaration] #define __my_cpu_offset per_cpu_offset(raw_smp_processor_id()) ^~~~ ./include/asm-generic/bug.h:145:27: note: in definition of macro 'WARN_ON_ONCE' int __ret_warn_once = !!(condition); \ ^ ././include/linux/compiler_types.h:307:2: note: in expansion of macro '__compiletime_assert' __compiletime_assert(condition, msg, prefix, suffix) ^~~~ ././include/linux/compiler_types.h:319:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) ^~~ ./include/asm-generic/rwonce.h:36:2: note: in expansion of macro 'compiletime_assert' compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \ ^~ ./include/asm-generic/rwonce.h:36:21: note: in expansion of macro '__native_word' compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \ ^ ./include/asm-generic/rwonce.h:49:2: note: in expansion of macro 'compiletime_assert_rwonce_type' compiletime_assert_rwonce_type(x);\ ^~ ./include/asm-generic/percpu.h:119:10: note: in expansion of macro 'READ_ONCE' __ret = READ_ONCE(*raw_cpu_ptr(&(pcp))); \ ^ ./include/linux/percpu-defs.h:231:2: note: in expansion of macro 'RELOC_HIDE' RELOC_HIDE((typeof(*(__p)) __kernel __force *)(__p), (__offset)) ^~ ./include/asm-generic/percpu.h:44:31: note: in expansion of macro 'SHIFT_PERCPU_PTR' #define arch_raw_cpu_ptr(ptr) SHIFT_PERCPU_PTR(ptr, __my_cpu_offset) ^~~~ ./include/asm-generic/percpu.h:31:25: note: in expansion of macro 'per_cpu_offset' #define __my_cpu_offset per_cpu_offset(raw_smp_processor_id()) ^~ ./include/asm-generic/percpu.h:44:53: note: in expansion of macro '__my_cpu_offset' #define arch_raw_cpu_ptr(ptr) SHIFT_PERCPU_PTR(ptr, __my_cpu_offset) ^~~ ./include/linux/percpu-defs.h:242:2: note: in expansion of macro 'arch_raw_cpu_ptr' arch_raw_cpu_ptr(ptr); \ ^~~~ ./include/asm-generic/percpu.h:119:21: note: in expansion of macro 'raw_cpu_ptr' __ret = READ_ONCE(*raw_cpu_ptr(&(pcp))); \ ^~~ ./include/asm-generic/percpu.h:138:11: note: in expansion of macro '__this_cpu_generic_read_nopreempt' __ret = __this_cpu_generic_read_nopreempt(pcp); \ ^ ./include/asm-generic/percpu.h:320:31: note: in expansion of macro 'this_cpu_generic_read' #define this_cpu_read_1(pcp) this_cpu_generic_read(pcp) ^ ./include/linux/percpu-defs.h:321:23: note: in expansion of macro 'this_cpu_read_1' case 1: pscr_ret__ = stem##1(variable); break; \ ^~~~ ./include/linux/percpu-defs.h:507:29: note: in expansion of macro '__pcpu_size_call_return' #define this_cpu_read(pcp) __pcpu_size_call_return(this_cpu_read_, pcp) ^~~ ./include/linux/lockdep.h:565:9: note: in expansion of macro 'this_cpu_read' this_cpu_read(hardirqs_enabled))); \ ^ ./include/linux/seqlock.h:285:2: note: in expansion of macro 'lockdep_assert_preemption_disabled' lockdep_assert_preemption_disabled(); ^~ cc1: some warnings being treated as errors make[1]: *** [scripts/Makefile.build:117: arch/arm/kernel/asm-offsets.s] Error 1 make: *** [Makefile:1203: prepare0] Error 2 pgpIssg6mNjkm.pgp Description: PGP signature
Re: kbuild: separate kerneldoc warnings from compiler warnings
On Mon, 22 Jun 2020 14:10:13 +0900, Masahiro Yamada said: > > This patch introduces a new build flag 'K=1' which controls whether > > kerneldoc > > warnings should be issued, separating them from the compiler warnings that > > W= > > controls. > I do not understand why this change is needed. > IIRC, our goal was to enable this check by default. > https://patchwork.kernel.org/patch/10030521/ > but there are so many warnings. So are we getting any closer to actually achieving that goal? I've done a fair number of cleanup patches to make the kernel safe(r) to build with W=1, but there's still quite the pile. And actually, if you want people to actually fix up the kerneldoc issues, making it easier helps the chances of getting patches. If somebody is in the mood to do kerneldoc clean-ups, having an easy way to get just the kerneldoc messages rather than having to find them mixed in with all the rest helps... So I ran some numbers... A plain "make" for an arm allmodconfig weighs in at 40,184 lines. Building with K=1 produces 10,358 additional lines of output - that's what needs patching if you want the kerneldocs cleaned up. Building with W=1 (w/ this patch) adds 155,773 lines. Not A Typo. Of those, a whole whopping 116,699 are complaints from DTS issues, and 39,074 for all other gcc warnings. (Though I have 2 patches that I'll send later that will knock about 3,000 off the "all other gcc warnings" numbers). (And for completeness, building with C=1 for sparse adds 18,936 lines that say 'CHECK', and 56,915 lines of sparse warnings) > Meanwhile, this is checked only when W= is given > because 0-day bot tests with W=1 to > block new kerneldoc warnings. Looking at the numbers, I really need to say "So how is that working out for us, anyhow?" In particular, is it just flagging them, or do we have an actual procedure that stops patches from being accepted if they add new kerneldoc warnings? Another issue that needs to be considered is how high-quality a fix for a kerneldoc warning is. Getting rid of a warning by adding a comment line that says the 3rd parameter is a pointer to a 'struct wombat' does nobody any good if looking at the formal parameter list clearly states that the third parameter is a 'struct wombat *foo'. Heck, I could probably create a Perl script that automates that level of fixing. But making an *informative* comment requires doing a bunch of research so that you understand why *this* struct wombat is the one we care about (and whether we care *so* much that somebody better be holding a lock) > K=1 ? Do people need to learn this new switch? pgpOvchKNFX55.pgp Description: PGP signature
kbuild: separate kerneldoc warnings from compiler warnings
This patch introduces a new build flag 'K=1' which controls whether kerneldoc
warnings should be issued, separating them from the compiler warnings that W=
controls.
Signed-off-by: Valdis Kletnieks
diff --git a/Makefile b/Makefile
index 29abe44ada91..b1c0f9484a66 100644
--- a/Makefile
+++ b/Makefile
@@ -1605,6 +1605,7 @@ PHONY += help
@echo ' (sparse by default)'
@echo ' make C=2 [targets] Force check of all c source with $$CHECK'
@echo ' make RECORDMCOUNT_WARN=1 [targets] Warn about ignored mcount
sections'
+ @echo ' make K=1 [targets] Warn about problems in kerneldoc
comments'
@echo ' make W=n [targets] Enable extra build checks, n=1,2,3 where'
@echo '1: warnings which may be relevant and do not
occur too often'
@echo '2: warnings which occur quite often but may
still be relevant'
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 2e8810b7e5ed..9bcb77f5a5f1 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -100,7 +100,7 @@ else ifeq ($(KBUILD_CHECKSRC),2)
cmd_force_checksrc = $(CHECK) $(CHECKFLAGS) $(c_flags) $<
endif
-ifneq ($(KBUILD_EXTRA_WARN),)
+ifneq ($(KBUILD_KDOC_WARN),)
cmd_checkdoc = $(srctree)/scripts/kernel-doc -none $<
endif
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 4aea7cf71d11..3fd5881c91b0 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -17,6 +17,12 @@ endif
export KBUILD_EXTRA_WARN
+ifeq ("$(origin K)", "command line")
+ KBUILD_KDOC_WARN := $(K)
+endif
+
+export KBUILD_KDOC_WARN
+
#
# W=1 - warnings which may be relevant and do not occur too often
#
opp: core: Add missing export to core.c
After this commit, a 'make allmodconfig' fails due to a missing export.
commit 5f2430fb40c74db85764c8a472ecd6849025dd3f
Author: Sibi Sankar
Date: Sat Jun 6 03:03:31 2020 +0530
cpufreq: qcom: Update the bandwidth levels on frequency change
ERROR: modpost: "dev_pm_opp_adjust_voltage"
[drivers/cpufreq/qcom-cpufreq-hw.ko] undefined!
Provide the export.
Fixes: 5f2430fb40c7 ("cpufreq: qcom: Update the bandwidth levels on frequency
change")
Signed-off-by: Valdis Kletnieks
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 6937bf45f497..c9336aac74e9 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -2302,6 +2302,7 @@ int dev_pm_opp_adjust_voltage(struct device *dev,
unsigned long freq,
dev_pm_opp_put_opp_table(opp_table);
return r;
}
+EXPORT_SYMBOL_GPL(dev_pm_opp_adjust_voltage);
/**
* dev_pm_opp_enable() - Enable a specific OPP
pgpPkMuOIVuWj.pgp
Description: PGP signature
Re: [RFC][PATCH 7/7] sched: Replace rq::wake_list
On Tue, 26 May 2020 18:11:04 +0200, Peter Zijlstra said:
> The recent commit: 90b5363acd47 ("sched: Clean up scheduler_ipi()")
> got smp_call_function_single_async() subtly wrong. Even though it will
> return -EBUSY when trying to re-use a csd, that condition is not
> atomic and still requires external serialization.
> kernel/smp.c | 47 ---
> --- a/kernel/smp.c
> +++ b/kernel/smp.c
> @@ -659,6 +685,13 @@ void __init smp_init(void)
> BUILD_BUG_ON(offsetof(struct irq_work, flags) !=
>offsetof(struct __call_single_data, flags));
>
> + /*
> + * Assert the CSD_TYPE_TTWU layout is similar enough
> + * for task_struct to be on the @call_single_queue.
> + */
> + BUILD_BUG_ON(offsetof(struct task_struct, wake_entry_type) -
> offsetof(struct task_struct, wake_entry) !=
> + offsetof(struct __call_single_data, flags) -
> offsetof(struct __call_single_data, llist));
> +
> idle_threads_init();
> cpuhp_threads_init();
This blows up on a 32-bit ARM allmodconfig:
CC kernel/smp.o
kernel/smp.c:319:6: warning: no previous prototype for
'flush_smp_call_function_from_idle' [-Wmissing-prototypes]
void flush_smp_call_function_from_idle(void)
^
In file included from ./arch/arm/include/asm/atomic.h:11,
from ./include/linux/atomic.h:7,
from ./include/linux/llist.h:51,
from ./include/linux/irq_work.h:5,
from kernel/smp.c:10:
kernel/smp.c: In function 'smp_init':
./include/linux/compiler.h:392:38: error: call to '__compiletime_assert_152'
declared with attribute error: BUILD_BUG_ON failed: offsetof(struct
task_struct, wake_entry_type) - offsetof(struct task_struct, wake_entry) !=
offsetof(struct __call_single_data, flags) - offsetof(struct
__call_single_data, llist)
_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
^
./include/linux/compiler.h:373:4: note: in definition of macro
'__compiletime_assert'
prefix ## suffix();\
^~
./include/linux/compiler.h:392:2: note: in expansion of macro
'_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
^~~
./include/linux/build_bug.h:39:37: note: in expansion of macro
'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~
./include/linux/build_bug.h:50:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
^~~~
kernel/smp.c:689:2: note: in expansion of macro 'BUILD_BUG_ON'
BUILD_BUG_ON(offsetof(struct task_struct, wake_entry_type) - offsetof(struct
task_struct, wake_entry) !=
^~~~
pgpUQgnzW1jbP.pgp
Description: PGP signature
Re: next-20200528 - build error in kernel/rcu/refperf.c
On Thu, 28 May 2020 21:48:18 -0700, Randy Dunlap said: > > ERROR: modpost: "__aeabi_uldivmod" [kernel/rcu/refperf.ko] undefined! Gaah. And the reason I didn't spot Paul's post while grepping my linux-kernel mailbox is because *that* thread had a different undefined reference: > > > > > > m68k-linux-ld: kernel/rcu/refperf.o: in function `main_func': > > > > > > >> refperf.c:(.text+0x762): undefined reference to `__umoddi3' > Paul has already responded: (unfortunately) > > "So I am restricting to 64BIT for the time being. Yeah, I know, lazy of > me. ;-)" It's the sort of issue that's well into "as long as it gets mostly fixed before it hits Linus's tree" territory. I've seen lots of far worse work-arounds in the years since the 2.5.47 kernel. :) pgphPzLQIiLb8.pgp Description: PGP signature
next-20200528 - build error in kernel/rcu/refperf.c
commit 9088b449814f788d24f35a5840b6b2c2a23cd32a Author: Paul E. McKenney Date: Mon May 25 17:22:24 2020 -0700 refperf: Provide module parameter to specify number of experiments changes this line of code (line 389) - reader_tasks[exp].result_avg = 1000 * process_durations(exp) / ((exp + 1) * loops); + result_avg[exp] = 1000 * process_durations(nreaders) / (nreaders * loops); On a 32-bit ARM make allmodconfig with gcc 8.3, this results in: ERROR: modpost: "__aeabi_uldivmod" [kernel/rcu/refperf.ko] undefined! make[1]: *** [scripts/Makefile.modpost:103: __modpost] Error 1 I admit not understanding why the original line of code worked and the new one doesn't. Maybe gcc is smarter/dumber about the ranges of 'exp' and 'nreaders' than we thought? pgpnulXfV2CN0.pgp Description: PGP signature
Re: inux-next: build failure after merge of the drm-msm tree
On Tue, 26 May 2020 21:16:18 -0700, Nathan Chancellor said: > Additionally, I see a failure with clang due to the use of Bps_to_icc, > which does a straight division by 1000, which is treated as an integer > literal, with avg_bw as the dividend, which is a u64. > > Below is the "hack" in my tree. Also needed with gcc 8.3 for arm allmodconfig. > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c > index 85c2a4190840..5ea725d8da6c 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c > @@ -250,7 +250,7 @@ static int _dpu_core_perf_crtc_update_bus(struct dpu_kms > *kms, > > for (i = 0; i < kms->num_paths; i++) > icc_set_bw(kms->path[i], > - Bps_to_icc(avg_bw), (perf.max_per_pipe_ib)); > + div_u64(avg_bw, 1000), (perf.max_per_pipe_ib)); > > return ret; > } > pgpgX5vYBXPSX.pgp Description: PGP signature
Re: [PATCH] binfmt_elf_fdpic: fix execfd build regression
On Wed, 27 May 2020 17:08:57 -0500, Eric W. Biederman said: > Is there an easy to build-test configuration that includes > binfmt_elf_fdpic? I tripped over it with a 'make ARM=arch allmodconfig', but any config that includes CONFIG_BINFMT_ELF_FDPIC should suffice. I haven't checked the 'depends' for that variable though... > I have this sense that it might be smart to unify binfmt_elf > and binftm_elf_fdpic to the extent possible, and that will take build > tests. Bring it on! :) pgpg6MOEouTwM.pgp Description: PGP signature
Re: [PATCH] power: reset: vexpress: fix build issue
On Sun, 24 May 2020 15:20:25 -0700, Nathan Chancellor said: > arm-linux-gnueabi-ld: drivers/power/reset/vexpress-poweroff.o: in function > `vexpress_reset_probe': > vexpress-poweroff.c:(.text+0x36c): undefined reference to > `devm_regmap_init_vexpress_config' The part I can't figure out is that git blame tells me there's already an export: 3b9334ac835bb (Pawel Moll 2014-04-30 16:46:29 +0100 154) return regmap; 3b9334ac835bb (Pawel Moll 2014-04-30 16:46:29 +0100 155) } b33cdd283bd91 (Arnd Bergmann 2014-05-26 17:25:22 +0200 156) EXPORT_SYMBOL_GPL(devm_regmap_init_vexpress_config); 3b9334ac835bb (Pawel Moll 2014-04-30 16:46:29 +0100 157) but I can't figure out where or if drivers/power/reset/vexpress-poweroff.c gets a MODULE_LICENSE from... pgpS8h1erSztY.pgp Description: PGP signature
next-20200522 - build fail in fs/binfmt_elf_fdpic.c
Eric: looks like you missed one? commit b8a61c9e7b4a0fec493d191429e9653d66a79ccc Author: Eric W. Biederman Date: Thu May 14 15:17:40 2020 -0500 exec: Generic execfd support CHECK fs/binfmt_elf_fdpic.c fs/binfmt_elf_fdpic.c:591:34: error: undefined identifier 'BINPRM_FLAGS_EXECFD' CC fs/binfmt_elf_fdpic.o fs/binfmt_elf_fdpic.c: In function 'create_elf_fdpic_tables': fs/binfmt_elf_fdpic.c:591:27: error: 'BINPRM_FLAGS_EXECFD' undeclared (first use in this function); did you mean 'VM_DATA_FLAGS_EXEC'? if (bprm->interp_flags & BINPRM_FLAGS_EXECFD) ^~~ VM_DATA_FLAGS_EXEC fs/binfmt_elf_fdpic.c:591:27: note: each undeclared identifier is reported only once for each function it appears in make[1]: *** [scripts/Makefile.build:273: fs/binfmt_elf_fdpic.o] Error 1 23:14:07 0 [/usr/src/linux-next]2 pgpWk3LL93PKY.pgp Description: PGP signature
Re: [PATCH v2 2/2] i2c: mediatek: Add i2c ac-timing adjust support
On Tue, 19 May 2020 10:57:53 +0800, Qii Wang said: > (10 * (sample_cnt + 1)) / clk_src value is a 32-bit, (10 > * (sample_cnt + 1)) will over 32-bit if sample_cnt is 7. > > I think 10 and clk_src is too big, maybe I can reduce then with > be divided all by 1000. Yes, it's definitely too big, the 3 DIV_ROUND_UP calls in mtk_i2c_check_ac_timing() end up causing a build issue during modpost on a 32-bit RPi4: ERROR: modpost: "__aeabi_uldivmod" [drivers/i2c/busses/i2c-mt65xx.ko] undefined! ERROR: modpost: "__aeabi_ldivmod" [drivers/i2c/busses/i2c-mt65xx.ko] undefined! pgpBCSBQ46GbV.pgp Description: PGP signature
[PATCH] remoteproc: wcss: Fix function call for new API
commit 8a226e2c71: remoteproc: wcss: add support for rpmsg communication throws a compile error: CC [M] drivers/remoteproc/qcom_q6v5_wcss.o drivers/remoteproc/qcom_q6v5_wcss.c: In function 'q6v5_wcss_probe': drivers/remoteproc/qcom_q6v5_wcss.c:563:2: error: too few arguments to function 'qcom_add_glink_subdev' qcom_add_glink_subdev(rproc, >glink_subdev); ^ In file included from drivers/remoteproc/qcom_q6v5_wcss.c:16: drivers/remoteproc/qcom_common.h:35:6: note: declared here void qcom_add_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink, ^ Update to API change from commit cd9fc8f: remoteproc: qcom: Pass ssr_name to glink subdevice Signed-off-by: Valdis Kletnieks diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c index 48d16d81f94d..99ecc0e6276e 100644 --- a/drivers/remoteproc/qcom_q6v5_wcss.c +++ b/drivers/remoteproc/qcom_q6v5_wcss.c @@ -560,7 +560,7 @@ static int q6v5_wcss_probe(struct platform_device *pdev) if (ret) goto free_rproc; - qcom_add_glink_subdev(rproc, >glink_subdev); + qcom_add_glink_subdev(rproc, >glink_subdev,"q6wcss"); qcom_add_ssr_subdev(rproc, >ssr_subdev, "q6wcss"); ret = rproc_add(rproc);
Re: next-20200514 - build issue in drivers/md/dm-zoned-target.c
On Mon, 18 May 2020 12:44:49 -0400, Mike Snitzer said: > Unless I'm missing something it was fixed up with this commit last > wednesday (13th): > > https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-5.8=81a3a1453ec4e5da081e1395732801a600feb352 That says: author Nathan Chancellor 2020-05-13 01:45:22 -0700 committer Mike Snitzer2020-05-15 10:29:39 -0400 So it didn't make it into next-0514, which is why I got bit by it. It's in today's linux-next and life is good. :) pgp7JF9vN60El.pgp Description: PGP signature
Re: general protection fault vs Oops
On Sat, 16 May 2020 18:05:07 +0530, Subhashini Rao Beerisetty said: > In the first attempt when I run that test case I landed into âgeneral > protection fault: [#1] SMP" .. Next I rebooted and ran the same > test , but now it resulted the âOops: 0002 [#1] SMP". And the 0002 is telling you that there's been 2 previous bug/oops since the reboot, so you need to go back through your dmesg and find the *first* one. > In both cases the call trace looks exactly same and RIP points to > ânative_queued_spin_lock_slowpath+0xfe/0x170".. The first few entries in the call trace are the oops handler itself. So... > May 16 12:06:17 test-pc kernel: [96934.567347] Call Trace: > May 16 12:06:17 test-pc kernel: [96934.569475] > []__raw_spin_lock_irqsave+0x37/0x40 > May 16 12:06:17 test-pc kernel: [96934.571686] [] > event_raise+0x22/0x60 [osa] > May 16 12:06:17 test-pc kernel: [96934.573935] [] > multi_q_completed_one_buffer+0x34/0x40 [mcore] The above line is the one where you hit the wall. > May 16 12:59:22 test-pc kernel: [ 3011.405602] Call Trace: > May 16 12:59:22 test-pc kernel: [ 3011.407892] [] > _raw_spin_lock_irqsave+0x37/0x40 > May 16 12:59:22 test-pc kernel: [ 3011.410256] [] > event_raise+0x22/0x60 [osa] > May 16 12:59:22 test-pc kernel: [ 3011.412652] [] > multi_q_completed_one_buffer+0x34/0x40 [mcore] And again. However, given that it's a 4.4 kernel from 4 years ago, it's going to be hard to find anybody who really cares. In fact. I'm wondering if this is from some out-of-tree or vendor patch, because I'm not finding any sign of that function in either the 5.7 or 4.4 tree. Not even a sign of ## catenation abuse - no relevant hits for "completed_one_buffer" or "multi_q" either I don't think anybody's going to be able to help unless somebody first identifies where that function is pgpwMREyVwScu.pgp Description: PGP signature
next-20200514 - build issue in drivers/md/dm-zoned-target.c
Am seeing a build error in next-0514. -0420 built OK. building a 'make allmodconfig' on a RPi4 in 32-bit mode. MODPOST 7575 modules ERROR: modpost: "__aeabi_uldivmod" [drivers/md/dm-zoned.ko] undefined! objdump and 'make drivers/md/dm-zoned-target.s' tells me that the problem is in function dmz_fixup_devices(), near here: @ drivers/md/dm-zoned-target.c:806: reg_dev->nr_zones = DIV_ROUND_UP(reg_dev->capacity, ldr r0, [r6, #56] @ reg_dev_166->capacity, reg_dev_166->capacity addsr1, r3, r1 @ tmp316, _227, reg_dev_166->capacity adc r0, r2, r0 @ tmp315, _227, reg_dev_166->capacity subsr1, r1, #1 @, tmp316, @ drivers/md/dm-zoned-target.c:805: reg_dev->zone_nr_sectors = zoned_dev->zone_nr_sectors; strdr2, [r6, #80] @, reg_dev, @ drivers/md/dm-zoned-target.c:806: reg_dev->nr_zones = DIV_ROUND_UP(reg_dev->capacity, sbc r0, r0, #0 @, tmp315, bl __aeabi_uldivmod@ @ drivers/md/dm-zoned-target.c:806: reg_dev->nr_zones = DIV_ROUND_UP(reg_dev->capacity, str r1, [r6, #64] @ tmp306, reg_dev_166->nr_zones git blame points at this commit: commit 70978208ec91d798066f4c291bc98ff914bea222 Author: Hannes Reinecke Date: Mon May 11 10:24:30 2020 +0200 dm zoned: metadata version 2 Reverting that commit lets the build complete. pgpJsrzVKWly2.pgp Description: PGP signature
[PATCH] watch_queue: sample: Update makefile to fix deprecated variables
A recent commit started warning for deprecated makefile variables. Turns out there was an in-tree user, so update it. Signed-off-by: Valdis Kletnieks diff --git a/samples/watch_queue/Makefile b/samples/watch_queue/Makefile index eec00dd0a8df..8511fb6c53d2 100644 --- a/samples/watch_queue/Makefile +++ b/samples/watch_queue/Makefile @@ -1,7 +1,7 @@ # List of programs to build -hostprogs-y := watch_test +hostprogs := watch_test # Tell kbuild to always build the programs -always := $(hostprogs-y) +always-y := $(hostprogs) HOSTCFLAGS_watch_test.o += -I$(objtree)/usr/include
linux-next 20200508 - build failure in kernel/resource.c w/ SPARSEMEM=n
So I did a 'make allmodconfig' and then a 'make' on an RPi4 ARM box, and it decided that CONFIG_SPARSEMEM=n was OK (so an include of linux/mmzone.h doesn't define some needed values). The offending code in resource.c is wrapped in a #ifdef CONFIG_DEVICE_PRIVATE, which throws a whinge during 'make menuconfig' or 'make allmodconfig': WARNING: unmet direct dependencies detected for DEVICE_PRIVATE Depends on [n]: ZONE_DEVICE [=n] Selected by [m]: - DRM_NOUVEAU_SVM [=y] && HAS_IOMEM [=y] && DRM_NOUVEAU [=m] && MMU [=y] && STAGING [=y] after which I end up with CONFIG_DEVICE_PRIVATE=y in the .config file. make menuconfig tells me: Symbol: ZONE_DEVICE [=n] Type : bool Defined at mm/Kconfig:779 Prompt: Device memory (pmem, HMM, etc...) hotplug support Depends on: MEMORY_HOTPLUG [=n] && MEMORY_HOTREMOVE [=n] && SPARSEMEM_VMEMMAP [=n] && ARCH_HAS_PTE_DEVMAP [=n] Location: (1) -> Memory Management options Selects: XARRAY_MULTI [=n] Pretty obviously a Kconfig whoops, but I have no idea what the proper Kconfig fix is for this.. May be related to: commit 0092908d16c604b8207c2141ec64b0fa4473bb03 Author: Christoph Hellwig Date: Wed Jun 26 14:27:06 2019 +0200 mm: factor out a devm_request_free_mem_region helper which added the #ifdef CONFIG_DEVICE_PRIVATE code in question, except that's a pretty old commit... The only thing I'm sure of is that DEVICE_PRIVATE=y and SPARSEMEM=n blows up. :) CC kernel/resource.o In file included from ./include/linux/cache.h:5, from ./include/linux/printk.h:9, from ./include/linux/kernel.h:15, from ./include/asm-generic/bug.h:19, from ./arch/arm/include/asm/bug.h:60, from ./include/linux/bug.h:5, from ./include/linux/mmdebug.h:5, from ./include/linux/gfp.h:5, from ./include/linux/slab.h:15, from kernel/resource.c:17: kernel/resource.c: In function '__request_free_mem_region': kernel/resource.c:1653:28: error: 'PA_SECTION_SHIFT' undeclared (first use in this function); did you mean 'SECTION_SHIFT'? size = ALIGN(size, 1UL << PA_SECTION_SHIFT); ^~~~ ./include/uapi/linux/kernel.h:11:47: note: in definition of macro '__ALIGN_KERNEL_MASK' #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask)) ^~~~ ./include/linux/kernel.h:33:22: note: in expansion of macro '__ALIGN_KERNEL' #define ALIGN(x, a) __ALIGN_KERNEL((x), (a)) ^~ kernel/resource.c:1653:9: note: in expansion of macro 'ALIGN' size = ALIGN(size, 1UL << PA_SECTION_SHIFT); ^ kernel/resource.c:1653:28: note: each undeclared identifier is reported only once for each function it appears in size = ALIGN(size, 1UL << PA_SECTION_SHIFT); ^~~~ ./include/uapi/linux/kernel.h:11:47: note: in definition of macro '__ALIGN_KERNEL_MASK' #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask)) ^~~~ ./include/linux/kernel.h:33:22: note: in expansion of macro '__ALIGN_KERNEL' #define ALIGN(x, a) __ALIGN_KERNEL((x), (a)) ^~ kernel/resource.c:1653:9: note: in expansion of macro 'ALIGN' size = ALIGN(size, 1UL << PA_SECTION_SHIFT); ^ In file included from ./include/asm-generic/bug.h:19, from ./arch/arm/include/asm/bug.h:60, from ./include/linux/bug.h:5, from ./include/linux/mmdebug.h:5, from ./include/linux/gfp.h:5, from ./include/linux/slab.h:15, from kernel/resource.c:17: kernel/resource.c:1654:48: error: 'MAX_PHYSMEM_BITS' undeclared (first use in this function); did you mean 'MAX_UINSN_BYTES'? end = min_t(unsigned long, base->end, (1UL << MAX_PHYSMEM_BITS) - 1); ^~~~ ./include/linux/kernel.h:848:40: note: in definition of macro '__typecheck' (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) ^ ./include/linux/kernel.h:872:24: note: in expansion of macro '__safe_cmp' __builtin_choose_expr(__safe_cmp(x, y), \ ^~ ./include/linux/kernel.h:940:27: note: in expansion of macro '__careful_cmp' #define min_t(type, x, y) __careful_cmp((type)(x), (type)(y), <) ^ kernel/resource.c:1654:8: note: in expansion of macro 'min_t' end = min_t(unsigned long, base->end, (1UL << MAX_PHYSMEM_BITS) - 1); ^ ./include/linux/kernel.h:872:2: error: first argument to '__builtin_choose_expr' not a constant __builtin_choose_expr(__safe_cmp(x, y), \ ^ ./include/linux/kernel.h:940:27: note: in expansion of macro '__careful_cmp' #define min_t(type, x, y) __careful_cmp((type)(x),
Re: linux-next 20200506 - build failure with net/bpfilter/bpfilter_umh
On Sat, 09 May 2020 12:45:22 +0900, Masahiro Yamada said: > > LD [U] net/bpfilter/bpfilter_umh > > /usr/bin/ld: cannot find -lc > > collect2: error: ld returned 1 exit status > > make[2]: *** [scripts/Makefile.userprogs:36: net/bpfilter/bpfilter_umh] > > Error 1 > > make[1]: *** [scripts/Makefile.build:494: net/bpfilter] Error 2 > > make: *** [Makefile:1726: net] Error 2 > > > > The culprit is this commit: > > Thanks. I will try to fix it, > but my commit is innocent because > it is just textual cleanups. > No functional change is intended. Hmm... 'git show' made it look like a totally new line... Proposed patch following in separate email... pgpASvU1HdMMA.pgp Description: PGP signature
[PATCH] bpfilter: document build requirements for bpfilter_umh
It's not intuitively obvious that bpfilter_umh is a statically linked binary. Mention the toolchain requirement in the Kconfig help, so people have an easier time figuring out what's needed. Signed-off-by: Valdis Kletnieks diff --git a/net/bpfilter/Kconfig b/net/bpfilter/Kconfig index fed9290e3b41..0ec6c7958c20 100644 --- a/net/bpfilter/Kconfig +++ b/net/bpfilter/Kconfig @@ -13,4 +13,8 @@ config BPFILTER_UMH default m help This builds bpfilter kernel module with embedded user mode helper + + Note: your toolchain must support building static binaries, since + rootfs isn't mounted at the time when __init functions are called + and do_execv won't be able to find the elf interpreter. endif
linux-next 20200506 - build failure with net/bpfilter/bpfilter_umh
My kernel build came to a screeching halt with: CHECK net/bpfilter/bpfilter_kern.c CC [M] net/bpfilter/bpfilter_kern.o CC [U] net/bpfilter/main.o LD [U] net/bpfilter/bpfilter_umh /usr/bin/ld: cannot find -lc collect2: error: ld returned 1 exit status make[2]: *** [scripts/Makefile.userprogs:36: net/bpfilter/bpfilter_umh] Error 1 make[1]: *** [scripts/Makefile.build:494: net/bpfilter] Error 2 make: *** [Makefile:1726: net] Error 2 The culprit is this commit: commit 0592c3c367c4c823f2a939968e72d39360fce1f4 Author: Masahiro Yamada Date: Wed Apr 29 12:45:15 2020 +0900 bpfilter: use 'userprogs' syntax to build bpfilter_umh and specifically, this line: +userldflags += -static At least on Fedora, this dies an ugly death unless you have the glibc-static RPM installed (which is *not* part of the glibc-devel RPM). Not sure how to fix this, or give a heads-up that there's a new requirement that might break the build. pgpNI6zjmun43.pgp Description: PGP signature
Re: [PATCH] staging: exfat: add exfat filesystem code to staging
On Wed, 16 Oct 2019 16:33:17 -0400, Sasha Levin said: > It looks like the reason this wasn't made public along with the exFAT > spec is that TexFAT is pretty much dead - it's old, there are no users > we are aware of, and digging it out of it's grave to make it public is > actually quite the headache. Ahh.. For some reason I had convinced myself that base exfat implementations used at least the 'keep a backup FAT' part of TexFAT, because on a terabyte external USB drive, losing the FAT would be painful. But if Windows 10 doesn't do that either, then it's no great sin for Linux to not do it (and may cause problems if Linux says "currently using FAT 2", and the disk is next used on a Windows 10 box that only looks at FAT 1) pgp36Nhqfju4G.pgp Description: PGP signature
Re: [PATCH] staging: exfat: add exfat filesystem code to staging
On Wed, 16 Oct 2019 10:31:13 -0400, Sasha Levin said: > On Wed, Oct 16, 2019 at 04:03:53PM +0200, Pali Roh�r wrote: > >Now one month passed, so do you have some information when missing parts > >of documentation like TexFAT would be released to public? > > Sure, I'll see if I can get an approval to open it up. > > Can I assume you will be implementing TexFAT support once the spec is > available? It's certainly something that *should* be supported. The exact timeframe, and who the "you" that actually writes the patch is of course up in the air (and will likely end up being a collaborative effort between the first author and corrections from others). pgpuzy44yR0z4.pgp Description: PGP signature
[PATCH] drivers/staging/exfat - fix fs_sync() calls.
The majority of them were totally backwards. Change the logic
so that if DELAYED_SYNC *isn't* in the config, we actually flush to disk
before flagging the file system as clean.
That leaves two calls in the DELAYED_SYNC case. More detailed
analysis is needed to make sure that's what's really needed, or if other
call sites also need a fs_sync() call. This patch is at least "less wrong"
than the code was, but further changes should be another patch.
Signed-off-by: Valdis Kletnieks
diff --git a/drivers/staging/exfat/exfat_super.c
b/drivers/staging/exfat/exfat_super.c
index 5f6caee819a6..2526044569ee 100644
--- a/drivers/staging/exfat/exfat_super.c
+++ b/drivers/staging/exfat/exfat_super.c
@@ -458,7 +458,7 @@ static int ffsUmountVol(struct super_block *sb)
/* acquire the lock for file system critical section */
down(_fs->v_sem);
- fs_sync(sb, false);
+ fs_sync(sb, true);
fs_set_vol_flags(sb, VOL_CLEAN);
if (p_fs->vol_type == EXFAT) {
@@ -666,8 +666,8 @@ static int ffsCreateFile(struct inode *inode, char *path,
u8 mode,
/* create a new file */
ret = create_file(inode, , _name, mode, fid);
-#ifdef CONFIG_EXFAT_DELAYED_SYNC
- fs_sync(sb, false);
+#ifndef CONFIG_EXFAT_DELAYED_SYNC
+ fs_sync(sb, true);
fs_set_vol_flags(sb, VOL_CLEAN);
#endif
@@ -1039,8 +1039,8 @@ static int ffsWriteFile(struct inode *inode, struct
file_id_t *fid,
release_entry_set(es);
}
-#ifdef CONFIG_EXFAT_DELAYED_SYNC
- fs_sync(sb, false);
+#ifndef CONFIG_EXFAT_DELAYED_SYNC
+ fs_sync(sb, true);
fs_set_vol_flags(sb, VOL_CLEAN);
#endif
@@ -1179,8 +1179,8 @@ static int ffsTruncateFile(struct inode *inode, u64
old_size, u64 new_size)
if (fid->rwoffset > fid->size)
fid->rwoffset = fid->size;
-#ifdef CONFIG_EXFAT_DELAYED_SYNC
- fs_sync(sb, false);
+#ifndef CONFIG_EXFAT_DELAYED_SYNC
+ fs_sync(sb, true);
fs_set_vol_flags(sb, VOL_CLEAN);
#endif
@@ -1327,8 +1327,8 @@ static int ffsMoveFile(struct inode *old_parent_inode,
struct file_id_t *fid,
num_entries + 1);
}
out:
-#ifdef CONFIG_EXFAT_DELAYED_SYNC
- fs_sync(sb, false);
+#ifndef CONFIG_EXFAT_DELAYED_SYNC
+ fs_sync(sb, true);
fs_set_vol_flags(sb, VOL_CLEAN);
#endif
@@ -1389,8 +1389,8 @@ static int ffsRemoveFile(struct inode *inode, struct
file_id_t *fid)
fid->start_clu = CLUSTER_32(~0);
fid->flags = (p_fs->vol_type == EXFAT) ? 0x03 : 0x01;
-#ifdef CONFIG_EXFAT_DELAYED_SYNC
- fs_sync(sb, false);
+#ifndef CONFIG_EXFAT_DELAYED_SYNC
+ fs_sync(sb, true);
fs_set_vol_flags(sb, VOL_CLEAN);
#endif
@@ -1478,8 +1478,8 @@ static int ffsSetAttr(struct inode *inode, u32 attr)
release_entry_set(es);
}
-#ifdef CONFIG_EXFAT_DELAYED_SYNC
- fs_sync(sb, false);
+#ifndef CONFIG_EXFAT_DELAYED_SYNC
+ fs_sync(sb, true);
fs_set_vol_flags(sb, VOL_CLEAN);
#endif
@@ -1916,8 +1916,8 @@ static int ffsCreateDir(struct inode *inode, char *path,
struct file_id_t *fid)
ret = create_dir(inode, , _name, fid);
-#ifdef CONFIG_EXFAT_DELAYED_SYNC
- fs_sync(sb, false);
+#ifndef CONFIG_EXFAT_DELAYED_SYNC
+ fs_sync(sb, true);
fs_set_vol_flags(sb, VOL_CLEAN);
#endif
@@ -2177,8 +2177,8 @@ static int ffsRemoveDir(struct inode *inode, struct
file_id_t *fid)
fid->start_clu = CLUSTER_32(~0);
fid->flags = (p_fs->vol_type == EXFAT) ? 0x03 : 0x01;
-#ifdef CONFIG_EXFAT_DELAYED_SYNC
- fs_sync(sb, false);
+#ifndef CONFIG_EXFAT_DELAYED_SYNC
+ fs_sync(sb, true);
fs_set_vol_flags(sb, VOL_CLEAN);
#endif
Re: [PATCH] staging: exfat: use bdev_sync function directly where needed
On Wed, 02 Oct 2019 20:47:03 +0530, Saiyam Doshi said: > fs_sync() is wrapper to bdev_sync(). When fs_sync is called with > non-zero argument, bdev_sync gets called. > > Most instances of fs_sync is called with false and very few with > true. Refactor this and makes direct call to bdev_sync() where > needed and removes fs_sync definition. Did you do an actual analysis of where bdev_sync() *should* be called? The note in the TODO was intended to point out that many of the calls are called with 'false' and probably should not be. #ifdef CONFIG_EXFAT_DELAYED_SYNC - fs_sync(sb, false); fs_set_vol_flags(sb, VOL_CLEAN); #endif Consider - with this patch, we are now setting the volume state to clean - but we've not actually flushed the filesystem to disk, which means it's almost guaranteed that the file system is *NOT* in fact clean. Changing all the 'false' that happen before a VOL_CLEAN to 'true' is probably more correct - but still totally wrong if DELAYED_SYNC isn't defined because then we *aren't* syncing to disk at all. pgpIy3gRkOS9L.pgp Description: PGP signature
[PATCH] drivers/staging/exfat - explain the fs_sync() issue in TODO
We've seen several incorrect patches for fs_sync() calls in the exfat driver.
Add code to the TODO that explains this isn't just a delete code and refactor,
but that actual analysis of when the filesystem should be flushed to disk
needs to be done.
Signed-off-by: Valdis Kletnieks
---
diff --git a/drivers/staging/exfat/TODO b/drivers/staging/exfat/TODO
index a3eb282f9efc..77c302acfcb8 100644
--- a/drivers/staging/exfat/TODO
+++ b/drivers/staging/exfat/TODO
@@ -3,6 +3,15 @@ same for ffsWriteFile.
exfat_core.c - fs_sync(sb,0) all over the place looks fishy as hell.
There's only one place that calls it with a non-zero argument.
+Randomly removing fs_sync() calls is *not* the right answer, especially
+if the removal then leaves a call to fs_set_vol_flags(VOL_CLEAN), as that
+says the file system is clean and synced when we *know* it isn't.
+The proper fix here is to go through and actually analyze how DELAYED_SYNC
+should work, and any time we're setting VOL_CLEAN, ensure the file system
+has in fact been synced to disk. In other words, changing the 'false' to
+'true' is probably more correct. Also, it's likely that the one current
+place where it actually does an bdev_sync isn't sufficient in the DELAYED_SYNC
+case.
ffsTruncateFile - if (old_size <= new_size) {
That doesn't look right. How did it ever work? Are they relying on lazy
[PATCH] samples/watch_test - fix build error
We're missing a depends in the Kconfig, which can lead to trying to build without the required headers being present.. HOSTCC samples/watch_queue/watch_test samples/watch_queue/watch_test.c:23:10: fatal error: linux/watch_queue.h: No such file or directory 23 | #include | ^ compilation terminated. make[2]: *** [scripts/Makefile.host:107: samples/watch_queue/watch_test] Error 1 Signed-off-by: Valdis Kletnieks --- diff --git a/samples/Kconfig b/samples/Kconfig index 2c3e07addd38..d0761f29ccb0 100644 --- a/samples/Kconfig +++ b/samples/Kconfig @@ -171,6 +171,7 @@ config SAMPLE_VFS config SAMPLE_WATCH_QUEUE bool "Build example /dev/watch_queue notification consumer" + depends on HEADERS_INSTALL help Build example userspace program to use the new mount_notify(), sb_notify() syscalls and the KEYCTL_WATCH_KEY keyctl() function.
[PATCH] drivers/staging/exfat - fix SPDX tags..
The copyright notices as I got them said "GPLv2 or later", which I screwed up when putting in the SPDX tags. Fix them to match the license I got the code under. Signed-off-by: Valdis Kletnieks --- diff --git a/drivers/staging/exfat/Makefile b/drivers/staging/exfat/Makefile index 84944dfbae28..6c90aec83feb 100644 --- a/drivers/staging/exfat/Makefile +++ b/drivers/staging/exfat/Makefile @@ -1,4 +1,4 @@ -# SPDX-License-Identifier: GPL-2.0 +# SPDX-License-Identifier: GPL-2.0-or-later obj-$(CONFIG_EXFAT_FS) += exfat.o diff --git a/drivers/staging/exfat/exfat.h b/drivers/staging/exfat/exfat.h index 6c12f2d79f4d..3abab33e932c 100644 --- a/drivers/staging/exfat/exfat.h +++ b/drivers/staging/exfat/exfat.h @@ -1,4 +1,4 @@ -/* SPDX-License-Identifier: GPL-2.0 */ +/* SPDX-License-Identifier: GPL-2.0-or-later */ /* * Copyright (C) 2012-2013 Samsung Electronics Co., Ltd. */ diff --git a/drivers/staging/exfat/exfat_blkdev.c b/drivers/staging/exfat/exfat_blkdev.c index f086c75e7076..81d20e6241c6 100644 --- a/drivers/staging/exfat/exfat_blkdev.c +++ b/drivers/staging/exfat/exfat_blkdev.c @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: GPL-2.0 +// SPDX-License-Identifier: GPL-2.0-or-later /* * Copyright (C) 2012-2013 Samsung Electronics Co., Ltd. */ diff --git a/drivers/staging/exfat/exfat_cache.c b/drivers/staging/exfat/exfat_cache.c index 1565ce65d39f..e1b001718709 100644 --- a/drivers/staging/exfat/exfat_cache.c +++ b/drivers/staging/exfat/exfat_cache.c @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: GPL-2.0 +// SPDX-License-Identifier: GPL-2.0-or-later /* * Copyright (C) 2012-2013 Samsung Electronics Co., Ltd. */ diff --git a/drivers/staging/exfat/exfat_core.c b/drivers/staging/exfat/exfat_core.c index b3e9cf725cf5..79174e5c4145 100644 --- a/drivers/staging/exfat/exfat_core.c +++ b/drivers/staging/exfat/exfat_core.c @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: GPL-2.0 +// SPDX-License-Identifier: GPL-2.0-or-later /* * Copyright (C) 2012-2013 Samsung Electronics Co., Ltd. */ diff --git a/drivers/staging/exfat/exfat_nls.c b/drivers/staging/exfat/exfat_nls.c index 03cb8290b5d2..a5c4b68925fb 100644 --- a/drivers/staging/exfat/exfat_nls.c +++ b/drivers/staging/exfat/exfat_nls.c @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: GPL-2.0 +// SPDX-License-Identifier: GPL-2.0-or-later /* * Copyright (C) 2012-2013 Samsung Electronics Co., Ltd. */ diff --git a/drivers/staging/exfat/exfat_super.c b/drivers/staging/exfat/exfat_super.c index 5f6caee819a6..229ecabe7a93 100644 --- a/drivers/staging/exfat/exfat_super.c +++ b/drivers/staging/exfat/exfat_super.c @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: GPL-2.0 +// SPDX-License-Identifier: GPL-2.0-or-later /* * Copyright (C) 2012-2013 Samsung Electronics Co., Ltd. */ diff --git a/drivers/staging/exfat/exfat_upcase.c b/drivers/staging/exfat/exfat_upcase.c index 366082fb3dab..b91a1faa0e50 100644 --- a/drivers/staging/exfat/exfat_upcase.c +++ b/drivers/staging/exfat/exfat_upcase.c @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: GPL-2.0 +// SPDX-License-Identifier: GPL-2.0-or-later /* * Copyright (C) 2012-2013 Samsung Electronics Co., Ltd. */
Re: [PATCH] staging: exfat: rebase to sdFAT v2.2.0
On Thu, 19 Sep 2019 05:31:21 +0900, Ju Hyung Park said: > We should probably ask Valdis on what happened there. > > Even the old exFAT v1.2.24 from Galaxy S7 is using "either version 2 > of the License, or (at your option) any later version". > You can go check it yourself by searching "G930F" from > http://opensource.samsung.com. > > I'm guessing he probably used "GPL-2.0" during his clean-up. My screw-up. Original had: /* * Copyright (C) 2012-2013 Samsung Electronics Co., Ltd. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License * as published by the Free Software Foundation; either version 2 * of the License, or (at your option) any later version. So yes, I dorked up the SPDX tags, as I didn't realize GPL-2.0-or-later was an actual thing for those... pgpDb8oDzjMmb.pgp Description: PGP signature
Re: [PATCH] staging: exfat: add exfat filesystem code to
On Tue, 17 Sep 2019 12:02:01 +0900, "Namjae Jeon" said: > We are excited to see this happening and would like to state that we > appreciate time and > effort which people put into upstreaming exfat. Thank you! The hard part - getting Microsoft to OK merging an exfat driver - is done. All we need now is to get a driver cleaned up. :) > However, if possible, can we step back a little bit and re-consider it? We > would prefer to > see upstream the code which we are currently using in our products - sdfat - > as > this can be mutually benefitial from various points of view. I'm working off a somewhat cleaned up copy of Samsung's original driver, because that's what I had knowledge of. If the sdfat driver is closer to being mergeable, I'd not object if that got merged instead. But here's the problem... Samsung has their internal sdfat code, Park Yu Hyung has what appears to be a fork of that code from some point (and it's unclear , and it's unclear which one has had more bugfixes and cleanups to get it to somewhere near mainline mergeable. Can you provide a pointer to what Samsung is *currently* using? We probably need to stop and actually look at the code bases and see what's in the best shape currently. pgpPCazp_DYH8.pgp Description: PGP signature
Re: staging: exfat: issue with FFS_MEDIAERR error return assignment
On Tue, 10 Sep 2019 16:09:35 +0300, Dan Carpenter said:
> On Tue, Sep 10, 2019 at 01:58:35PM +0100, Colin Ian King wrote:
> > On 10/09/2019 13:44, Dan Carpenter wrote:
> > > On Fri, Aug 30, 2019 at 07:38:00PM +0100, Colin Ian King wrote:
> > >> Hi,
> > >>
> > >> Static analysis on exfat with Coverity has picked up an assignment of
> > >> FFS_MEDIAERR that gets over-written:
> > >>
> > >>
> > >> 1750if (is_dir) {
> > >> 1751if ((fid->dir.dir == p_fs->root_dir) &&
> > >> 1752(fid->entry == -1)) {
> > >> 1753if (p_fs->dev_ejected)
> > >
> > > Idealy we would have both a filename and a function name but this email
> > > doesn't have either so no one knows what code you are talking about. :P
> >
> > Oops, my bad.
> >
> > drivers/staging/exfat/exfat_super.c ffsWriteStat()
>
> Yes, your solution is correct.
Actually, you can skip the else, because we initialized 'ret' at the start of
the function.
The *bigger* issue - what should 'ret' be if dev_ejected is *false*?
pgp_e7xFbV9AW.pgp
Description: PGP signature
Re: [PATCH v3 1/4] staging: exfat: drop unused function parameter
On Sun, 08 Sep 2019 17:35:36 -, Valentin Vidic said: > sbi parameter not used inside the function so remove it. > Also cleanup unused variables generated by this change. Tread carefully with this sort of patch - there's still a lot of places in the code where we have matching pairs of exfat_foo() and fat_foo() functions which need to have the same signatures because they're called through a function pointer. This particular one looks OK, but there's other functions that come in pairs that you need to watch out for... pgpTeMYOQawkl.pgp Description: PGP signature
Re: [PATCH v2 2/3] staging: exfat: drop unused field access_time_ms
On Sun, 08 Sep 2019 16:10:14 -, Valentin Vidic said: > Not used in the exfat-fuse implementation and spec defines > this position should hold the value for CreateUtcOffset. In that case, rather than removing it, shouldn't we be *adding* code to properly set it instead? pgp9KZVl0RIgO.pgp Description: PGP signature
Re: perf_event wakeup_events = 0
On Sat, 07 Sep 2019 09:14:49 -0700, Theodore Dubois said: Reading what it actually says rather than what I thought it said.. :) Events come in two flavors: counting and sampled. A counting event is one that is used for counting the aggregate number of events that occur. In general, counting event results are gathered with a read(2) call. A sampling event periodically writes measurements to a buffer that can then be accessed via mmap(2). For some reason, I was thinking counting events. -ENOCAFFEINE. :) > sample_freq is 4000 (and freq is 1). Hereâs the man page on this field: > >sample_period, sample_freq > A "sampling" event is one that generates an overflow > notificaâ > tion every N events, where N is given by sample_period. A > samâ > pling event has sample_period > 0. There's this part: > pling event has sample_period > 0. When an overflow occurs, > requested data is recorded in the mmap buffer. The sample_type > field controls what data is recorded on each overflow. So an entry is made in the buffer. It's not clear that this immediately triggers a signal... MMAP layout When using perf_event_open() in sampled mode, asynchronous events (like counter overflow or PROT_EXEC mmap tracking) are logged into a ring- buffer. This ring-buffer is created and accessed through mmap(2). The mmap size should be 1+2^n pages, where the first page is a metadata page (struct perf_event_mmap_page) that contains various bits of infor? mation such as where the ring-buffer head is. So you need to look at what size mmap buffer is being allocated. It's *probably* on the order of megabytes, so that you can buffer a fairly large number of entries and not take several user/kernel transitions on every single entry... > If Iâm reading this right, this is a sampling event which overflows 4000 > times a second. And 4,000 entries are made in the buffer per second.. > But perf then does a poll call which wakes up on this FD with POLLIN after > 1.637 seconds, instead of 0.00025 seconds At which point perf goes and looks at several thousand entries in the ring buffer... pgp2wjxgbcJF2.pgp Description: PGP signature
Re: perf_event wakeup_events = 0
On Sat, 07 Sep 2019 09:14:49 -0700, Theodore Dubois said: > If Iâm reading this right, this is a sampling event which overflows 4000 > times a second. But perf then does a poll call which wakes up on this FD with > POLLIN after 1.637 seconds, instead of 0.00025 seconds. No, it *takes a sample* 4,000 times a second. For instance, number of cache line misses since the last sample. You get an overflow when the counter wraps because there have been more than 2^32 events since you read the counter. At least that's my understanding of it. pgp4B98dVc2cw.pgp Description: PGP signature
[PATCH] scripts/checkpatch.pl - don't check for const structs if list is empty
If the list of structures we expect to be const is empty (due to file
permissions,
or the file being empty, etc), we get odd complaints about structures:
[/usr/src/linux-next] scripts/checkpatch.pl -f
drivers/staging/netlogic/platform_net.h
No structs that should be const will be found - file
'/usr/src/linux-next/scripts/const_structs.checkpatch': Permission denied
WARNING: struct should normally be const
#9: FILE: drivers/staging/netlogic/platform_net.h:9:
+struct xlr_net_data {
WARNING: struct should normally be const
#20: FILE: drivers/staging/netlogic/platform_net.h:20:
+ struct xlr_fmn_info *gmac_fmn_info;
total: 0 errors, 2 warnings, 0 checks, 21 lines checked
Fix it so that it actually *obeys* what it said about not finding structures.
Reported-by: Pablo Pellecchia
Signed-off-by: Valdis Kletnieks
---
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index f4b6127ff469..103c67665f61 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6497,7 +6497,7 @@ sub process {
# check for various structs that are normally const (ops, kgdb, device_tree)
# and avoid what seem like struct definitions 'struct foo {'
- if ($line !~ /\bconst\b/ &&
+ if ($line !~ /\bconst\b/ && $const_structs ne "" &&
$line =~ /\bstruct\s+($const_structs)\b(?!\s*\{)/) {
WARN("CONST_STRUCT",
"struct $1 should normally be const\n" .
$herecurr);
[tip: perf/core] perf/x86: Make more stuff static
The following commit has been merged into the perf/core branch of tip:
Commit-ID: d9f3b450f206332b7ef3d78b5a85b6c20ad00fd2
Gitweb:
https://git.kernel.org/tip/d9f3b450f206332b7ef3d78b5a85b6c20ad00fd2
Author:Valdis Klētnieks
AuthorDate:Thu, 08 Aug 2019 13:44:02 -04:00
Committer: Ingo Molnar
CommitterDate: Tue, 03 Sep 2019 09:22:32 +02:00
perf/x86: Make more stuff static
When building with C=2, sparse makes note of a number of things:
arch/x86/events/intel/rapl.c:637:30: warning: symbol 'rapl_attr_update' was
not declared. Should it be static?
arch/x86/events/intel/cstate.c:449:30: warning: symbol 'core_attr_update' was
not declared. Should it be static?
arch/x86/events/intel/cstate.c:457:30: warning: symbol 'pkg_attr_update' was
not declared. Should it be static?
arch/x86/events/msr.c:170:30: warning: symbol 'attr_update' was not declared.
Should it be static?
arch/x86/events/intel/lbr.c:276:1: warning: symbol 'lbr_from_quirk_key' was
not declared. Should it be static?
And they can all indeed be static.
Signed-off-by: Valdis Kletnieks
Signed-off-by: Peter Zijlstra (Intel)
Cc: Alexander Shishkin
Cc: Arnaldo Carvalho de Melo
Cc: Jiri Olsa
Cc: Linus Torvalds
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Link: https://lkml.kernel.org/r/128059.1565286242@turing-police
Signed-off-by: Ingo Molnar
---
arch/x86/events/intel/cstate.c | 4 ++--
arch/x86/events/intel/lbr.c| 2 +-
arch/x86/events/intel/rapl.c | 2 +-
arch/x86/events/msr.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/x86/events/intel/cstate.c b/arch/x86/events/intel/cstate.c
index 688592b..db498b5 100644
--- a/arch/x86/events/intel/cstate.c
+++ b/arch/x86/events/intel/cstate.c
@@ -446,7 +446,7 @@ static int cstate_cpu_init(unsigned int cpu)
return 0;
}
-const struct attribute_group *core_attr_update[] = {
+static const struct attribute_group *core_attr_update[] = {
_cstate_core_c1,
_cstate_core_c3,
_cstate_core_c6,
@@ -454,7 +454,7 @@ const struct attribute_group *core_attr_update[] = {
NULL,
};
-const struct attribute_group *pkg_attr_update[] = {
+static const struct attribute_group *pkg_attr_update[] = {
_cstate_pkg_c2,
_cstate_pkg_c3,
_cstate_pkg_c6,
diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 6f814a2..ea54634 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -273,7 +273,7 @@ static inline bool lbr_from_signext_quirk_needed(void)
return !tsx_support && (lbr_desc[lbr_format] & LBR_TSX);
}
-DEFINE_STATIC_KEY_FALSE(lbr_from_quirk_key);
+static DEFINE_STATIC_KEY_FALSE(lbr_from_quirk_key);
/* If quirk is enabled, ensure sign extension is 63 bits: */
inline u64 lbr_from_signext_quirk_wr(u64 val)
diff --git a/arch/x86/events/intel/rapl.c b/arch/x86/events/intel/rapl.c
index 64ab51f..f34b949 100644
--- a/arch/x86/events/intel/rapl.c
+++ b/arch/x86/events/intel/rapl.c
@@ -634,7 +634,7 @@ static void cleanup_rapl_pmus(void)
kfree(rapl_pmus);
}
-const struct attribute_group *rapl_attr_update[] = {
+static const struct attribute_group *rapl_attr_update[] = {
_events_cores_group,
_events_pkg_group,
_events_ram_group,
diff --git a/arch/x86/events/msr.c b/arch/x86/events/msr.c
index 9431447..5812e87 100644
--- a/arch/x86/events/msr.c
+++ b/arch/x86/events/msr.c
@@ -167,7 +167,7 @@ static const struct attribute_group *attr_groups[] = {
NULL,
};
-const struct attribute_group *attr_update[] = {
+static const struct attribute_group *attr_update[] = {
_aperf,
_mperf,
_pperf,
Re: [PATCH] drivers/staging/exfat - by default, prohibit mount of fat/vfat
On Mon, 02 Sep 2019 17:25:24 +0200, Greg Kroah-Hartman said: > I dug up my old discussion with the current vfat maintainer and he said > something to the affect of, "leave the existing code alone, make a new > filesystem, I don't want anything to do with exfat". > > And I don't blame them, vfat is fine as-is and stable and shouldn't be > touched for new things. > > We can keep non-vfat filesystems from being mounted with the exfat > codebase, and make things simpler for everyone involved. Ogawa: Is this still your position, that you want exfat to be a separate module? pgpviGRDDf6sR.pgp Description: PGP signature
Re: [PATCH] drivers/staging/exfat - by default, prohibit mount of fat/vfat
On Mon, 02 Sep 2019 08:43:29 +1000, Dave Chinner said: > I don't know the details of the exfat spec or the code to know what > the best approach is. I've worked fairly closely with Christoph for > more than a decade - you need to think about what he says rather > than /how he says it/ because there's a lot of thought and knowledge > behind his reasoning. Hence if I were implementing exfat and > Christoph was saying "throw it away and extend fs/fat" > then that's what I'd be doing. Again, I'm not ruling that out if that's the consensus direction. After all, the goal is to merge a working driver - and for that, I need to produce something that the file system maintainers will be willing to merge, which means doing it in a way they want it... Hopefully next week a few other people will weigh in with what they prefer as far as approach goes. Only definite statement I've heard so far was Christoph's... > and we don't want more. Implementing exfat on top of fs/fat kills > two birds with one stone - it modernises the fs/fat code base and > brings new functionality that will have more developers interested > in maintaining it over the long term. Any recommendations on how to approach that? Clone the current fs/fat code and develop on top of that, or create a branch of it and on occasion do the merging needed to track further fs/fat development? Mostly asking for workflow suggestions - what's known to work well for this sort of situation, where we know we won't be merging until we have several thousand lines of new code? And any "don't do or you'll regret it later" advice is also appreciated. :) pgp5MXKs2UlE7.pgp Description: PGP signature
Re: [PATCH] drivers/staging/exfat - by default, prohibit mount of fat/vfat
On Sun, 01 Sep 2019 11:07:21 +1000, Dave Chinner said: > Totally irrelevant to the issue at hand. You can easily co-ordinate > out of tree contributions through a github tree, or a tree on > kernel.org, etc. Well.. I'm not personally wedded to the staging tree. I'm just interested in getting a driver done and upstreamed with as little pain as possible. :) Is there any preference for github versus kernel.org? I can set up a github tree on my own, no idea who needs to do what for a kernel.org tree. Also, this (from another email of yours) was (at least to me) the most useful thing said so far: > look at what other people have raised w.r.t. to that filesystem - > on-disk format validation, re-implementation of largely generic > code, lack of namespacing of functions leading to conflicts with > generic/VFS functionality, etc. All of which are now on the to-do list, thanks. Now one big question: Should I heave all the vfat stuff overboard and make a module that *only* does exfat, or is there enough interest in an extended FAT module that does vfat and extfat, in which case the direction should be to re-align this module's code with vfat? > That's the choice you have to make now: listen to the reviewers > saying "resolve the fundamental issues before goign any further", Well... *getting* a laundry list of what the reviewers see as the fundamental issues is the first step in resolving them ;) pgp9pn06SBf1i.pgp Description: PGP signature
Re: [PATCH] drivers/staging/exfat - by default, prohibit mount of fat/vfat
On Sat, 31 Aug 2019 17:24:47 +0300, Andy Shevchenko said: > Side note: > > % git shortlog -n --no-merges -- fs/ext4 | grep '^[^ ]' > > kinda faster and groups by name. Thanks... I rarely do statistical analyses of this sort of thing, so 'git shortlog' isn't on my list of most-used git subcommands... pgp9lSc7nJfbt.pgp Description: PGP signature
Re: [PATCH] staging: exfat: add exfat filesystem code to staging
On Sat, 31 Aug 2019 07:54:10 +1000, Dave Chinner said: > The correct place for new filesystem review is where all the > experienced filesystem developers hang out - that's linux-fsdevel, > not the driver staging tree. So far everything's been cc'ed to linux-fsdevel, which has been spending more time discussing unlikely() usage in a different filesystem. pgpEuWl9W6Eg_.pgp Description: PGP signature
Re: [PATCH] staging: exfat: remove redundant goto
On Fri, 30 Aug 2019 19:15:23 +0100, Colin King said:
> From: Colin Ian King
>
> The goto after a return is never executed, so it is redundant and can
> be removed.
>
> Addresses-Coverity: ("Structurally dead code")
> Signed-off-by: Colin Ian King
Good catch
> - if (dentry < -1) {
> + if (dentry < -1)
> return FFS_NOTFOUND;
> - goto out;
> - }
But the wrong fix. The code *used* to have returns like this all over the
place, but that meant it returns with a lock held - whoops. The *other* 287 or
so places I changed to 'ret = FFS_yaddayadda', followed by a 'goto out' but I
apparently missed one.
And thanks a bunch for feeding it to Coverity :)
pgp8BgZzsRFoU.pgp
Description: PGP signature
Re: [PATCH] drivers/staging/exfat - by default, prohibit mount of fat/vfat
On Fri, 30 Aug 2019 23:46:16 -0700, Christoph Hellwig said:
> Since when did Linux kernel submissions become "show me a better patch"
> to reject something obviously bad?
Well, do you even have a *suggestion* for a better idea? Other than "just rip
it out"? Keeping in mind that:
> As I said the right approach is to probably (pending comments from the
> actual fat maintainer) to merge exfat support into the existing fs/fat/
> codebase. You obviously seem to disagree (and at the same time not).
At this point, there isn't any true consensus on whether that's the best
approach at the current. "Just rip it out" prevents following some directions
that people have voiced interest in. Now, if there was consensus that we
wanted a separate module that only did exfat, you'd be correct.
And just for the record, I've been around since the 2.5.47 or so kernel, and
I've seen *plenty* of times when somebody has submitted a better alternative
patch.
> But using this as a pre-text of adding a non-disabled second fat16/32
> implementation and actually allowing that to build with no reason is
> just not how it works.
Well.. let's see... It won't build unless you select CONFIG_STAGING. *And* you
select CONFIG_EXFAT_FS. *And* it wont mount anything other than exfat
unless you change CONFIG_EXFAT_DONT_MOUNT_VFAT
I think at that point, we can safely say that if it mounts a vfat filesystem,
it's because the person building the kernel has gone out of their way to
request that it do so.
Either that, or we have a major bug in kbuild. In general, kbuild doesn't build
things "for no reason".
Now, if what you want is "Please make it so the fat16/fat32 code is in separate
files that aren't built unless requested", that's in fact doable and a
reasonable request, and one that both doesn't conflict with anything other
directions we might want to go, and also prepares the code for more easy
separation if it's decided we really do want an exfat-only module.
> > > done. Given that you signed up as the maintainer for this what is your
> > > plan forward on it? What development did you on the code and what are
> > > your next steps?
> >
> > Well, the *original* plan was to get it into the tree someplace so it can
> > get
> > review and updates from others.
>
> In other words you have no actual plan and no idea what to do and want to
> rely on "others" to do anything, but at the same time reject the
> comments from others how to do things right?
So far, the only comment I've rejected is one "just rip it out" that conflicts
with
directions that others have indicated we may want to pursue.
And by the way, it seems like other filesystems rely on "others" to help out.
Let's look at the non-merge commit for fs/ext4. And these are actual patches,
not just reviewer comments
(For those who don't feel like scrolling - 77.6% of the non-merge ext4 commits
are from a total of 463 somebodies other than Ted Ts'o)
Even some guy named Christoph Hellwig gave Ted Ts'o a bunch of help.
[/usr/src/linux-next] git log -- fs/ext4 | awk '/^commit/ {merge=0} /^Merge: /
{merge=1} /^Author:/ { if (!merge) {print $0}}' | sort | uniq -c | sort -t: -k
1,1nr -k 2
718 Author: Theodore Ts'o
260 Author: Jan Kara
151 Author: Aneesh Kumar K.V
120 Author: Eric Sandeen
119 Author: Lukas Czerner
99 Author: Dmitry Monakhov
84 Author: Al Viro
67 Author: Eric Biggers
63 Author: Tao Ma
61 Author: Yongqiang Yang
49 Author: Zheng Liu
47 Author: Christoph Hellwig
42 Author: Darrick J. Wong
40 Author: Tahsin Erdogan
36 Author: Mingming Cao
32 Author: Eric Whitney
28 Author: Christoph Hellwig
27 Author: Curt Wohlgemuth
24 Author: Darrick J. Wong
22 Author: Akira Fujita
20 Author: Ross Zwisler
18 Author: Eryu Guan
16 Author: Vasily Averin
15 Author: Akinobu Mita
15 Author: Allison Henderson
15 Author: Robin Dong
14 Author: Dan Carpenter
13 Author: Michael Halcrow
13 Author: Miklos Szeredi
13 Author: Wang Shilong
12 Author: Daeho Jeong
12 Author: Jan Kara
12 Author: Joe Perches
12 Author: Tejun Heo
11 Author: Jiaying Zhang
11 Author: Namjae Jeon
11 Author: Shen Feng
10 Author: David Howells
10 Author: Guo Chao
10 Author: Matthew Wilcox
9 Author: Alexey Dobriyan
9 Author: Andreas Gruenbacher
9 Author: Fabian Frederick
9 Author: Linus Torvalds
8 Author: Amir Goldstein
8 Author: Amir Goldstein
8 Author: Andrew Morton
8 Author: Artem Bityutskiy
8 Author: Dan Williams
7 Author: Aditya Kali
7 Author: Al Viro
7 Author: Alex Tomas
7 Author: Arnd Bergmann
7 Author: Colin Ian King
7 Author: Dave Jiang
7 Author: Hugh Dickins
7 Author: Kazuya Mio
7 Author: Toshiyuki Okajima
7 Author: Zheng Liu
7 Author:
Re: [PATCH] drivers/staging/exfat - by default, prohibit mount of fat/vfat
On Fri, 30 Aug 2019 09:45:03 -0700, Christoph Hellwig said: > On Fri, Aug 30, 2019 at 12:42:39PM -0400, Valdis KlÄtnieks wrote: > > Concerns have been raised about the exfat driver accidentally mounting > > fat/vfat file systems. Add an extra configure option to help prevent that. > > Just remove that code. This is exactly what I fear about this staging > crap, all kinds of half-a***ed patches instead of trying to get anything Explain how it's half-a**ed. You worry about accidental mounting, meanwhile down in the embedded space there are memory-constrained machines that don't want separate vfat and exfat drivers sitting around in memory. If you have a better patch that addresses both concerns, feel free to submit it. > done. Given that you signed up as the maintainer for this what is your > plan forward on it? What development did you on the code and what are > your next steps? Well, the *original* plan was to get it into the tree someplace so it can get review and updates from others. Given the amount of press the Microsoft announcement had, we were *hoping* there would be some momentum and people actually looking at the code and feeding me patches. I've gotten a half dozen already today Although if you prefer, it can just sit out-of-tree until I've got a perfect driver without input or review from anybody. But I can't think of *any* instance where that model has actually worked. pgp5SKFaJ926z.pgp Description: PGP signature
Re: linux-next: Tree for Aug 30 (exfat)
On Fri, 30 Aug 2019 09:52:15 -0700, Randy Dunlap said: > on x86_64: > when CONFIG_VFAT_FS is not set/enabled: > > ../drivers/staging/exfat/exfat_super.c:46:41: error: > �CONFIG_FAT_DEFAULT_IOCHARSET� undeclared here (not in a function); did you > mean �CONFIG_EXFAT_DEFAULT_IOCHARSET�? Thanks. I'll fix this tonight pgpxe_9ABOc_H.pgp Description: PGP signature
[PATCH] drivers/staging/exfat - by default, prohibit mount of fat/vfat
Concerns have been raised about the exfat driver accidentally mounting
fat/vfat file systems. Add an extra configure option to help prevent that.
Suggested-by: Christoph Hellwig
Signed-off-by: Valdis Kletnieks
diff --git a/drivers/staging/exfat/Kconfig b/drivers/staging/exfat/Kconfig
index 78b32aa2ca19..1df177b1dc72 100644
--- a/drivers/staging/exfat/Kconfig
+++ b/drivers/staging/exfat/Kconfig
@@ -4,6 +4,14 @@ config EXFAT_FS
help
This adds support for the exFAT file system.
+config EXFAT_DONT_MOUNT_VFAT
+ bool "Prohibit mounting of fat/vfat filesysems by exFAT"
+ default y
+ help
+ By default, the exFAT driver will only mount exFAT filesystems, and
refuse
+ to mount fat/vfat filesystems. Set this to 'n' to allow the exFAT
driver
+ to mount these filesystems.
+
config EXFAT_DISCARD
bool "enable discard support"
depends on EXFAT_FS
diff --git a/drivers/staging/exfat/exfat_super.c
b/drivers/staging/exfat/exfat_super.c
index 5b5c2ca8c9aa..7fdb5b8bc928 100644
--- a/drivers/staging/exfat/exfat_super.c
+++ b/drivers/staging/exfat/exfat_super.c
@@ -486,10 +486,16 @@ static int ffsMountVol(struct super_block *sb)
break;
if (i < 53) {
+#ifdef CONFIG_EXFAT_DONT_MOUNT_VFAT
+ ret = -EINVAL;
+ printk(KERN_INFO "EXFAT: Attempted to mount VFAT filesystem\n");
+ goto out;
+#else
if (GET16(p_pbr->bpb + 11)) /* num_fat_sectors */
ret = fat16_mount(sb, p_pbr);
else
ret = fat32_mount(sb, p_pbr);
+#endif
} else {
ret = exfat_mount(sb, p_pbr);
}
Re: [PATCH] staging: exfat: add exfat filesystem code to staging
On Thu, 29 Aug 2019 22:56:31 +0200, Pali Roh�r said: > I'm not really sure if this exfat implementation is fully suitable for > mainline linux kernel. > > In my opinion, proper way should be to implement exFAT support into > existing fs/fat/ code instead of replacing whole vfat/msdosfs by this > new (now staging) fat implementation. > In linux kernel we really do not need two different implementation of > VFAT32. This patch however does have one major advantage over "patch vfat to support exfat" - which is that the patch exists. If somebody comes forward with an actual "extend vfat to do exfat" patch, we should at that point have a discussion about relative merits pgp7mhV7YWTlo.pgp Description: PGP signature
Re: [PATCH] staging: exfat: add exfat filesystem code to staging
On Thu, 29 Aug 2019 14:14:35 +0200, Pali Roh�r said: > On Wednesday 28 August 2019 18:08:17 Greg Kroah-Hartman wrote: > > The full specification of the filesystem can be found at: > > https://docs.microsoft.com/en-us/windows/win32/fileio/exfat-specification > > This is not truth. This specification is not "full". There are missing > important details, like how is TexFAT implemented. Well..given that the spec says it's an extension used by Windows CE... > 1.5 Windows CE and TexFAT > TexFAT is an extension to exFAT that adds transaction-safe operational > semantics on top of the base file system. TexFAT is used by Windows CE. TexFAT > requires the use of the two FATs and allocation bitmaps for use in > transactions. It also defines several additional structures including padding > descriptors and security descriptors. And these two pieces of info: > 3.1.13.1 ActiveFat Field > The ActiveFat field shall describe which FAT and Allocation Bitmap are active > (and implementations shall use), as follows: > 0, which means the First FAT and First Allocation Bitmap are active > 1, which means the Second FAT and Second Allocation Bitmap are active and is > possible only when the NumberOfFats field contains the value 2 > Implementations shall consider the inactive FAT and Allocation Bitmap as > stale. > Only TexFAT-aware implementations shall switch the active FAT and Allocation > Bitmaps (see Section 7.1). > 3.1.16 NumberOfFats Field > The NumberOfFats field shall describe the number of FATs and Allocation > Bitmaps > the volume contains. > The valid range of values for this field shall be: > 1, which indicates the volume only contains the First FAT and First > Allocation Bitmap > 2, which indicates the volume contains the First FAT, Second FAT, First > Allocation Bitmap, and Second Allocation Bitmap; this value is only valid for > TexFAT volumes I think we're OK if we just set ActiveFat to 0 and NumberOfFats to 1. Unless somebody has actual evidence of a non-WindowsCE extfat that has NumberOfFats == 2 pgpzRQ8Fc3Jm7.pgp Description: PGP signature
Re: next-20190826 - objtool fails to build.
On Wed, 28 Aug 2019 14:51:00 -0500, Josh Poimboeuf said: > > The real question then becomes - should the Makefile sanitize CFLAGS or just > > append to whatever the user supplied as it does currently? The rest of the > > tree > > sanitizes CFLAG, because I don't get deluged in -Wsign-compare warnings all > > over the place... > > Agreed. I assume this fixes it? > > diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile > index 88158239622b..20f67fcf378d 100644 > -CFLAGS += -Werror $(WARNINGS) $(KBUILD_HOSTCFLAGS) -g $(INCLUDES) > $(LIBELF_FLAGS) > +CFLAGS := -Werror $(WARNINGS) $(KBUILD_HOSTCFLAGS) -g $(INCLUDES) > $(LIBELF_FLAGS) Yep, thanks. Feel free to stick these on the final version: Reported-by: Valdis Kletnieks Tested-by: Valdis Kletnieks
Re: next-20190826 - objtool fails to build.
On Wed, 28 Aug 2019 10:10:04 -0500, Josh Poimboeuf said: > But I don't see how those warnings could get enabled: -Wsign-compare > -Wunused-parameter. > > Can you "make clean" and do "make V=1 tools/objtool" to show the actual > flags? And that tells me those warnings in fact don't get specifically enabled. (I've added some line breaks for sanity) gcc -Wp,-MD,/usr/src/linux-next/tools/objtool/.special.o.d -Wp,-MT,/usr/src/linux-next/tools/objtool/special.o -O2 -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wbad-function-cast Found the cause of the mystery - I changed something in a bash profile, and as a result... export CFLAGS="-O2 -D_FORTIFY_SOURCE=2 -Wall -Wextra" And -Wextra pulls in the things that cause problems. So this is mostly self-inflicted. The real question then becomes - should the Makefile sanitize CFLAGS or just append to whatever the user supplied as it does currently? The rest of the tree sanitizes CFLAG, because I don't get deluged in -Wsign-compare warnings all over the place... pgpLq0_j7jazf.pgp Description: PGP signature
Re: [PATCHv5] drivers/amba: add reset control to amba bus probe
On Wed, 28 Aug 2019 08:34:20 -0500, Dinh Nguyen said: > > Does this DTRT for both old and new U-Boots? My naive reading of > > this patch > > What is a DTRT? Do The Right Thing, sorry... > > says on an old U-Boot, we end up attempting to bring it out of > > reset even though they had already been brought out. > > > > If the peripheral is already out of reset, de-asserting the reset has > no affect. OK, thanks. There's been hardware where doing that sort of thing twice will confuse the device and cause issues. pgpf3ae3l4gWA.pgp Description: PGP signature
next-20190826 - objtool fails to build.
OK. I'm mystified. next-20190806 built fine. -0818 and -0826 died a
glorious death indeed. All 3 were build using the same Fedora Rawhide 9.1.1
compiler (installed on July 30). 'git log -- tools/objtool' comes up empty.
Local hack-around was to remove the -Werror from tools/objtool/Makefile
Am particularly mystified by the errors on lines 808/809 - the same compiler
that
was happy with the same code in next-0806 is now upset. The others could
possibly
be a stray typedef in an include someplace, but those two errors have me
stumped.
For that matter, I'm not even seeing how -Wsign-compare was even getting set,
given
a command 'make' and not W=1 or W=2
(cue twilight zone theme)
CALLscripts/checksyscalls.sh
CALLscripts/atomic/check-atomics.sh
DESCEND objtool
HOSTCC /usr/src/linux-next/tools/objtool/fixdep.o
HOSTLD /usr/src/linux-next/tools/objtool/fixdep-in.o
LINK /usr/src/linux-next/tools/objtool/fixdep
CC /usr/src/linux-next/tools/objtool/exec-cmd.o
CC /usr/src/linux-next/tools/objtool/help.o
CC /usr/src/linux-next/tools/objtool/pager.o
CC /usr/src/linux-next/tools/objtool/parse-options.o
CC /usr/src/linux-next/tools/objtool/run-command.o
CC /usr/src/linux-next/tools/objtool/sigchain.o
CC /usr/src/linux-next/tools/objtool/subcmd-config.o
LD /usr/src/linux-next/tools/objtool/libsubcmd-in.o
AR /usr/src/linux-next/tools/objtool/libsubcmd.a
CC /usr/src/linux-next/tools/objtool/builtin-check.o
CC /usr/src/linux-next/tools/objtool/builtin-orc.o
CC /usr/src/linux-next/tools/objtool/check.o
check.c: In function '__dead_end_function':
check.c:158:17: warning: comparison of integer expressions of different
signedness: 'int' and 'long unsigned int' [-Wsign-compare]
158 | for (i = 0; i < ARRAY_SIZE(global_noreturns); i++)
| ^
check.c: In function 'add_dead_ends':
check.c:330:25: warning: comparison of integer expressions of different
signedness: 'int' and 'unsigned int' [-Wsign-compare]
330 | else if (rela->addend == rela->sym->sec->len) {
| ^~
check.c:372:25: warning: comparison of integer expressions of different
signedness: 'int' and 'unsigned int' [-Wsign-compare]
372 | else if (rela->addend == rela->sym->sec->len) {
| ^~
check.c: In function 'add_jump_destinations':
check.c:560:36: warning: comparison of integer expressions of different
signedness: 'long unsigned int' and 'int' [-Wsign-compare]
560 | if (insn->ignore || insn->offset == FAKE_JUMP_OFFSET)
|^~
check.c: In function 'handle_jump_alt':
check.c:808:49: warning: unused parameter 'file' [-Wunused-parameter]
808 | static int handle_jump_alt(struct objtool_file *file,
|~^~~~
check.c:809:27: warning: unused parameter 'special_alt' [-Wunused-parameter]
809 | struct special_alt *special_alt,
| ^~~
check.c: In function 'add_jump_table':
check.c:925:20: warning: comparison of integer expressions of different
signedness: 'int' and 'long unsigned int' [-Wsign-compare]
925 | rela->addend == pfunc->offset)
|^~
check.c: In function 'read_unwind_hints':
check.c:1187:16: warning: comparison of integer expressions of different
signedness: 'int' and 'long unsigned int' [-Wsign-compare]
1187 | for (i = 0; i < sec->len / sizeof(struct unwind_hint); i++) {
|^
CC /usr/src/linux-next/tools/objtool/orc_gen.o
CC /usr/src/linux-next/tools/objtool/orc_dump.o
orc_dump.c: In function 'orc_dump':
orc_dump.c:104:16: warning: comparison of integer expressions of different
signedness: 'int' and 'size_t' {aka 'long unsigned int'} [-Wsign-compare]
104 | for (i = 0; i < nr_sections; i++) {
|^
CC /usr/src/linux-next/tools/objtool/elf.o
elf.c: In function 'find_section_by_index':
elf.c:41:16: warning: comparison of integer expressions of different
signedness: 'int' and 'unsigned int' [-Wsign-compare]
41 | if (sec->idx == idx)
|^~
elf.c: In function 'read_sections':
elf.c:148:16: warning: comparison of integer expressions of different
signedness: 'int' and 'size_t' {aka 'long unsigned int'} [-Wsign-compare]
148 | for (i = 0; i < sections_nr; i++) {
|^
elf.c: In function 'read_relas':
elf.c:370:17: warning: comparison of integer expressions of different
signedness: 'int' and 'Elf64_Xword' {aka 'long unsigned int'} [-Wsign-compare]
370 | for (i = 0; i < sec->sh.sh_size / sec->sh.sh_entsize; i++) {
| ^
CC /usr/src/linux-next/tools/objtool/special.o
special.c: In function 'get_alt_entry':
special.c:71:38: warning: unused parameter 'elf' [-Wunused-parameter]
71 | static int get_alt_entry(struct elf *elf, struct
Re: [PATCHv5] drivers/amba: add reset control to amba bus probe
On Mon, 26 Aug 2019 10:42:52 -0500, Dinh Nguyen said: > The primecell controller on some SoCs, i.e. SoCFPGA, is held in reset by > default. Until recently, the DMA controller was brought out of reset by the > bootloader(i.e. U-Boot). But a recent change in U-Boot, the peripherals > that are not used are held in reset and are left to Linux to bring them > out of reset. > > Add a mechanism for getting the reset property and de-assert the primecell > module from reset if found. This is a not a hard fail if the reset properti > is not present in the device tree node, so the driver will continue to > probe. Does this DTRT for both old and new U-Boots? My naive reading of this patch says on an old U-Boot, we end up attempting to bring it out of reset even though they had already been brought out. pgpoksGuYfPUv.pgp Description: PGP signature
Re: [PATCH] arch/x86/kernel/cpu/umwait.c - remove unused variable
On Thu, 08 Aug 2019 22:32:38 +0200, Thomas Gleixner said: > Care to resend the last few with fixed subject lines, so I don't have to > clean them up? Will do that later this evening... > The right thing to do is to have a cpu_offline callback which clears the > umwait MSR. That covers also the failure in the cpu hotplug setup. Then > handling an error return makes sense and keeps everything in a workable > shape. OK, in that case, toss the umwait patch for now pgpCWY7sbFom_.pgp Description: PGP signature
Re: [PATCH] arch/x86/kernel/cpu/umwait.c - remove unused variable
On Thu, 08 Aug 2019 22:04:03 +0200, Thomas Gleixner said: > I really appreciate your work, but can you please refrain from using file > names as prefixes? OK, will do so going forward.. > > We get a warning when building with W=1: > > Please avoid 'We/I' in changelogs. OK.. > > CC arch/x86/kernel/cpu/umwait.o > > arch/x86/kernel/cpu/umwait.c: In function 'umwait_init': > > arch/x86/kernel/cpu/umwait.c:183:6: warning: variable 'ret' set but not > > used [-Wunused-but-set-variable] > > 183 | int ret; > > | ^~~ > > > > And indeed, we don't do anything with it, so clean it up. > > Well, the question is whether removing the variable is the right thing to > do. > If that fails then umwait is broken. So instead of removing it, this should > actually check the return code and act accordingly. Fenghua? [/usr/src/linux-next] git grep umwait_init arch/x86/kernel/cpu/umwait.c:static int __init umwait_init(void) arch/x86/kernel/cpu/umwait.c:device_initcall(umwait_init); It isn't clear that whatever is doing the device_initcall()s will be able to do any reasonable recovery if we return an error, so any error recovery is going to have to happen before the function returns. It might make sense to do an 'if (ret) return;' before going further in the function, but given the comment a few lines further down about ignoring errors, it was apparently considered more important to struggle through and register stuff in sysfs even if umwait was broken pgpQlntWlfAlm.pgp Description: PGP signature
[PATCH] arch/x86/kernel/cpu/common.c - add proper prototypes
When building withW=1, we get a warning.. CC arch/x86/kernel/cpu/common.o arch/x86/kernel/cpu/common.c:1952:6: warning: no previous prototype for 'arch_smt_update' [-Wmissing-prototypes] 1952 | void arch_smt_update(void) | ^~~ Provide the proper #include so the prototype is found. Signed-off-by: Valdis Kletnieks diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index e0489d2860d3..b8ed6d8e55df 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include
