Re: The linux devs can rescind their license grant. - Additional restrictive terms
Version 2 of the GPL forbids the incorporation of additional restrictive terms, relating to the distribution, modification, etc of the article licensed under the terms. Those that violate this section are declared, by operation of the terms, to have their grant automatically revoked. An additional term need-not be present in the same writing. Such terms simply need to be present to or made known to the taker(sub-licensee) by the distributor. They may be proffered in writing, orally, or implied in the course of doing business dealings. They simply must relate back or involve the article in question (the licensed code or product.) The proffering of additional restrictive terms is a violation of the text of the license grant in and of itself. Here we have a situation where an additional writing has been proffered. The additional writing promises both in it's own text and by implication consequences against those who violate the terms of this additional writing. The additional writing restricts those subject to it from expressing certain views publicly - promising retribution against those who do. No consideration is paid to those subject to the additional writing for their assent; it is simply imposed unilaterally against the subjects. The violators of the additional writing are promised: Additional, unwanted, public scrutiny (to which they were not subject to prior) Public ridicule. Loss of public standing. as-well as an implied loss of future income. These are the enforcement mechanisms of the additional writing to enforce its restrictions against those who publish derivative works of the kernel. The additional writing is activated when (with the prerequisite of being a derivative work of the linux kernel) the work of a rights-holder is incorporated into the kernel, when such a work is made known to the kernel-team to exist where any one person on this earth has seen fit to present it for inclusion, or by simple prior-inclusion into the kernel. Thus all current and past rights-holders who have code in, or have published for distribution, derivative works of the kernel are subject to the retributive promises made to them in the additional writing, drafted to restrict their actions and utterances. This is tantamount to an additional restrictive term regarding the modification and distribution of works under the linux kernel license grant. It is a violation of the license terms of the rights-holders past incorporated works in much the same way that GRSecurity's Contributor Access Agreement was and is.
Re: The linux devs can rescind their license grant. - Additional restrictive terms
Version 2 of the GPL forbids the incorporation of additional restrictive terms, relating to the distribution, modification, etc of the article licensed under the terms. Those that violate this section are declared, by operation of the terms, to have their grant automatically revoked. An additional term need-not be present in the same writing. Such terms simply need to be present to or made known to the taker(sub-licensee) by the distributor. They may be proffered in writing, orally, or implied in the course of doing business dealings. They simply must relate back or involve the article in question (the licensed code or product.) The proffering of additional restrictive terms is a violation of the text of the license grant in and of itself. Here we have a situation where an additional writing has been proffered. The additional writing promises both in it's own text and by implication consequences against those who violate the terms of this additional writing. The additional writing restricts those subject to it from expressing certain views publicly - promising retribution against those who do. No consideration is paid to those subject to the additional writing for their assent; it is simply imposed unilaterally against the subjects. The violators of the additional writing are promised: Additional, unwanted, public scrutiny (to which they were not subject to prior) Public ridicule. Loss of public standing. as-well as an implied loss of future income. These are the enforcement mechanisms of the additional writing to enforce its restrictions against those who publish derivative works of the kernel. The additional writing is activated when (with the prerequisite of being a derivative work of the linux kernel) the work of a rights-holder is incorporated into the kernel, when such a work is made known to the kernel-team to exist where any one person on this earth has seen fit to present it for inclusion, or by simple prior-inclusion into the kernel. Thus all current and past rights-holders who have code in, or have published for distribution, derivative works of the kernel are subject to the retributive promises made to them in the additional writing, drafted to restrict their actions and utterances. This is tantamount to an additional restrictive term regarding the modification and distribution of works under the linux kernel license grant. It is a violation of the license terms of the rights-holders past incorporated works in much the same way that GRSecurity's Contributor Access Agreement was and is.
[GIT PULL] C-SKY(csky) Port for Linux 4.20
The following changes since commit 84df9525b0c27f3ebc2ebb1864fa62a97fdedb7d: Linux 4.19 (2018-10-22 07:37:37 +0100) are available in the git repository at: https://github.com/c-sky/csky-linux.git tags/csky-for-linus-4.20 for you to fetch changes up to 2347e7e1aea410865e3c3f92014b07ff7d4c5b02: dt-bindings: interrupt-controller: C-SKY APB intc (2018-10-26 00:54:31 +0800) This tag contains the Linux port for C-SKY(csky) based on linux-4.19 Release, which has been through 10 rounds of review on mailing list. We almost got the Acked-by/Reviewed-by of all patches except "Process management and Signal", but all've been tested. Here is the LTP-20180118 test report: --- Total Tests: 1298 Total Skipped Tests: 281 Total Failures: 10 Kernel Version: 4.19.0+ Machine Architecture: csky Hostname: buildroot --- This patchset adds architecture support to Linux for C-SKY's 32-bit embedded There are two ABI versions with several CPU cores in this patchset: ABIv1: 610 (16-bit instruction, 32-bit data path, VIPT Cache ...) ABIv2: 807 810 860 (16/32-bit variable length instruction, PIPT Cache, SMP ...) More information: http://en.c-sky.com The development repo: https://github.com/c-sky/csky-linux ABI Documentation: https://github.com/c-sky/csky-doc Here is the pre-built cross compiler for fast test from our CI: https://gitlab.com/c-sky/buildroot/-/jobs/101608095/artifacts/file/output/images/csky_toolchain_qemu_csky_ck807f_4.18_glibc_defconfig_482b221e52908be1c9b2ccb444255e1562bb7025.tar.xz We use buildroot as our CI-test enviornment. "LTP, Lmbench ..." will be tested for every commit. See here for more details: https://gitlab.com/c-sky/buildroot/pipelines We'll continouslly improve csky subsystem in future. Changes in v10: - Remove duplicated headers in asm/Kbuild and uapi/asm/Kbuild. - Change to (__NR_arch_specific_syscall + 1) in unistd.h. - Drop dword access for get_user_size patch. - Involve the interrupt controller drivers after got Reviewed-by. Changes in v9: - Remove unused code in smp.c and use per_cpu for ipi_data. - Fixup r15 register access in abiv1/alignment.c. - Improve the changelog comment in commit-msg. Changes in v8: - Pass make allmodconfig. - Implement abiv1 get_user_dword(). - Remove set_irq_mapping() used by driver in smp.c. Changes in v7: - Use checkpatch.pl to check all patches and fixup as possible. - Remove github.com/c-sky print in bootup. - Give a return in DMA_ATTR_NON_CONSISTENT in csky_dma_alloc_atomic(). - Remove the NSIGXXX in fpu.c and use force_sig_fault() in fpu.c. - Remove irq.h and add it in asm/Kbuild. - Use byteswap helpers in abiv1/bswapXi.c. - Fixup arch_sync_dma() only with one page problem. Changes in v6: - use asm-generic/bitops/atomic.h for all in asm/bitops.h - fix flush_cache_range and tlb_start_vma - fix compile error with include linux/bug.h in cmpxchg.h - improve the comment Changes in v5: - remove redundant smp_mb operations in spinlock.h - add commit message for dt-bindings docs - add CPUHP_AP_CSKY_TIMER_STARTING in hotplug.h for csky_mptimer - add COMPILE_TEST for timer-gx6605s Kconfig - seperate csky two interrupt controllers with 2 patches - add MAINTAINERS patch for csky - move IPI_IRQ into csky_mptimer, fixup irq_mapping problem - coding convension Changes in v4: - cleanup defconfig - use ksys_ in syscall.c - remove wrong comment in vdso.c - Use GENERIC_IRQ_MULTI_HANDLER - optimize the memset.c - fixup dts warnings - remove big-endian in byteorder.h Changes in v3: dc560f1 csky: change to EM_CSKY 252 for elf.h 2ac3ddf csky: remove gx6605s.dts af00b8c csky: add defconfig and qemu.dts 6c87efb csky: remove the deprecate name. f6dda39 csky: add dt-bindings doc. d9f02a8 csky: remove KERNEL_VERSION in upstream branch 7bd663c csky: Use kernel/dma/noncoherent.c 1544c09 csky: bugfix emmc hang up LINS-976 e963271 csky: cleanup include/asm/Kbuild cd267ba csky: remove CSKY_DEBUG_INFO 78950da csky: remove dcache invalid. 13fe51d csky: remove csum_ipv6_magic(), use generic one. a7372db csky: bugfix CK810 access twice error. 1bb7c69 csky: bugfix add gcc asm memory for barrier. 5ea3257 csky: add -msoft-float instead of -mfloat-abi=soft. 38b037d csky: bugfix losing cache flush range. ab5e8c4 csky: Add ticket-spinlock and qrwlock support. c9aaec5 csky: rename cskyksyms.c to libgcc_ksyms.c 28c5e48 csky: avoid the MB on failure: trylock f929c97 csky: bugfix idly4 may cause exception. 09dc496 csky: Use GENERIC_ASHLDI3/ASHRDI3 etc 6ecc99d csky: optimize smp boot code. 16f50df csky: asm/bug.h simple implement. 0ba532a csky: csky asm/atomic.h added. df66947 csky: asm/compat.h added 275a06f csky: String operations optimization 4c021dd csky: ck860 SMP memory barrier optimize fc39c66 csky: Add wait/doze/stop d005144 csky: add GENERIC_ALLOCATOR 4a10074 csky: bugfix cma
[GIT PULL] C-SKY(csky) Port for Linux 4.20
The following changes since commit 84df9525b0c27f3ebc2ebb1864fa62a97fdedb7d: Linux 4.19 (2018-10-22 07:37:37 +0100) are available in the git repository at: https://github.com/c-sky/csky-linux.git tags/csky-for-linus-4.20 for you to fetch changes up to 2347e7e1aea410865e3c3f92014b07ff7d4c5b02: dt-bindings: interrupt-controller: C-SKY APB intc (2018-10-26 00:54:31 +0800) This tag contains the Linux port for C-SKY(csky) based on linux-4.19 Release, which has been through 10 rounds of review on mailing list. We almost got the Acked-by/Reviewed-by of all patches except "Process management and Signal", but all've been tested. Here is the LTP-20180118 test report: --- Total Tests: 1298 Total Skipped Tests: 281 Total Failures: 10 Kernel Version: 4.19.0+ Machine Architecture: csky Hostname: buildroot --- This patchset adds architecture support to Linux for C-SKY's 32-bit embedded There are two ABI versions with several CPU cores in this patchset: ABIv1: 610 (16-bit instruction, 32-bit data path, VIPT Cache ...) ABIv2: 807 810 860 (16/32-bit variable length instruction, PIPT Cache, SMP ...) More information: http://en.c-sky.com The development repo: https://github.com/c-sky/csky-linux ABI Documentation: https://github.com/c-sky/csky-doc Here is the pre-built cross compiler for fast test from our CI: https://gitlab.com/c-sky/buildroot/-/jobs/101608095/artifacts/file/output/images/csky_toolchain_qemu_csky_ck807f_4.18_glibc_defconfig_482b221e52908be1c9b2ccb444255e1562bb7025.tar.xz We use buildroot as our CI-test enviornment. "LTP, Lmbench ..." will be tested for every commit. See here for more details: https://gitlab.com/c-sky/buildroot/pipelines We'll continouslly improve csky subsystem in future. Changes in v10: - Remove duplicated headers in asm/Kbuild and uapi/asm/Kbuild. - Change to (__NR_arch_specific_syscall + 1) in unistd.h. - Drop dword access for get_user_size patch. - Involve the interrupt controller drivers after got Reviewed-by. Changes in v9: - Remove unused code in smp.c and use per_cpu for ipi_data. - Fixup r15 register access in abiv1/alignment.c. - Improve the changelog comment in commit-msg. Changes in v8: - Pass make allmodconfig. - Implement abiv1 get_user_dword(). - Remove set_irq_mapping() used by driver in smp.c. Changes in v7: - Use checkpatch.pl to check all patches and fixup as possible. - Remove github.com/c-sky print in bootup. - Give a return in DMA_ATTR_NON_CONSISTENT in csky_dma_alloc_atomic(). - Remove the NSIGXXX in fpu.c and use force_sig_fault() in fpu.c. - Remove irq.h and add it in asm/Kbuild. - Use byteswap helpers in abiv1/bswapXi.c. - Fixup arch_sync_dma() only with one page problem. Changes in v6: - use asm-generic/bitops/atomic.h for all in asm/bitops.h - fix flush_cache_range and tlb_start_vma - fix compile error with include linux/bug.h in cmpxchg.h - improve the comment Changes in v5: - remove redundant smp_mb operations in spinlock.h - add commit message for dt-bindings docs - add CPUHP_AP_CSKY_TIMER_STARTING in hotplug.h for csky_mptimer - add COMPILE_TEST for timer-gx6605s Kconfig - seperate csky two interrupt controllers with 2 patches - add MAINTAINERS patch for csky - move IPI_IRQ into csky_mptimer, fixup irq_mapping problem - coding convension Changes in v4: - cleanup defconfig - use ksys_ in syscall.c - remove wrong comment in vdso.c - Use GENERIC_IRQ_MULTI_HANDLER - optimize the memset.c - fixup dts warnings - remove big-endian in byteorder.h Changes in v3: dc560f1 csky: change to EM_CSKY 252 for elf.h 2ac3ddf csky: remove gx6605s.dts af00b8c csky: add defconfig and qemu.dts 6c87efb csky: remove the deprecate name. f6dda39 csky: add dt-bindings doc. d9f02a8 csky: remove KERNEL_VERSION in upstream branch 7bd663c csky: Use kernel/dma/noncoherent.c 1544c09 csky: bugfix emmc hang up LINS-976 e963271 csky: cleanup include/asm/Kbuild cd267ba csky: remove CSKY_DEBUG_INFO 78950da csky: remove dcache invalid. 13fe51d csky: remove csum_ipv6_magic(), use generic one. a7372db csky: bugfix CK810 access twice error. 1bb7c69 csky: bugfix add gcc asm memory for barrier. 5ea3257 csky: add -msoft-float instead of -mfloat-abi=soft. 38b037d csky: bugfix losing cache flush range. ab5e8c4 csky: Add ticket-spinlock and qrwlock support. c9aaec5 csky: rename cskyksyms.c to libgcc_ksyms.c 28c5e48 csky: avoid the MB on failure: trylock f929c97 csky: bugfix idly4 may cause exception. 09dc496 csky: Use GENERIC_ASHLDI3/ASHRDI3 etc 6ecc99d csky: optimize smp boot code. 16f50df csky: asm/bug.h simple implement. 0ba532a csky: csky asm/atomic.h added. df66947 csky: asm/compat.h added 275a06f csky: String operations optimization 4c021dd csky: ck860 SMP memory barrier optimize fc39c66 csky: Add wait/doze/stop d005144 csky: add GENERIC_ALLOCATOR 4a10074 csky: bugfix cma
Re: Linux kernel crash
On Wednesday, October 24, 2018 10:20:05 PM MST Stephen Smith wrote: > > Whenever I run "shutdown -h now" or "reboot" I receive an immediate kernel > crash with a dump that has: > > "Code: Bad RIP value" I Canonical response noted the following from the dump: [ 42.640541] resource sanity check: requesting [mem 0x000c-0x000f], which spans more than PCI Bus :00 [mem 0x000c-0x000d window] I've since updated to the latest BIOS at there suggestion. I see the following on the screen when the kernel crashes: [ 43.115817] WARNING: CPU: 2 PID: 1847 at mm/usercopy.c:81 usercopy_warn+0x81/0xa0 [ 43.115818] Modules linked in: binfmt_misc snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic edac_mce_amd snd_hda_intel kvm_amd kvm snd_hda_codec snd_hda_core irqbypass crct10dif_pclmul crc32_pclmul snd_hwdep snd_pcm snd_seq_midi snd_seq_midi_event ghash_clmulni_intel snd_rawmidi snd_seq pcbc aesni_intel snd_seq_device snd_timer eeepc_wmi asus_wmi sparse_keymap aes_x86_64 joydev snd video crypto_simd cryptd soundcore wmi_bmof glue_helper input_leds wmi k10temp ccp serio_raw nvidia_uvm(POE) mac_hid sch_fq_codel parport_pc ppdev lp parport ip_tables x_tables autofs4 raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor raid6_pq libcrc32c raid0 multipath linear raid1 hid_generic usbhid hid nvidia_drm(POE) nvidia_modeset(POE) nvidia(POE) drm_kms_helper syscopyarea [ 43.115849] sysfillrect sysimgblt fb_sys_fops drm r8169 ipmi_devintf i2c_piix4 ipmi_msghandler mii ahci libahci gpio_amdpt gpio_generic [ 43.115856] CPU: 2 PID: 1847 Comm: Xorg Tainted: P OE 4.18.0-10-generic #11-Ubuntu [ 43.115857] Hardware name: System manufacturer System Product Name/PRIME B350-PLUS, BIOS 0406 02/07/2017 [ 43.115859] RIP: 0010:usercopy_warn+0x81/0xa0 [ 43.115859] Code: 10 99 41 51 4d 89 d8 48 c7 c0 89 8d 0f 99 49 89 f1 48 89 f9 48 0f 45 c2 48 c7 c7 f0 a1 10 99 4c 89 d2 48 89 c6 e8 f1 cf df ff <0f> 0b 48 83 c4 18 c9 c3 48 c7 c6 b2 8a 12 99 49 89 f1 49 89 f3 eb [ 43.115875] RSP: 0018:b0c741f07b08 EFLAGS: 00010286 [ 43.115876] RAX: RBX: 9beec5c42cb0 RCX: 0006 [ 43.115876] RDX: 0007 RSI: 0096 RDI: 9beece6964b0 [ 43.115877] RBP: b0c741f07b20 R08: 0001 R09: 0392 [ 43.115878] R10: 0004 R11: R12: 0003 [ 43.115878] R13: 0001 R14: 9beec5c42cb3 R15: 9beec5c42cf8 [ 43.115879] FS: 7f6267654a80() GS:9beece68() knlGS: [ 43.115880] CS: 0010 DS: ES: CR0: 80050033 [ 43.115881] CR2: 7f6263cc5d20 CR3: 000402fb CR4: 003406e0 [ 43.115881] Call Trace: [ 43.115886] __check_heap_object+0xc2/0x110 [ 43.115887] __check_object_size+0x14c/0x178 [ 43.116024] os_memcpy_to_user+0x26/0x50 [nvidia] [ 43.116158] _nv009384rm+0xbf/0xe0 [nvidia] [ 43.116159] WARNING: kernel stack frame pointer at e73eb0f3 in Xorg:1847 has bad value 74044542
Re: Linux kernel crash
On Wednesday, October 24, 2018 10:20:05 PM MST Stephen Smith wrote: > > Whenever I run "shutdown -h now" or "reboot" I receive an immediate kernel > crash with a dump that has: > > "Code: Bad RIP value" I Canonical response noted the following from the dump: [ 42.640541] resource sanity check: requesting [mem 0x000c-0x000f], which spans more than PCI Bus :00 [mem 0x000c-0x000d window] I've since updated to the latest BIOS at there suggestion. I see the following on the screen when the kernel crashes: [ 43.115817] WARNING: CPU: 2 PID: 1847 at mm/usercopy.c:81 usercopy_warn+0x81/0xa0 [ 43.115818] Modules linked in: binfmt_misc snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic edac_mce_amd snd_hda_intel kvm_amd kvm snd_hda_codec snd_hda_core irqbypass crct10dif_pclmul crc32_pclmul snd_hwdep snd_pcm snd_seq_midi snd_seq_midi_event ghash_clmulni_intel snd_rawmidi snd_seq pcbc aesni_intel snd_seq_device snd_timer eeepc_wmi asus_wmi sparse_keymap aes_x86_64 joydev snd video crypto_simd cryptd soundcore wmi_bmof glue_helper input_leds wmi k10temp ccp serio_raw nvidia_uvm(POE) mac_hid sch_fq_codel parport_pc ppdev lp parport ip_tables x_tables autofs4 raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor raid6_pq libcrc32c raid0 multipath linear raid1 hid_generic usbhid hid nvidia_drm(POE) nvidia_modeset(POE) nvidia(POE) drm_kms_helper syscopyarea [ 43.115849] sysfillrect sysimgblt fb_sys_fops drm r8169 ipmi_devintf i2c_piix4 ipmi_msghandler mii ahci libahci gpio_amdpt gpio_generic [ 43.115856] CPU: 2 PID: 1847 Comm: Xorg Tainted: P OE 4.18.0-10-generic #11-Ubuntu [ 43.115857] Hardware name: System manufacturer System Product Name/PRIME B350-PLUS, BIOS 0406 02/07/2017 [ 43.115859] RIP: 0010:usercopy_warn+0x81/0xa0 [ 43.115859] Code: 10 99 41 51 4d 89 d8 48 c7 c0 89 8d 0f 99 49 89 f1 48 89 f9 48 0f 45 c2 48 c7 c7 f0 a1 10 99 4c 89 d2 48 89 c6 e8 f1 cf df ff <0f> 0b 48 83 c4 18 c9 c3 48 c7 c6 b2 8a 12 99 49 89 f1 49 89 f3 eb [ 43.115875] RSP: 0018:b0c741f07b08 EFLAGS: 00010286 [ 43.115876] RAX: RBX: 9beec5c42cb0 RCX: 0006 [ 43.115876] RDX: 0007 RSI: 0096 RDI: 9beece6964b0 [ 43.115877] RBP: b0c741f07b20 R08: 0001 R09: 0392 [ 43.115878] R10: 0004 R11: R12: 0003 [ 43.115878] R13: 0001 R14: 9beec5c42cb3 R15: 9beec5c42cf8 [ 43.115879] FS: 7f6267654a80() GS:9beece68() knlGS: [ 43.115880] CS: 0010 DS: ES: CR0: 80050033 [ 43.115881] CR2: 7f6263cc5d20 CR3: 000402fb CR4: 003406e0 [ 43.115881] Call Trace: [ 43.115886] __check_heap_object+0xc2/0x110 [ 43.115887] __check_object_size+0x14c/0x178 [ 43.116024] os_memcpy_to_user+0x26/0x50 [nvidia] [ 43.116158] _nv009384rm+0xbf/0xe0 [nvidia] [ 43.116159] WARNING: kernel stack frame pointer at e73eb0f3 in Xorg:1847 has bad value 74044542
Re: 4.18: early boot crash in thermal_cooling_device_destroy_sysfs
On 10/26/18 2:14 AM, Rafael J. Wysocki wrote: > On Monday, October 22, 2018 8:37:25 PM CEST Randy Dunlap wrote: >> >> On 8/16/18 2:33 PM, Randy Dunlap wrote: >>> Hi, >>> >>> Sorry for the photo. That's all I have available so far. >>> >>> https://www.infradead.org/~rdunlap/doc/IMG_20180816_133254743_HDR.jpg >>> >>> >>> Does anyone recognize this? >>> >>> This is an (older) Toshiba laptop. The kernel .config is mostly an >>> allmodconfig with some DEBUG options disabled and other options enabled >>> so that it can boot without using an initramfs. (and with COMPILE_TEST >>> disabled :) >>> >>> >>> The full kernel .config file is attached. >>> >>> Thanks, >>> >> >> This is a result of CONFIG_DEBUG_TEST_DRIVER_REMOVE=y. >> [switch from 64-bit to 32-bit machine] >> >> >> When using CONFIG_DEBUG_VM=y, it BUGs at: >> [5.553603] [ cut here ] >> [5.553733] kernel BUG at arch/x86/mm/physaddr.c:75! >> [5.557788] invalid opcode: [#1] PREEMPT SMP DEBUG_PAGEALLOC >> [5.558738] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.19.0-rc7 #4 >> [5.558738] Hardware name: Dell Inc. Inspiron 1318 >> /0C236D, BIOS A04 01/15/2009 >> [5.558738] EIP: __phys_addr+0x40/0x90 >> [5.558738] Code: 00 40 75 2e 8b 15 00 57 23 d5 85 d2 74 12 89 d9 c1 e9 >> 0c 39 ca 72 5b e8 2e ca ff ff 39 d8 75 4a 89 d8 5b 5d c3 8d 74 26 00 90 <0f> >> 0b 8d b6 00 00 00 00 8b 0d 80 56 23 d5 8d 91 00 00 80 00 39 d0 >> [5.558738] EAX: 6b6b6b6b EBX: 6b6b6b6b ECX: 00140011 EDX: >> [5.558738] ESI: f489 EDI: d4a58d60 EBP: f40c1e0c ESP: f40c1e08 >> [5.558738] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 EFLAGS: 00210a97 >> [5.558738] CR0: 80050033 CR2: CR3: 14cad000 CR4: 000406d0 >> [5.558738] Call Trace: >> [5.558738] kfree+0x1f/0x160 >> [5.558738] thermal_cooling_device_destroy_sysfs+0x11/0x20 >> [5.558738] thermal_cooling_device_unregister+0x168/0x180 >> [5.558738] acpi_pss_perf_exit.isra.4+0x32/0x50 >> [5.558738] acpi_processor_stop+0x4d/0x60 >> [5.558738] really_probe+0xa3/0x3e0 >> [5.558738] driver_probe_device+0x5b/0x120 >> [5.558738] __driver_attach+0xd9/0x100 >> [5.558738] ? driver_probe_device+0x120/0x120 >> [5.558738] bus_for_each_dev+0x56/0x90 >> [5.558738] driver_attach+0x14/0x20 >> [5.558738] ? driver_probe_device+0x120/0x120 >> [5.558738] bus_add_driver+0x117/0x210 >> [5.558738] driver_register+0x61/0xb0 >> [5.558738] acpi_processor_driver_init+0x19/0x88 >> [5.558738] ? acpi_pci_slot_init+0xf/0xf >> [5.558738] do_one_initcall+0x3e/0x15a >> [5.558738] ? do_early_param+0x75/0x75 >> [5.558738] kernel_init_freeable+0x170/0x1f3 >> [5.558738] ? rest_init+0xcd/0xcd >> [5.558738] kernel_init+0x8/0xdb >> [5.558738] ret_from_fork+0x2e/0x38 >> [5.558738] Modules linked in: >> [5.625269] _warn_unseeded_randomness: 1 callbacks suppressed >> [5.625272] random: get_random_bytes called from init_oops_id+0x3a/0x40 >> with crng_init=0 >> [5.629758] ---[ end trace 65b17bf4d18e7692 ]--- >> [5.631573] EIP: __phys_addr+0x40/0x90 >> [5.633242] Code: 00 40 75 2e 8b 15 00 57 23 d5 85 d2 74 12 89 d9 c1 e9 >> 0c 39 ca 72 5b e8 2e ca ff ff 39 d8 75 4a 89 d8 5b 5d c3 8d 74 26 00 90 <0f> >> 0b 8d b6 00 00 00 00 8b 0d 80 56 23 d5 8d 91 00 00 80 00 39 d0 >> [5.638618] EAX: 6b6b6b6b EBX: 6b6b6b6b ECX: 00140011 EDX: >> [5.640703] ESI: f489 EDI: d4a58d60 EBP: f40c1e0c ESP: d4cb13dc >> [5.642801] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 EFLAGS: 00210a97 >> [5.645053] CR0: 80050033 CR2: CR3: 14cad000 CR4: 000406d0 >> [5.647179] Kernel panic - not syncing: Fatal exception >> [5.648172] Kernel Offset: 0x1300 from 0xc100 (relocation range: >> 0xc000-0xf77fdfff) >> [5.648172] ---[ end Kernel panic - not syncing: Fatal exception ]--- >> >> >> When not using CONFIG_DEBUG_VM, it BUGs in kfree: >> [5.497864] [ cut here ] >> [5.498215] kernel BUG at mm/slub.c:3901! >> [5.501739] invalid opcode: [#1] PREEMPT SMP >> [5.502720] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.19.0-rc7 #3 >> [5.502720] Hardware name: Dell Inc. Inspiron 1318 >> /0C236D, BIOS A04 01/15/2009 >> [5.502720] EIP: kfree+0x117/0x150 >> [5.502720] Code: 74 21 8b 06 31 d2 f6 c4 80 74 04 0f b6 56 31 89 f0 e8 >> 7d e0 fa ff e9 7b ff ff ff 8d b4 26 00 00 00 00 90 8b 46 04 a8 01 75 d8 <0f> >> 0b 8d b4 26 00 00 00 00 8b 75 f0 ff 75 ec 89 d9 89 f8 6a 01 53 >> [5.502720] EAX: 0100 EBX: 6b6b6b6b ECX: 00140011 EDX: >> [5.502720] ESI: f67dac70 EDI: ccc4aca0 EBP: f4083e28 ESP: f4083e10 >> [5.502720] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 EFLAGS: 00210246 >> [5.502720] CR0: 80050033 CR2: ffd14000 CR3: 0ce94000 CR4: 000406d0 >> [5.502720] Call Trace: >> [5.502720]
Re: 4.18: early boot crash in thermal_cooling_device_destroy_sysfs
On 10/26/18 2:14 AM, Rafael J. Wysocki wrote: > On Monday, October 22, 2018 8:37:25 PM CEST Randy Dunlap wrote: >> >> On 8/16/18 2:33 PM, Randy Dunlap wrote: >>> Hi, >>> >>> Sorry for the photo. That's all I have available so far. >>> >>> https://www.infradead.org/~rdunlap/doc/IMG_20180816_133254743_HDR.jpg >>> >>> >>> Does anyone recognize this? >>> >>> This is an (older) Toshiba laptop. The kernel .config is mostly an >>> allmodconfig with some DEBUG options disabled and other options enabled >>> so that it can boot without using an initramfs. (and with COMPILE_TEST >>> disabled :) >>> >>> >>> The full kernel .config file is attached. >>> >>> Thanks, >>> >> >> This is a result of CONFIG_DEBUG_TEST_DRIVER_REMOVE=y. >> [switch from 64-bit to 32-bit machine] >> >> >> When using CONFIG_DEBUG_VM=y, it BUGs at: >> [5.553603] [ cut here ] >> [5.553733] kernel BUG at arch/x86/mm/physaddr.c:75! >> [5.557788] invalid opcode: [#1] PREEMPT SMP DEBUG_PAGEALLOC >> [5.558738] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.19.0-rc7 #4 >> [5.558738] Hardware name: Dell Inc. Inspiron 1318 >> /0C236D, BIOS A04 01/15/2009 >> [5.558738] EIP: __phys_addr+0x40/0x90 >> [5.558738] Code: 00 40 75 2e 8b 15 00 57 23 d5 85 d2 74 12 89 d9 c1 e9 >> 0c 39 ca 72 5b e8 2e ca ff ff 39 d8 75 4a 89 d8 5b 5d c3 8d 74 26 00 90 <0f> >> 0b 8d b6 00 00 00 00 8b 0d 80 56 23 d5 8d 91 00 00 80 00 39 d0 >> [5.558738] EAX: 6b6b6b6b EBX: 6b6b6b6b ECX: 00140011 EDX: >> [5.558738] ESI: f489 EDI: d4a58d60 EBP: f40c1e0c ESP: f40c1e08 >> [5.558738] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 EFLAGS: 00210a97 >> [5.558738] CR0: 80050033 CR2: CR3: 14cad000 CR4: 000406d0 >> [5.558738] Call Trace: >> [5.558738] kfree+0x1f/0x160 >> [5.558738] thermal_cooling_device_destroy_sysfs+0x11/0x20 >> [5.558738] thermal_cooling_device_unregister+0x168/0x180 >> [5.558738] acpi_pss_perf_exit.isra.4+0x32/0x50 >> [5.558738] acpi_processor_stop+0x4d/0x60 >> [5.558738] really_probe+0xa3/0x3e0 >> [5.558738] driver_probe_device+0x5b/0x120 >> [5.558738] __driver_attach+0xd9/0x100 >> [5.558738] ? driver_probe_device+0x120/0x120 >> [5.558738] bus_for_each_dev+0x56/0x90 >> [5.558738] driver_attach+0x14/0x20 >> [5.558738] ? driver_probe_device+0x120/0x120 >> [5.558738] bus_add_driver+0x117/0x210 >> [5.558738] driver_register+0x61/0xb0 >> [5.558738] acpi_processor_driver_init+0x19/0x88 >> [5.558738] ? acpi_pci_slot_init+0xf/0xf >> [5.558738] do_one_initcall+0x3e/0x15a >> [5.558738] ? do_early_param+0x75/0x75 >> [5.558738] kernel_init_freeable+0x170/0x1f3 >> [5.558738] ? rest_init+0xcd/0xcd >> [5.558738] kernel_init+0x8/0xdb >> [5.558738] ret_from_fork+0x2e/0x38 >> [5.558738] Modules linked in: >> [5.625269] _warn_unseeded_randomness: 1 callbacks suppressed >> [5.625272] random: get_random_bytes called from init_oops_id+0x3a/0x40 >> with crng_init=0 >> [5.629758] ---[ end trace 65b17bf4d18e7692 ]--- >> [5.631573] EIP: __phys_addr+0x40/0x90 >> [5.633242] Code: 00 40 75 2e 8b 15 00 57 23 d5 85 d2 74 12 89 d9 c1 e9 >> 0c 39 ca 72 5b e8 2e ca ff ff 39 d8 75 4a 89 d8 5b 5d c3 8d 74 26 00 90 <0f> >> 0b 8d b6 00 00 00 00 8b 0d 80 56 23 d5 8d 91 00 00 80 00 39 d0 >> [5.638618] EAX: 6b6b6b6b EBX: 6b6b6b6b ECX: 00140011 EDX: >> [5.640703] ESI: f489 EDI: d4a58d60 EBP: f40c1e0c ESP: d4cb13dc >> [5.642801] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 EFLAGS: 00210a97 >> [5.645053] CR0: 80050033 CR2: CR3: 14cad000 CR4: 000406d0 >> [5.647179] Kernel panic - not syncing: Fatal exception >> [5.648172] Kernel Offset: 0x1300 from 0xc100 (relocation range: >> 0xc000-0xf77fdfff) >> [5.648172] ---[ end Kernel panic - not syncing: Fatal exception ]--- >> >> >> When not using CONFIG_DEBUG_VM, it BUGs in kfree: >> [5.497864] [ cut here ] >> [5.498215] kernel BUG at mm/slub.c:3901! >> [5.501739] invalid opcode: [#1] PREEMPT SMP >> [5.502720] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.19.0-rc7 #3 >> [5.502720] Hardware name: Dell Inc. Inspiron 1318 >> /0C236D, BIOS A04 01/15/2009 >> [5.502720] EIP: kfree+0x117/0x150 >> [5.502720] Code: 74 21 8b 06 31 d2 f6 c4 80 74 04 0f b6 56 31 89 f0 e8 >> 7d e0 fa ff e9 7b ff ff ff 8d b4 26 00 00 00 00 90 8b 46 04 a8 01 75 d8 <0f> >> 0b 8d b4 26 00 00 00 00 8b 75 f0 ff 75 ec 89 d9 89 f8 6a 01 53 >> [5.502720] EAX: 0100 EBX: 6b6b6b6b ECX: 00140011 EDX: >> [5.502720] ESI: f67dac70 EDI: ccc4aca0 EBP: f4083e28 ESP: f4083e10 >> [5.502720] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 EFLAGS: 00210246 >> [5.502720] CR0: 80050033 CR2: ffd14000 CR3: 0ce94000 CR4: 000406d0 >> [5.502720] Call Trace: >> [5.502720]
[PATCH] sched/rt: Fix a comment in pick_next_task_rt()
Commit f4ebcbc0d7e0 ("sched/rt: Substract number of tasks of throttled
queues from rq->nr_running") added a new member rt_rq->rt_queued, which
is used to indicate the status of rq->rt enqueue or dequeue. So, the
check rt_nr_running was removed and we now check rt_queued.
Fix the comment in pick_next_task_rt() to avoid confusing.
Signed-off-by: Muchun Song
---
kernel/sched/rt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 2e2955a8cf8f..a21ea6021929 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1561,7 +1561,7 @@ pick_next_task_rt(struct rq *rq, struct task_struct
*prev, struct rq_flags *rf)
/*
* We may dequeue prev's rt_rq in put_prev_task().
-* So, we update time before rt_nr_running check.
+* So, we update time before rt_queued check.
*/
if (prev->sched_class == _sched_class)
update_curr_rt(rq);
--
2.17.1
[PATCH] sched/rt: Fix a comment in pick_next_task_rt()
Commit f4ebcbc0d7e0 ("sched/rt: Substract number of tasks of throttled
queues from rq->nr_running") added a new member rt_rq->rt_queued, which
is used to indicate the status of rq->rt enqueue or dequeue. So, the
check rt_nr_running was removed and we now check rt_queued.
Fix the comment in pick_next_task_rt() to avoid confusing.
Signed-off-by: Muchun Song
---
kernel/sched/rt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 2e2955a8cf8f..a21ea6021929 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1561,7 +1561,7 @@ pick_next_task_rt(struct rq *rq, struct task_struct
*prev, struct rq_flags *rf)
/*
* We may dequeue prev's rt_rq in put_prev_task().
-* So, we update time before rt_nr_running check.
+* So, we update time before rt_queued check.
*/
if (prev->sched_class == _sched_class)
update_curr_rt(rq);
--
2.17.1
[PATCH -next] sgi-xp: drop pointless static qualifier in xpc_setup_msg_structures_uv
There is no need to have the 'enum xp_retval ret' variable static since
new value always be assigned before use it.
Signed-off-by: YueHaibing
---
drivers/misc/sgi-xp/xpc_uv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/misc/sgi-xp/xpc_uv.c b/drivers/misc/sgi-xp/xpc_uv.c
index 0441abe..0323bfe 100644
--- a/drivers/misc/sgi-xp/xpc_uv.c
+++ b/drivers/misc/sgi-xp/xpc_uv.c
@@ -1151,7 +1151,7 @@ struct uv_IO_APIC_route_entry {
static enum xp_retval
xpc_setup_msg_structures_uv(struct xpc_channel *ch)
{
- static enum xp_retval ret;
+ enum xp_retval ret;
struct xpc_channel_uv *ch_uv = >sn.uv;
DBUG_ON(ch->flags & XPC_C_SETUP);
[PATCH -next] sgi-xp: drop pointless static qualifier in xpc_setup_msg_structures_uv
There is no need to have the 'enum xp_retval ret' variable static since
new value always be assigned before use it.
Signed-off-by: YueHaibing
---
drivers/misc/sgi-xp/xpc_uv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/misc/sgi-xp/xpc_uv.c b/drivers/misc/sgi-xp/xpc_uv.c
index 0441abe..0323bfe 100644
--- a/drivers/misc/sgi-xp/xpc_uv.c
+++ b/drivers/misc/sgi-xp/xpc_uv.c
@@ -1151,7 +1151,7 @@ struct uv_IO_APIC_route_entry {
static enum xp_retval
xpc_setup_msg_structures_uv(struct xpc_channel *ch)
{
- static enum xp_retval ret;
+ enum xp_retval ret;
struct xpc_channel_uv *ch_uv = >sn.uv;
DBUG_ON(ch->flags & XPC_C_SETUP);
[PATCH] ASoC: stm32: sai: fix invalid use of sizeof in stm32_sai_add_mclk_provider()
sizeof() when applied to a pointer typed expression gives the
size of the pointer, not that of the pointed data.
Fixes: 8307b2afd386 ("ASoC: stm32: sai: set sai as mclk clock provider")
Signed-off-by: Wei Yongjun
---
sound/soc/stm/stm32_sai_sub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/stm/stm32_sai_sub.c b/sound/soc/stm/stm32_sai_sub.c
index ea05cc9..211589b 100644
--- a/sound/soc/stm/stm32_sai_sub.c
+++ b/sound/soc/stm/stm32_sai_sub.c
@@ -390,7 +390,7 @@ static int stm32_sai_add_mclk_provider(struct
stm32_sai_sub_data *sai)
char *mclk_name, *p, *s = (char *)pname;
int ret, i = 0;
- mclk = devm_kzalloc(dev, sizeof(mclk), GFP_KERNEL);
+ mclk = devm_kzalloc(dev, sizeof(*mclk), GFP_KERNEL);
if (!mclk)
return -ENOMEM;
[PATCH] ASoC: stm32: sai: fix invalid use of sizeof in stm32_sai_add_mclk_provider()
sizeof() when applied to a pointer typed expression gives the
size of the pointer, not that of the pointed data.
Fixes: 8307b2afd386 ("ASoC: stm32: sai: set sai as mclk clock provider")
Signed-off-by: Wei Yongjun
---
sound/soc/stm/stm32_sai_sub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/stm/stm32_sai_sub.c b/sound/soc/stm/stm32_sai_sub.c
index ea05cc9..211589b 100644
--- a/sound/soc/stm/stm32_sai_sub.c
+++ b/sound/soc/stm/stm32_sai_sub.c
@@ -390,7 +390,7 @@ static int stm32_sai_add_mclk_provider(struct
stm32_sai_sub_data *sai)
char *mclk_name, *p, *s = (char *)pname;
int ret, i = 0;
- mclk = devm_kzalloc(dev, sizeof(mclk), GFP_KERNEL);
+ mclk = devm_kzalloc(dev, sizeof(*mclk), GFP_KERNEL);
if (!mclk)
return -ENOMEM;
[PATCH v2 6/6] staging:iio:ad2s90: Check channel type at read_raw
This patch adds a channel type check at the beginning of the
ad2s90_read_raw function. Since ad2s90 has only one channel, it just
checks if the given channel is the expected one and if not, return
-EINVAL.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 52b656875ed1..24002042a5c5 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -34,6 +34,9 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
int ret;
struct ad2s90_state *st = iio_priv(indio_dev);
+ if (chan->type != IIO_ANGL)
+ return -EINVAL;
+
switch (m) {
case IIO_CHAN_INFO_SCALE:
/* 2 * Pi / (2^12 - 1) ~= 0.001534355 */
--
2.18.0
[PATCH v2 6/6] staging:iio:ad2s90: Check channel type at read_raw
This patch adds a channel type check at the beginning of the
ad2s90_read_raw function. Since ad2s90 has only one channel, it just
checks if the given channel is the expected one and if not, return
-EINVAL.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 52b656875ed1..24002042a5c5 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -34,6 +34,9 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
int ret;
struct ad2s90_state *st = iio_priv(indio_dev);
+ if (chan->type != IIO_ANGL)
+ return -EINVAL;
+
switch (m) {
case IIO_CHAN_INFO_SCALE:
/* 2 * Pi / (2^12 - 1) ~= 0.001534355 */
--
2.18.0
[PATCH v2 2/6] staging:iio:ad2s90: Make probe handle spi_setup failure
Previously, ad2s90_probe ignored the return code from spi_setup, not
handling its possible failure. This patch makes ad2s90_probe check if
the code is an error code and, if so, do the following:
- Call dev_err with an appropriate error message.
- Return the spi_setup's error code, aborting the execution of the
rest of the function.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 11fac9f90148..d6a42e3f1bd8 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -88,7 +88,12 @@ static int ad2s90_probe(struct spi_device *spi)
/* need 600ns between CS and the first falling edge of SCLK */
spi->max_speed_hz = 83;
spi->mode = SPI_MODE_3;
- spi_setup(spi);
+ ret = spi_setup(spi);
+
+ if (ret < 0) {
+ dev_err(>dev, "spi_setup failed!\n");
+ return ret;
+ }
return 0;
}
--
2.18.0
[PATCH v2 1/6] staging:iio:ad2s90: Make read_raw return spi_read's error code
Previously, when spi_read returned an error code inside ad2s90_read_raw,
the code was ignored and IIO_VAL_INT was returned. This patch makes the
function return the error code returned by spi_read when it fails.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 59586947a936..11fac9f90148 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -35,12 +35,15 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
struct ad2s90_state *st = iio_priv(indio_dev);
mutex_lock(>lock);
+
ret = spi_read(st->sdev, st->rx, 2);
- if (ret)
- goto error_ret;
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
+ }
+
*val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
-error_ret:
mutex_unlock(>lock);
return IIO_VAL_INT;
--
2.18.0
[PATCH v2 0/6] staging:iio:ad2s90: Add scale info and improve error handling
This patch set adds scale info to ad2s90's single channel, improve error handling in it's functions and fix a possible race condition issue. The goal with this patch set is to address the points discussed in the mailing list in an effort to move ad2s90.c out of staging. Changes in v2: - Added my S-o-B in patch 5. Matheus Tavares (5): staging:iio:ad2s90: Make read_raw return spi_read's error code staging:iio:ad2s90: Make probe handle spi_setup failure staging:iio:ad2s90: Remove always overwritten assignment staging:iio:ad2s90: Move device registration to the end of probe staging:iio:ad2s90: Check channel type at read_raw Victor Colombo (1): staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw drivers/staging/iio/resolver/ad2s90.c | 55 ++- 1 file changed, 37 insertions(+), 18 deletions(-) -- 2.18.0
[PATCH v2 2/6] staging:iio:ad2s90: Make probe handle spi_setup failure
Previously, ad2s90_probe ignored the return code from spi_setup, not
handling its possible failure. This patch makes ad2s90_probe check if
the code is an error code and, if so, do the following:
- Call dev_err with an appropriate error message.
- Return the spi_setup's error code, aborting the execution of the
rest of the function.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 11fac9f90148..d6a42e3f1bd8 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -88,7 +88,12 @@ static int ad2s90_probe(struct spi_device *spi)
/* need 600ns between CS and the first falling edge of SCLK */
spi->max_speed_hz = 83;
spi->mode = SPI_MODE_3;
- spi_setup(spi);
+ ret = spi_setup(spi);
+
+ if (ret < 0) {
+ dev_err(>dev, "spi_setup failed!\n");
+ return ret;
+ }
return 0;
}
--
2.18.0
[PATCH v2 1/6] staging:iio:ad2s90: Make read_raw return spi_read's error code
Previously, when spi_read returned an error code inside ad2s90_read_raw,
the code was ignored and IIO_VAL_INT was returned. This patch makes the
function return the error code returned by spi_read when it fails.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index 59586947a936..11fac9f90148 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -35,12 +35,15 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
struct ad2s90_state *st = iio_priv(indio_dev);
mutex_lock(>lock);
+
ret = spi_read(st->sdev, st->rx, 2);
- if (ret)
- goto error_ret;
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
+ }
+
*val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
-error_ret:
mutex_unlock(>lock);
return IIO_VAL_INT;
--
2.18.0
[PATCH v2 0/6] staging:iio:ad2s90: Add scale info and improve error handling
This patch set adds scale info to ad2s90's single channel, improve error handling in it's functions and fix a possible race condition issue. The goal with this patch set is to address the points discussed in the mailing list in an effort to move ad2s90.c out of staging. Changes in v2: - Added my S-o-B in patch 5. Matheus Tavares (5): staging:iio:ad2s90: Make read_raw return spi_read's error code staging:iio:ad2s90: Make probe handle spi_setup failure staging:iio:ad2s90: Remove always overwritten assignment staging:iio:ad2s90: Move device registration to the end of probe staging:iio:ad2s90: Check channel type at read_raw Victor Colombo (1): staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw drivers/staging/iio/resolver/ad2s90.c | 55 ++- 1 file changed, 37 insertions(+), 18 deletions(-) -- 2.18.0
[PATCH v2 5/6] staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw
This patch adds the IIO_CHAN_INFO_SCALE mask to ad2s90_chan and
implements the relative read behavior at ad2s90_read_raw.
Signed-off-by: Victor Colombo
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 32 ++-
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index b4a6a89c11b0..52b656875ed1 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -34,19 +34,31 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
int ret;
struct ad2s90_state *st = iio_priv(indio_dev);
- mutex_lock(>lock);
+ switch (m) {
+ case IIO_CHAN_INFO_SCALE:
+ /* 2 * Pi / (2^12 - 1) ~= 0.001534355 */
+ *val = 0;
+ *val2 = 1534355;
+ return IIO_VAL_INT_PLUS_NANO;
+ case IIO_CHAN_INFO_RAW:
+ mutex_lock(>lock);
+
+ ret = spi_read(st->sdev, st->rx, 2);
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
+ }
+
+ *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
- ret = spi_read(st->sdev, st->rx, 2);
- if (ret < 0) {
mutex_unlock(>lock);
- return ret;
- }
- *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
-
- mutex_unlock(>lock);
+ return IIO_VAL_INT;
+ default:
+ break;
+ }
- return IIO_VAL_INT;
+ return -EINVAL;
}
static const struct iio_info ad2s90_info = {
@@ -57,7 +69,7 @@ static const struct iio_chan_spec ad2s90_chan = {
.type = IIO_ANGL,
.indexed = 1,
.channel = 0,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
};
static int ad2s90_probe(struct spi_device *spi)
--
2.18.0
[PATCH v2 5/6] staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw
This patch adds the IIO_CHAN_INFO_SCALE mask to ad2s90_chan and
implements the relative read behavior at ad2s90_read_raw.
Signed-off-by: Victor Colombo
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 32 ++-
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index b4a6a89c11b0..52b656875ed1 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -34,19 +34,31 @@ static int ad2s90_read_raw(struct iio_dev *indio_dev,
int ret;
struct ad2s90_state *st = iio_priv(indio_dev);
- mutex_lock(>lock);
+ switch (m) {
+ case IIO_CHAN_INFO_SCALE:
+ /* 2 * Pi / (2^12 - 1) ~= 0.001534355 */
+ *val = 0;
+ *val2 = 1534355;
+ return IIO_VAL_INT_PLUS_NANO;
+ case IIO_CHAN_INFO_RAW:
+ mutex_lock(>lock);
+
+ ret = spi_read(st->sdev, st->rx, 2);
+ if (ret < 0) {
+ mutex_unlock(>lock);
+ return ret;
+ }
+
+ *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
- ret = spi_read(st->sdev, st->rx, 2);
- if (ret < 0) {
mutex_unlock(>lock);
- return ret;
- }
- *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
-
- mutex_unlock(>lock);
+ return IIO_VAL_INT;
+ default:
+ break;
+ }
- return IIO_VAL_INT;
+ return -EINVAL;
}
static const struct iio_info ad2s90_info = {
@@ -57,7 +69,7 @@ static const struct iio_chan_spec ad2s90_chan = {
.type = IIO_ANGL,
.indexed = 1,
.channel = 0,
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
};
static int ad2s90_probe(struct spi_device *spi)
--
2.18.0
[PATCH v2 3/6] staging:iio:ad2s90: Remove always overwritten assignment
This patch removes an initial assignment to the variable ret at probe,
that was always overwritten.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index d6a42e3f1bd8..c20d37dc065a 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -64,7 +64,7 @@ static int ad2s90_probe(struct spi_device *spi)
{
struct iio_dev *indio_dev;
struct ad2s90_state *st;
- int ret = 0;
+ int ret;
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
--
2.18.0
[PATCH v2 4/6] staging:iio:ad2s90: Move device registration to the end of probe
Previously, devm_iio_device_register was being called before the
spi_setup call and the spi_device's max_speed_hz and mode assignments.
This could lead to a race condition since the driver was still being
set up after it was already made ready to use. To fix it, this patch
moves the device registration to the end of ad2s90_probe.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index c20d37dc065a..b4a6a89c11b0 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -81,10 +81,6 @@ static int ad2s90_probe(struct spi_device *spi)
indio_dev->num_channels = 1;
indio_dev->name = spi_get_device_id(spi)->name;
- ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
- if (ret)
- return ret;
-
/* need 600ns between CS and the first falling edge of SCLK */
spi->max_speed_hz = 83;
spi->mode = SPI_MODE_3;
@@ -95,7 +91,7 @@ static int ad2s90_probe(struct spi_device *spi)
return ret;
}
- return 0;
+ return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
}
static const struct spi_device_id ad2s90_id[] = {
--
2.18.0
[PATCH v2 3/6] staging:iio:ad2s90: Remove always overwritten assignment
This patch removes an initial assignment to the variable ret at probe,
that was always overwritten.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index d6a42e3f1bd8..c20d37dc065a 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -64,7 +64,7 @@ static int ad2s90_probe(struct spi_device *spi)
{
struct iio_dev *indio_dev;
struct ad2s90_state *st;
- int ret = 0;
+ int ret;
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
--
2.18.0
[PATCH v2 4/6] staging:iio:ad2s90: Move device registration to the end of probe
Previously, devm_iio_device_register was being called before the
spi_setup call and the spi_device's max_speed_hz and mode assignments.
This could lead to a race condition since the driver was still being
set up after it was already made ready to use. To fix it, this patch
moves the device registration to the end of ad2s90_probe.
Signed-off-by: Matheus Tavares
---
drivers/staging/iio/resolver/ad2s90.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s90.c
b/drivers/staging/iio/resolver/ad2s90.c
index c20d37dc065a..b4a6a89c11b0 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -81,10 +81,6 @@ static int ad2s90_probe(struct spi_device *spi)
indio_dev->num_channels = 1;
indio_dev->name = spi_get_device_id(spi)->name;
- ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
- if (ret)
- return ret;
-
/* need 600ns between CS and the first falling edge of SCLK */
spi->max_speed_hz = 83;
spi->mode = SPI_MODE_3;
@@ -95,7 +91,7 @@ static int ad2s90_probe(struct spi_device *spi)
return ret;
}
- return 0;
+ return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
}
static const struct spi_device_id ad2s90_id[] = {
--
2.18.0
Re: [PATCH v2 1/3] namei: implement O_BENEATH-style AT_* flags
On Tue, 9 Oct 2018 at 02:53, Aleksa Sarai wrote: > > +#ifndef O_BENEATH > +#define O_BENEATH 0004000 /* *Not* the same as capsicum's > O_BENEATH! */ > +#endif I had originally followed up privately to Aleksa about this comment (to suggest that it's outdated and should be removed), but the reference Capsicum implementation now supports O_BENEATH and I think it's sensible to follow up here with the additional context. O_BENEATH originally came from the Capsicum Linux port, and inherited the restriction against ".." path components from years ago when the port was done. In addition, FreeBSD did not originally implement O_BENEATH as the "beneath" behaviour is inherently provided once a process enters a capability mode sandbox. However, Capsicum now allows ".." paths, and FreeBSD supports O_BENEATH separately from capability mode. Absolute paths are not yet allowed with O_BENEATH but a change is in review to permit them. On FreeBSD a lookup prevented by O_BENEATH semantics returns ENOTCAPABLE, the errno coming from the Capsicum implementation. Ideally I would like to see us have the same API; none of this work has yet shipped in a FreeBSD release and there is an opportunity for us to make changes to match the interface and errors Linux may adopt.
Re: [PATCH v2 1/3] namei: implement O_BENEATH-style AT_* flags
On Tue, 9 Oct 2018 at 02:53, Aleksa Sarai wrote: > > +#ifndef O_BENEATH > +#define O_BENEATH 0004000 /* *Not* the same as capsicum's > O_BENEATH! */ > +#endif I had originally followed up privately to Aleksa about this comment (to suggest that it's outdated and should be removed), but the reference Capsicum implementation now supports O_BENEATH and I think it's sensible to follow up here with the additional context. O_BENEATH originally came from the Capsicum Linux port, and inherited the restriction against ".." path components from years ago when the port was done. In addition, FreeBSD did not originally implement O_BENEATH as the "beneath" behaviour is inherently provided once a process enters a capability mode sandbox. However, Capsicum now allows ".." paths, and FreeBSD supports O_BENEATH separately from capability mode. Absolute paths are not yet allowed with O_BENEATH but a change is in review to permit them. On FreeBSD a lookup prevented by O_BENEATH semantics returns ENOTCAPABLE, the errno coming from the Capsicum implementation. Ideally I would like to see us have the same API; none of this work has yet shipped in a FreeBSD release and there is an opportunity for us to make changes to match the interface and errors Linux may adopt.
Re: [Ksummit-discuss] Call to Action Re: [PATCH 0/7] Code of Conduct: Fix some wording, and add an interpretation document
On Fri, Oct 26, 2018 at 08:14:51AM +1100, NeilBrown wrote: > On Wed, Oct 24 2018, Josh Triplett wrote: > > > On Tue, Oct 23, 2018 at 07:26:06AM +1100, NeilBrown wrote: > >> On Sun, Oct 21 2018, Josh Triplett wrote: > >> > >> > On Mon, Oct 22, 2018 at 08:20:11AM +1100, NeilBrown wrote: > >> >> I call on you, Greg: > >> >> - to abandon this divisive attempt to impose a "Code of Conduct" > >> >> - to revert 8a104f8b5867c68 > >> >> - to return to your core competence of building a great team around > >> >>a great kernel > >> >> > >> >> #Isupportreversion > >> >> > >> >> I call on the community to consider what *does* need to be said, about > >> >> conduct, to people outside the community and who have recently joined. > >> >> What is the document that you would have liked to have read as you were > >> >> starting out? It is all too long ago for me to remember clearly, and so > >> >> much has changed. > >> > > >> > The document I would have liked to have read when starting out is > >> > currently checked into the source tree in > >> > Documentation/process/code-of-conduct.rst . > >> > >> I'm curious - what would you have gained by reading that document? > > > > I would have then had rather less of a pervasive feeling of "if I make > > even a single mistake I get made an example of in ways that will feed > > people's quotes files for years to come". > > Thanks for your reply. Certainly feeling safe is important, and having > clear statements that the community values and promotes psychological > safety is valuable. > > The old "code of conflict" said >If however, anyone feels personally abused, threatened, or otherwise >uncomfortable due to this process, that is not acceptable. > > would you have not found this a strong enough statement to ward off that > pervasive feeling? Not when that document started out effectively saying, in an elaborate way, "code > people". (Leaving aside that the more important detail would be the community actually acting consistently with the code of conduct it espoused.) > In the current code, would The "Our Pledge" section have been > sufficient, or do you think the other sections would have actually > helped you? "Our Standards" would have been at least as important to me personally, as would "Enforcement" (and more importantly, examples of that applying in practice and not just as empty words).
Re: [Ksummit-discuss] Call to Action Re: [PATCH 0/7] Code of Conduct: Fix some wording, and add an interpretation document
On Fri, Oct 26, 2018 at 08:14:51AM +1100, NeilBrown wrote: > On Wed, Oct 24 2018, Josh Triplett wrote: > > > On Tue, Oct 23, 2018 at 07:26:06AM +1100, NeilBrown wrote: > >> On Sun, Oct 21 2018, Josh Triplett wrote: > >> > >> > On Mon, Oct 22, 2018 at 08:20:11AM +1100, NeilBrown wrote: > >> >> I call on you, Greg: > >> >> - to abandon this divisive attempt to impose a "Code of Conduct" > >> >> - to revert 8a104f8b5867c68 > >> >> - to return to your core competence of building a great team around > >> >>a great kernel > >> >> > >> >> #Isupportreversion > >> >> > >> >> I call on the community to consider what *does* need to be said, about > >> >> conduct, to people outside the community and who have recently joined. > >> >> What is the document that you would have liked to have read as you were > >> >> starting out? It is all too long ago for me to remember clearly, and so > >> >> much has changed. > >> > > >> > The document I would have liked to have read when starting out is > >> > currently checked into the source tree in > >> > Documentation/process/code-of-conduct.rst . > >> > >> I'm curious - what would you have gained by reading that document? > > > > I would have then had rather less of a pervasive feeling of "if I make > > even a single mistake I get made an example of in ways that will feed > > people's quotes files for years to come". > > Thanks for your reply. Certainly feeling safe is important, and having > clear statements that the community values and promotes psychological > safety is valuable. > > The old "code of conflict" said >If however, anyone feels personally abused, threatened, or otherwise >uncomfortable due to this process, that is not acceptable. > > would you have not found this a strong enough statement to ward off that > pervasive feeling? Not when that document started out effectively saying, in an elaborate way, "code > people". (Leaving aside that the more important detail would be the community actually acting consistently with the code of conduct it espoused.) > In the current code, would The "Our Pledge" section have been > sufficient, or do you think the other sections would have actually > helped you? "Our Standards" would have been at least as important to me personally, as would "Enforcement" (and more importantly, examples of that applying in practice and not just as empty words).
Re: [RFC PATCH 2/2] memcg: do not report racy no-eligible OOM tasks
On 2018/10/27 4:25, Michal Hocko wrote:
>> out_of_memory() bails on task_will_free_mem(current), which
>> specifically *excludes* already reaped tasks. Why are we then adding a
>> separate check before that to bail on already reaped victims?
>
> 696453e66630a has introduced the bail out.
>
>> Do we want to bail if current is a reaped victim or not?
>>
>> I don't see how we could skip it safely in general: the current task
>> might have been killed and reaped and gotten access to the memory
>> reserve and still fail to allocate on its way out. It needs to kill
>> the next task if there is one, or warn if there isn't another
>> one. Because we're genuinely oom without reclaimable tasks.
>
> Yes, this would be the case for the global case which is a real OOM
> situation. Memcg oom is somehow more relaxed because the oom is local.
We can handle possibility of genuinely OOM without reclaimable tasks.
Only __GFP_NOFAIL OOM has to select next OOM victim. There is no need to
select next OOM victim unless __GFP_NOFAIL. Commit 696453e66630ad45
("mm, oom: task_will_free_mem should skip oom_reaped tasks") was too simple.
On 2018/10/27 4:33, Michal Hocko wrote:
> On Fri 26-10-18 21:25:51, Michal Hocko wrote:
>> On Fri 26-10-18 10:25:31, Johannes Weiner wrote:
> [...]
>>> There is of course the scenario brought forward in this thread, where
>>> multiple threads of a process race and the second one enters oom even
>>> though it doesn't need to anymore. What the global case does to catch
>>> this is to grab the oom lock and do one last alloc attempt. Should
>>> memcg lock the oom_lock and try one more time to charge the memcg?
>>
>> That would be another option. I agree that making it more towards the
>> global case makes it more attractive. My tsk_is_oom_victim is more
>> towards "plug this particular case".
>
> Nevertheless let me emphasise that tsk_is_oom_victim will close the race
> completely, while mem_cgroup_margin will always be racy. So the question
> is whether we want to close the race because it is just too easy for
> userspace to hit it or keep the global and memcg oom handling as close
> as possible.
>
Yes, adding tsk_is_oom_victim(current) before calling out_of_memory() from
both global OOM and memcg OOM paths can close the race completely. (But
note that tsk_is_oom_victim(current) for global OOM path needs to check for
__GFP_NOFAIL in order to handle genuinely OOM case.)
Re: [RFC PATCH 2/2] memcg: do not report racy no-eligible OOM tasks
On 2018/10/27 4:25, Michal Hocko wrote:
>> out_of_memory() bails on task_will_free_mem(current), which
>> specifically *excludes* already reaped tasks. Why are we then adding a
>> separate check before that to bail on already reaped victims?
>
> 696453e66630a has introduced the bail out.
>
>> Do we want to bail if current is a reaped victim or not?
>>
>> I don't see how we could skip it safely in general: the current task
>> might have been killed and reaped and gotten access to the memory
>> reserve and still fail to allocate on its way out. It needs to kill
>> the next task if there is one, or warn if there isn't another
>> one. Because we're genuinely oom without reclaimable tasks.
>
> Yes, this would be the case for the global case which is a real OOM
> situation. Memcg oom is somehow more relaxed because the oom is local.
We can handle possibility of genuinely OOM without reclaimable tasks.
Only __GFP_NOFAIL OOM has to select next OOM victim. There is no need to
select next OOM victim unless __GFP_NOFAIL. Commit 696453e66630ad45
("mm, oom: task_will_free_mem should skip oom_reaped tasks") was too simple.
On 2018/10/27 4:33, Michal Hocko wrote:
> On Fri 26-10-18 21:25:51, Michal Hocko wrote:
>> On Fri 26-10-18 10:25:31, Johannes Weiner wrote:
> [...]
>>> There is of course the scenario brought forward in this thread, where
>>> multiple threads of a process race and the second one enters oom even
>>> though it doesn't need to anymore. What the global case does to catch
>>> this is to grab the oom lock and do one last alloc attempt. Should
>>> memcg lock the oom_lock and try one more time to charge the memcg?
>>
>> That would be another option. I agree that making it more towards the
>> global case makes it more attractive. My tsk_is_oom_victim is more
>> towards "plug this particular case".
>
> Nevertheless let me emphasise that tsk_is_oom_victim will close the race
> completely, while mem_cgroup_margin will always be racy. So the question
> is whether we want to close the race because it is just too easy for
> userspace to hit it or keep the global and memcg oom handling as close
> as possible.
>
Yes, adding tsk_is_oom_victim(current) before calling out_of_memory() from
both global OOM and memcg OOM paths can close the race completely. (But
note that tsk_is_oom_victim(current) for global OOM path needs to check for
__GFP_NOFAIL in order to handle genuinely OOM case.)
Re: [GIT] Sparc
On Fri, Oct 26, 2018 at 4:08 PM David Miller wrote: > > Some more sparc fixups, mostly aimed at getting the allmodconfig build > up and clean again. Pulled, Linus
Re: [GIT] Sparc
On Fri, Oct 26, 2018 at 4:08 PM David Miller wrote: > > Some more sparc fixups, mostly aimed at getting the allmodconfig build > up and clean again. Pulled, Linus
Re: [RFC 00/60] Coscheduling for Linux
On 27/10/2018 01.05, Subhra Mazumdar wrote: > > >> D) What can I *not* do with this? >> - >> >> Besides the missing load-balancing within coscheduled task-groups, this >> implementation has the following properties, which might be considered >> short-comings. >> >> This particular implementation focuses on SCHED_OTHER tasks managed by CFS >> and allows coscheduling them. Interrupts as well as tasks in higher >> scheduling classes are currently out-of-scope: they are assumed to be >> negligible interruptions as far as coscheduling is concerned and they do >> *not* cause a preemption of a whole group. This implementation could be >> extended to cover higher scheduling classes. Interrupts, however, are an >> orthogonal issue. >> >> The collective context switch from one coscheduled set of tasks to another >> -- while fast -- is not atomic. If a use-case needs the absolute guarantee >> that all tasks of the previous set have stopped executing before any task >> of the next set starts executing, an additional hand-shake/barrier needs to >> be added. >> > The leader doesn't kick the other cpus _immediately_ to switch to a > different cosched group. It does. (Or at least, it should, in case you found evidence that it does not.) Specifically, the logic to not preempt the currently running task before some minimum time has passed, is without effect for a collective context switch. > So threads from previous cosched group will keep > running in other HTs till their sched_slice is over (in worst case). This > can still keep the window of L1TF vulnerability open? No. Per the above, the window due to the collective context switch should not be as long as "the remaining time slice" but more towards the IPI delay. During this window, tasks of different coscheduling groups may execute simultaneously. In addition (as mentioned in the quoted text above), there more cases where a task of a coscheduled group on one SMT sibling may execute simultaneously with some other code not from the same coscheduled group: tasks in scheduling classes higher than CFS, and interrupts -- as both of them operate outside the scope of the coscheduler. Regards Jan
Re: [RFC 00/60] Coscheduling for Linux
On 27/10/2018 01.05, Subhra Mazumdar wrote: > > >> D) What can I *not* do with this? >> - >> >> Besides the missing load-balancing within coscheduled task-groups, this >> implementation has the following properties, which might be considered >> short-comings. >> >> This particular implementation focuses on SCHED_OTHER tasks managed by CFS >> and allows coscheduling them. Interrupts as well as tasks in higher >> scheduling classes are currently out-of-scope: they are assumed to be >> negligible interruptions as far as coscheduling is concerned and they do >> *not* cause a preemption of a whole group. This implementation could be >> extended to cover higher scheduling classes. Interrupts, however, are an >> orthogonal issue. >> >> The collective context switch from one coscheduled set of tasks to another >> -- while fast -- is not atomic. If a use-case needs the absolute guarantee >> that all tasks of the previous set have stopped executing before any task >> of the next set starts executing, an additional hand-shake/barrier needs to >> be added. >> > The leader doesn't kick the other cpus _immediately_ to switch to a > different cosched group. It does. (Or at least, it should, in case you found evidence that it does not.) Specifically, the logic to not preempt the currently running task before some minimum time has passed, is without effect for a collective context switch. > So threads from previous cosched group will keep > running in other HTs till their sched_slice is over (in worst case). This > can still keep the window of L1TF vulnerability open? No. Per the above, the window due to the collective context switch should not be as long as "the remaining time slice" but more towards the IPI delay. During this window, tasks of different coscheduling groups may execute simultaneously. In addition (as mentioned in the quoted text above), there more cases where a task of a coscheduled group on one SMT sibling may execute simultaneously with some other code not from the same coscheduled group: tasks in scheduling classes higher than CFS, and interrupts -- as both of them operate outside the scope of the coscheduler. Regards Jan
[git pull] Input updates for v4.20-rc0
Hi Linus, Please pull from: git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus to receive updates for the input subsystem. Just random driver fixups, nothing exiting. Changelog: - Brian Masney (1): Input: pwm-vibrator - correct pwms in DT binding example Dmitry Torokhov (1): Input: synaptics - avoid using uninitialized variable when probing George G. Davis (1): Input: atmel_mxt_ts - fix multiple includes Gustavo A. R. Silva (3): Input: cyapa - mark expected switch fall-throughs Input: atmel_mxt_ts - mark expected switch fall-through Input: xen-kbdfront - mark expected switch fall-through Hans de Goede (1): Input: of_touchscreen - add support for touchscreen-min-x|y Julian Sax (1): Input: silead - try firmware reload after unsuccessful resume Linus Walleij (1): Input: Fix DIR-685 touchkeys MAINTAINERS entry Martin Kepplinger (1): Input: st1232 - set INPUT_PROP_DIRECT property Randy Dunlap (1): Input: wm97xx-ts - fix exit path Rob Herring (2): Input: sun4i-lradc - convert to using %pOFn instead of device_node.name Input: xilinx_ps2 - convert to using %pOFn instead of device_node.name Stephen Boyd (1): Input: elants_i2c - use DMA safe i2c when possible Diffstat: .../devicetree/bindings/input/pwm-vibrator.txt | 4 +-- .../bindings/input/touchscreen/touchscreen.txt | 6 ++-- MAINTAINERS| 2 +- drivers/input/keyboard/sun4i-lradc-keys.c | 6 ++-- drivers/input/misc/xen-kbdfront.c | 2 +- drivers/input/mouse/cyapa_gen3.c | 4 +-- drivers/input/mouse/synaptics.c| 4 +-- drivers/input/serio/xilinx_ps2.c | 2 +- drivers/input/touchscreen/atmel_mxt_ts.c | 3 +- drivers/input/touchscreen/elants_i2c.c | 7 +++-- drivers/input/touchscreen/of_touchscreen.c | 36 +- drivers/input/touchscreen/silead.c | 13 drivers/input/touchscreen/st1232.c | 1 + drivers/input/touchscreen/wm97xx-core.c| 3 +- 14 files changed, 64 insertions(+), 29 deletions(-) Thanks. -- Dmitry
[git pull] Input updates for v4.20-rc0
Hi Linus, Please pull from: git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus to receive updates for the input subsystem. Just random driver fixups, nothing exiting. Changelog: - Brian Masney (1): Input: pwm-vibrator - correct pwms in DT binding example Dmitry Torokhov (1): Input: synaptics - avoid using uninitialized variable when probing George G. Davis (1): Input: atmel_mxt_ts - fix multiple includes Gustavo A. R. Silva (3): Input: cyapa - mark expected switch fall-throughs Input: atmel_mxt_ts - mark expected switch fall-through Input: xen-kbdfront - mark expected switch fall-through Hans de Goede (1): Input: of_touchscreen - add support for touchscreen-min-x|y Julian Sax (1): Input: silead - try firmware reload after unsuccessful resume Linus Walleij (1): Input: Fix DIR-685 touchkeys MAINTAINERS entry Martin Kepplinger (1): Input: st1232 - set INPUT_PROP_DIRECT property Randy Dunlap (1): Input: wm97xx-ts - fix exit path Rob Herring (2): Input: sun4i-lradc - convert to using %pOFn instead of device_node.name Input: xilinx_ps2 - convert to using %pOFn instead of device_node.name Stephen Boyd (1): Input: elants_i2c - use DMA safe i2c when possible Diffstat: .../devicetree/bindings/input/pwm-vibrator.txt | 4 +-- .../bindings/input/touchscreen/touchscreen.txt | 6 ++-- MAINTAINERS| 2 +- drivers/input/keyboard/sun4i-lradc-keys.c | 6 ++-- drivers/input/misc/xen-kbdfront.c | 2 +- drivers/input/mouse/cyapa_gen3.c | 4 +-- drivers/input/mouse/synaptics.c| 4 +-- drivers/input/serio/xilinx_ps2.c | 2 +- drivers/input/touchscreen/atmel_mxt_ts.c | 3 +- drivers/input/touchscreen/elants_i2c.c | 7 +++-- drivers/input/touchscreen/of_touchscreen.c | 36 +- drivers/input/touchscreen/silead.c | 13 drivers/input/touchscreen/st1232.c | 1 + drivers/input/touchscreen/wm97xx-core.c| 3 +- 14 files changed, 64 insertions(+), 29 deletions(-) Thanks. -- Dmitry
[RFC 00/60] Coscheduling for Linux
On 19/10/2018 02.26, Subhra Mazumdar wrote: > Hi Jan, Hi. Sorry for the delay. > On 9/7/18 2:39 PM, Jan H. Schönherr wrote: >> The collective context switch from one coscheduled set of tasks to another >> -- while fast -- is not atomic. If a use-case needs the absolute guarantee >> that all tasks of the previous set have stopped executing before any task >> of the next set starts executing, an additional hand-shake/barrier needs to >> be added. >> > Do you know how much is the delay? i.e what is overlap time when a thread > of new group starts executing on one HT while there is still thread of > another group running on the other HT? The delay is roughly equivalent to the IPI latency, if we're just talking about coscheduling at SMT level: one sibling decides to schedule another group, sends an IPI to the other sibling(s), and may already start executing a task of that other group, before the IPI is received on the other end. Now, there are some things that may delay processing an IPI, but in those cases the target CPU isn't executing user code. I've yet to produce some current numbers for SMT-only coscheduling. An older ballpark number I have is about 2 microseconds for the collective context switch of one hierarchy level, but take that with a grain of salt. Regards Jan
[RFC 00/60] Coscheduling for Linux
On 19/10/2018 02.26, Subhra Mazumdar wrote: > Hi Jan, Hi. Sorry for the delay. > On 9/7/18 2:39 PM, Jan H. Schönherr wrote: >> The collective context switch from one coscheduled set of tasks to another >> -- while fast -- is not atomic. If a use-case needs the absolute guarantee >> that all tasks of the previous set have stopped executing before any task >> of the next set starts executing, an additional hand-shake/barrier needs to >> be added. >> > Do you know how much is the delay? i.e what is overlap time when a thread > of new group starts executing on one HT while there is still thread of > another group running on the other HT? The delay is roughly equivalent to the IPI latency, if we're just talking about coscheduling at SMT level: one sibling decides to schedule another group, sends an IPI to the other sibling(s), and may already start executing a task of that other group, before the IPI is received on the other end. Now, there are some things that may delay processing an IPI, but in those cases the target CPU isn't executing user code. I've yet to produce some current numbers for SMT-only coscheduling. An older ballpark number I have is about 2 microseconds for the collective context switch of one hierarchy level, but take that with a grain of salt. Regards Jan
[RFC PATCH 5/7] kernel/kthread.c: do runtime check of format string in kthread_create_on_cpu()
One is supposed to pass in a format string containing (at most) one %u
instance. Use fmtcheck() to enforce that at runtime, WARNing and falling
back to a harmless "kthread/%u" in case verification fails.
Signed-off-by: Rasmus Villemoes
---
kernel/kthread.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 087d18d771b5..fddfe605632b 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -441,8 +441,8 @@ struct task_struct *kthread_create_on_cpu(int
(*threadfn)(void *data),
{
struct task_struct *p;
- p = kthread_create_on_node(threadfn, data, cpu_to_node(cpu), namefmt,
- cpu);
+ p = kthread_create_on_node(threadfn, data, cpu_to_node(cpu),
+ fmtcheck(namefmt, "kthread/%u", 0), cpu);
if (IS_ERR(p))
return p;
kthread_bind(p, cpu);
--
2.19.1.6.gbde171bbf5
[RFC PATCH 2/7] lib/vsprintf.c: add fmtcheck utility
We have a few places in the kernel where a *printf function is used with
a non-constant format string, making the ordinary static type checking
done by gcc et al. impossible. With extra instrumentation, some things
can still be caught at build time, but that still leaves a number of
places unchecked. So this patch adds a function for doing run-time
verification of a given format string against a template.
The fmtcheck() function takes two format string arguments and checks
whether they contain the same printf specifiers. If they do, the
first (the string-to-be-checked) string is returned. If not, the
second (the template) is returned - the resulting formatted string is
likely garbage, but this should still be better than using arguments of
the wrong type.
Regardless of which string is returned at run-time, the __format_arg
attribute allows the compiler to do type-checking if the fmtcheck()
function is used inside a *printf call, e.g.
sprintf(buf, fmtcheck(what->ever, "%d %lx", 0), i, m)
This also serves as documentation for whoever creates the string found
at what->ever that it should contain these two specifiers.
We actually make fmtcheck() a macro that tries very hard to ensure the
template argument is a string literal - partly to help avoid mixing up
the two "const char*" arguments, partly because much of the point of
this sanity checking vanishes if the template is not a literal (e.g.,
the __format_arg annotation becomes useless).
We don't treat "%*.*s" and "%d %d %s" as equivalent, despite them
taking the same vararg types, since they're morally very distinct. In
fact, at least for now, we don't even treat "%d" and "%u" as
equivalent. We can relax that, possibly via FMTCHECK_* flags, but let's
first see which users there might be and what they'd want.
If either string contains a %p, we really should check the following
alphanumerics to see which (if any) extension is used and check that
they match as well. For now, just complain loudly, partly because I'm
lazy, partly because I don't know any in-tree code that might use
fmtcheck() with a %p in the template, and I can't really imagine
anyone would use a %pXX extension in a non-constant format string.
I'm making this optional, but default y, since I don't suppose
fmtcheck() will ever appear in a hot path.
The BSDs (and libbsd on linux) contain a fmtcheck() function; I took the
name and return semantics from that.
Signed-off-by: Rasmus Villemoes
---
include/linux/kernel.h | 18
lib/Kconfig.debug | 9 ++
lib/vsprintf.c | 65 ++
3 files changed, 92 insertions(+)
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index d6aac75b51ba..8e9154e100c3 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -495,6 +495,24 @@ char *kvasprintf(gfp_t gfp, const char *fmt, va_list args);
extern __printf(2, 0)
const char *kvasprintf_const(gfp_t gfp, const char *fmt, va_list args);
+#define FMTCHECK_SILENT0x01
+#define FMTCHECK_NO_EXTRA_ARGS 0x02
+#ifdef CONFIG_FMTCHECK
+__format_arg(2)
+const char *_fmtcheck(const char *fmt, const char *tmpl, unsigned flags);
+#else
+static inline __format_arg(2) const char *
+_fmtcheck(const char *fmt, const char *tmpl, unsigned flags)
+{
+ return fmt;
+}
+#endif
+/*
+ * Use of fmtcheck is pointless if the template is not a string
+ * literal, so try to enforce that.
+ */
+#define fmtcheck(fmt, tmpl, flags) _fmtcheck(fmt, "" tmpl "", flags)
+
extern __scanf(2, 3)
int sscanf(const char *, const char *, ...);
extern __scanf(2, 0)
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 4966c4fbe7f7..adfd431c6876 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1037,6 +1037,15 @@ config DEBUG_PREEMPT
if kernel code uses it in a preemption-unsafe way. Also, the kernel
will detect preemption count underflows.
+config FMTCHECK
+ bool "Runtime format string checking"
+ default y
+ help
+ If you say Y here, the kernel performs runtime sanity checks
+ of non-constant format strings against builtin templates,
+ issuing a warning and using the template as a fallback in
+ case of mismatch.
+
menu "Lock Debugging (spinlocks, mutexes, etc...)"
config LOCK_DEBUGGING_SUPPORT
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index d5b3a3f95c01..81b7cda71158 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -3201,3 +3201,68 @@ int sscanf(const char *buf, const char *fmt, ...)
return i;
}
EXPORT_SYMBOL(sscanf);
+
+#ifdef CONFIG_FMTCHECK
+static int
+next_interesting_spec(const char **s, struct printf_spec *spec)
+{
+ int len;
+
+ while (1) {
+ len = format_decode(*s, spec);
+ if (!len)
+ return 0;
+ *s += len;
+ if (spec->type == FORMAT_TYPE_NONE ||
+ spec->type == FORMAT_TYPE_PERCENT_CHAR)
+ continue;
+
[RFC PATCH 1/7] compiler_attributes.h: add __attribute__((format_arg)) shorthand
The __format_arg attribute tells gcc that it can use a specific argument to the annotated function as the format string for the purpose of type-checking a surrounding __printf function call. For example, assuming one has a fmtcheck function declared as const char *fmtcheck(const char *, const char *, unsigned) __format_arg(2); and this is used in sprintf(buf, fmtcheck(what->ever, "%d %lx", 0), i, m) gcc checks that the varargs (i and m) matches the second argument to the fmtcheck function, i.e. that they are (int, long). With sprintf(buf, what->ever, i, m) the compiler cannot do any type checking. Even a static inline fmtcheck() that just returns its first argument would provide documentation for which specifiers what->ever is supposed to contain, but we'll implement an actual run-time check later. Signed-off-by: Rasmus Villemoes --- include/linux/compiler_attributes.h | 13 + 1 file changed, 13 insertions(+) diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h index 6b28c1b7310c..08264df52322 100644 --- a/include/linux/compiler_attributes.h +++ b/include/linux/compiler_attributes.h @@ -32,6 +32,7 @@ # define __GCC4_has_attribute___assume_aligned__ (__GNUC_MINOR__ >= 9) # define __GCC4_has_attribute___designated_init__ 0 # define __GCC4_has_attribute___externally_visible__ 1 +# define __GCC4_has_attribute___format_arg__ 1 # define __GCC4_has_attribute___noclone__ 1 # define __GCC4_has_attribute___optimize__1 # define __GCC4_has_attribute___nonstring__ 0 @@ -140,6 +141,18 @@ #define __printf(a, b) __attribute__((__format__(printf, a, b))) #define __scanf(a, b) __attribute__((__format__(scanf, a, b))) +/* + * Optional + * + * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-format_005farg-function-attribute + * clang: apparently supported, but undocumented + */ +#if __has_attribute(__format_arg__) +# define __format_arg(n) __attribute__((__format_arg__(n))) +#else +# define __format_arg(n) +#endif + /* * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-gnu_005finline-function-attribute * clang: https://clang.llvm.org/docs/AttributeReference.html#gnu-inline -- 2.19.1.6.gbde171bbf5
[RFC PATCH 6/7] nfs: use fmtcheck() in root_nfs_data
tmp is initially the string "/tftpboot/%s", but it may be changed from the calls to root_nfs_parse_options. While an nfsroot= command line option can probably be trusted (or the user gets to keep both pieces), it's also possible for contents to come via a BOOTP option. Do a sanity check of fmt to ensure it doesn't contain odd printf specifiers that would make snprintf go off into the weeds. The lack of the FMTCHECK_NO_EXTRA_ARGS flag (i.e., the last 0 argument) means we allow either no specifiers or precisely one occurrence of %s in tmp. Signed-off-by: Rasmus Villemoes --- fs/nfs/nfsroot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/nfs/nfsroot.c b/fs/nfs/nfsroot.c index effaa4247b91..71db0149eb49 100644 --- a/fs/nfs/nfsroot.c +++ b/fs/nfs/nfsroot.c @@ -261,7 +261,7 @@ static int __init root_nfs_data(char *cmdline) * mess into nfs_root_device. */ len = snprintf(nfs_export_path, sizeof(nfs_export_path), - tmp, utsname()->nodename); + fmtcheck(tmp, "%s", 0), utsname()->nodename); if (len >= (int)sizeof(nfs_export_path)) goto out_devnametoolong; len = snprintf(nfs_root_device, sizeof(nfs_root_device), -- 2.19.1.6.gbde171bbf5
[RFC PATCH 4/7] lib/test_printf.c: add a few fmtcheck() test cases
It should be trivial to add more test cases, once we figure out the
exact rules for being compatible or not. Perhaps we'll have to extend
the struct test with a flags element if we add flags that affect the
return value.
Signed-off-by: Rasmus Villemoes
---
lib/test_printf.c | 43 +++
1 file changed, 43 insertions(+)
diff --git a/lib/test_printf.c b/lib/test_printf.c
index 53527ea822b5..5b6ba0e817d5 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -535,6 +535,47 @@ test_pointer(void)
flags();
}
+static void __init
+test_fmtcheck(void)
+{
+#ifdef CONFIG_FMTCHECK
+ struct test { const char *fmt; const char *tmpl; };
+ static const struct test compatible[] __initconst = {
+ {"", ""},
+ {"wlan%d", "%d"},
+ {"aa%llxbb", "cc%Lxdd%%"},
+ };
+ static const struct test incompatible[] __initconst = {
+ {"a %d b %lx", "%d %x"},
+ {"%llo", "%Lx"},
+ };
+ unsigned i;
+ const struct test *t;
+ const char *ret;
+
+ for (i = 0; i < ARRAY_SIZE(compatible); ++i) {
+ total_tests++;
+ t = [i];
+ ret = _fmtcheck(t->fmt, t->tmpl, FMTCHECK_SILENT);
+ if (ret != t->fmt) {
+ failed_tests++;
+ pr_warn("'%s' and '%s' deemed incompatible by
fmtcheck()",
+ t->fmt, t->tmpl);
+ }
+ }
+ for (i = 0; i < ARRAY_SIZE(incompatible); ++i) {
+ total_tests++;
+ t = [i];
+ ret = _fmtcheck(t->fmt, t->tmpl, FMTCHECK_SILENT);
+ if (ret != t->tmpl) {
+ failed_tests++;
+ pr_warn("'%s' and '%s' deemed compatible by fmtcheck()",
+ t->fmt, t->tmpl);
+ }
+ }
+#endif
+}
+
static int __init
test_printf_init(void)
{
@@ -548,6 +589,8 @@ test_printf_init(void)
test_string();
test_pointer();
+ test_fmtcheck();
+
kfree(alloced_buffer);
if (failed_tests == 0)
--
2.19.1.6.gbde171bbf5
[RFC PATCH 5/7] kernel/kthread.c: do runtime check of format string in kthread_create_on_cpu()
One is supposed to pass in a format string containing (at most) one %u
instance. Use fmtcheck() to enforce that at runtime, WARNing and falling
back to a harmless "kthread/%u" in case verification fails.
Signed-off-by: Rasmus Villemoes
---
kernel/kthread.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 087d18d771b5..fddfe605632b 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -441,8 +441,8 @@ struct task_struct *kthread_create_on_cpu(int
(*threadfn)(void *data),
{
struct task_struct *p;
- p = kthread_create_on_node(threadfn, data, cpu_to_node(cpu), namefmt,
- cpu);
+ p = kthread_create_on_node(threadfn, data, cpu_to_node(cpu),
+ fmtcheck(namefmt, "kthread/%u", 0), cpu);
if (IS_ERR(p))
return p;
kthread_bind(p, cpu);
--
2.19.1.6.gbde171bbf5
[RFC PATCH 2/7] lib/vsprintf.c: add fmtcheck utility
We have a few places in the kernel where a *printf function is used with
a non-constant format string, making the ordinary static type checking
done by gcc et al. impossible. With extra instrumentation, some things
can still be caught at build time, but that still leaves a number of
places unchecked. So this patch adds a function for doing run-time
verification of a given format string against a template.
The fmtcheck() function takes two format string arguments and checks
whether they contain the same printf specifiers. If they do, the
first (the string-to-be-checked) string is returned. If not, the
second (the template) is returned - the resulting formatted string is
likely garbage, but this should still be better than using arguments of
the wrong type.
Regardless of which string is returned at run-time, the __format_arg
attribute allows the compiler to do type-checking if the fmtcheck()
function is used inside a *printf call, e.g.
sprintf(buf, fmtcheck(what->ever, "%d %lx", 0), i, m)
This also serves as documentation for whoever creates the string found
at what->ever that it should contain these two specifiers.
We actually make fmtcheck() a macro that tries very hard to ensure the
template argument is a string literal - partly to help avoid mixing up
the two "const char*" arguments, partly because much of the point of
this sanity checking vanishes if the template is not a literal (e.g.,
the __format_arg annotation becomes useless).
We don't treat "%*.*s" and "%d %d %s" as equivalent, despite them
taking the same vararg types, since they're morally very distinct. In
fact, at least for now, we don't even treat "%d" and "%u" as
equivalent. We can relax that, possibly via FMTCHECK_* flags, but let's
first see which users there might be and what they'd want.
If either string contains a %p, we really should check the following
alphanumerics to see which (if any) extension is used and check that
they match as well. For now, just complain loudly, partly because I'm
lazy, partly because I don't know any in-tree code that might use
fmtcheck() with a %p in the template, and I can't really imagine
anyone would use a %pXX extension in a non-constant format string.
I'm making this optional, but default y, since I don't suppose
fmtcheck() will ever appear in a hot path.
The BSDs (and libbsd on linux) contain a fmtcheck() function; I took the
name and return semantics from that.
Signed-off-by: Rasmus Villemoes
---
include/linux/kernel.h | 18
lib/Kconfig.debug | 9 ++
lib/vsprintf.c | 65 ++
3 files changed, 92 insertions(+)
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index d6aac75b51ba..8e9154e100c3 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -495,6 +495,24 @@ char *kvasprintf(gfp_t gfp, const char *fmt, va_list args);
extern __printf(2, 0)
const char *kvasprintf_const(gfp_t gfp, const char *fmt, va_list args);
+#define FMTCHECK_SILENT0x01
+#define FMTCHECK_NO_EXTRA_ARGS 0x02
+#ifdef CONFIG_FMTCHECK
+__format_arg(2)
+const char *_fmtcheck(const char *fmt, const char *tmpl, unsigned flags);
+#else
+static inline __format_arg(2) const char *
+_fmtcheck(const char *fmt, const char *tmpl, unsigned flags)
+{
+ return fmt;
+}
+#endif
+/*
+ * Use of fmtcheck is pointless if the template is not a string
+ * literal, so try to enforce that.
+ */
+#define fmtcheck(fmt, tmpl, flags) _fmtcheck(fmt, "" tmpl "", flags)
+
extern __scanf(2, 3)
int sscanf(const char *, const char *, ...);
extern __scanf(2, 0)
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 4966c4fbe7f7..adfd431c6876 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1037,6 +1037,15 @@ config DEBUG_PREEMPT
if kernel code uses it in a preemption-unsafe way. Also, the kernel
will detect preemption count underflows.
+config FMTCHECK
+ bool "Runtime format string checking"
+ default y
+ help
+ If you say Y here, the kernel performs runtime sanity checks
+ of non-constant format strings against builtin templates,
+ issuing a warning and using the template as a fallback in
+ case of mismatch.
+
menu "Lock Debugging (spinlocks, mutexes, etc...)"
config LOCK_DEBUGGING_SUPPORT
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index d5b3a3f95c01..81b7cda71158 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -3201,3 +3201,68 @@ int sscanf(const char *buf, const char *fmt, ...)
return i;
}
EXPORT_SYMBOL(sscanf);
+
+#ifdef CONFIG_FMTCHECK
+static int
+next_interesting_spec(const char **s, struct printf_spec *spec)
+{
+ int len;
+
+ while (1) {
+ len = format_decode(*s, spec);
+ if (!len)
+ return 0;
+ *s += len;
+ if (spec->type == FORMAT_TYPE_NONE ||
+ spec->type == FORMAT_TYPE_PERCENT_CHAR)
+ continue;
+
[RFC PATCH 1/7] compiler_attributes.h: add __attribute__((format_arg)) shorthand
The __format_arg attribute tells gcc that it can use a specific argument to the annotated function as the format string for the purpose of type-checking a surrounding __printf function call. For example, assuming one has a fmtcheck function declared as const char *fmtcheck(const char *, const char *, unsigned) __format_arg(2); and this is used in sprintf(buf, fmtcheck(what->ever, "%d %lx", 0), i, m) gcc checks that the varargs (i and m) matches the second argument to the fmtcheck function, i.e. that they are (int, long). With sprintf(buf, what->ever, i, m) the compiler cannot do any type checking. Even a static inline fmtcheck() that just returns its first argument would provide documentation for which specifiers what->ever is supposed to contain, but we'll implement an actual run-time check later. Signed-off-by: Rasmus Villemoes --- include/linux/compiler_attributes.h | 13 + 1 file changed, 13 insertions(+) diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h index 6b28c1b7310c..08264df52322 100644 --- a/include/linux/compiler_attributes.h +++ b/include/linux/compiler_attributes.h @@ -32,6 +32,7 @@ # define __GCC4_has_attribute___assume_aligned__ (__GNUC_MINOR__ >= 9) # define __GCC4_has_attribute___designated_init__ 0 # define __GCC4_has_attribute___externally_visible__ 1 +# define __GCC4_has_attribute___format_arg__ 1 # define __GCC4_has_attribute___noclone__ 1 # define __GCC4_has_attribute___optimize__1 # define __GCC4_has_attribute___nonstring__ 0 @@ -140,6 +141,18 @@ #define __printf(a, b) __attribute__((__format__(printf, a, b))) #define __scanf(a, b) __attribute__((__format__(scanf, a, b))) +/* + * Optional + * + * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-format_005farg-function-attribute + * clang: apparently supported, but undocumented + */ +#if __has_attribute(__format_arg__) +# define __format_arg(n) __attribute__((__format_arg__(n))) +#else +# define __format_arg(n) +#endif + /* * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-gnu_005finline-function-attribute * clang: https://clang.llvm.org/docs/AttributeReference.html#gnu-inline -- 2.19.1.6.gbde171bbf5
[RFC PATCH 6/7] nfs: use fmtcheck() in root_nfs_data
tmp is initially the string "/tftpboot/%s", but it may be changed from the calls to root_nfs_parse_options. While an nfsroot= command line option can probably be trusted (or the user gets to keep both pieces), it's also possible for contents to come via a BOOTP option. Do a sanity check of fmt to ensure it doesn't contain odd printf specifiers that would make snprintf go off into the weeds. The lack of the FMTCHECK_NO_EXTRA_ARGS flag (i.e., the last 0 argument) means we allow either no specifiers or precisely one occurrence of %s in tmp. Signed-off-by: Rasmus Villemoes --- fs/nfs/nfsroot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/nfs/nfsroot.c b/fs/nfs/nfsroot.c index effaa4247b91..71db0149eb49 100644 --- a/fs/nfs/nfsroot.c +++ b/fs/nfs/nfsroot.c @@ -261,7 +261,7 @@ static int __init root_nfs_data(char *cmdline) * mess into nfs_root_device. */ len = snprintf(nfs_export_path, sizeof(nfs_export_path), - tmp, utsname()->nodename); + fmtcheck(tmp, "%s", 0), utsname()->nodename); if (len >= (int)sizeof(nfs_export_path)) goto out_devnametoolong; len = snprintf(nfs_root_device, sizeof(nfs_root_device), -- 2.19.1.6.gbde171bbf5
[RFC PATCH 4/7] lib/test_printf.c: add a few fmtcheck() test cases
It should be trivial to add more test cases, once we figure out the
exact rules for being compatible or not. Perhaps we'll have to extend
the struct test with a flags element if we add flags that affect the
return value.
Signed-off-by: Rasmus Villemoes
---
lib/test_printf.c | 43 +++
1 file changed, 43 insertions(+)
diff --git a/lib/test_printf.c b/lib/test_printf.c
index 53527ea822b5..5b6ba0e817d5 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -535,6 +535,47 @@ test_pointer(void)
flags();
}
+static void __init
+test_fmtcheck(void)
+{
+#ifdef CONFIG_FMTCHECK
+ struct test { const char *fmt; const char *tmpl; };
+ static const struct test compatible[] __initconst = {
+ {"", ""},
+ {"wlan%d", "%d"},
+ {"aa%llxbb", "cc%Lxdd%%"},
+ };
+ static const struct test incompatible[] __initconst = {
+ {"a %d b %lx", "%d %x"},
+ {"%llo", "%Lx"},
+ };
+ unsigned i;
+ const struct test *t;
+ const char *ret;
+
+ for (i = 0; i < ARRAY_SIZE(compatible); ++i) {
+ total_tests++;
+ t = [i];
+ ret = _fmtcheck(t->fmt, t->tmpl, FMTCHECK_SILENT);
+ if (ret != t->fmt) {
+ failed_tests++;
+ pr_warn("'%s' and '%s' deemed incompatible by
fmtcheck()",
+ t->fmt, t->tmpl);
+ }
+ }
+ for (i = 0; i < ARRAY_SIZE(incompatible); ++i) {
+ total_tests++;
+ t = [i];
+ ret = _fmtcheck(t->fmt, t->tmpl, FMTCHECK_SILENT);
+ if (ret != t->tmpl) {
+ failed_tests++;
+ pr_warn("'%s' and '%s' deemed compatible by fmtcheck()",
+ t->fmt, t->tmpl);
+ }
+ }
+#endif
+}
+
static int __init
test_printf_init(void)
{
@@ -548,6 +589,8 @@ test_printf_init(void)
test_string();
test_pointer();
+ test_fmtcheck();
+
kfree(alloced_buffer);
if (failed_tests == 0)
--
2.19.1.6.gbde171bbf5
[RFC PATCH 3/7] kernel.h: implement fmtmatch() wrapper around fmtcheck()
Some users may prefer to check a "user-supplied" string upfront and
return EINVAL rather than using the the template as a fallback for
printf'ing later. fmtmatch() is simply a shorthand for
fmtcheck(a, b, c | FMTCHECK_SILENT) == a.
Signed-off-by: Rasmus Villemoes
---
include/linux/kernel.h | 7 +++
1 file changed, 7 insertions(+)
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 8e9154e100c3..c2b710426227 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -507,11 +507,18 @@ _fmtcheck(const char *fmt, const char *tmpl, unsigned
flags)
return fmt;
}
#endif
+
+static inline bool
+_fmtmatch(const char *fmt, const char *tmpl, unsigned flags)
+{
+ return _fmtcheck(fmt, tmpl, flags | FMTCHECK_SILENT) == fmt;
+}
/*
* Use of fmtcheck is pointless if the template is not a string
* literal, so try to enforce that.
*/
#define fmtcheck(fmt, tmpl, flags) _fmtcheck(fmt, "" tmpl "", flags)
+#define fmtmatch(fmt, tmpl, flags) _fmtmatch(fmt, "" tmpl "", flags)
extern __scanf(2, 3)
int sscanf(const char *, const char *, ...);
--
2.19.1.6.gbde171bbf5
[RFC PATCH 3/7] kernel.h: implement fmtmatch() wrapper around fmtcheck()
Some users may prefer to check a "user-supplied" string upfront and
return EINVAL rather than using the the template as a fallback for
printf'ing later. fmtmatch() is simply a shorthand for
fmtcheck(a, b, c | FMTCHECK_SILENT) == a.
Signed-off-by: Rasmus Villemoes
---
include/linux/kernel.h | 7 +++
1 file changed, 7 insertions(+)
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 8e9154e100c3..c2b710426227 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -507,11 +507,18 @@ _fmtcheck(const char *fmt, const char *tmpl, unsigned
flags)
return fmt;
}
#endif
+
+static inline bool
+_fmtmatch(const char *fmt, const char *tmpl, unsigned flags)
+{
+ return _fmtcheck(fmt, tmpl, flags | FMTCHECK_SILENT) == fmt;
+}
/*
* Use of fmtcheck is pointless if the template is not a string
* literal, so try to enforce that.
*/
#define fmtcheck(fmt, tmpl, flags) _fmtcheck(fmt, "" tmpl "", flags)
+#define fmtmatch(fmt, tmpl, flags) _fmtmatch(fmt, "" tmpl "", flags)
extern __scanf(2, 3)
int sscanf(const char *, const char *, ...);
--
2.19.1.6.gbde171bbf5
Re: [PATCH 11/11] perf tools: Stop fallbacking to kallsyms for vdso symbols lookup
Hi,
Adrian Hunter writes:
> On 18/10/18 1:55 AM, Arnaldo Carvalho de Melo wrote:
>> From: Arnaldo Carvalho de Melo
>>
>> David reports that:
>>
>>
>> Perf has this hack where it uses the kernel symbol map as a backup when
>> a symbol can't be found in the user's symbol table(s).
>
> I don't think this is a complete fix because it exposes new problems.
This commit broke function name resolution for 'perf record -g' for me.
What I mean is, with this commit applied:
$ ./tools/perf/perf record -g -- sleep 1
$ ./tools/perf/perf report
'perf report' doesn't seem to be able to show the function names of the
trace.
If I revert this commit, function names are resolved fine.
> This code caters for branches from kernel space to user space and vice
> versa. That is, since there is only one cpumode so it is certain to be
> wrong for either 'ip' or 'addr' when they are not both in the kernel
> or both in userspace.
>
>>
>>
>> Cc: Adrian Hunter
>> Cc: David Ahern
>> Cc: Jiri Olsa
>> Cc: Namhyung Kim
>> Cc: Wang Nan
>> Link: https://lkml.kernel.org/n/tip-cs7skq9pp0kjypiju6o7t...@git.kernel.org
>> Signed-off-by: Arnaldo Carvalho de Melo
>> ---
>> tools/perf/util/event.c | 21 ++---
>> 1 file changed, 2 insertions(+), 19 deletions(-)
>>
>> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
>> index 0988eb3b844b..bc646185f8d9 100644
>> --- a/tools/perf/util/event.c
>> +++ b/tools/perf/util/event.c
>> @@ -1561,26 +1561,9 @@ struct map *thread__find_map(struct thread *thread,
>> u8 cpumode, u64 addr,
>>
>> return NULL;
>> }
>> -try_again:
>> +
>> al->map = map_groups__find(mg, al->addr);
>> -if (al->map == NULL) {
>> -/*
>> - * If this is outside of all known maps, and is a negative
>> - * address, try to look it up in the kernel dso, as it might be
>> - * a vsyscall or vdso (which executes in user-mode).
>> - *
>> - * XXX This is nasty, we should have a symbol list in the
>> - * "[vdso]" dso, but for now lets use the old trick of looking
>> - * in the whole kernel symbol list.
>> - */
>> -if (cpumode == PERF_RECORD_MISC_USER && machine &&
>> -mg != >kmaps &&
>> -machine__kernel_ip(machine, al->addr)) {
>> -mg = >kmaps;
>> -load_map = true;
>> -goto try_again;
>> -}
>> -} else {
>> +if (al->map != NULL) {
>> /*
>> * Kernel maps might be changed when loading symbols so loading
>> * must be done prior to using kernel maps.
>>
--
Vinicius
Re: [PATCH 11/11] perf tools: Stop fallbacking to kallsyms for vdso symbols lookup
Hi,
Adrian Hunter writes:
> On 18/10/18 1:55 AM, Arnaldo Carvalho de Melo wrote:
>> From: Arnaldo Carvalho de Melo
>>
>> David reports that:
>>
>>
>> Perf has this hack where it uses the kernel symbol map as a backup when
>> a symbol can't be found in the user's symbol table(s).
>
> I don't think this is a complete fix because it exposes new problems.
This commit broke function name resolution for 'perf record -g' for me.
What I mean is, with this commit applied:
$ ./tools/perf/perf record -g -- sleep 1
$ ./tools/perf/perf report
'perf report' doesn't seem to be able to show the function names of the
trace.
If I revert this commit, function names are resolved fine.
> This code caters for branches from kernel space to user space and vice
> versa. That is, since there is only one cpumode so it is certain to be
> wrong for either 'ip' or 'addr' when they are not both in the kernel
> or both in userspace.
>
>>
>>
>> Cc: Adrian Hunter
>> Cc: David Ahern
>> Cc: Jiri Olsa
>> Cc: Namhyung Kim
>> Cc: Wang Nan
>> Link: https://lkml.kernel.org/n/tip-cs7skq9pp0kjypiju6o7t...@git.kernel.org
>> Signed-off-by: Arnaldo Carvalho de Melo
>> ---
>> tools/perf/util/event.c | 21 ++---
>> 1 file changed, 2 insertions(+), 19 deletions(-)
>>
>> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
>> index 0988eb3b844b..bc646185f8d9 100644
>> --- a/tools/perf/util/event.c
>> +++ b/tools/perf/util/event.c
>> @@ -1561,26 +1561,9 @@ struct map *thread__find_map(struct thread *thread,
>> u8 cpumode, u64 addr,
>>
>> return NULL;
>> }
>> -try_again:
>> +
>> al->map = map_groups__find(mg, al->addr);
>> -if (al->map == NULL) {
>> -/*
>> - * If this is outside of all known maps, and is a negative
>> - * address, try to look it up in the kernel dso, as it might be
>> - * a vsyscall or vdso (which executes in user-mode).
>> - *
>> - * XXX This is nasty, we should have a symbol list in the
>> - * "[vdso]" dso, but for now lets use the old trick of looking
>> - * in the whole kernel symbol list.
>> - */
>> -if (cpumode == PERF_RECORD_MISC_USER && machine &&
>> -mg != >kmaps &&
>> -machine__kernel_ip(machine, al->addr)) {
>> -mg = >kmaps;
>> -load_map = true;
>> -goto try_again;
>> -}
>> -} else {
>> +if (al->map != NULL) {
>> /*
>> * Kernel maps might be changed when loading symbols so loading
>> * must be done prior to using kernel maps.
>>
--
Vinicius
[GIT] Sparc
Some more sparc fixups, mostly aimed at getting the allmodconfig build up and clean again. Please pull, thanks a lot! The following changes since commit caf539cd1087f7c36b9c4df271575e9aee49fde5: sparc: Fix VDSO build with older binutils. (2018-10-25 10:36:19 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc.git for you to fetch changes up to 6c2fc9cddc1ffdef8ada1dc8404e5affae849953: sparc64: Rework xchg() definition to avoid warnings. (2018-10-26 15:39:49 -0700) David Miller (1): sparc64: Make corrupted user stacks more debuggable. David S. Miller (2): sparc64: Export __node_distance. sparc64: Rework xchg() definition to avoid warnings. arch/sparc/include/asm/cmpxchg_64.h | 7 ++- arch/sparc/include/asm/switch_to_64.h | 3 ++- arch/sparc/kernel/process_64.c| 25 +++-- arch/sparc/kernel/rtrap_64.S | 1 + arch/sparc/kernel/signal32.c | 12 ++-- arch/sparc/kernel/signal_64.c | 6 +- arch/sparc/mm/init_64.c | 1 + 7 files changed, 44 insertions(+), 11 deletions(-)
[PATCH] platform/x86: ideapad: Add Y530-15ICH to no_hw_rfkill
Lenovo Legion Y530-15ICH is another model without
hardware radio switch. Add it to no_hw_rfkill to
enable wireless.
Signed-off-by: Misha Komarovskiy
---
drivers/platform/x86/ideapad-laptop.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/platform/x86/ideapad-laptop.c
b/drivers/platform/x86/ideapad-laptop.c
index f856d59361f2..b6489cba2985 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -1147,6 +1147,13 @@ static const struct dmi_system_id no_hw_rfkill_list[] = {
},
},
{
+ .ident = "Lenovo Legion Y530-15ICH",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo Legion
Y530-15ICH"),
+ },
+ },
+ {
.ident = "Lenovo Legion Y720-15IKB",
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
--
2.13.7
[GIT] Sparc
Some more sparc fixups, mostly aimed at getting the allmodconfig build up and clean again. Please pull, thanks a lot! The following changes since commit caf539cd1087f7c36b9c4df271575e9aee49fde5: sparc: Fix VDSO build with older binutils. (2018-10-25 10:36:19 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc.git for you to fetch changes up to 6c2fc9cddc1ffdef8ada1dc8404e5affae849953: sparc64: Rework xchg() definition to avoid warnings. (2018-10-26 15:39:49 -0700) David Miller (1): sparc64: Make corrupted user stacks more debuggable. David S. Miller (2): sparc64: Export __node_distance. sparc64: Rework xchg() definition to avoid warnings. arch/sparc/include/asm/cmpxchg_64.h | 7 ++- arch/sparc/include/asm/switch_to_64.h | 3 ++- arch/sparc/kernel/process_64.c| 25 +++-- arch/sparc/kernel/rtrap_64.S | 1 + arch/sparc/kernel/signal32.c | 12 ++-- arch/sparc/kernel/signal_64.c | 6 +- arch/sparc/mm/init_64.c | 1 + 7 files changed, 44 insertions(+), 11 deletions(-)
[PATCH] platform/x86: ideapad: Add Y530-15ICH to no_hw_rfkill
Lenovo Legion Y530-15ICH is another model without
hardware radio switch. Add it to no_hw_rfkill to
enable wireless.
Signed-off-by: Misha Komarovskiy
---
drivers/platform/x86/ideapad-laptop.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/platform/x86/ideapad-laptop.c
b/drivers/platform/x86/ideapad-laptop.c
index f856d59361f2..b6489cba2985 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -1147,6 +1147,13 @@ static const struct dmi_system_id no_hw_rfkill_list[] = {
},
},
{
+ .ident = "Lenovo Legion Y530-15ICH",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo Legion
Y530-15ICH"),
+ },
+ },
+ {
.ident = "Lenovo Legion Y720-15IKB",
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
--
2.13.7
Re: [RFC 00/60] Coscheduling for Linux
D) What can I *not* do with this? - Besides the missing load-balancing within coscheduled task-groups, this implementation has the following properties, which might be considered short-comings. This particular implementation focuses on SCHED_OTHER tasks managed by CFS and allows coscheduling them. Interrupts as well as tasks in higher scheduling classes are currently out-of-scope: they are assumed to be negligible interruptions as far as coscheduling is concerned and they do *not* cause a preemption of a whole group. This implementation could be extended to cover higher scheduling classes. Interrupts, however, are an orthogonal issue. The collective context switch from one coscheduled set of tasks to another -- while fast -- is not atomic. If a use-case needs the absolute guarantee that all tasks of the previous set have stopped executing before any task of the next set starts executing, an additional hand-shake/barrier needs to be added. The leader doesn't kick the other cpus _immediately_ to switch to a different cosched group. So threads from previous cosched group will keep running in other HTs till their sched_slice is over (in worst case). This can still keep the window of L1TF vulnerability open?
Re: [RFC 00/60] Coscheduling for Linux
D) What can I *not* do with this? - Besides the missing load-balancing within coscheduled task-groups, this implementation has the following properties, which might be considered short-comings. This particular implementation focuses on SCHED_OTHER tasks managed by CFS and allows coscheduling them. Interrupts as well as tasks in higher scheduling classes are currently out-of-scope: they are assumed to be negligible interruptions as far as coscheduling is concerned and they do *not* cause a preemption of a whole group. This implementation could be extended to cover higher scheduling classes. Interrupts, however, are an orthogonal issue. The collective context switch from one coscheduled set of tasks to another -- while fast -- is not atomic. If a use-case needs the absolute guarantee that all tasks of the previous set have stopped executing before any task of the next set starts executing, an additional hand-shake/barrier needs to be added. The leader doesn't kick the other cpus _immediately_ to switch to a different cosched group. So threads from previous cosched group will keep running in other HTs till their sched_slice is over (in worst case). This can still keep the window of L1TF vulnerability open?
Re: [PATCH 1/2] x86/pkeys: copy pkey state at fork()
On Fri, 26 Oct 2018 at 18:12, Andy Lutomirski wrote: > > > > > On Oct 26, 2018, at 2:39 PM, Daniel Micay wrote: > > > > I ended up working around this with a pthread_atfork handler disabling > > my usage of the feature in the child process for the time being. I > > don't have an easy way to detect if the bug is present within a > > library so > > Can you not just make sure that the fix is backported to all relevant kernels? There are too many different distribution kernels and I won't be in control of where the software is used. > I suppose we could add a new flag for pkey_get() or something. That would work, since I can apply the workaround (disabling the feature in child processes) if I get EINVAL. The flag wouldn't need to do anything, just existing and being tied to this patch so I have a way to detect that I can safely use MPK after fork. > > I'm going to need a kernel version check with a table of > > kernel releases fixing the problem for each stable branch. > > That won’t work right on district kernels. Please don’t go there. If it's backported to an earlier version, it will just mean missing a chance to use it. I'm not going to assume that it behaves a certain way based on having an old kernel, but rather I just won't use it in a forked child on an older kernel version if I don't have a way to detect the problem. It's for a few different uses in library code so I can't have a runtime test trying to detect it with clone(...). I'd definitely prefer a proper way to detect that I can use it after fork. I really don't want to have a hack like that which is why I'm bringing it up.
Re: [PATCH 1/2] x86/pkeys: copy pkey state at fork()
On Fri, 26 Oct 2018 at 18:12, Andy Lutomirski wrote: > > > > > On Oct 26, 2018, at 2:39 PM, Daniel Micay wrote: > > > > I ended up working around this with a pthread_atfork handler disabling > > my usage of the feature in the child process for the time being. I > > don't have an easy way to detect if the bug is present within a > > library so > > Can you not just make sure that the fix is backported to all relevant kernels? There are too many different distribution kernels and I won't be in control of where the software is used. > I suppose we could add a new flag for pkey_get() or something. That would work, since I can apply the workaround (disabling the feature in child processes) if I get EINVAL. The flag wouldn't need to do anything, just existing and being tied to this patch so I have a way to detect that I can safely use MPK after fork. > > I'm going to need a kernel version check with a table of > > kernel releases fixing the problem for each stable branch. > > That won’t work right on district kernels. Please don’t go there. If it's backported to an earlier version, it will just mean missing a chance to use it. I'm not going to assume that it behaves a certain way based on having an old kernel, but rather I just won't use it in a forked child on an older kernel version if I don't have a way to detect the problem. It's for a few different uses in library code so I can't have a runtime test trying to detect it with clone(...). I'd definitely prefer a proper way to detect that I can use it after fork. I really don't want to have a hack like that which is why I'm bringing it up.
[PATCH] MIPS: OCTEON: fix out of bounds array access on CN68XX
The maximum number of interfaces is returned by
cvmx_helper_get_number_of_interfaces(), and the value is used to access
interface_port_count[]. When CN68XX support was added, we forgot
to increase the array size. Fix that.
Fixes: 2c8c3f0201333 ("MIPS: Octeon: Support additional interfaces on CN68XX")
Signed-off-by: Aaro Koskinen
---
arch/mips/cavium-octeon/executive/cvmx-helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/mips/cavium-octeon/executive/cvmx-helper.c
b/arch/mips/cavium-octeon/executive/cvmx-helper.c
index 75108ec669eb..6c79e8a16a26 100644
--- a/arch/mips/cavium-octeon/executive/cvmx-helper.c
+++ b/arch/mips/cavium-octeon/executive/cvmx-helper.c
@@ -67,7 +67,7 @@ void (*cvmx_override_pko_queue_priority) (int pko_port,
void (*cvmx_override_ipd_port_setup) (int ipd_port);
/* Port count per interface */
-static int interface_port_count[5];
+static int interface_port_count[9];
/**
* Return the number of interfaces the chip has. Each interface
--
2.17.0
[PATCH] MIPS: OCTEON: fix out of bounds array access on CN68XX
The maximum number of interfaces is returned by
cvmx_helper_get_number_of_interfaces(), and the value is used to access
interface_port_count[]. When CN68XX support was added, we forgot
to increase the array size. Fix that.
Fixes: 2c8c3f0201333 ("MIPS: Octeon: Support additional interfaces on CN68XX")
Signed-off-by: Aaro Koskinen
---
arch/mips/cavium-octeon/executive/cvmx-helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/mips/cavium-octeon/executive/cvmx-helper.c
b/arch/mips/cavium-octeon/executive/cvmx-helper.c
index 75108ec669eb..6c79e8a16a26 100644
--- a/arch/mips/cavium-octeon/executive/cvmx-helper.c
+++ b/arch/mips/cavium-octeon/executive/cvmx-helper.c
@@ -67,7 +67,7 @@ void (*cvmx_override_pko_queue_priority) (int pko_port,
void (*cvmx_override_ipd_port_setup) (int ipd_port);
/* Port count per interface */
-static int interface_port_count[5];
+static int interface_port_count[9];
/**
* Return the number of interfaces the chip has. Each interface
--
2.17.0
Re: [PATCH net-next] net/ncsi: Add NCSI Mellanox OEM command
Thanks David On 10/26/18, 10:36 AM, "David Miller" wrote: From: Vijay Khemka Date: Fri, 26 Oct 2018 17:19:49 + > Do you have any timeline when it is going to open next or how do I > know. I always announce net-next openning and closing here on the list. There is also a web site: http://vger.kernel.org/~davem/net-next.html Thanks.
Re: [PATCH net-next] net/ncsi: Add NCSI Mellanox OEM command
Thanks David On 10/26/18, 10:36 AM, "David Miller" wrote: From: Vijay Khemka Date: Fri, 26 Oct 2018 17:19:49 + > Do you have any timeline when it is going to open next or how do I > know. I always announce net-next openning and closing here on the list. There is also a web site: http://vger.kernel.org/~davem/net-next.html Thanks.
[PATCH v4 1/2] arm64: Get rid of __early_init_dt_declare_initrd()
ARM64 is the only architecture that re-defines
__early_init_dt_declare_initrd() in order for that function to populate
initrd_start/initrd_end with physical addresses instead of virtual
addresses. Instead of having an override, just get rid of that
implementation and perform the virtual to physical conversion of these
addresses in arm64_memblock_init() where relevant.
Signed-off-by: Florian Fainelli
---
arch/arm64/include/asm/memory.h | 8
arch/arm64/mm/init.c| 26 --
2 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index b96442960aea..dc3ca21ba240 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -168,14 +168,6 @@
#define IOREMAP_MAX_ORDER (PMD_SHIFT)
#endif
-#ifdef CONFIG_BLK_DEV_INITRD
-#define __early_init_dt_declare_initrd(__start, __end) \
- do {\
- initrd_start = (__start); \
- initrd_end = (__end); \
- } while (0)
-#endif
-
#ifndef __ASSEMBLY__
#include
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 3cf87341859f..98ff0f7a0f7a 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -364,6 +364,9 @@ static void __init fdt_enforce_memory_region(void)
void __init arm64_memblock_init(void)
{
const s64 linear_region_size = -(s64)PAGE_OFFSET;
+ unsigned long __maybe_unused phys_initrd_size;
+ u64 __maybe_unused phys_initrd_start;
+ u64 __maybe_unused base, size;
/* Handle linux,usable-memory-range property */
fdt_enforce_memory_region();
@@ -414,8 +417,11 @@ void __init arm64_memblock_init(void)
* initrd to become inaccessible via the linear mapping.
* Otherwise, this is a no-op
*/
- u64 base = initrd_start & PAGE_MASK;
- u64 size = PAGE_ALIGN(initrd_end) - base;
+ phys_initrd_start = __pa(initrd_start);
+ phys_initrd_size = __pa(initrd_end) - phys_initrd_start;
+
+ base = phys_initrd_start & PAGE_MASK;
+ size = PAGE_ALIGN(phys_initrd_size);
/*
* We can only add back the initrd memory if we don't end up
@@ -459,15 +465,15 @@ void __init arm64_memblock_init(void)
* pagetables with memblock.
*/
memblock_reserve(__pa_symbol(_text), _end - _text);
-#ifdef CONFIG_BLK_DEV_INITRD
- if (initrd_start) {
- memblock_reserve(initrd_start, initrd_end - initrd_start);
-
- /* the generic initrd code expects virtual addresses */
- initrd_start = __phys_to_virt(initrd_start);
- initrd_end = __phys_to_virt(initrd_end);
+ if (IS_ENABLED(CONFIG_BLK_DEV_INITRD) && initrd_start) {
+ memblock_reserve(phys_initrd_start, phys_initrd_size);
+ /*
+* initrd_below_start_ok can be changed by
+* __early_init_dt_declare_initrd(), set it back to what
+* we want here.
+*/
+ initrd_below_start_ok = 0;
}
-#endif
early_init_fdt_scan_reserved_mem();
--
2.17.1
Re: Call to Action Re: [PATCH 0/7] Code of Conduct: Fix some wording, and add an interpretation document
On Fri, Oct 26 2018, Rainer Fiebig wrote: > NeilBrown schrieb: >> On Thu, Oct 25 2018, Rainer Fiebig wrote: >> >>> Am Montag, 22. Oktober 2018, 08:20:11 schrieb NeilBrown: On Sat, Oct 20 2018, Greg Kroah-Hartman wrote: > Hi all, > > As everyone knows by now, we added a new Code of Conduct to the kernel > tree a few weeks ago. I wanted to stay detached from all this, but as remaining (publicly) silent might be seen (publicly) as acquiescing, I hereby declare that: I reject, as illegitimate, this Code and the process by which it is being "developed". It is clear from the surrounding discussions that this is well outside our core competencies. It will be flawed, it isn't what we need. I call on any other community members who reject this process to say so, not to remain silent. #Iobject We don't need a "Code of Conduct" nearly as much as we need "Leadership in conduct". Without the leadership, any code looks like a joke. >>> [...] >>> I call on you, Greg: - to abandon this divisive attempt to impose a "Code of Conduct" - to revert 8a104f8b5867c68 >>> >>> Yes but this seems increasingly unlikely now. However, there may be an >>> alternative. >>> >>> Jugding by the release-message for 4.19, some people here are fans of >>> Monty Python's. No wonder - as those guys are famous for being unrelenting >>> supporters of Political Correctness. >>> >>> So one would be on the safe side if one just supplemented "Our Pledge" >>> with this: >>> >>> "Everybody has the right to be offended." >>> >>> I think, John Cleese would also welcome this.[1] >>> >>> What do you think? >> >> I do think that giving certain rights to the community is a good thing: >> - the right to tell anyone that their speech is hurtful >> - the right to (patch) review by a third party. >> >> I don't think the right to be offended really needs to be given. >> Yes, I know it is a joke and I do like Monty Python. I just don't think >> it is particular helpful in this context. Maybe I missed something. > > Of course it's a joke and iirc it was indeed John Cleese who made it. But he > made it for a reason, out of concern. It has a serious core. > > The question is: what *is* helpful in this matter? > > Just saying "this is not helpful" isn't helpful either. It's a well-known > killer-phrase that has been used ad nauseam in this discussion. But an > alternative is never given. And thus it's just an other way of saying > "Eat it. And shut tf up!" You asked me what I thought, and I told you what I thought. If you think differently, you are quite welcome tell us - to explain the way in which you think the addition would be helpful. > > Not even *constructive* criticism is helpful. AFAIK I'm the only one here who > came up with a *complete* alternative - ignored. Others provided patches for > certain sections - ignored. Data that indicate that this may be detrimental to > Linux - ignored. Almost anything that was provided by me or others - ignored. From my perspective, providing a complete alternative is no better than what Greg did - provided a "complete" "code of conduct". Engage in discussion, present your case, make me *want* to read your document because you've shown me how it relates to me. > > What kind of community or attitude is this? This feels more like "The Wall" or > North Korea than an Open-Source-project. > > And what beat everything was to misuse famously politically *in*correct Monty > Python to malign criticism of this Political-Correctness-monster. The > "People's Front" - message will forever be a prominent exhibit in "Monty > Python's Hall of Shame". And the author should be banned from laughing about > MP-sketches until he recants. Perhaps one should also report this incident to > the "Ministry of Silly CoCs". ;) > >> For myself, I relinquish my right to be offended. I just don't do it. >> It doesn't seem to be worth the effort. > > I don't. John Cleese is a smart guy. And he has a point. > > > OK, thanks for your reply! But I think it's time for me to move on. "Cut your > losses", as they say. > Thanks for participating! NeilBrown > > Good luck and regards! > > Rainer Fiebig signature.asc Description: PGP signature
[PATCH v4 0/2] arm64: Get rid of __early_init_dt_declare_initrd()
Hi all, I numbered this v4 because this is still revolving around the same initial desire to cut the build time of an ARM64 kernel when toggling CONFIG_BLK_DEV_INITRD. This 4th version is basically the 3rd possible way to just get rid of __early_init_dt_declare_initrd() for ARM64. I previously mentioned that I was working on making phys_initrd_start/phys_initrd_size generic across architectures that make use of it, which would cover ARM (32-bit), unicore32 and possibly arm64, but upon second glance, this does not necessarily help, or rather this patch series actually helps make a smoother conversion in the future. Comments welcome, sorry for making so many iterations, it's Friday. Previous discussions/submissions list here: v3: https://www.spinics.net/lists/arm-kernel/msg683566.html v2: https://lkml.org/lkml/2018/10/25/4 Changes in v4: - perform the physical to virtual initrd conversion straight within arm64_memblock_init() to get the correct memblock reservation to occur Changes in v3: - attempt to change drivers/of/fdt.c to absorb ARM64's specific behavior Changes in v2: - put an /* empty */ comment in the asm-generic/initrd.h file - trim down the CC list to maximize the chances of people receiving this Florian Fainelli (2): arm64: Get rid of __early_init_dt_declare_initrd() of/fdt: Remove definition check for __early_init_dt_declare_initrd() arch/arm64/include/asm/memory.h | 8 arch/arm64/mm/init.c| 26 -- drivers/of/fdt.c| 2 -- 3 files changed, 16 insertions(+), 20 deletions(-) -- 2.17.1
[PATCH v4 2/2] of/fdt: Remove definition check for __early_init_dt_declare_initrd()
With the one and only architecture (ARM64) no longer defining a custom
__early_init_dt_declare_initrd() function, just get rid of the check for
that function being already defined.
Signed-off-by: Florian Fainelli
---
drivers/of/fdt.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 800ad252cf9c..797c9e2d6d57 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -892,7 +892,6 @@ const void * __init of_flat_dt_match_machine(const void
*default_match,
}
#ifdef CONFIG_BLK_DEV_INITRD
-#ifndef __early_init_dt_declare_initrd
static void __early_init_dt_declare_initrd(unsigned long start,
unsigned long end)
{
@@ -900,7 +899,6 @@ static void __early_init_dt_declare_initrd(unsigned long
start,
initrd_end = (unsigned long)__va(end);
initrd_below_start_ok = 1;
}
-#endif
/**
* early_init_dt_check_for_initrd - Decode initrd location from flat tree
--
2.17.1
[PATCH v4 0/2] arm64: Get rid of __early_init_dt_declare_initrd()
Hi all, I numbered this v4 because this is still revolving around the same initial desire to cut the build time of an ARM64 kernel when toggling CONFIG_BLK_DEV_INITRD. This 4th version is basically the 3rd possible way to just get rid of __early_init_dt_declare_initrd() for ARM64. I previously mentioned that I was working on making phys_initrd_start/phys_initrd_size generic across architectures that make use of it, which would cover ARM (32-bit), unicore32 and possibly arm64, but upon second glance, this does not necessarily help, or rather this patch series actually helps make a smoother conversion in the future. Comments welcome, sorry for making so many iterations, it's Friday. Previous discussions/submissions list here: v3: https://www.spinics.net/lists/arm-kernel/msg683566.html v2: https://lkml.org/lkml/2018/10/25/4 Changes in v4: - perform the physical to virtual initrd conversion straight within arm64_memblock_init() to get the correct memblock reservation to occur Changes in v3: - attempt to change drivers/of/fdt.c to absorb ARM64's specific behavior Changes in v2: - put an /* empty */ comment in the asm-generic/initrd.h file - trim down the CC list to maximize the chances of people receiving this Florian Fainelli (2): arm64: Get rid of __early_init_dt_declare_initrd() of/fdt: Remove definition check for __early_init_dt_declare_initrd() arch/arm64/include/asm/memory.h | 8 arch/arm64/mm/init.c| 26 -- drivers/of/fdt.c| 2 -- 3 files changed, 16 insertions(+), 20 deletions(-) -- 2.17.1
[PATCH v4 2/2] of/fdt: Remove definition check for __early_init_dt_declare_initrd()
With the one and only architecture (ARM64) no longer defining a custom
__early_init_dt_declare_initrd() function, just get rid of the check for
that function being already defined.
Signed-off-by: Florian Fainelli
---
drivers/of/fdt.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 800ad252cf9c..797c9e2d6d57 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -892,7 +892,6 @@ const void * __init of_flat_dt_match_machine(const void
*default_match,
}
#ifdef CONFIG_BLK_DEV_INITRD
-#ifndef __early_init_dt_declare_initrd
static void __early_init_dt_declare_initrd(unsigned long start,
unsigned long end)
{
@@ -900,7 +899,6 @@ static void __early_init_dt_declare_initrd(unsigned long
start,
initrd_end = (unsigned long)__va(end);
initrd_below_start_ok = 1;
}
-#endif
/**
* early_init_dt_check_for_initrd - Decode initrd location from flat tree
--
2.17.1
[PATCH v4 1/2] arm64: Get rid of __early_init_dt_declare_initrd()
ARM64 is the only architecture that re-defines
__early_init_dt_declare_initrd() in order for that function to populate
initrd_start/initrd_end with physical addresses instead of virtual
addresses. Instead of having an override, just get rid of that
implementation and perform the virtual to physical conversion of these
addresses in arm64_memblock_init() where relevant.
Signed-off-by: Florian Fainelli
---
arch/arm64/include/asm/memory.h | 8
arch/arm64/mm/init.c| 26 --
2 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index b96442960aea..dc3ca21ba240 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -168,14 +168,6 @@
#define IOREMAP_MAX_ORDER (PMD_SHIFT)
#endif
-#ifdef CONFIG_BLK_DEV_INITRD
-#define __early_init_dt_declare_initrd(__start, __end) \
- do {\
- initrd_start = (__start); \
- initrd_end = (__end); \
- } while (0)
-#endif
-
#ifndef __ASSEMBLY__
#include
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 3cf87341859f..98ff0f7a0f7a 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -364,6 +364,9 @@ static void __init fdt_enforce_memory_region(void)
void __init arm64_memblock_init(void)
{
const s64 linear_region_size = -(s64)PAGE_OFFSET;
+ unsigned long __maybe_unused phys_initrd_size;
+ u64 __maybe_unused phys_initrd_start;
+ u64 __maybe_unused base, size;
/* Handle linux,usable-memory-range property */
fdt_enforce_memory_region();
@@ -414,8 +417,11 @@ void __init arm64_memblock_init(void)
* initrd to become inaccessible via the linear mapping.
* Otherwise, this is a no-op
*/
- u64 base = initrd_start & PAGE_MASK;
- u64 size = PAGE_ALIGN(initrd_end) - base;
+ phys_initrd_start = __pa(initrd_start);
+ phys_initrd_size = __pa(initrd_end) - phys_initrd_start;
+
+ base = phys_initrd_start & PAGE_MASK;
+ size = PAGE_ALIGN(phys_initrd_size);
/*
* We can only add back the initrd memory if we don't end up
@@ -459,15 +465,15 @@ void __init arm64_memblock_init(void)
* pagetables with memblock.
*/
memblock_reserve(__pa_symbol(_text), _end - _text);
-#ifdef CONFIG_BLK_DEV_INITRD
- if (initrd_start) {
- memblock_reserve(initrd_start, initrd_end - initrd_start);
-
- /* the generic initrd code expects virtual addresses */
- initrd_start = __phys_to_virt(initrd_start);
- initrd_end = __phys_to_virt(initrd_end);
+ if (IS_ENABLED(CONFIG_BLK_DEV_INITRD) && initrd_start) {
+ memblock_reserve(phys_initrd_start, phys_initrd_size);
+ /*
+* initrd_below_start_ok can be changed by
+* __early_init_dt_declare_initrd(), set it back to what
+* we want here.
+*/
+ initrd_below_start_ok = 0;
}
-#endif
early_init_fdt_scan_reserved_mem();
--
2.17.1
Re: Call to Action Re: [PATCH 0/7] Code of Conduct: Fix some wording, and add an interpretation document
On Fri, Oct 26 2018, Rainer Fiebig wrote: > NeilBrown schrieb: >> On Thu, Oct 25 2018, Rainer Fiebig wrote: >> >>> Am Montag, 22. Oktober 2018, 08:20:11 schrieb NeilBrown: On Sat, Oct 20 2018, Greg Kroah-Hartman wrote: > Hi all, > > As everyone knows by now, we added a new Code of Conduct to the kernel > tree a few weeks ago. I wanted to stay detached from all this, but as remaining (publicly) silent might be seen (publicly) as acquiescing, I hereby declare that: I reject, as illegitimate, this Code and the process by which it is being "developed". It is clear from the surrounding discussions that this is well outside our core competencies. It will be flawed, it isn't what we need. I call on any other community members who reject this process to say so, not to remain silent. #Iobject We don't need a "Code of Conduct" nearly as much as we need "Leadership in conduct". Without the leadership, any code looks like a joke. >>> [...] >>> I call on you, Greg: - to abandon this divisive attempt to impose a "Code of Conduct" - to revert 8a104f8b5867c68 >>> >>> Yes but this seems increasingly unlikely now. However, there may be an >>> alternative. >>> >>> Jugding by the release-message for 4.19, some people here are fans of >>> Monty Python's. No wonder - as those guys are famous for being unrelenting >>> supporters of Political Correctness. >>> >>> So one would be on the safe side if one just supplemented "Our Pledge" >>> with this: >>> >>> "Everybody has the right to be offended." >>> >>> I think, John Cleese would also welcome this.[1] >>> >>> What do you think? >> >> I do think that giving certain rights to the community is a good thing: >> - the right to tell anyone that their speech is hurtful >> - the right to (patch) review by a third party. >> >> I don't think the right to be offended really needs to be given. >> Yes, I know it is a joke and I do like Monty Python. I just don't think >> it is particular helpful in this context. Maybe I missed something. > > Of course it's a joke and iirc it was indeed John Cleese who made it. But he > made it for a reason, out of concern. It has a serious core. > > The question is: what *is* helpful in this matter? > > Just saying "this is not helpful" isn't helpful either. It's a well-known > killer-phrase that has been used ad nauseam in this discussion. But an > alternative is never given. And thus it's just an other way of saying > "Eat it. And shut tf up!" You asked me what I thought, and I told you what I thought. If you think differently, you are quite welcome tell us - to explain the way in which you think the addition would be helpful. > > Not even *constructive* criticism is helpful. AFAIK I'm the only one here who > came up with a *complete* alternative - ignored. Others provided patches for > certain sections - ignored. Data that indicate that this may be detrimental to > Linux - ignored. Almost anything that was provided by me or others - ignored. From my perspective, providing a complete alternative is no better than what Greg did - provided a "complete" "code of conduct". Engage in discussion, present your case, make me *want* to read your document because you've shown me how it relates to me. > > What kind of community or attitude is this? This feels more like "The Wall" or > North Korea than an Open-Source-project. > > And what beat everything was to misuse famously politically *in*correct Monty > Python to malign criticism of this Political-Correctness-monster. The > "People's Front" - message will forever be a prominent exhibit in "Monty > Python's Hall of Shame". And the author should be banned from laughing about > MP-sketches until he recants. Perhaps one should also report this incident to > the "Ministry of Silly CoCs". ;) > >> For myself, I relinquish my right to be offended. I just don't do it. >> It doesn't seem to be worth the effort. > > I don't. John Cleese is a smart guy. And he has a point. > > > OK, thanks for your reply! But I think it's time for me to move on. "Cut your > losses", as they say. > Thanks for participating! NeilBrown > > Good luck and regards! > > Rainer Fiebig signature.asc Description: PGP signature
Re: [PATCH v4] kernel/signal: Signal-based pre-coredump notification
Hi, Olge:
This is really a good idea given that "parent" is declared as RCU-protected.
Just a bit odd, though, that the "parent" has not been accessed this way in
the code base.
So just to confirm: the revised code would look like the following:
static void do_notify_parent_predump(void)
{
struct task_struct *parent;
int sig;
rcu_read_lock();
parent = rcu_dereference(current->parent);
sig = parent->signal->predump_signal;
if (sig != 0)
do_send_sig_info(sig, SEND_SIG_NOINFO, parent, PIDTYPE_TGID);
rcu_read_unlock();
}
Thank you so much for your help during this review. I would like to ack your
contribution in the "Reviewed-by:" field.
-- Enke
On 10/26/18 1:28 AM, Oleg Nesterov wrote:
> On 10/25, Enke Chen wrote:
>>
>> +static void do_notify_parent_predump(void)
>> +{
>> +struct task_struct *parent;
>> +int sig;
>> +
>> +read_lock(_lock);
>> +parent = current->parent;
>> +sig = parent->signal->predump_signal;
>> +if (sig != 0)
>> +do_send_sig_info(sig, SEND_SIG_NOINFO, parent, PIDTYPE_TGID);
>> +read_unlock(_lock);
>
> Ah. It is strange I didn't think about this before... Please, do not take
> tasklist_lock, use rcu_read_lock() instead. do_send_sig_info() uses the
> rcu-friendly lock_task_sighand(), so rcu_dereference(parent) should work
> fine.
>
> Oleg.
>
Re: [PATCH v4] kernel/signal: Signal-based pre-coredump notification
Hi, Olge:
This is really a good idea given that "parent" is declared as RCU-protected.
Just a bit odd, though, that the "parent" has not been accessed this way in
the code base.
So just to confirm: the revised code would look like the following:
static void do_notify_parent_predump(void)
{
struct task_struct *parent;
int sig;
rcu_read_lock();
parent = rcu_dereference(current->parent);
sig = parent->signal->predump_signal;
if (sig != 0)
do_send_sig_info(sig, SEND_SIG_NOINFO, parent, PIDTYPE_TGID);
rcu_read_unlock();
}
Thank you so much for your help during this review. I would like to ack your
contribution in the "Reviewed-by:" field.
-- Enke
On 10/26/18 1:28 AM, Oleg Nesterov wrote:
> On 10/25, Enke Chen wrote:
>>
>> +static void do_notify_parent_predump(void)
>> +{
>> +struct task_struct *parent;
>> +int sig;
>> +
>> +read_lock(_lock);
>> +parent = current->parent;
>> +sig = parent->signal->predump_signal;
>> +if (sig != 0)
>> +do_send_sig_info(sig, SEND_SIG_NOINFO, parent, PIDTYPE_TGID);
>> +read_unlock(_lock);
>
> Ah. It is strange I didn't think about this before... Please, do not take
> tasklist_lock, use rcu_read_lock() instead. do_send_sig_info() uses the
> rcu-friendly lock_task_sighand(), so rcu_dereference(parent) should work
> fine.
>
> Oleg.
>
[PATCH] x86: traps.c: use format string with panic() call
Building with -Wformat-nonliteral gives
arch/x86/kernel/traps.c:334:2: warning: format not a string literal and no
format arguments [-Wformat-nonliteral]
panic(message);
handle_stack_overflow can only be called from two places (kernel/traps.c
and via inline asm in mm/fault.c), in both cases with a string not
containing format specifiers, but we might as well silence this warning
using "%s" as format string.
Signed-off-by: Rasmus Villemoes
---
arch/x86/kernel/traps.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 8f6dcd88202e..9b7c4ca8f0a7 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -306,7 +306,7 @@ __visible void __noreturn handle_stack_overflow(const char
*message,
die(message, regs, 0);
/* Be absolutely certain we don't return. */
- panic(message);
+ panic("%s", message);
}
#endif
--
2.19.1.6.gbde171bbf5
[PATCH] x86: traps.c: use format string with panic() call
Building with -Wformat-nonliteral gives
arch/x86/kernel/traps.c:334:2: warning: format not a string literal and no
format arguments [-Wformat-nonliteral]
panic(message);
handle_stack_overflow can only be called from two places (kernel/traps.c
and via inline asm in mm/fault.c), in both cases with a string not
containing format specifiers, but we might as well silence this warning
using "%s" as format string.
Signed-off-by: Rasmus Villemoes
---
arch/x86/kernel/traps.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 8f6dcd88202e..9b7c4ca8f0a7 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -306,7 +306,7 @@ __visible void __noreturn handle_stack_overflow(const char
*message,
die(message, regs, 0);
/* Be absolutely certain we don't return. */
- panic(message);
+ panic("%s", message);
}
#endif
--
2.19.1.6.gbde171bbf5
Re: [PATCH 1/2] x86/pkeys: copy pkey state at fork()
> On Oct 26, 2018, at 2:39 PM, Daniel Micay wrote: > > I ended up working around this with a pthread_atfork handler disabling > my usage of the feature in the child process for the time being. I > don't have an easy way to detect if the bug is present within a > library so Can you not just make sure that the fix is backported to all relevant kernels? I suppose we could add a new flag for pkey_get() or something. > I'm going to need a kernel version check with a table of > kernel releases fixing the problem for each stable branch. That won’t work right on district kernels. Please don’t go there. > > It would be helpful if there was a new cpuinfo flag to check if the > MPK state is preserved on fork in addition to the existing ospke flag. > The problem will fade away over time but in my experience there are a > lot of people using distributions with kernels not incorporating all > of the stable fixes. I expect other people will run into the problem > once hardware with MPK is more widely available and other people try > to use it for various things like moving GC or assorted security > features. Someone will end up running software adopting it on an older > kernel with the problem. > > The clobbering issue I found with MAP_FIXED_NOREPLACE isn't quite > as annoying because it was easy to make a runtime test usable in a library > to see if the feature works properly.
Re: [PATCH 1/2] x86/pkeys: copy pkey state at fork()
> On Oct 26, 2018, at 2:39 PM, Daniel Micay wrote: > > I ended up working around this with a pthread_atfork handler disabling > my usage of the feature in the child process for the time being. I > don't have an easy way to detect if the bug is present within a > library so Can you not just make sure that the fix is backported to all relevant kernels? I suppose we could add a new flag for pkey_get() or something. > I'm going to need a kernel version check with a table of > kernel releases fixing the problem for each stable branch. That won’t work right on district kernels. Please don’t go there. > > It would be helpful if there was a new cpuinfo flag to check if the > MPK state is preserved on fork in addition to the existing ospke flag. > The problem will fade away over time but in my experience there are a > lot of people using distributions with kernels not incorporating all > of the stable fixes. I expect other people will run into the problem > once hardware with MPK is more widely available and other people try > to use it for various things like moving GC or assorted security > features. Someone will end up running software adopting it on an older > kernel with the problem. > > The clobbering issue I found with MAP_FIXED_NOREPLACE isn't quite > as annoying because it was easy to make a runtime test usable in a library > to see if the feature works properly.
Re: [PATCH 1/3] tracing: merge seq_print_sym_short() and seq_print_sym_offset()
Hi Ramus,
Thanks for sending these patches. I have some small nits though.
First, please send a cover letter whenever sending more than one patch.
It just groups them better in my inbox.
The second is embedded below.
On Fri, 26 Oct 2018 23:13:44 +0200
Rasmus Villemoes wrote:
> These two functions are nearly identical, so we can avoid some code
> duplication by moving the conditional into a common implementation.
>
> Signed-off-by: Rasmus Villemoes
> ---
> kernel/trace/trace_output.c | 34 +++---
> 1 file changed, 7 insertions(+), 27 deletions(-)
>
> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index 6e6cc64faa38..501311dcfca6 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -339,34 +339,17 @@ static inline const char *kretprobed(const char *name)
> #endif /* CONFIG_KRETPROBES */
>
> static void
> -seq_print_sym_short(struct trace_seq *s, const char *fmt, unsigned long
> address)
> +seq_print_sym(struct trace_seq *s, const char *fmt, unsigned long address,
> + bool with_offset)
Just call the variable "offset". It's rather obvious what that means.
The other patches look good, but can you send a v2 of the series, with
these updates?
Thanks!
-- Steve
> {
> char str[KSYM_SYMBOL_LEN];
> #ifdef CONFIG_KALLSYMS
> const char *name;
>
> - kallsyms_lookup(address, NULL, NULL, NULL, str);
> -
> - name = kretprobed(str);
> -
> - if (name && strlen(name)) {
> - trace_seq_printf(s, fmt, name);
> - return;
> - }
> -#endif
> - snprintf(str, KSYM_SYMBOL_LEN, "0x%08lx", address);
> - trace_seq_printf(s, fmt, str);
> -}
> -
> -static void
> -seq_print_sym_offset(struct trace_seq *s, const char *fmt,
> - unsigned long address)
> -{
> - char str[KSYM_SYMBOL_LEN];
> -#ifdef CONFIG_KALLSYMS
> - const char *name;
> -
> - sprint_symbol(str, address);
> + if (with_offset)
> + sprint_symbol(str, address);
> + else
> + kallsyms_lookup(address, NULL, NULL, NULL, str);
> name = kretprobed(str);
>
> if (name && strlen(name)) {
> @@ -424,10 +407,7 @@ seq_print_ip_sym(struct trace_seq *s, unsigned long ip,
> unsigned long sym_flags)
> goto out;
> }
>
> - if (sym_flags & TRACE_ITER_SYM_OFFSET)
> - seq_print_sym_offset(s, "%s", ip);
> - else
> - seq_print_sym_short(s, "%s", ip);
> + seq_print_sym(s, "%s", ip, sym_flags & TRACE_ITER_SYM_OFFSET);
>
> if (sym_flags & TRACE_ITER_SYM_ADDR)
> trace_seq_printf(s, " <" IP_FMT ">", ip);
Re: [PATCH 1/3] tracing: merge seq_print_sym_short() and seq_print_sym_offset()
Hi Ramus,
Thanks for sending these patches. I have some small nits though.
First, please send a cover letter whenever sending more than one patch.
It just groups them better in my inbox.
The second is embedded below.
On Fri, 26 Oct 2018 23:13:44 +0200
Rasmus Villemoes wrote:
> These two functions are nearly identical, so we can avoid some code
> duplication by moving the conditional into a common implementation.
>
> Signed-off-by: Rasmus Villemoes
> ---
> kernel/trace/trace_output.c | 34 +++---
> 1 file changed, 7 insertions(+), 27 deletions(-)
>
> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index 6e6cc64faa38..501311dcfca6 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -339,34 +339,17 @@ static inline const char *kretprobed(const char *name)
> #endif /* CONFIG_KRETPROBES */
>
> static void
> -seq_print_sym_short(struct trace_seq *s, const char *fmt, unsigned long
> address)
> +seq_print_sym(struct trace_seq *s, const char *fmt, unsigned long address,
> + bool with_offset)
Just call the variable "offset". It's rather obvious what that means.
The other patches look good, but can you send a v2 of the series, with
these updates?
Thanks!
-- Steve
> {
> char str[KSYM_SYMBOL_LEN];
> #ifdef CONFIG_KALLSYMS
> const char *name;
>
> - kallsyms_lookup(address, NULL, NULL, NULL, str);
> -
> - name = kretprobed(str);
> -
> - if (name && strlen(name)) {
> - trace_seq_printf(s, fmt, name);
> - return;
> - }
> -#endif
> - snprintf(str, KSYM_SYMBOL_LEN, "0x%08lx", address);
> - trace_seq_printf(s, fmt, str);
> -}
> -
> -static void
> -seq_print_sym_offset(struct trace_seq *s, const char *fmt,
> - unsigned long address)
> -{
> - char str[KSYM_SYMBOL_LEN];
> -#ifdef CONFIG_KALLSYMS
> - const char *name;
> -
> - sprint_symbol(str, address);
> + if (with_offset)
> + sprint_symbol(str, address);
> + else
> + kallsyms_lookup(address, NULL, NULL, NULL, str);
> name = kretprobed(str);
>
> if (name && strlen(name)) {
> @@ -424,10 +407,7 @@ seq_print_ip_sym(struct trace_seq *s, unsigned long ip,
> unsigned long sym_flags)
> goto out;
> }
>
> - if (sym_flags & TRACE_ITER_SYM_OFFSET)
> - seq_print_sym_offset(s, "%s", ip);
> - else
> - seq_print_sym_short(s, "%s", ip);
> + seq_print_sym(s, "%s", ip, sym_flags & TRACE_ITER_SYM_OFFSET);
>
> if (sym_flags & TRACE_ITER_SYM_ADDR)
> trace_seq_printf(s, " <" IP_FMT ">", ip);
Re: [PATCH 1/2] x86/pkeys: copy pkey state at fork()
I ended up working around this with a pthread_atfork handler disabling my usage of the feature in the child process for the time being. I don't have an easy way to detect if the bug is present within a library so I'm going to need a kernel version check with a table of kernel releases fixing the problem for each stable branch. It would be helpful if there was a new cpuinfo flag to check if the MPK state is preserved on fork in addition to the existing ospke flag. The problem will fade away over time but in my experience there are a lot of people using distributions with kernels not incorporating all of the stable fixes. I expect other people will run into the problem once hardware with MPK is more widely available and other people try to use it for various things like moving GC or assorted security features. Someone will end up running software adopting it on an older kernel with the problem. The clobbering issue I found with MAP_FIXED_NOREPLACE isn't quite as annoying because it was easy to make a runtime test usable in a library to see if the feature works properly.
Re: [PATCH 1/2] x86/pkeys: copy pkey state at fork()
I ended up working around this with a pthread_atfork handler disabling my usage of the feature in the child process for the time being. I don't have an easy way to detect if the bug is present within a library so I'm going to need a kernel version check with a table of kernel releases fixing the problem for each stable branch. It would be helpful if there was a new cpuinfo flag to check if the MPK state is preserved on fork in addition to the existing ospke flag. The problem will fade away over time but in my experience there are a lot of people using distributions with kernels not incorporating all of the stable fixes. I expect other people will run into the problem once hardware with MPK is more widely available and other people try to use it for various things like moving GC or assorted security features. Someone will end up running software adopting it on an older kernel with the problem. The clobbering issue I found with MAP_FIXED_NOREPLACE isn't quite as annoying because it was easy to make a runtime test usable in a library to see if the feature works properly.
[PATCH] fs: proc: move linux_proc_banner to where it is used
With -Wformat-literal, gcc complains
fs/proc/version.c:11:16: warning: format not a string literal, argument types
not checked [-Wformat-nonliteral]
seq_printf(m, linux_proc_banner,
linux_proc_banner is only used in this one place, so move the definition
here, to allow the compiler to complain in the unlikely case one of the
LINUX_* strings ended up containing a %. This also avoids compiling it
in for !CONFIG_PROC_FS.
Signed-off-by: Rasmus Villemoes
---
fs/proc/version.c | 6 ++
include/linux/printk.h | 1 -
init/version.c | 5 -
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/proc/version.c b/fs/proc/version.c
index b449f186577f..40a33e1434d7 100644
--- a/fs/proc/version.c
+++ b/fs/proc/version.c
@@ -1,4 +1,5 @@
// SPDX-License-Identifier: GPL-2.0
+#include
#include
#include
#include
@@ -6,6 +7,11 @@
#include
#include
+#define linux_proc_banner \
+ "%s version %s" \
+ " (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ")" \
+ " (" LINUX_COMPILER ") %s\n"
+
static int version_proc_show(struct seq_file *m, void *v)
{
seq_printf(m, linux_proc_banner,
diff --git a/include/linux/printk.h b/include/linux/printk.h
index cf3eccfe1543..c7edf4398e4d 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -9,7 +9,6 @@
#include
extern const char linux_banner[];
-extern const char linux_proc_banner[];
#define PRINTK_MAX_SINGLE_HEADER_LEN 2
diff --git a/init/version.c b/init/version.c
index ef4012ec4375..ead5e21fa3a3 100644
--- a/init/version.c
+++ b/init/version.c
@@ -46,9 +46,4 @@ const char linux_banner[] =
"Linux version " UTS_RELEASE " (" LINUX_COMPILE_BY "@"
LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION "\n";
-const char linux_proc_banner[] =
- "%s version %s"
- " (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ")"
- " (" LINUX_COMPILER ") %s\n";
-
BUILD_SALT;
--
2.19.1.6.gbde171bbf5
[PATCH] fs: proc: move linux_proc_banner to where it is used
With -Wformat-literal, gcc complains
fs/proc/version.c:11:16: warning: format not a string literal, argument types
not checked [-Wformat-nonliteral]
seq_printf(m, linux_proc_banner,
linux_proc_banner is only used in this one place, so move the definition
here, to allow the compiler to complain in the unlikely case one of the
LINUX_* strings ended up containing a %. This also avoids compiling it
in for !CONFIG_PROC_FS.
Signed-off-by: Rasmus Villemoes
---
fs/proc/version.c | 6 ++
include/linux/printk.h | 1 -
init/version.c | 5 -
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/proc/version.c b/fs/proc/version.c
index b449f186577f..40a33e1434d7 100644
--- a/fs/proc/version.c
+++ b/fs/proc/version.c
@@ -1,4 +1,5 @@
// SPDX-License-Identifier: GPL-2.0
+#include
#include
#include
#include
@@ -6,6 +7,11 @@
#include
#include
+#define linux_proc_banner \
+ "%s version %s" \
+ " (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ")" \
+ " (" LINUX_COMPILER ") %s\n"
+
static int version_proc_show(struct seq_file *m, void *v)
{
seq_printf(m, linux_proc_banner,
diff --git a/include/linux/printk.h b/include/linux/printk.h
index cf3eccfe1543..c7edf4398e4d 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -9,7 +9,6 @@
#include
extern const char linux_banner[];
-extern const char linux_proc_banner[];
#define PRINTK_MAX_SINGLE_HEADER_LEN 2
diff --git a/init/version.c b/init/version.c
index ef4012ec4375..ead5e21fa3a3 100644
--- a/init/version.c
+++ b/init/version.c
@@ -46,9 +46,4 @@ const char linux_banner[] =
"Linux version " UTS_RELEASE " (" LINUX_COMPILE_BY "@"
LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION "\n";
-const char linux_proc_banner[] =
- "%s version %s"
- " (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ")"
- " (" LINUX_COMPILER ") %s\n";
-
BUILD_SALT;
--
2.19.1.6.gbde171bbf5
Re: Git pull ack emails..
On Fri, Oct 26, 2018 at 12:36:14PM -0500, Rob Herring wrote: > On Thu, Oct 25, 2018 at 9:14 AM Linus Torvalds > wrote: > > Are there other situations where you might want to track something > > _outside_ of a pull request? Maybe. I can't really think of a lot of > > them, though. Patches etc don't have commit ID's to track, but it patchwork gives them IDs and lets you do lookups using them, that's what I'm doing. You can get the ID from a git commit by piping the output of git show into parser.py from the patchwork source, it works a lot of the time but things like editing the commit message will break it (this is a theme with my scripting around the mail stuff...). > submissions. For example, with Greg and Mark B you can expect an > automated replies. Mark's reply gets threaded with the original, but > Greg's do not. For networking, you may or may not get a manual reply, Mine *mostly* gets threaded, it's relying on being able to talk to patchwork to figure out the message ID at the minute so if the patchwork lookup fails for whatever reason it'll just use on what's in the commit for the CC list and not thread. That isn't ideal, especially when I'm travelling and my network connection isn't the best, I keep meaning to try to figure out a better way which would probably be based on git notes as discussed earlier. > maintainers apply. I've somewhat solved it for myself by automating > checking linux-next, but maybe automated email replies to patches > being in linux-next would be nice. While that's not immediate, it Yeah, I do that as well (I have an outbound patch queue I rebase against -next, this also tells me if stuff starts conflicting with other work). I can see the automated e-mails be useful but it might be tricky for a bot that's only looking at git to figure out which people and lists to CC to ensure visibility unless we do the annotation thing, it's not just the patch submitters that want visibility - I did the patchwork stuff due to user demand for that with some help from Brian Norris. > BTW, patchwork tracks pull requests too, so maybe there's a common solution. Ooh, interesting... signature.asc Description: PGP signature
Re: Git pull ack emails..
On Fri, Oct 26, 2018 at 12:36:14PM -0500, Rob Herring wrote: > On Thu, Oct 25, 2018 at 9:14 AM Linus Torvalds > wrote: > > Are there other situations where you might want to track something > > _outside_ of a pull request? Maybe. I can't really think of a lot of > > them, though. Patches etc don't have commit ID's to track, but it patchwork gives them IDs and lets you do lookups using them, that's what I'm doing. You can get the ID from a git commit by piping the output of git show into parser.py from the patchwork source, it works a lot of the time but things like editing the commit message will break it (this is a theme with my scripting around the mail stuff...). > submissions. For example, with Greg and Mark B you can expect an > automated replies. Mark's reply gets threaded with the original, but > Greg's do not. For networking, you may or may not get a manual reply, Mine *mostly* gets threaded, it's relying on being able to talk to patchwork to figure out the message ID at the minute so if the patchwork lookup fails for whatever reason it'll just use on what's in the commit for the CC list and not thread. That isn't ideal, especially when I'm travelling and my network connection isn't the best, I keep meaning to try to figure out a better way which would probably be based on git notes as discussed earlier. > maintainers apply. I've somewhat solved it for myself by automating > checking linux-next, but maybe automated email replies to patches > being in linux-next would be nice. While that's not immediate, it Yeah, I do that as well (I have an outbound patch queue I rebase against -next, this also tells me if stuff starts conflicting with other work). I can see the automated e-mails be useful but it might be tricky for a bot that's only looking at git to figure out which people and lists to CC to ensure visibility unless we do the annotation thing, it's not just the patch submitters that want visibility - I did the patchwork stuff due to user demand for that with some help from Brian Norris. > BTW, patchwork tracks pull requests too, so maybe there's a common solution. Ooh, interesting... signature.asc Description: PGP signature
[PATCH 3/3] tracing: simplify printf'ing in seq_print_sym
trace_seq_printf(..., "%s", ...) can be done with trace_seq_puts()
instead, avoiding printf overhead. In the second instance, the string
we're copying was just created from an snprintf() to a stack buffer, so
we might as well do that printf directly. This naturally leads to moving
the declaration of the str buffer inside the CONFIG_KALLSYMS guard,
which in turn will make gcc inline the function for !CONFIG_KALLSYMS (it
only has a single caller, but the huge stack frame seems to make gcc not
inline it for CONFIG_KALLSYMS).
Signed-off-by: Rasmus Villemoes
---
kernel/trace/trace_output.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 7417ce5fe4bb..0f96262d23be 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -341,8 +341,8 @@ static inline const char *kretprobed(const char *name)
static void
seq_print_sym(struct trace_seq *s, unsigned long address, bool with_offset)
{
- char str[KSYM_SYMBOL_LEN];
#ifdef CONFIG_KALLSYMS
+ char str[KSYM_SYMBOL_LEN];
const char *name;
if (with_offset)
@@ -352,12 +352,11 @@ seq_print_sym(struct trace_seq *s, unsigned long address,
bool with_offset)
name = kretprobed(str);
if (name && strlen(name)) {
- trace_seq_printf(s, "%s", name);
+ trace_seq_puts(s, name);
return;
}
#endif
- snprintf(str, KSYM_SYMBOL_LEN, "0x%08lx", address);
- trace_seq_printf(s, "%s", str);
+ trace_seq_printf(s, "0x%08lx", address);
}
#ifndef CONFIG_64BIT
--
2.19.1.6.gbde171bbf5
[PATCH 1/3] tracing: merge seq_print_sym_short() and seq_print_sym_offset()
These two functions are nearly identical, so we can avoid some code
duplication by moving the conditional into a common implementation.
Signed-off-by: Rasmus Villemoes
---
kernel/trace/trace_output.c | 34 +++---
1 file changed, 7 insertions(+), 27 deletions(-)
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 6e6cc64faa38..501311dcfca6 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -339,34 +339,17 @@ static inline const char *kretprobed(const char *name)
#endif /* CONFIG_KRETPROBES */
static void
-seq_print_sym_short(struct trace_seq *s, const char *fmt, unsigned long
address)
+seq_print_sym(struct trace_seq *s, const char *fmt, unsigned long address,
+ bool with_offset)
{
char str[KSYM_SYMBOL_LEN];
#ifdef CONFIG_KALLSYMS
const char *name;
- kallsyms_lookup(address, NULL, NULL, NULL, str);
-
- name = kretprobed(str);
-
- if (name && strlen(name)) {
- trace_seq_printf(s, fmt, name);
- return;
- }
-#endif
- snprintf(str, KSYM_SYMBOL_LEN, "0x%08lx", address);
- trace_seq_printf(s, fmt, str);
-}
-
-static void
-seq_print_sym_offset(struct trace_seq *s, const char *fmt,
-unsigned long address)
-{
- char str[KSYM_SYMBOL_LEN];
-#ifdef CONFIG_KALLSYMS
- const char *name;
-
- sprint_symbol(str, address);
+ if (with_offset)
+ sprint_symbol(str, address);
+ else
+ kallsyms_lookup(address, NULL, NULL, NULL, str);
name = kretprobed(str);
if (name && strlen(name)) {
@@ -424,10 +407,7 @@ seq_print_ip_sym(struct trace_seq *s, unsigned long ip,
unsigned long sym_flags)
goto out;
}
- if (sym_flags & TRACE_ITER_SYM_OFFSET)
- seq_print_sym_offset(s, "%s", ip);
- else
- seq_print_sym_short(s, "%s", ip);
+ seq_print_sym(s, "%s", ip, sym_flags & TRACE_ITER_SYM_OFFSET);
if (sym_flags & TRACE_ITER_SYM_ADDR)
trace_seq_printf(s, " <" IP_FMT ">", ip);
--
2.19.1.6.gbde171bbf5
[PATCH 2/3] tracing: avoid -Wformat-nonliteral warning
Building with -Wformat-nonliteral, gcc complains
kernel/trace/trace_output.c: In function ‘seq_print_sym’:
kernel/trace/trace_output.c:356:3: warning: format not a string literal,
argument types not checked [-Wformat-nonliteral]
trace_seq_printf(s, fmt, name);
But seq_print_sym only has a single caller which passes "%s" as fmt, so
we might as well just use that directly. That also paves the way for
further cleanups that will actually make that format string go away
entirely.
Signed-off-by: Rasmus Villemoes
---
kernel/trace/trace_output.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 501311dcfca6..7417ce5fe4bb 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -339,8 +339,7 @@ static inline const char *kretprobed(const char *name)
#endif /* CONFIG_KRETPROBES */
static void
-seq_print_sym(struct trace_seq *s, const char *fmt, unsigned long address,
- bool with_offset)
+seq_print_sym(struct trace_seq *s, unsigned long address, bool with_offset)
{
char str[KSYM_SYMBOL_LEN];
#ifdef CONFIG_KALLSYMS
@@ -353,12 +352,12 @@ seq_print_sym(struct trace_seq *s, const char *fmt,
unsigned long address,
name = kretprobed(str);
if (name && strlen(name)) {
- trace_seq_printf(s, fmt, name);
+ trace_seq_printf(s, "%s", name);
return;
}
#endif
snprintf(str, KSYM_SYMBOL_LEN, "0x%08lx", address);
- trace_seq_printf(s, fmt, str);
+ trace_seq_printf(s, "%s", str);
}
#ifndef CONFIG_64BIT
@@ -407,7 +406,7 @@ seq_print_ip_sym(struct trace_seq *s, unsigned long ip,
unsigned long sym_flags)
goto out;
}
- seq_print_sym(s, "%s", ip, sym_flags & TRACE_ITER_SYM_OFFSET);
+ seq_print_sym(s, ip, sym_flags & TRACE_ITER_SYM_OFFSET);
if (sym_flags & TRACE_ITER_SYM_ADDR)
trace_seq_printf(s, " <" IP_FMT ">", ip);
--
2.19.1.6.gbde171bbf5
[PATCH 3/3] tracing: simplify printf'ing in seq_print_sym
trace_seq_printf(..., "%s", ...) can be done with trace_seq_puts()
instead, avoiding printf overhead. In the second instance, the string
we're copying was just created from an snprintf() to a stack buffer, so
we might as well do that printf directly. This naturally leads to moving
the declaration of the str buffer inside the CONFIG_KALLSYMS guard,
which in turn will make gcc inline the function for !CONFIG_KALLSYMS (it
only has a single caller, but the huge stack frame seems to make gcc not
inline it for CONFIG_KALLSYMS).
Signed-off-by: Rasmus Villemoes
---
kernel/trace/trace_output.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 7417ce5fe4bb..0f96262d23be 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -341,8 +341,8 @@ static inline const char *kretprobed(const char *name)
static void
seq_print_sym(struct trace_seq *s, unsigned long address, bool with_offset)
{
- char str[KSYM_SYMBOL_LEN];
#ifdef CONFIG_KALLSYMS
+ char str[KSYM_SYMBOL_LEN];
const char *name;
if (with_offset)
@@ -352,12 +352,11 @@ seq_print_sym(struct trace_seq *s, unsigned long address,
bool with_offset)
name = kretprobed(str);
if (name && strlen(name)) {
- trace_seq_printf(s, "%s", name);
+ trace_seq_puts(s, name);
return;
}
#endif
- snprintf(str, KSYM_SYMBOL_LEN, "0x%08lx", address);
- trace_seq_printf(s, "%s", str);
+ trace_seq_printf(s, "0x%08lx", address);
}
#ifndef CONFIG_64BIT
--
2.19.1.6.gbde171bbf5
[PATCH 1/3] tracing: merge seq_print_sym_short() and seq_print_sym_offset()
These two functions are nearly identical, so we can avoid some code
duplication by moving the conditional into a common implementation.
Signed-off-by: Rasmus Villemoes
---
kernel/trace/trace_output.c | 34 +++---
1 file changed, 7 insertions(+), 27 deletions(-)
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 6e6cc64faa38..501311dcfca6 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -339,34 +339,17 @@ static inline const char *kretprobed(const char *name)
#endif /* CONFIG_KRETPROBES */
static void
-seq_print_sym_short(struct trace_seq *s, const char *fmt, unsigned long
address)
+seq_print_sym(struct trace_seq *s, const char *fmt, unsigned long address,
+ bool with_offset)
{
char str[KSYM_SYMBOL_LEN];
#ifdef CONFIG_KALLSYMS
const char *name;
- kallsyms_lookup(address, NULL, NULL, NULL, str);
-
- name = kretprobed(str);
-
- if (name && strlen(name)) {
- trace_seq_printf(s, fmt, name);
- return;
- }
-#endif
- snprintf(str, KSYM_SYMBOL_LEN, "0x%08lx", address);
- trace_seq_printf(s, fmt, str);
-}
-
-static void
-seq_print_sym_offset(struct trace_seq *s, const char *fmt,
-unsigned long address)
-{
- char str[KSYM_SYMBOL_LEN];
-#ifdef CONFIG_KALLSYMS
- const char *name;
-
- sprint_symbol(str, address);
+ if (with_offset)
+ sprint_symbol(str, address);
+ else
+ kallsyms_lookup(address, NULL, NULL, NULL, str);
name = kretprobed(str);
if (name && strlen(name)) {
@@ -424,10 +407,7 @@ seq_print_ip_sym(struct trace_seq *s, unsigned long ip,
unsigned long sym_flags)
goto out;
}
- if (sym_flags & TRACE_ITER_SYM_OFFSET)
- seq_print_sym_offset(s, "%s", ip);
- else
- seq_print_sym_short(s, "%s", ip);
+ seq_print_sym(s, "%s", ip, sym_flags & TRACE_ITER_SYM_OFFSET);
if (sym_flags & TRACE_ITER_SYM_ADDR)
trace_seq_printf(s, " <" IP_FMT ">", ip);
--
2.19.1.6.gbde171bbf5
[PATCH 2/3] tracing: avoid -Wformat-nonliteral warning
Building with -Wformat-nonliteral, gcc complains
kernel/trace/trace_output.c: In function ‘seq_print_sym’:
kernel/trace/trace_output.c:356:3: warning: format not a string literal,
argument types not checked [-Wformat-nonliteral]
trace_seq_printf(s, fmt, name);
But seq_print_sym only has a single caller which passes "%s" as fmt, so
we might as well just use that directly. That also paves the way for
further cleanups that will actually make that format string go away
entirely.
Signed-off-by: Rasmus Villemoes
---
kernel/trace/trace_output.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 501311dcfca6..7417ce5fe4bb 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -339,8 +339,7 @@ static inline const char *kretprobed(const char *name)
#endif /* CONFIG_KRETPROBES */
static void
-seq_print_sym(struct trace_seq *s, const char *fmt, unsigned long address,
- bool with_offset)
+seq_print_sym(struct trace_seq *s, unsigned long address, bool with_offset)
{
char str[KSYM_SYMBOL_LEN];
#ifdef CONFIG_KALLSYMS
@@ -353,12 +352,12 @@ seq_print_sym(struct trace_seq *s, const char *fmt,
unsigned long address,
name = kretprobed(str);
if (name && strlen(name)) {
- trace_seq_printf(s, fmt, name);
+ trace_seq_printf(s, "%s", name);
return;
}
#endif
snprintf(str, KSYM_SYMBOL_LEN, "0x%08lx", address);
- trace_seq_printf(s, fmt, str);
+ trace_seq_printf(s, "%s", str);
}
#ifndef CONFIG_64BIT
@@ -407,7 +406,7 @@ seq_print_ip_sym(struct trace_seq *s, unsigned long ip,
unsigned long sym_flags)
goto out;
}
- seq_print_sym(s, "%s", ip, sym_flags & TRACE_ITER_SYM_OFFSET);
+ seq_print_sym(s, ip, sym_flags & TRACE_ITER_SYM_OFFSET);
if (sym_flags & TRACE_ITER_SYM_ADDR)
trace_seq_printf(s, " <" IP_FMT ">", ip);
--
2.19.1.6.gbde171bbf5
Re: [PATCH v2 4/4] dts/arm64/layerscape: Clean PCIe controller compatible strings
On Thu, Oct 25, 2018 at 4:53 AM Z.q. Hou wrote:
The correct prefix for arm64 dts patches should be: "arm64: dts:
layerscape: ...", and it should be better to mention the string
removed in the title too.
>
> From: Hou Zhiqiang
>
> Removed the wrong compatible string "snps,dw-pcie", in case
> match incorrect driver.
>
> Signed-off-by: Hou Zhiqiang
> ---
> V2:
> - no change
>
> arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 2 +-
> arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 6 +++---
> arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 6 +++---
> arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 6 +++---
> arch/arm64/boot/dts/freescale/fsl-ls2088a.dtsi | 8
> arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 12
> 6 files changed, 18 insertions(+), 22 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
> b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
> index 5da732f82fa0..028a6daa5597 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
> @@ -475,7 +475,7 @@
> };
>
> pcie@340 {
> - compatible = "fsl,ls1012a-pcie", "snps,dw-pcie";
> + compatible = "fsl,ls1012a-pcie";
> reg = <0x00 0x0340 0x0 0x0010 /* controller
> registers */
>0x40 0x 0x0 0x2000>; /*
> configuration space */
> reg-names = "regs", "config";
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> index 3fed504b5381..5480d6c4c548 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> @@ -661,7 +661,7 @@
> };
>
> pcie@340 {
> - compatible = "fsl,ls1043a-pcie", "snps,dw-pcie";
> + compatible = "fsl,ls1043a-pcie";
> reg = <0x00 0x0340 0x0 0x0010 /* controller
> registers */
>0x40 0x 0x0 0x2000>; /*
> configuration space */
> reg-names = "regs", "config";
> @@ -686,7 +686,7 @@
> };
>
> pcie@350 {
> - compatible = "fsl,ls1043a-pcie", "snps,dw-pcie";
> + compatible = "fsl,ls1043a-pcie";
> reg = <0x00 0x0350 0x0 0x0010 /* controller
> registers */
>0x48 0x 0x0 0x2000>; /*
> configuration space */
> reg-names = "regs", "config";
> @@ -711,7 +711,7 @@
> };
>
> pcie@360 {
> - compatible = "fsl,ls1043a-pcie", "snps,dw-pcie";
> + compatible = "fsl,ls1043a-pcie";
> reg = <0x00 0x0360 0x0 0x0010 /* controller
> registers */
>0x50 0x 0x0 0x2000>; /*
> configuration space */
> reg-names = "regs", "config";
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> index 51cbd50012d6..519315c5d507 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> @@ -630,7 +630,7 @@
> };
>
> pcie@340 {
> - compatible = "fsl,ls1046a-pcie", "snps,dw-pcie";
> + compatible = "fsl,ls1046a-pcie";
> reg = <0x00 0x0340 0x0 0x0010 /* controller
> registers */
>0x40 0x 0x0 0x2000>; /*
> configuration space */
> reg-names = "regs", "config";
> @@ -655,7 +655,7 @@
> };
>
> pcie@350 {
> - compatible = "fsl,ls1046a-pcie", "snps,dw-pcie";
> + compatible = "fsl,ls1046a-pcie";
> reg = <0x00 0x0350 0x0 0x0010 /* controller
> registers */
>0x48 0x 0x0 0x2000>; /*
> configuration space */
> reg-names = "regs", "config";
> @@ -680,7 +680,7 @@
> };
>
> pcie@360 {
> - compatible = "fsl,ls1046a-pcie", "snps,dw-pcie";
> + compatible = "fsl,ls1046a-pcie";
> reg = <0x00 0x0360 0x0 0x0010 /* controller
> registers */
>0x50 0x 0x0 0x2000>; /*
> configuration space */
> reg-names = "regs", "config";
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> index a07f612ab56b..10b253c88a16 100644
> ---
Re: [PATCH v2 4/4] dts/arm64/layerscape: Clean PCIe controller compatible strings
On Thu, Oct 25, 2018 at 4:53 AM Z.q. Hou wrote:
The correct prefix for arm64 dts patches should be: "arm64: dts:
layerscape: ...", and it should be better to mention the string
removed in the title too.
>
> From: Hou Zhiqiang
>
> Removed the wrong compatible string "snps,dw-pcie", in case
> match incorrect driver.
>
> Signed-off-by: Hou Zhiqiang
> ---
> V2:
> - no change
>
> arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 2 +-
> arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 6 +++---
> arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 6 +++---
> arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 6 +++---
> arch/arm64/boot/dts/freescale/fsl-ls2088a.dtsi | 8
> arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 12
> 6 files changed, 18 insertions(+), 22 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
> b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
> index 5da732f82fa0..028a6daa5597 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
> @@ -475,7 +475,7 @@
> };
>
> pcie@340 {
> - compatible = "fsl,ls1012a-pcie", "snps,dw-pcie";
> + compatible = "fsl,ls1012a-pcie";
> reg = <0x00 0x0340 0x0 0x0010 /* controller
> registers */
>0x40 0x 0x0 0x2000>; /*
> configuration space */
> reg-names = "regs", "config";
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> index 3fed504b5381..5480d6c4c548 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> @@ -661,7 +661,7 @@
> };
>
> pcie@340 {
> - compatible = "fsl,ls1043a-pcie", "snps,dw-pcie";
> + compatible = "fsl,ls1043a-pcie";
> reg = <0x00 0x0340 0x0 0x0010 /* controller
> registers */
>0x40 0x 0x0 0x2000>; /*
> configuration space */
> reg-names = "regs", "config";
> @@ -686,7 +686,7 @@
> };
>
> pcie@350 {
> - compatible = "fsl,ls1043a-pcie", "snps,dw-pcie";
> + compatible = "fsl,ls1043a-pcie";
> reg = <0x00 0x0350 0x0 0x0010 /* controller
> registers */
>0x48 0x 0x0 0x2000>; /*
> configuration space */
> reg-names = "regs", "config";
> @@ -711,7 +711,7 @@
> };
>
> pcie@360 {
> - compatible = "fsl,ls1043a-pcie", "snps,dw-pcie";
> + compatible = "fsl,ls1043a-pcie";
> reg = <0x00 0x0360 0x0 0x0010 /* controller
> registers */
>0x50 0x 0x0 0x2000>; /*
> configuration space */
> reg-names = "regs", "config";
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> index 51cbd50012d6..519315c5d507 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> @@ -630,7 +630,7 @@
> };
>
> pcie@340 {
> - compatible = "fsl,ls1046a-pcie", "snps,dw-pcie";
> + compatible = "fsl,ls1046a-pcie";
> reg = <0x00 0x0340 0x0 0x0010 /* controller
> registers */
>0x40 0x 0x0 0x2000>; /*
> configuration space */
> reg-names = "regs", "config";
> @@ -655,7 +655,7 @@
> };
>
> pcie@350 {
> - compatible = "fsl,ls1046a-pcie", "snps,dw-pcie";
> + compatible = "fsl,ls1046a-pcie";
> reg = <0x00 0x0350 0x0 0x0010 /* controller
> registers */
>0x48 0x 0x0 0x2000>; /*
> configuration space */
> reg-names = "regs", "config";
> @@ -680,7 +680,7 @@
> };
>
> pcie@360 {
> - compatible = "fsl,ls1046a-pcie", "snps,dw-pcie";
> + compatible = "fsl,ls1046a-pcie";
> reg = <0x00 0x0360 0x0 0x0010 /* controller
> registers */
>0x50 0x 0x0 0x2000>; /*
> configuration space */
> reg-names = "regs", "config";
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> index a07f612ab56b..10b253c88a16 100644
> ---
[PATCH] mfd: ti_am335x_tscadc: release device nodes in ti_tscadc_probe()
ti_tscadc_probe() increments refcnt of tsc and adc device nodes
and leaves it undecremented.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov
---
drivers/mfd/ti_am335x_tscadc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index c2d47d78705b..6572232a5cef 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -142,16 +142,19 @@ staticint ti_tscadc_probe(struct platform_device
*pdev)
node = of_get_child_by_name(pdev->dev.of_node, "tsc");
of_property_read_u32(node, "ti,wires", _wires);
of_property_read_u32(node, "ti,coordiante-readouts", );
+ of_node_put(node);
node = of_get_child_by_name(pdev->dev.of_node, "adc");
of_property_for_each_u32(node, "ti,adc-channels", prop, cur, val) {
adc_channels++;
if (val > 7) {
+ of_node_put(node);
dev_err(>dev, " PIN numbers are 0..7 (not %d)\n",
val);
return -EINVAL;
}
}
+ of_node_put(node);
total_channels = tsc_wires + adc_channels;
if (total_channels > 8) {
dev_err(>dev, "Number of i/p channels more than 8\n");
--
2.7.4
[PATCH] mfd: ti_am335x_tscadc: release device nodes in ti_tscadc_probe()
ti_tscadc_probe() increments refcnt of tsc and adc device nodes
and leaves it undecremented.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov
---
drivers/mfd/ti_am335x_tscadc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index c2d47d78705b..6572232a5cef 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -142,16 +142,19 @@ staticint ti_tscadc_probe(struct platform_device
*pdev)
node = of_get_child_by_name(pdev->dev.of_node, "tsc");
of_property_read_u32(node, "ti,wires", _wires);
of_property_read_u32(node, "ti,coordiante-readouts", );
+ of_node_put(node);
node = of_get_child_by_name(pdev->dev.of_node, "adc");
of_property_for_each_u32(node, "ti,adc-channels", prop, cur, val) {
adc_channels++;
if (val > 7) {
+ of_node_put(node);
dev_err(>dev, " PIN numbers are 0..7 (not %d)\n",
val);
return -EINVAL;
}
}
+ of_node_put(node);
total_channels = tsc_wires + adc_channels;
if (total_channels > 8) {
dev_err(>dev, "Number of i/p channels more than 8\n");
--
2.7.4
