[PATCH] mm/zswap: Fix passing zero to 'PTR_ERR' warning
Fix smatch warning: mm/zswap.c:425 zswap_cpu_comp_prepare() warn: passing zero to 'PTR_ERR' crypto_alloc_comp() never return NULL, use IS_ERR instead of IS_ERR_OR_NULL to fix this. Fixes: f1c54846ee45 ("zswap: dynamic pool creation") Signed-off-by: YueHaibing --- mm/zswap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/zswap.c b/mm/zswap.c index 1eced701b3bd..55a2f72557a8 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -421,7 +421,7 @@ static int zswap_cpu_comp_prepare(unsigned int cpu, struct hlist_node *node) return 0; tfm = crypto_alloc_comp(pool->tfm_name, 0, 0); - if (IS_ERR_OR_NULL(tfm)) { + if (IS_ERR(tfm)) { pr_err("could not alloc crypto comp %s : %ld\n", pool->tfm_name, PTR_ERR(tfm)); return -ENOMEM; -- 2.17.1
ltp::mmap05 --> BUG: using __this_cpu_read() in preemptible
[ 138.620544] BUG: using __this_cpu_read() in preemptible [] code: mmap05/4858 [ 138.620737] caller is lockdep_hardirqs_on_prepare+0x2f/0x1b0 [ 138.620880] CPU: 2 PID: 4858 Comm: mmap05 Kdump: loaded Tainted: G S E 5.10.0.g07e0887-master #18 [ 138.621097] Hardware name: MEDION MS-7848/MS-7848, BIOS M7848W08.20C 09/23/2013 [ 138.621243] Call Trace: [ 138.621330] dump_stack+0x77/0x97 [ 138.621434] check_preemption_disabled+0xb4/0xc0 [ 138.621551] ? __bad_area_nosemaphore+0x63/0x1e0 [ 138.621636] lockdep_hardirqs_on_prepare+0x2f/0x1b0 [ 138.621697] ? __bad_area_nosemaphore+0x63/0x1e0 [ 138.621758] trace_hardirqs_on+0x33/0xf0 [ 138.621813] __bad_area_nosemaphore+0x63/0x1e0 [ 138.621887] exc_page_fault+0x1a4/0x670 [ 138.621956] ? asm_exc_page_fault+0x8/0x30 [ 138.622017] asm_exc_page_fault+0x1e/0x30 [ 138.622073] RIP: 0033:0x403ef6 [ 138.622118] Code: 48 83 c0 01 89 15 aa 1f 22 00 0f 84 d7 00 00 00 be 01 00 00 00 bf 00 07 62 00 e8 65 fd ff ff 85 c0 75 0a 48 8b 05 da c8 21 00 <0f> b6 00 8b 05 c9 c8 21 00 85 c0 74 43 b9 49 4e 41 00 31 d2 be 6e [ 138.622314] RSP: 002b:7fff18c776e0 EFLAGS: 00010246 [ 138.622384] RAX: 7faf439cc000 RBX: 0003 RCX: 7faf432077d0 [ 138.622466] RDX: 00620748 RSI: 0001 RDI: [ 138.622547] RBP: 7faf4397f6c0 R08: 0003 R09: [ 138.622628] R10: 0008 R11: 0246 R12: 004040a0 [ 138.622709] R13: 7fff18c777e0 R14: R15: [ 138.622840] BUG: using __this_cpu_read() in preemptible [] code: mmap05/4858 [ 138.622931] caller is lockdep_hardirqs_on+0x38/0x110 [ 138.622994] CPU: 2 PID: 4858 Comm: mmap05 Kdump: loaded Tainted: G S E 5.10.0.g07e0887-master #18 [ 138.623102] Hardware name: MEDION MS-7848/MS-7848, BIOS M7848W08.20C 09/23/2013 [ 138.623184] Call Trace: [ 138.623234] dump_stack+0x77/0x97 [ 138.623292] check_preemption_disabled+0xb4/0xc0 [ 138.623355] ? __bad_area_nosemaphore+0x63/0x1e0 [ 138.623422] lockdep_hardirqs_on+0x38/0x110 [ 138.623484] __bad_area_nosemaphore+0x63/0x1e0 [ 138.623561] exc_page_fault+0x1a4/0x670 [ 138.623630] ? asm_exc_page_fault+0x8/0x30 [ 138.623695] asm_exc_page_fault+0x1e/0x30 [ 138.623751] RIP: 0033:0x403ef6 [ 138.623797] Code: 48 83 c0 01 89 15 aa 1f 22 00 0f 84 d7 00 00 00 be 01 00 00 00 bf 00 07 62 00 e8 65 fd ff ff 85 c0 75 0a 48 8b 05 da c8 21 00 <0f> b6 00 8b 05 c9 c8 21 00 85 c0 74 43 b9 49 4e 41 00 31 d2 be 6e [ 138.623993] RSP: 002b:7fff18c776e0 EFLAGS: 00010246 [ 138.624065] RAX: 7faf439cc000 RBX: 0003 RCX: 7faf432077d0 [ 138.624150] RDX: 00620748 RSI: 0001 RDI: [ 138.624235] RBP: 7faf4397f6c0 R08: 0003 R09: [ 138.624319] R10: 0008 R11: 0246 R12: 004040a0 [ 138.624404] R13: 7fff18c777e0 R14: R15: A.
kernel/trace/bpf_trace.c:1181:23: warning: Uninitialized variable: t
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 5fc6b075e165f641fbc366b58b578055762d5f8c commit: c4d0bfb45068d853a478b9067a95969b1886a30f bpf: Add bpf_snprintf_btf helper date: 5 weeks ago compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot "cppcheck warnings: (new ones prefixed by >>)" >> kernel/trace/bpf_trace.c:1181:23: warning: Uninitialized variable: t >> [uninitvar] if (*btf_id <= 0 || !t) ^ vim +1181 kernel/trace/bpf_trace.c 1153 1154 #define BTF_F_ALL (BTF_F_COMPACT | BTF_F_NONAME | \ 1155 BTF_F_PTR_RAW | BTF_F_ZERO) 1156 1157 static int bpf_btf_printf_prepare(struct btf_ptr *ptr, u32 btf_ptr_size, 1158u64 flags, const struct btf **btf, 1159s32 *btf_id) 1160 { 1161 const struct btf_type *t; 1162 1163 if (unlikely(flags & ~(BTF_F_ALL))) 1164 return -EINVAL; 1165 1166 if (btf_ptr_size != sizeof(struct btf_ptr)) 1167 return -EINVAL; 1168 1169 *btf = bpf_get_btf_vmlinux(); 1170 1171 if (IS_ERR_OR_NULL(*btf)) 1172 return PTR_ERR(*btf); 1173 1174 if (ptr->type_id > 0) 1175 *btf_id = ptr->type_id; 1176 else 1177 return -EINVAL; 1178 1179 if (*btf_id > 0) 1180 t = btf_type_by_id(*btf, *btf_id); > 1181 if (*btf_id <= 0 || !t) 1182 return -ENOENT; 1183 1184 return 0; 1185 } 1186 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
Re: [PATCH 0/3] Fix kernfs node reference count leak issues
Hi Reinette, Thank you for correcting this! The subject of this "cover letter" should be: x86/resctrl: Fix kernfs node reference count leak issues On 10/31/2020 5:18, Reinette Chatre wrote: Apologies, the Subject intended to have a "x86/resctrl:" prefix. On 10/30/2020 12:02 PM, Xiaochen Shen wrote: Fix several kernfs node reference count leak issues: (1) Remove superfluous kernfs_get() calls to prevent refcount leak (2) Add necessary kernfs_put() calls to prevent refcount leak (3) Follow-up cleanup for the change in previous patch. Xiaochen Shen (3): x86/resctrl: Remove superfluous kernfs_get() calls to prevent refcount leak x86/resctrl: Add necessary kernfs_put() calls to prevent refcount leak x86/resctrl: Clean up unused function parameter in rmdir path arch/x86/kernel/cpu/resctrl/rdtgroup.c | 82 ++ 1 file changed, 33 insertions(+), 49 deletions(-) -- Best regards, Xiaochen
Re: [PATCH net-next 00/19] net: phy: add support for shared interrupts (part 1)
On Fri, Oct 30, 2020 at 10:56:24PM +0100, Heiner Kallweit wrote: > On 29.10.2020 11:07, Ioana Ciornei wrote: > > From: Ioana Ciornei > > > > This patch set aims to actually add support for shared interrupts in > > phylib and not only for multi-PHY devices. While we are at it, > > streamline the interrupt handling in phylib. > > > > For a bit of context, at the moment, there are multiple phy_driver ops > > that deal with this subject: > > > > - .config_intr() - Enable/disable the interrupt line. > > > > - .ack_interrupt() - Should quiesce any interrupts that may have been > > fired. It's also used by phylib in conjunction with .config_intr() to > > clear any pending interrupts after the line was disabled, and before > > it is going to be enabled. > > > > - .did_interrupt() - Intended for multi-PHY devices with a shared IRQ > > line and used by phylib to discern which PHY from the package was the > > one that actually fired the interrupt. > > > > - .handle_interrupt() - Completely overrides the default interrupt > > handling logic from phylib. The PHY driver is responsible for checking > > if any interrupt was fired by the respective PHY and choose > > accordingly if it's the one that should trigger the link state machine. > > > >>From my point of view, the interrupt handling in phylib has become > > somewhat confusing with all these callbacks that actually read the same > > PHY register - the interrupt status. A more streamlined approach would > > be to just move the responsibility to write an interrupt handler to the > > driver (as any other device driver does) and make .handle_interrupt() > > the only way to deal with interrupts. > > > > Another advantage with this approach would be that phylib would gain > > support for shared IRQs between different PHY (not just multi-PHY > > devices), something which at the moment would require extending every > > PHY driver anyway in order to implement their .did_interrupt() callback > > and duplicate the same logic as in .ack_interrupt(). The disadvantage > > of making .did_interrupt() mandatory would be that we are slightly > > changing the semantics of the phylib API and that would increase > > confusion instead of reducing it. > > > > What I am proposing is the following: > > > > - As a first step, make the .ack_interrupt() callback optional so that > > we do not break any PHY driver amid the transition. > > > > - Every PHY driver gains a .handle_interrupt() implementation that, for > > the most part, would look like below: > > > > irq_status = phy_read(phydev, INTR_STATUS); > > if (irq_status < 0) { > > phy_error(phydev); > > return IRQ_NONE; > > } > > > > if (irq_status == 0) > > return IRQ_NONE; > > > > phy_trigger_machine(phydev); > > > > return IRQ_HANDLED; > > > > - Remove each PHY driver's implementation of the .ack_interrupt() by > > actually taking care of quiescing any pending interrupts before > > enabling/after disabling the interrupt line. > > > > - Finally, after all drivers have been ported, remove the > > .ack_interrupt() and .did_interrupt() callbacks from phy_driver. > > > > Looks good to me. The current interrupt support in phylib basically > just covers the link change interrupt and we need more flexibility. > > And even in the current limited use case we face smaller issues. > One reason is that INTR_STATUS typically is self-clearing on read. > phylib has to deal with the case that did_interrupt may or may not > have read INTR_STATUS already. > > I'd just like to avoid the term "shared interrupt", because it has > a well-defined meaning. Our major concern isn't shared interrupts > but support for multiple interrupt sources (in addition to > link change) in a PHY. > I am not going to address this part, Vladimir did a good job in the following emails describing exactly the problem that I am trying to fix - shared interrupts even between PHYs which are not in the same package or even the same type of device. > WRT implementing a shutdown hook another use case was mentioned > recently: https://lkml.org/lkml/2020/9/30/451 > But that's not really relevant here and just fyi. > I missed this thread. Thanks for the link! Ioana
Re: [PATCH net-next 00/19] net: phy: add support for shared interrupts (part 1)
On Sat, Oct 31, 2020 at 12:36:27AM +0100, Andrew Lunn wrote: > > > - Every PHY driver gains a .handle_interrupt() implementation that, for > > > the most part, would look like below: > > > > > > irq_status = phy_read(phydev, INTR_STATUS); > > > if (irq_status < 0) { > > > phy_error(phydev); > > > return IRQ_NONE; > > > } > > > > > > if (irq_status == 0) > > > > Here I have a concern, bits may be set even if the respective interrupt > > source isn't enabled. Therefore we may falsely blame a device to have > > triggered the interrupt. irq_status should be masked with the actually > > enabled irq source bits. > > Hi Heiner > > I would say that is a driver implementation detail, for each driver to > handle how it needs to handle it. I've seen some hardware where the > interrupt status is already masked with the interrupt enabled > bits. I've soon other hardware where it is not. > > For example code, what is listed above is O.K. The real implementation > in a driver need knowledge of the hardware. > Hi, As Andrew said, that is just an example code that could work for some devices but should be extended depending on how the actual PHY is working. For example, the VSC8584 will still be trigerring the link state machine just on the link change interrupt, I am not changing this: static irqreturn_t vsc8584_handle_interrupt(struct phy_device *phydev) { irqreturn_t ret; int irq_status; irq_status = phy_read(phydev, MII_VSC85XX_INT_STATUS); if (irq_status < 0) return IRQ_NONE; /* Timestamping IRQ does not set a bit in the global INT_STATUS, so * irq_status would be 0. */ ret = vsc8584_handle_ts_interrupt(phydev); if (!(irq_status & MII_VSC85XX_INT_MASK_MASK)) return ret; if (irq_status & MII_VSC85XX_INT_MASK_EXT) vsc8584_handle_macsec_interrupt(phydev); if (irq_status & MII_VSC85XX_INT_MASK_LINK_CHG) phy_trigger_machine(phydev); return IRQ_HANDLED; } Ioana
Re: [PATCH v4 11/14] clk: imx: Add blk-ctl driver for i.MX8MP
On Mon, Oct 26, 2020 at 2:33 PM Abel Vesa wrote: > > This driver is intended to work with the following BLK_CTL IPs found in > i.MX8MP: > - Audio > - Media > - HDMI > > Signed-off-by: Abel Vesa > --- > drivers/clk/imx/Makefile | 2 +- > drivers/clk/imx/clk-blk-ctl-imx8mp.c | 316 > +++ > 2 files changed, 317 insertions(+), 1 deletion(-) > create mode 100644 drivers/clk/imx/clk-blk-ctl-imx8mp.c > > diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile > index 3d6d9cb..6c9b595 100644 > --- a/drivers/clk/imx/Makefile > +++ b/drivers/clk/imx/Makefile > @@ -23,7 +23,7 @@ obj-$(CONFIG_MXC_CLK) += mxc-clk.o > > obj-$(CONFIG_CLK_IMX8MM) += clk-imx8mm.o > obj-$(CONFIG_CLK_IMX8MN) += clk-imx8mn.o > -obj-$(CONFIG_CLK_IMX8MP) += clk-imx8mp.o clk-blk-ctl.o > +obj-$(CONFIG_CLK_IMX8MP) += clk-imx8mp.o clk-blk-ctl.o clk-blk-ctl-imx8mp.o > obj-$(CONFIG_CLK_IMX8MQ) += clk-imx8mq.o > > obj-$(CONFIG_MXC_CLK_SCU) += clk-imx-scu.o clk-imx-lpcg-scu.o > diff --git a/drivers/clk/imx/clk-blk-ctl-imx8mp.c > b/drivers/clk/imx/clk-blk-ctl-imx8mp.c > new file mode 100644 > index ..cee146e > --- /dev/null > +++ b/drivers/clk/imx/clk-blk-ctl-imx8mp.c > @@ -0,0 +1,316 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright 2020 NXP. > + */ > + > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "clk.h" > +#include "clk-blk-ctl.h" > + > +#defineIMX_AUDIO_BLK_CTL_CLKEN00x0 > +#defineIMX_AUDIO_BLK_CTL_CLKEN10x4 > +#defineIMX_AUDIO_BLK_CTL_EARC 0x200 > +#defineIMX_AUDIO_BLK_CTL_SAI1_MCLK_SEL 0x300 > +#defineIMX_AUDIO_BLK_CTL_SAI2_MCLK_SEL 0x304 > +#defineIMX_AUDIO_BLK_CTL_SAI3_MCLK_SEL 0x308 > +#defineIMX_AUDIO_BLK_CTL_SAI5_MCLK_SEL 0x30C > +#defineIMX_AUDIO_BLK_CTL_SAI6_MCLK_SEL 0x310 > +#defineIMX_AUDIO_BLK_CTL_SAI7_MCLK_SEL 0x314 > +#defineIMX_AUDIO_BLK_CTL_PDM_CLK 0x318 > +#defineIMX_AUDIO_BLK_CTL_SAI_PLL_GNRL_CTL 0x400 > +#defineIMX_AUDIO_BLK_CTL_SAI_PLL_FDIVL_CTL00x404 > +#defineIMX_AUDIO_BLK_CTL_SAI_PLL_FDIVL_CTL10x408 > +#defineIMX_AUDIO_BLK_CTL_SAI_PLL_SSCG_CTL 0x40C > +#defineIMX_AUDIO_BLK_CTL_SAI_PLL_MNIT_CTL 0x410 > +#defineIMX_AUDIO_BLK_CTL_IPG_LP_CTRL 0x504 > + > +#define IMX_MEDIA_BLK_CTL_SFT_RSTN 0x0 > +#define IMX_MEDIA_BLK_CTL_CLK_EN 0x4 > + > +static int shared_count_pdm; > + > +static const struct imx_pll14xx_rate_table imx_blk_ctl_sai_pll_tbl[] = { > + PLL_1443X_RATE(65000U, 325, 3, 2, 0), > +}; > + > +static const struct imx_pll14xx_clk imx_blk_ctl_sai_pll = { > + .type = PLL_1443X, > + .rate_table = imx_blk_ctl_sai_pll_tbl, > +}; > + > +static const char * const imx_sai_mclk2_sels[] = {"sai1_root", "sai2_root", > "sai3_root", "dummy", > + "sai5_root", "sai6_root", > "sai7_root", "sai1_mclk", > + "sai2_mclk", "sai3_mclk", "dummy", > + "sai5_mclk", "sai6_mclk", > "sai7_mclk", "spdif1_ext_clk"}; > +static const char * const imx_sai1_mclk1_sels[] = {"sai1_root", "sai1_mclk", > }; > +static const char * const imx_sai2_mclk1_sels[] = {"sai2_root", "sai2_mclk", > }; > +static const char * const imx_sai3_mclk1_sels[] = {"sai3_root", "sai3_mclk", > }; > +static const char * const imx_sai5_mclk1_sels[] = {"sai5_root", "sai5_mclk", > }; > +static const char * const imx_sai6_mclk1_sels[] = {"sai6_root", "sai6_mclk", > }; > +static const char * const imx_sai7_mclk1_sels[] = {"sai7_root", "sai7_mclk", > }; > +static const char * const imx_pdm_sels[] = {"pdm_root", "sai_pll_div2", > "dummy", "dummy" }; > +static const char * const imx_sai_pll_ref_sels[] = {"osc_24m", "dummy", > "dummy", "dummy", }; > +static const char * const imx_sai_pll_bypass_sels[] = {"sai_pll", > "sai_pll_ref_sel", }; > + > +static const char * const imx_hdmi_phy_clks_sels[] = {"hdmi_glb_24m", > "dummy", }; > +static const char * const imx_lcdif_clks_sels[] = {"dummy", "hdmi_glb_pix", > }; > +static const char * const imx_hdmi_pipe_clks_sels[] = {"dummy", > "hdmi_glb_pix", }; > + > +static struct imx_blk_ctl_hw imx8mp_hdmi_blk_ctl_hws[] = { > + /* clocks */ > + IMX_BLK_CTL_CLK_GATE("hdmi_glb_apb", > IMX8MP_CLK_HDMI_BLK_CTL_GLOBAL_APB_CLK, 0x40, 0, "hdmi_apb"), > + IMX_BLK_CTL_CLK_GATE("hdmi_glb_b", > IMX8MP_CLK_HDMI_BLK_CTL_GLOBAL_B_CLK, 0x40, 1, "hdmi_axi"), > + IMX_BLK_CTL_CLK_GATE("hdmi_glb_ref_266m", > IMX8MP_CLK_HDMI_BLK_CTL_GLOBAL_REF266M_CLK, 0x40, 2, "hdmi_ref_266m"), > + IMX_BLK_CTL_CLK_GATE("hdmi_glb_24m", > IMX8MP_CLK_HDMI_BLK_CTL_GLOBAL_XTAL24M_CLK, 0x40, 4, "hdmi_24m"), > +
Re: [PATCH net-next] net: dlci: Deprecate the DLCI driver (aka the Frame Relay layer)
On Fri, Oct 30, 2020 at 8:07 PM Jakub Kicinski wrote: > > This code has only seen cleanup patches since the git era begun - > do you think that it may still have users? Or is it completely unused? I don't think it is still used. But I don't have solid evidence. So I asked people in the warning messages to move to the newer HDLC Frame Relay layer. Some evidence indicating this is not in use might be: 1. In "Documentation/networking/framerelay.rst", in the last paragraph, the link for the user space programs needed for configuration has been broken. 2. The Frame Relay layer supports only one hardware driver in the kernel. I looked up the hardware on the vendor's website, and it seemed they had their own drivers for the hardware. I guess most users would use the driver provided by the vendor instead. The names of the hardware supported by our sdla.c driver appear in this page: http://ftp.sangoma.com/technote/INDEX According to this file in the same directory, the driver provided by the vendor is named WANPIPE and supports multi-protocol (unlike sdla.c / dlci.c but similar to hdlc_fr.c): http://ftp.sangoma.com/technote/tn0015.txt According to the WANPIPE installation guide here, the vendor explicitly requested users to disable dlci.c in the kernel, by answering NO to CONFIG_DLCI: http://ftp.sangoma.com/linux/current_wanpipe/doc/WanpipeInstallation.pdf 3. According to drivers/net/wan/Kconfig, sdla.c (the only hardware driver dlci.c supports) depends on CONFIG_ISA. According to Wikipedia, ISA is an old 16-bit bus system. I think this makes users of this hardware driver rare. 4. Frame Relay itself is an old technology. I'm not clear about its overall usage but if it's still used its usage must be declining. > The usual way of getting rid of old code is to move it to staging/ > for a few releases then delete it, like Arnd just did with wimax. Oh. OK. But I see "include/linux/if_frad.h" is included in "net/socket.c", and there's still some code in "net/socket.c" related to it. If we move all these files to "staging/", we need to change the "include" line in "net/socket.c" to point to the new location, and we still need to keep a little code in "net/socket.c". So I think if we move it to "staging/", we can't do this in a clean way.
Re: [PATCH 1/1] Fonts: font_acorn_8x8: Replace discarded const qualifier
Hi Lee, On Fri, Oct 30, 2020 at 06:18:22PM +, Lee Jones wrote: > Commit 09e5b3fd5672 ("Fonts: Support FONT_EXTRA_WORDS macros for > built-in fonts") introduced the following error when building > rpc_defconfig (only this build appears to be affected): > > `acorndata_8x8' referenced in section `.text' of > arch/arm/boot/compressed/ll_char_wr.o: > defined in discarded section `.data' of arch/arm/boot/compressed/font.o > `acorndata_8x8' referenced in section `.data.rel.ro' of > arch/arm/boot/compressed/font.o: > defined in discarded section `.data' of arch/arm/boot/compressed/font.o > make[3]: *** [/scratch/linux/arch/arm/boot/compressed/Makefile:191: > arch/arm/boot/compressed/vmlinux] Error 1 > make[2]: *** [/scratch/linux/arch/arm/boot/Makefile:61: > arch/arm/boot/compressed/vmlinux] Error 2 > make[1]: *** [/scratch/linux/arch/arm/Makefile:317: zImage] Error 2 > > The .data section is discarded at link time. Reinstating > acorndata_8x8 as const ensures it is still available after linking. Thanks a lot for fixing this up! I wasn't aware that the symbol is being referenced in arch/arm/boot/compressed/ll_char_wr.S. I'm sorry for the trouble. The patch is, > Cc: > Cc: Russell King > Signed-off-by: Lee Jones Tested-by: Peilin Ye Thank you, Peilin Ye
Re: [PATCH] checkpatch: improve handling of email comments
On Fri, 2020-10-30 at 12:58 +0100, Lukas Bulwahn wrote: > > On Fri, 30 Oct 2020, Joe Perches wrote: > > > On Fri, 2020-10-30 at 14:37 +0530, Dwaipayan Ray wrote: > > > checkpatch has limited support for parsing email comments. It only > > > support single name comments or single after address comments. > > > Whereas, RFC 5322 specifies that comments can be inserted in > > > between any tokens of the email fields. > > > > > > Improve comment parsing mechanism in checkpatch. > > > > > > What is handled now: > > > > > > - Multiple name/address comments > > > - Comments anywhere in between name/address > > > - Nested comments like (John (Doe)) > > > > > > A brief analysis of checkpatch output on v5.0..v5.7 showed that > > > after these modifications, the number of BAD_SIGN_OFF warnings > > > came down from 2944 to 1424, and FROM_SIGN_OFF_MISMATCH came > > > down from 2366 to 2330. > > > > > > So, a total of 1556 false positives were resolved in total. > > > > A mere reduction in messages emitted isn't necessarily good. > > > > Agree. That is why I also went through the list of those warnings. > > I could not spot any obvious true positive among the reduced ones. > > > > Please send me privately a complete list of these nominally > > false positive messages that are no longer emitted. > > > > I believe one of the relatively common incorrect messages is > > for the cc: where a version number is > > continued on the same line after a #. > > > > CC: sta...@vger.kernel.org # for versions x.y.z and above > > > > That was one, It's not just one, it's ~90% of the list that Dwaipayan sent me. $ wc -l mismatches 831 mismatches $ grep -v -i stable mismatches | wc -l 98 > another common pattern was just quotes put inconsistently at > different places. Yes, there are some defects there. But there are also now false negatives. For instance, this is not appropriate to ignore: WARNING:BAD_SIGN_OFF: email address 'jacek.anaszew...@gmail.com, linux-l...@vger.kernel.org, linux-kernel@vger.kernel.org, dmur...@ti.com' might be better as 'jacek.anaszew...@gmail.com,linux-l...@vger.kernel.org, linux-kernel@vger.kernel.org, dmur...@ti.com' >From the file that Dwaipayan sent me, all the rest not including the stable variants, which IMO should be handled separately, are below. Of these 98 in total, 60+% are unicode which IMO should always be quoted and most are doubled with BAD_SIGN_OFF doubling FROM_SIGN_OFF_MISMATCH (and I don't quite understand why it's "From:/" then "Signed-off-by:" $ grep -v -i stable dwai | sort | uniq -c | sort -rn 31 WARNING:BAD_SIGN_OFF: email address '周琰杰 (Zhou Yanjie) ' might be better as '"周琰杰"(Zhou Yanjie) ' 30 WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email name mismatch: 'From: "周琰杰"(Zhou Yanjie) ' != 'Signed-off-by: 周琰杰 (Zhou Yanjie) ' These 29 in total would be better stripping any bits in parentheses from the name portion only when _not_ inside quotes. 20 WARNING:BAD_SIGN_OFF: email address 'Thomas Hellström (VMware) ' might be better as '"Thomas Hellström"(VMware) ' 5 WARNING:BAD_SIGN_OFF: email address 'H. Peter Anvin (Intel) ' might be better as '"H. Peter Anvin"(Intel) ' 1 WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email name mismatch: 'From: "Thomas Hellström"(VMware) ' != 'Signed-off-by: Thomas Hellström (VMware) ' 1 WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email name mismatch: 'From: "Srivatsa S. Bhat"(VMware) ' != 'Signed-off-by: Srivatsa S. Bhat (VMware) ' 1 WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email name mismatch: 'From: JanNieuwenhuizen(janneke) ' != 'Signed-off-by: Jan Nieuwenhuizen ' 1 WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email name mismatch: 'From: "Frédéric Pierret"(fepitre) ' != 'Signed-off-by: Frédéric Pierret (fepitre) ' So these 8 others are ones where quotes are either oddly placed or perhaps should always exist and the comment in parentheses is suggested poorly. 7 of these should be fixed and one should still be reported. 1 WARNING:BAD_SIGN_OFF: email address '"Thomas Hellström (VMware)" ' might be better as '"Thomas Hellström"(VMware) ' 1 WARNING:BAD_SIGN_OFF: email address 'Srivatsa S. Bhat (VMware) ' might be better as '"Srivatsa S. Bhat"(VMware) ' 1 WARNING:BAD_SIGN_OFF: email address '"Rantala, Tommi T. (Nokia - FI/Espoo)" ' might be better as '"Rantala, Tommi T."(Nokia - FI/Espoo) ' 1 WARNING:BAD_SIGN_OFF: email address '"Kai Mäkisara (Kolumbus)" ' might be better as '"Kai Mäkisara"(Kolumbus) ' 1 WARNING:BAD_SIGN_OFF: email address 'jacek.anaszew...@gmail.com, linux-l...@vger.kernel.org, linux-kernel@vger.kernel.org, dmur...@ti.com' might be better as 'jacek.anaszew...@gmail.com,linux-l...@vger.kernel.org, linux-kernel@vger.kernel.org, dmur...@ti.com' 1 WARNING:BAD_SIGN_OFF: email address 'Frédéric Pierret (fepitre) ' might be better as
Re: [PATCH v11 01/14] s390/vfio-ap: No need to disable IRQ after queue reset
On Fri, 30 Oct 2020 16:37:04 -0400 Tony Krowiak wrote: > On 10/30/20 1:42 PM, Halil Pasic wrote: > > On Thu, 29 Oct 2020 19:29:35 -0400 > > Tony Krowiak wrote: > > > @@ -1177,7 +1166,10 @@ static int vfio_ap_mdev_reset_queues(struct > mdev_device *mdev) > */ > if (ret) > rc = ret; > -vfio_ap_irq_disable_apqn(AP_MKQID(apid, apqi)); > +q = vfio_ap_get_queue(matrix_mdev, > + AP_MKQID(apid, apqi)); > +if (q) > +vfio_ap_free_aqic_resources(q); > >>> Is it safe to do vfio_ap_free_aqic_resources() at this point? I don't > >>> think so. I mean does the current code (and vfio_ap_mdev_reset_queue() > >>> in particular guarantee that the reset is actually done when we arrive > >>> here)? BTW, I think we have a similar problem with the current code as > >>> well. > >> If the return code from the vfio_ap_mdev_reset_queue() function > >> is zero, then yes, we are guaranteed the reset was done and the > >> queue is empty. > > I've read up on this and I disagree. We should discuss this offline. > > Maybe you are confusing things here; my statement is specific to the return > code from the vfio_ap_mdev_reset_queue() function, not the response code > from the PQAP(ZAPQ) instruction. The vfio_ap_mdev_reset_queue() > function issues the PQAP(ZAPQ) instruction and if the status response code > is 0 indicating the reset was successfully initiated, it waits for the > queue to empty. When the queue is empty, it returns 0 to indicate > the queue is reset. > If the queue does not become empty after a period of > time, > it will issue a warning (WARN_ON_ONCE) and return 0. In that case, I suppose > there is no guarantee the reset was done, so maybe a change needs to be > made there such as a non-zero return code. > I've overlooked the wait for empty. Maybe that return 0 had a part in it. I now remember me insisting on having the wait code added when the interrupt support was in the make. Sorry! If we have given up on out of retries retries, we are in trouble anyway. > > > >> The function returns a non-zero return code if > >> the reset fails or the queue the reset did not complete within a given > >> amount of time, so maybe we shouldn't free AQIC resources when > >> we get a non-zero return code from the reset function? > >> > > If the queue is gone, or broken, it won't produce interrupts or poke the > > notifier bit, and we should clean up the AQIC resources. > > True, which is what the code provided by this patch does; however, > the AQIC resources should be cleaned up only if the KVM pointer is > not NULL for reasons discussed elsewhere. Yes, but these should be cleaned up before the KVM pointer becomes null. We don't want to keep the page with the notifier byte pinned forever, or?
Re: [PATCH] kunit: tool: fix extra trailing \n in raw + parsed test output
On Sat, Oct 31, 2020 at 6:39 AM Daniel Latypov wrote: > > For simplcity, strip all trailing whitespace from parsed output. > I imagine no one is printing out meaningful trailing whitespace via > KUNIT_FAIL() or similar, and that if they are, they really shouldn't. > > `isolate_kunit_output()` yielded liens with trailing \n, which results > in artifacty output like this: > > $ ./tools/testing/kunit/kunit.py run > [16:16:46] [FAILED] example_simple_test > [16:16:46] # example_simple_test: EXPECTATION FAILED at > lib/kunit/kunit-example-test.c:29 > > [16:16:46] Expected 1 + 1 == 3, but > > [16:16:46] 1 + 1 == 2 > > [16:16:46] 3 == 3 > > [16:16:46] not ok 1 - example_simple_test > > [16:16:46] > > After this change: > [16:16:46] # example_simple_test: EXPECTATION FAILED at > lib/kunit/kunit-example-test.c:29 > [16:16:46] Expected 1 + 1 == 3, but > [16:16:46] 1 + 1 == 2 > [16:16:46] 3 == 3 > [16:16:46] not ok 1 - example_simple_test > [16:16:46] > > We should *not* be expecting lines to end with \n in kunit_tool_test.py > for this reason. > > Do the same for `raw_output()` as well which suffers from the same > issue. > > This is a followup to [1], but rebased onto kunit-fixes to pick up the > other raw_output() fix and fixes for kunit_tool_test.py. > > [1] > https://lore.kernel.org/linux-kselftest/20201020233219.4146059-1-dlaty...@google.com/ > > Signed-off-by: Daniel Latypov > --- Thanks! I tried this out against everything I could (including the nastier --alltests option), and didn't hit any problems, so it looks good to go to me! Reviewed-by: David Gow Tested-by: David Gow Cheers, -- David
[PATCH] spi: spi-mem: Fix passing zero to 'PTR_ERR' warning
Fix smatch warning: drivers/spi/spi-mem.c:746 spi_mem_probe() warn: passing zero to 'PTR_ERR' Fixes: 5d27a9c8ea9e ("spi: spi-mem: Extend the SPI mem interface to set a custom memory name") Signed-off-by: YueHaibing --- drivers/spi/spi-mem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c index ef53290b7d24..a1b4d085834a 100644 --- a/drivers/spi/spi-mem.c +++ b/drivers/spi/spi-mem.c @@ -743,7 +743,7 @@ static int spi_mem_probe(struct spi_device *spi) mem->name = dev_name(>dev); if (IS_ERR_OR_NULL(mem->name)) - return PTR_ERR(mem->name); + return PTR_ERR_OR_ZERO(mem->name); spi_set_drvdata(spi, mem); -- 2.17.1
Re: [PATCH 1/4] dt-bindings: clock: Add SDX55 GCC clock bindings
Hi Rob, On Fri, Oct 30, 2020 at 02:22:25PM -0500, Rob Herring wrote: > On Wed, Oct 28, 2020 at 01:12:29PM +0530, Manivannan Sadhasivam wrote: > > From: Vinod Koul > > > > Add device tree bindings for global clock controller on SDX55 SoCs. > > > > Signed-off-by: Vinod Koul > > This should carry your S-o-b too. > Ah yes! > > --- > > .../bindings/clock/qcom,gcc-sdx55.yaml| 71 +++ > > include/dt-bindings/clock/qcom,gcc-sdx55.h| 112 ++ > > 2 files changed, 183 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/clock/qcom,gcc-sdx55.yaml > > create mode 100644 include/dt-bindings/clock/qcom,gcc-sdx55.h > > [...] > > diff --git a/include/dt-bindings/clock/qcom,gcc-sdx55.h > > b/include/dt-bindings/clock/qcom,gcc-sdx55.h > > new file mode 100644 > > index ..09ca45c6de73 > > --- /dev/null > > +++ b/include/dt-bindings/clock/qcom,gcc-sdx55.h > > @@ -0,0 +1,112 @@ > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > Dual license? > The downstream code just lists the GPL2.0 and I'm not sure if I can make it as dual license. Whereas the binding we made it dual license since we authored it. Thanks, Mani > > +/* > > + * Copyright (c) 2018, The Linux Foundation. All rights reserved. > > + * Copyright (c) 2020, Linaro Ltd. > > + */ > > + > > +#ifndef _DT_BINDINGS_CLK_QCOM_GCC_SDX55_H > > +#define _DT_BINDINGS_CLK_QCOM_GCC_SDX55_H > > + > > +#define GPLL0 3 > > +#define GPLL0_OUT_EVEN 4 > > +#define GPLL4 5 > > +#define GPLL4_OUT_EVEN 6 > > +#define GPLL5 7 > > +#define GCC_AHB_PCIE_LINK_CLK 8 > > +#define GCC_BLSP1_AHB_CLK 9 > > +#define GCC_BLSP1_QUP1_I2C_APPS_CLK10 > > +#define GCC_BLSP1_QUP1_I2C_APPS_CLK_SRC11 > > +#define GCC_BLSP1_QUP1_SPI_APPS_CLK12 > > +#define GCC_BLSP1_QUP1_SPI_APPS_CLK_SRC13 > > +#define GCC_BLSP1_QUP2_I2C_APPS_CLK14 > > +#define GCC_BLSP1_QUP2_I2C_APPS_CLK_SRC15 > > +#define GCC_BLSP1_QUP2_SPI_APPS_CLK16 > > +#define GCC_BLSP1_QUP2_SPI_APPS_CLK_SRC17 > > +#define GCC_BLSP1_QUP3_I2C_APPS_CLK18 > > +#define GCC_BLSP1_QUP3_I2C_APPS_CLK_SRC19 > > +#define GCC_BLSP1_QUP3_SPI_APPS_CLK20 > > +#define GCC_BLSP1_QUP3_SPI_APPS_CLK_SRC21 > > +#define GCC_BLSP1_QUP4_I2C_APPS_CLK22 > > +#define GCC_BLSP1_QUP4_I2C_APPS_CLK_SRC23 > > +#define GCC_BLSP1_QUP4_SPI_APPS_CLK24 > > +#define GCC_BLSP1_QUP4_SPI_APPS_CLK_SRC25 > > +#define GCC_BLSP1_UART1_APPS_CLK 26 > > +#define GCC_BLSP1_UART1_APPS_CLK_SRC 27 > > +#define GCC_BLSP1_UART2_APPS_CLK 28 > > +#define GCC_BLSP1_UART2_APPS_CLK_SRC 29 > > +#define GCC_BLSP1_UART3_APPS_CLK 30 > > +#define GCC_BLSP1_UART3_APPS_CLK_SRC 31 > > +#define GCC_BLSP1_UART4_APPS_CLK 32 > > +#define GCC_BLSP1_UART4_APPS_CLK_SRC 33 > > +#define GCC_BOOT_ROM_AHB_CLK 34 > > +#define GCC_CE1_AHB_CLK35 > > +#define GCC_CE1_AXI_CLK36 > > +#define GCC_CE1_CLK37 > > +#define GCC_CPUSS_AHB_CLK 38 > > +#define GCC_CPUSS_AHB_CLK_SRC 39 > > +#define GCC_CPUSS_GNOC_CLK 40 > > +#define GCC_CPUSS_RBCPR_CLK41 > > +#define GCC_CPUSS_RBCPR_CLK_SRC42 > > +#define GCC_EMAC_CLK_SRC 43 > > +#define GCC_EMAC_PTP_CLK_SRC 44 > > +#define GCC_ETH_AXI_CLK45 > > +#define GCC_ETH_PTP_CLK46 > > +#define GCC_ETH_RGMII_CLK 47 > > +#define GCC_ETH_SLAVE_AHB_CLK 48 > > +#define GCC_GP1_CLK49 > > +#define GCC_GP1_CLK_SRC50 > > +#define GCC_GP2_CLK
[PATCH] drm/bridge: tpd12s015: Fix irq registering in tpd12s015_probe
gpiod_to_irq() return negative value in case of error, the existing code handle negative error codes wrongly. Fixes: cff5e6f7e83f ("drm/bridge: Add driver for the TI TPD12S015 HDMI level shifter") Signed-off-by: YueHaibing --- drivers/gpu/drm/bridge/ti-tpd12s015.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/ti-tpd12s015.c b/drivers/gpu/drm/bridge/ti-tpd12s015.c index 514cbf0eac75..a18d5197c16c 100644 --- a/drivers/gpu/drm/bridge/ti-tpd12s015.c +++ b/drivers/gpu/drm/bridge/ti-tpd12s015.c @@ -160,7 +160,7 @@ static int tpd12s015_probe(struct platform_device *pdev) /* Register the IRQ if the HPD GPIO is IRQ-capable. */ tpd->hpd_irq = gpiod_to_irq(tpd->hpd_gpio); - if (tpd->hpd_irq) { + if (tpd->hpd_irq > 0) { ret = devm_request_threaded_irq(>dev, tpd->hpd_irq, NULL, tpd12s015_hpd_isr, IRQF_TRIGGER_RISING | -- 2.17.1
Re: [PATCH RFC v3 4/4] Documentation/admin-guide: Change doc for split_lock_detect parameter
Hi, On 10/30/20 5:27 PM, Fenghua Yu wrote: > Since #DB for bus lock detect changes the split_lock_detect parameter, > update the documentation for the changes. > > Signed-off-by: Fenghua Yu > Reviewed-by: Tony Luck > --- > Change Log: > - Simplify the documentation (Randy). > > .../admin-guide/kernel-parameters.txt | 28 +++ > 1 file changed, 23 insertions(+), 5 deletions(-) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt > b/Documentation/admin-guide/kernel-parameters.txt > index 526d65d8573a..ee419ce659f5 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -5044,27 +5044,45 @@ > spia_peddr= > > split_lock_detect= > - [X86] Enable split lock detection > + [X86] Enable split lock detection or bus lock detection > > When enabled (and if hardware support is present), > atomic > instructions that access data across cache line > - boundaries will result in an alignment check exception. > + boundaries will result in an alignment check exception > + for split lock detection or a debug exception for > + bus lock detection. > > off - not enabled > > warn- the kernel will emit rate limited warnings rate-limited > about applications triggering the #AC > - exception. This mode is the default on CPUs > - that supports split lock detection. > + exception or the #DB exception. This mode is > + the default on CPUs that supports split lock support > + detection or bus lock detection. Default > + behavior is from #DB if both features are I would sayis by #DB > + enabled in hardware. > > fatal - the kernel will send SIGBUS to applications > - that trigger the #AC exception. > + that trigger the #AC exception or the #DB > + exception. Default behavior is from #AC and is by #AC > + if both features are enabled in hardware. > + > + ratelimit:N - > + Set rate limit to N bus locks per second > + for bus lock detection. 0 < N <= HZ/2 and > + N is approximate. Only applied to non root non-root > + user. users. > + > + N/A for split lock detection. > > If an #AC exception is hit in the kernel or in > firmware (i.e. not while executing in user mode) > the kernel will oops in either "warn" or "fatal" > mode. > > + #DB exception for bus lock is triggered only when > + CPL > 0. > + thanks. -- ~Randy
ld.lld: error: undefined symbol: bpf_preload_ops
Hi Alexei, FYI, the error/warning still remains. tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 5fc6b075e165f641fbc366b58b578055762d5f8c commit: d71fa5c9763c24dd997a2fa4feb7a13a95bab42c bpf: Add kernel module with user mode driver that populates bpffs. date: 2 months ago config: x86_64-randconfig-r021-20201030 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project fa5a13276764a2657b3571fa3c57b07ee5d2d661) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d71fa5c9763c24dd997a2fa4feb7a13a95bab42c git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git git fetch --no-tags linus master git checkout d71fa5c9763c24dd997a2fa4feb7a13a95bab42c # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): >> ld.lld: error: undefined symbol: bpf_preload_ops >>> referenced by bpf_preload_kern.c:83 (kernel/bpf/preload/bpf_preload_kern.c:83) >>> bpf/preload/bpf_preload_kern.o:(fini_umd) in archive kernel/built-in.a >>> referenced by bpf_preload_kern.c:83 (kernel/bpf/preload/bpf_preload_kern.c:83) >>> bpf/preload/bpf_preload_kern.o:(fini_umd) in archive kernel/built-in.a >>> referenced by bpf_preload_kern.c:77 (kernel/bpf/preload/bpf_preload_kern.c:77) >>> bpf/preload/bpf_preload_kern.o:(load_umd) in archive kernel/built-in.a >>> referenced 1 more times --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
[PATCH] sfp: Fix error handing in sfp_probe()
gpiod_to_irq() never return 0, but returns negative in case of error, check it and set gpio_irq to 0. Fixes: 73970055450e ("sfp: add SFP module support") Signed-off-by: YueHaibing --- drivers/net/phy/sfp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c index 1d18c10e8f82..34aa196b7465 100644 --- a/drivers/net/phy/sfp.c +++ b/drivers/net/phy/sfp.c @@ -2389,7 +2389,8 @@ static int sfp_probe(struct platform_device *pdev) continue; sfp->gpio_irq[i] = gpiod_to_irq(sfp->gpio[i]); - if (!sfp->gpio_irq[i]) { + if (sfp->gpio_irq[i] < 0) { + sfp->gpio_irq[i] = 0; sfp->need_poll = true; continue; } -- 2.17.1
Re: Using fixed LPI number for some Device ID
On 2020/10/31 10:59, Thomas Gleixner wrote: > On Sat, Oct 31 2020 at 10:19, Dongjiu Geng wrote: >> Hi Marc, >> Sorry to disturb you, Currently the LPI number is not fixed for >> the device. The LPI number is dynamically allocated start from 8092. >> For two OS which shares the ITS, One OS needs to configure the device >> interrupt required by another OS, and the other OS uses a fixed >> interrupt ID to respond the interrupt. Therefore, the LPI IRQ number >> of the device needed be fixed. I want to upstream this feature that >> allocate fixed LPI number for the device that is specified through >> the DTS. What is your meaning? Thanks > > What's the purpose of resending the same thing within less than 24 > hours? Do you really expect maintainers to be available 24/7 and being Sorry for the noise, Because Marc rarely uses the ARM email address, so I replace to use Marc's kernel.org address instead of ARM email address. > able to respond within less than a day? > > > . >
Re: [PATCH net-next] net: dlci: Deprecate the DLCI driver (aka the Frame Relay layer)
On Wed, 28 Oct 2020 00:05:04 -0700 Xie He wrote: > I wish to deprecate the Frame Relay layer (dlci.c) in the kernel because > we already have a newer and better "HDLC Frame Relay" layer (hdlc_fr.c). > > Reasons why hdlc_fr.c is better than dlci.c include: > > 1. > dlci.c is dated 1997, while hdlc_fr.c is dated 1999 - 2006, so the later > is newer than the former. > > 2. > hdlc_fr.c is working well (tested by me). For dlci.c, I cannot even find > the user space problem needed to use it. The link provided in > Documentation/networking/framerelay.rst (in the last paragraph) is no > longer valid. > > 3. > dlci.c supports only one hardware driver - sdla.c, while hdlc_fr.c > supports many hardware drivers through the generic HDLC layer (hdlc.c). > > WAN hardware devices are usually able to support several L2 protocols > at the same time, so the HDLC layer is more suitable for these devices. > > The hardware devices that sdla.c supports are also multi-protocol > (according to drivers/net/wan/Kconfig), so the HDLC layer is more > suitable for these devices, too. > > 4. > hdlc_fr.c supports LMI and supports Ethernet emulation. dlci.c supports > neither according to its code. > > 5. > include/uapi/linux/if_frad.h, which comes with dlci.c, contains two > structs for ioctl configs (dlci_conf and frad_conf). According to the > comments, these two structs are specially crafted for sdla.c (the only > hardware driver dlci.c supports). I think this makes dlci.c not generic > enough to support other hardware drivers. > > Signed-off-by: Xie He This code has only seen cleanup patches since the git era begun - do you think that it may still have users? Or is it completely unused? The usual way of getting rid of old code is to move it to staging/ for a few releases then delete it, like Arnd just did with wimax. > diff --git a/Documentation/networking/framerelay.rst > b/Documentation/networking/framerelay.rst > index 6d904399ec6d..92e66fc3dffc 100644 > --- a/Documentation/networking/framerelay.rst > +++ b/Documentation/networking/framerelay.rst > @@ -4,6 +4,9 @@ > Frame Relay (FR) > > > +(Note that this Frame Relay layer is deprecated. New drivers should use the > +HDLC Frame Relay layer instead.) > + > Frame Relay (FR) support for linux is built into a two tiered system of > device > drivers. The upper layer implements RFC1490 FR specification, and uses the > Data Link Connection Identifier (DLCI) as its hardware address. Usually > these > diff --git a/drivers/net/wan/dlci.c b/drivers/net/wan/dlci.c > index 3ca4daf63389..1f0eee10c13f 100644 > --- a/drivers/net/wan/dlci.c > +++ b/drivers/net/wan/dlci.c > @@ -514,6 +514,8 @@ static int __init init_dlci(void) > register_netdevice_notifier(_notifier); > > printk("%s.\n", version); > + pr_warn("The DLCI driver (the Frame Relay layer) is deprecated.\n" > + "Please move your driver to the HDLC Frame Relay layer.\n"); > > return 0; > } > diff --git a/drivers/net/wan/sdla.c b/drivers/net/wan/sdla.c > index bc2c1c7fb1a4..21d602f698fc 100644 > --- a/drivers/net/wan/sdla.c > +++ b/drivers/net/wan/sdla.c > @@ -1623,6 +1623,9 @@ static int __init init_sdla(void) > int err; > > printk("%s.\n", version); > + pr_warn("The SDLA driver is deprecated.\n" > + "If you are still using the hardware,\n" > + "please help move this driver to the HDLC Frame Relay > layer.\n"); > > sdla = alloc_netdev(sizeof(struct frad_local), "sdla0", > NET_NAME_UNKNOWN, setup_sdla);
[PATCH] serial: mctrl_gpio: Fix passing zero to 'ERR_PTR' warning
drivers/tty/serial/serial_mctrl_gpio.c:214 mctrl_gpio_init() warn: passing zero to 'ERR_PTR' gpiod_to_irq() never return 0, so remove the useless test and make code more clear. Signed-off-by: YueHaibing --- drivers/tty/serial/serial_mctrl_gpio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c index fb4781292d40..c41d8911ce95 100644 --- a/drivers/tty/serial/serial_mctrl_gpio.c +++ b/drivers/tty/serial/serial_mctrl_gpio.c @@ -207,7 +207,7 @@ struct mctrl_gpios *mctrl_gpio_init(struct uart_port *port, unsigned int idx) continue; ret = gpiod_to_irq(gpios->gpio[i]); - if (ret <= 0) { + if (ret < 0) { dev_err(port->dev, "failed to find corresponding irq for %s (idx=%d, err=%d)\n", mctrl_gpios_desc[i].name, idx, ret); -- 2.17.1
Re: [PATCH v11 4/4] bus: mhi: Add userspace client interface driver
hi Hemant, On Fri, Oct 30, 2020 at 06:26:38PM -0700, Hemant Kumar wrote: > Hi Mani, > > On 10/30/20 3:34 AM, Manivannan Sadhasivam wrote: > > Hi Hemant, > > > > On Thu, Oct 29, 2020 at 07:45:46PM -0700, Hemant Kumar wrote: > > > This MHI client driver allows userspace clients to transfer > > > raw data between MHI device and host using standard file operations. > > > Driver instantiates UCI device object which is associated to device > > > file node. UCI device object instantiates UCI channel object when device > > > file node is opened. UCI channel object is used to manage MHI channels > > > by calling MHI core APIs for read and write operations. MHI channels > > > are started as part of device open(). MHI channels remain in start > > > state until last release() is called on UCI device file node. Device > > > file node is created with format > > > > > > /dev/mhi__ > > > > > > Currently it supports LOOPBACK channel. > > > > > > Signed-off-by: Hemant Kumar > > > > Thanks for continuously updating the series based on reviews, now the > > locking > > part looks a _lot_ cleaner than it used to be. I just have one query > > (inline) > > regarding the usage of refcount for uci_chan and uci_dev. Once you fix that, > > I think this is good to go in. > Thanks for reviewing my changes. > > [..] > > > > +#define DEVICE_NAME "mhi" > > > +#define MHI_UCI_DRIVER_NAME "mhi_uci" > > > +#define MAX_UCI_MINORS 128 > > > > Prefix MHI for these. > Done. > > > > > > + > > > +static DEFINE_IDR(uci_idr); > > > +static DEFINE_MUTEX(uci_drv_mutex); > > > +static struct class *uci_dev_class; > > > +static int uci_dev_major; > > > + > > > +/** > > > + * struct uci_chan - MHI channel for a UCI device > > > + * @udev: associated UCI device object > > > + * @ul_wq: wait queue for writer > > > + * @write_lock: mutex write lock for ul channel > > > + * @dl_wq: wait queue for reader > > > + * @read_lock: mutex read lock for dl channel > > > + * @dl_pending_lock: spin lock for dl_pending list > > > + * @dl_pending: list of dl buffers userspace is waiting to read > > > + * @cur_buf: current buffer userspace is reading > > > + * @dl_size: size of the current dl buffer userspace is reading > > > + * @ref_count: uci_chan reference count > > > + */ > > > +struct uci_chan { > > > + struct uci_dev *udev; > > > + wait_queue_head_t ul_wq; > > > + > > > + /* ul channel lock to synchronize multiple writes */ > > > > I asked you to move these comments to Kdoc in previous iteration. > There are multiple revisions of UCI pushed after i responded on this one. On > V7 i responded to your comment :) > > "This was added because checkpatch --strict required to add a comment when > lock is added to struct, after adding inline comment, checkpatch error was > gone." > > i was sticking to --strict option. Considering it is best to address what > --strict is complaining for. Ah okay. > > > > > + struct mutex write_lock; > > > + > > > + wait_queue_head_t dl_wq; > > > + > > > + /* dl channel lock to synchronize multiple reads */ > > > + struct mutex read_lock; > > > + > > > + /* > > > + * protects pending list in bh context, channel release, read and > > > + * poll > > > + */ > > > + spinlock_t dl_pending_lock; > > > + > > > + struct list_head dl_pending; > > > + struct uci_buf *cur_buf; > > > + size_t dl_size; > > > + struct kref ref_count; > > > > I'm now thinking that instead of having two reference counts for uci_chan > > and > > uci_dev, why can't you club them together and just use uci_dev's refcount to > > handle the channel management also. > > > > For instance in uci_open, you are incrementing the refcount for uci_dev > > before > > starting the channel and then doing the same for uci_chan in > > mhi_uci_dev_start_chan(). So why can't you just use a single refcount once > > the > > mhi_uci_dev_start_chan() succeeds? The UCI device is useless without a > > channel, > > isn't it? > Main idea is to have the uci driver probed (uci device object is > instantiated) but it is possible that device node is not opened or if it was > opened before and release() was called after that. So UCI channel is not > active but device node would continue to exist. Which can be opened again > and channel would move to start state. So we dont want to couple mhi driver > probe with starting of channels. We start channels only when it is really > needed. This would allow MHI device to go to lower power state when channels > are disabled. > Okay, makes sense! Please make sure you add it in Documentation. > [..] > > > > + > > > +static int mhi_queue_inbound(struct uci_dev *udev) > > > +{ > > > + struct mhi_device *mhi_dev = udev->mhi_dev; > > > + struct device *dev = _dev->dev; > > > + int nr_trbs, i, ret = -EIO; > > > > s/nr_trbs/nr_desc > Done. > > > > > + size_t dl_buf_size; > > > + void *buf; > > > + struct uci_buf *ubuf; > > > + > > > + /* dont queue if dl channel is not supported */ > > > + if (!udev->mhi_dev->dl_chan) > > > +
[PATCH 5.9.y] erofs: avoid duplicated permission check for "trusted." xattrs
From: Gao Xiang commit d578b46db69d125a654f509bdc9091d84e924dc8 upstream. Don't recheck it since xattr_permission() already checks CAP_SYS_ADMIN capability. Just follow 5d3ce4f70172 ("f2fs: avoid duplicated permission check for "trusted." xattrs") Reported-by: Hongyu Jin [ Gao Xiang: since it could cause some complex Android overlay permission issue as well on android-5.4+, it'd be better to backport to 5.4+ rather than pure cleanup on mainline. ] Cc: # 5.4+ Link: https://lore.kernel.org/r/20200811070020.6339-1-hsiang...@redhat.com Reviewed-by: Chao Yu Signed-off-by: Gao Xiang --- fs/erofs/xattr.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c index c8c381eadcd6..5bde77d70852 100644 --- a/fs/erofs/xattr.c +++ b/fs/erofs/xattr.c @@ -473,8 +473,6 @@ static int erofs_xattr_generic_get(const struct xattr_handler *handler, return -EOPNOTSUPP; break; case EROFS_XATTR_INDEX_TRUSTED: - if (!capable(CAP_SYS_ADMIN)) - return -EPERM; break; case EROFS_XATTR_INDEX_SECURITY: break; -- 2.24.0
[PATCH 5.8.y] erofs: avoid duplicated permission check for "trusted." xattrs
From: Gao Xiang commit d578b46db69d125a654f509bdc9091d84e924dc8 upstream. Don't recheck it since xattr_permission() already checks CAP_SYS_ADMIN capability. Just follow 5d3ce4f70172 ("f2fs: avoid duplicated permission check for "trusted." xattrs") Reported-by: Hongyu Jin [ Gao Xiang: since it could cause some complex Android overlay permission issue as well on android-5.4+, it'd be better to backport to 5.4+ rather than pure cleanup on mainline. ] Cc: # 5.4+ Link: https://lore.kernel.org/r/20200811070020.6339-1-hsiang...@redhat.com Reviewed-by: Chao Yu Signed-off-by: Gao Xiang --- fs/erofs/xattr.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c index 87e437e7b34f..f86e3247febc 100644 --- a/fs/erofs/xattr.c +++ b/fs/erofs/xattr.c @@ -473,8 +473,6 @@ static int erofs_xattr_generic_get(const struct xattr_handler *handler, return -EOPNOTSUPP; break; case EROFS_XATTR_INDEX_TRUSTED: - if (!capable(CAP_SYS_ADMIN)) - return -EPERM; break; case EROFS_XATTR_INDEX_SECURITY: break; -- 2.24.0
[PATCH 5.4.y] erofs: avoid duplicated permission check for "trusted." xattrs
From: Gao Xiang commit d578b46db69d125a654f509bdc9091d84e924dc8 upstream. Don't recheck it since xattr_permission() already checks CAP_SYS_ADMIN capability. Just follow 5d3ce4f70172 ("f2fs: avoid duplicated permission check for "trusted." xattrs") Reported-by: Hongyu Jin [ Gao Xiang: since it could cause some complex Android overlay permission issue as well on android-5.4+, it'd be better to backport to 5.4+ rather than pure cleanup on mainline. ] Cc: # 5.4+ Link: https://lore.kernel.org/r/20200811070020.6339-1-hsiang...@redhat.com Reviewed-by: Chao Yu Signed-off-by: Gao Xiang --- fs/erofs/xattr.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c index b766c3ee5fa8..503bea20cde2 100644 --- a/fs/erofs/xattr.c +++ b/fs/erofs/xattr.c @@ -473,8 +473,6 @@ static int erofs_xattr_generic_get(const struct xattr_handler *handler, return -EOPNOTSUPP; break; case EROFS_XATTR_INDEX_TRUSTED: - if (!capable(CAP_SYS_ADMIN)) - return -EPERM; break; case EROFS_XATTR_INDEX_SECURITY: break; -- 2.24.0
Re: Using fixed LPI number for some Device ID
On Sat, Oct 31 2020 at 10:19, Dongjiu Geng wrote: > Hi Marc, > Sorry to disturb you, Currently the LPI number is not fixed for > the device. The LPI number is dynamically allocated start from 8092. > For two OS which shares the ITS, One OS needs to configure the device > interrupt required by another OS, and the other OS uses a fixed > interrupt ID to respond the interrupt. Therefore, the LPI IRQ number > of the device needed be fixed. I want to upstream this feature that > allocate fixed LPI number for the device that is specified through > the DTS. What is your meaning? Thanks What's the purpose of resending the same thing within less than 24 hours? Do you really expect maintainers to be available 24/7 and being able to respond within less than a day?
Re: [PATCH v5 05/15] mm/frame-vector: Use FOLL_LONGTERM
On 10/30/20 3:08 AM, Daniel Vetter wrote: This is used by media/videbuf2 for persistent dma mappings, not just for a single dma operation and then freed again, so needs FOLL_LONGTERM. Unfortunately current pup_locked doesn't support FOLL_LONGTERM due to locking issues. Rework the code to pull the pup path out from the mmap_sem critical section as suggested by Jason. By relying entirely on the vma checks in pin_user_pages and follow_pfn There are vma checks in pin_user_pages(), but this patch changes things to call pin_user_pages_fast(). And that does not have the vma checks. More below about this: (for vm_flags and vma_is_fsdax) we can also streamline the code a lot. Signed-off-by: Daniel Vetter Cc: Jason Gunthorpe Cc: Pawel Osciak Cc: Marek Szyprowski Cc: Kyungmin Park Cc: Tomasz Figa Cc: Mauro Carvalho Chehab Cc: Andrew Morton Cc: John Hubbard Cc: Jérôme Glisse Cc: Jan Kara Cc: Dan Williams Cc: linux...@kvack.org Cc: linux-arm-ker...@lists.infradead.org Cc: linux-samsung-...@vger.kernel.org Cc: linux-me...@vger.kernel.org Signed-off-by: Daniel Vetter -- v2: Streamline the code and further simplify the loop checks (Jason) v5: Review from Tomasz: - fix page counting for the follow_pfn case by resetting ret - drop gup_flags paramater, now unused --- .../media/common/videobuf2/videobuf2-memops.c | 3 +- include/linux/mm.h| 2 +- mm/frame_vector.c | 53 ++- 3 files changed, 19 insertions(+), 39 deletions(-) diff --git a/drivers/media/common/videobuf2/videobuf2-memops.c b/drivers/media/common/videobuf2/videobuf2-memops.c index 6e9e05153f4e..9dd6c27162f4 100644 --- a/drivers/media/common/videobuf2/videobuf2-memops.c +++ b/drivers/media/common/videobuf2/videobuf2-memops.c @@ -40,7 +40,6 @@ struct frame_vector *vb2_create_framevec(unsigned long start, unsigned long first, last; unsigned long nr; struct frame_vector *vec; - unsigned int flags = FOLL_FORCE | FOLL_WRITE; first = start >> PAGE_SHIFT; last = (start + length - 1) >> PAGE_SHIFT; @@ -48,7 +47,7 @@ struct frame_vector *vb2_create_framevec(unsigned long start, vec = frame_vector_create(nr); if (!vec) return ERR_PTR(-ENOMEM); - ret = get_vaddr_frames(start & PAGE_MASK, nr, flags, vec); + ret = get_vaddr_frames(start & PAGE_MASK, nr, vec); if (ret < 0) goto out_destroy; /* We accept only complete set of PFNs */ diff --git a/include/linux/mm.h b/include/linux/mm.h index ef360fe70aaf..d6b8e30dce2e 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -1765,7 +1765,7 @@ struct frame_vector { struct frame_vector *frame_vector_create(unsigned int nr_frames); void frame_vector_destroy(struct frame_vector *vec); int get_vaddr_frames(unsigned long start, unsigned int nr_pfns, -unsigned int gup_flags, struct frame_vector *vec); +struct frame_vector *vec); void put_vaddr_frames(struct frame_vector *vec); int frame_vector_to_pages(struct frame_vector *vec); void frame_vector_to_pfns(struct frame_vector *vec); diff --git a/mm/frame_vector.c b/mm/frame_vector.c index 10f82d5643b6..f8c34b895c76 100644 --- a/mm/frame_vector.c +++ b/mm/frame_vector.c @@ -32,13 +32,12 @@ * This function takes care of grabbing mmap_lock as necessary. */ int get_vaddr_frames(unsigned long start, unsigned int nr_frames, -unsigned int gup_flags, struct frame_vector *vec) +struct frame_vector *vec) { struct mm_struct *mm = current->mm; struct vm_area_struct *vma; int ret = 0; int err; - int locked; if (nr_frames == 0) return 0; @@ -48,40 +47,26 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames, start = untagged_addr(start); - mmap_read_lock(mm); - locked = 1; - vma = find_vma_intersection(mm, start, start + 1); - if (!vma) { - ret = -EFAULT; - goto out; - } - - /* -* While get_vaddr_frames() could be used for transient (kernel -* controlled lifetime) pinning of memory pages all current -* users establish long term (userspace controlled lifetime) -* page pinning. Treat get_vaddr_frames() like -* get_user_pages_longterm() and disallow it for filesystem-dax -* mappings. -*/ - if (vma_is_fsdax(vma)) { - ret = -EOPNOTSUPP; - goto out; - } - - if (!(vma->vm_flags & (VM_IO | VM_PFNMAP))) { By removing this check from this location, and changing from pin_user_pages_locked() to pin_user_pages_fast(), I *think* we end up losing the check entirely. Is that intended? If so it could use a comment somewhere to explain why. thanks, -- John Hubbard NVIDIA + ret = pin_user_pages_fast(start, nr_frames, +
Re: [PATCH] opp: Reduce the size of critical section in _opp_table_kref_release()
Quoting Viresh Kumar (2020-10-29 21:20:00) > On 29-10-20, 09:40, Viresh Kumar wrote: > > Thanks a lot. I was a bit worried about the crazy idea I had to solve > > this :) > > Hmm, I thought this is the other patch where I had that crazy idea. > This one was quite straight forward :) > What's the other crazy idea patch?
Re: [PATCH v4 00/17] Add VFIO mediated device support and DEV-MSI support for the idxd driver
Ashok, On Fri, Oct 30 2020 at 13:43, Ashok Raj wrote: > On Fri, Oct 30, 2020 at 04:30:45PM -0300, Jason Gunthorpe wrote: >> On Fri, Oct 30, 2020 at 12:23:25PM -0700, Raj, Ashok wrote: >> It is a different subsystem, different maintainer, and different >> reviewers. >> >> It is a development process problem, it doesn't matter what it is >> doing. < skip a lot of non-sensical arguments> > I know you aren't going to give up, but there is little we can do. I want > the maintainers to make that call and I'm not add more noise to this. Jason is absolutely right. Just because there is historical precendence which does not care about the differentiation of subsystems is not an argument at all to make the same mistakes which have been made years ago. IDXD is just infrastructure which provides the base for a variety of different functionalities. Very similar to what multi function devices provide. In fact IDXD is pretty much a MFD facility. Sticking all of it into dmaengine is sloppy at best. The dma engine related part of IDXD is only a part of the overall functionality. I'm well aware that it is conveniant to just throw everything into drivers/myturf/ but that does neither make it reviewable nor maintainable. What's the problem with restructuring your code in a way which makes it fit into existing subsystems? The whole thing - as I pointed out to Dave earlier - is based on 'works for me' wishful thinking with a blissful ignorance of the development process and the requirement to split a large problem into the proper bits and pieces aka. engineering 101. Thanks, tglx
[PATCH net-next] net/mlx5e: Remove duplicated include
Remove duplicated include. Signed-off-by: YueHaibing --- drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c index 599f5b5ebc97..58c177756dc4 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c @@ -52,7 +52,6 @@ #include "en/xsk/rx.h" #include "en/health.h" #include "en/params.h" -#include "en/txrx.h" static struct sk_buff * mlx5e_skb_from_cqe_mpwrq_linear(struct mlx5e_rq *rq, struct mlx5e_mpw_info *wi, -- 2.17.1
[PATCH net-next] net: hns3: Remove duplicated include
Remove duplicated include. Signed-off-by: YueHaibing --- drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c index 3606240025a8..f990f6915226 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c @@ -4,7 +4,6 @@ #include "hclge_main.h" #include "hclge_dcb.h" #include "hclge_tm.h" -#include "hclge_dcb.h" #include "hnae3.h" #define BW_PERCENT 100 -- 2.17.1
[PATCH net-next] liquidio: cn68xx: Remove duplicated include
Remove duplicated include. Signed-off-by: YueHaibing --- drivers/net/ethernet/cavium/liquidio/cn68xx_device.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/net/ethernet/cavium/liquidio/cn68xx_device.c b/drivers/net/ethernet/cavium/liquidio/cn68xx_device.c index 2a6d1cadac9e..30254e4cf70f 100644 --- a/drivers/net/ethernet/cavium/liquidio/cn68xx_device.c +++ b/drivers/net/ethernet/cavium/liquidio/cn68xx_device.c @@ -27,7 +27,6 @@ #include "cn66xx_device.h" #include "cn68xx_device.h" #include "cn68xx_regs.h" -#include "cn68xx_device.h" static void lio_cn68xx_set_dpi_regs(struct octeon_device *oct) { -- 2.17.1
Re: [PATCH v11 00/10] NTFS read-write driver GPL implementation by Paragon Software
On Fri, Oct 30, 2020 at 07:42:09PM -0700, Eric Biggers wrote: > Hi Konstantin, > > On Fri, Oct 30, 2020 at 06:02:29PM +0300, Konstantin Komarov wrote: > > This patch adds NTFS Read-Write driver to fs/ntfs3. > > > > Having decades of expertise in commercial file systems development and huge > > test coverage, we at Paragon Software GmbH want to make our contribution to > > the Open Source Community by providing implementation of NTFS Read-Write > > driver for the Linux Kernel. > > > > This is fully functional NTFS Read-Write driver. Current version works with > > NTFS(including v3.1) and normal/compressed/sparse files and supports > > journal replaying. > > > > We plan to support this version after the codebase once merged, and add new > > features and fix bugs. For example, full journaling support over JBD will be > > added in later updates. > > > > Have you tried testing this filesystem using some of the kernel debugging > options (lockdep, KASAN, etc.?). I tried a basic test just for fun, and I > immediately got a lockdep report: > > mkfs.ntfs -f /dev/vdb > mount /dev/vdb /mnt -t ntfs3 > echo foo > /mnt/foo > > == > WARNING: possible circular locking dependency detected > 5.10.0-rc1-00275-ga34a2c322380 #33 Not tainted > -- > bash/160 is trying to acquire lock: > 888011e68108 (>ni_lock){+.+.}-{3:3}, at: ni_lock > fs/ntfs3/ntfs_fs.h:959 [inline] > 888011e68108 (>ni_lock){+.+.}-{3:3}, at: ntfs_set_size+0xee/0x210 > fs/ntfs3/inode.c:880 > > but task is already holding lock: > 888011e68370 (>s_type->i_mutex_key#11){+.+.}-{3:3}, at: inode_trylock > include/linux/fs.h:794 [inline] > 888011e68370 (>s_type->i_mutex_key#11){+.+.}-{3:3}, at: > ntfs_file_write_iter+0x1bc/0x4e0 fs/ntfs3/file.c:1040 > > which lock already depends on the new lock. Also trying to create a symlink causes a stack out-of-bounds access: $ mkfs.ntfs -f /dev/vdb $ mount /dev/vdb /mnt -t ntfs3 $ ln -s target /mnt/symlink BUG: KASAN: stack-out-of-bounds in memcpy include/linux/string.h:399 [inline] BUG: KASAN: stack-out-of-bounds in hdr_insert_de+0x224/0x4d0 fs/ntfs3/index.c:851 Read of size 32 at addr c97b7b10 by task ln/181 CPU: 1 PID: 181 Comm: ln Not tainted 5.10.0-rc1-00275-ga34a2c322380 #33 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ArchLinux 1.14.0-1 04/01/2014 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0xa4/0xd9 lib/dump_stack.c:118 print_address_description.constprop.0+0x1f/0x160 mm/kasan/report.c:385 __kasan_report.cold+0x37/0x7f mm/kasan/report.c:545 kasan_report+0x3e/0x60 mm/kasan/report.c:562 check_memory_region_inline mm/kasan/generic.c:186 [inline] check_memory_region+0xfb/0x1d0 mm/kasan/generic.c:192 memcpy+0x24/0x60 mm/kasan/common.c:105 memcpy include/linux/string.h:399 [inline] hdr_insert_de+0x224/0x4d0 fs/ntfs3/index.c:851 indx_insert_into_root+0x5d0/0x1d10 fs/ntfs3/index.c:1585 indx_insert_entry+0x299/0x4e0 fs/ntfs3/index.c:1936 ntfs_insert_reparse+0x133/0x1b0 fs/ntfs3/fsntfs.c:2425 ntfs_create_inode+0x28ec/0x4590 fs/ntfs3/inode.c:1511 ntfs_symlink+0xb1/0xf0 fs/ntfs3/namei.c:198 vfs_symlink fs/namei.c:3960 [inline] vfs_symlink+0x237/0x380 fs/namei.c:3946 do_symlinkat+0x125/0x220 fs/namei.c:3987 __do_sys_symlinkat fs/namei.c:4001 [inline] __se_sys_symlinkat fs/namei.c:3998 [inline] __x64_sys_symlinkat+0x6e/0xb0 fs/namei.c:3998 do_syscall_64+0x32/0x50 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x7fdaf2fb2b2b Code: 73 01 c3 48 8b 0d 45 f3 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 0a 01 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 15 f3 0c 00 f7 d8 64 89 01 48 RSP: 002b:7ffe428da378 EFLAGS: 0246 ORIG_RAX: 010a RAX: ffda RBX: 7ffe428da598 RCX: 7fdaf2fb2b2b RDX: 7ffe428db812 RSI: ff9c RDI: 7ffe428db80b RBP: 7ffe428da588 R08: R09: R10: 5647f9c2b340 R11: 0246 R12: 0002 R13: 0001 R14: R15: addr c97b7b10 is located in stack of task ln/181 at offset 32 in frame: ntfs_insert_reparse+0x0/0x1b0 fs/ntfs3/fsntfs.c:2387 this frame has 1 object: [32, 60) 're' Memory state around the buggy address: c97b7a00: 00 00 00 00 00 f1 f1 f1 f1 f1 f1 04 f2 00 f3 f3 c97b7a80: f3 00 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 >c97b7b00: f1 f1 00 00 00 04 f3 f3 f3 f3 00 00 00 00 00 00 ^ c97b7b80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 c97b7c00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 ==
Re: [PATCH -next] drm/rockchip: cdn-dp: Mark cdn_dp_core_suspend/resume __maybe_unused
ping... On 2020/8/11 10:12, YueHaibing wrote: > If CONFIG_PM is not set, gcc warns: > > drivers/gpu/drm/rockchip/cdn-dp-core.c:1124:12: > warning: ‘cdn_dp_resume’ defined but not used [-Wunused-function] > > Mark them __maybe_unused to fix this. > > Reported-by: Hulk Robot > Signed-off-by: YueHaibing > --- > drivers/gpu/drm/rockchip/cdn-dp-core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c > b/drivers/gpu/drm/rockchip/cdn-dp-core.c > index a4a45daf93f2..413b0e90f10f 100644 > --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c > +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c > @@ -1107,7 +1107,7 @@ static const struct component_ops cdn_dp_component_ops > = { > .unbind = cdn_dp_unbind, > }; > > -static int cdn_dp_suspend(struct device *dev) > +static __maybe_unused int cdn_dp_suspend(struct device *dev) > { > struct cdn_dp_device *dp = dev_get_drvdata(dev); > int ret = 0; > @@ -1121,7 +1121,7 @@ static int cdn_dp_suspend(struct device *dev) > return ret; > } > > -static int cdn_dp_resume(struct device *dev) > +static __maybe_unused int cdn_dp_resume(struct device *dev) > { > struct cdn_dp_device *dp = dev_get_drvdata(dev); > >
Re: [PATCH v11 00/10] NTFS read-write driver GPL implementation by Paragon Software
Hi Konstantin, On Fri, Oct 30, 2020 at 06:02:29PM +0300, Konstantin Komarov wrote: > This patch adds NTFS Read-Write driver to fs/ntfs3. > > Having decades of expertise in commercial file systems development and huge > test coverage, we at Paragon Software GmbH want to make our contribution to > the Open Source Community by providing implementation of NTFS Read-Write > driver for the Linux Kernel. > > This is fully functional NTFS Read-Write driver. Current version works with > NTFS(including v3.1) and normal/compressed/sparse files and supports journal > replaying. > > We plan to support this version after the codebase once merged, and add new > features and fix bugs. For example, full journaling support over JBD will be > added in later updates. > Have you tried testing this filesystem using some of the kernel debugging options (lockdep, KASAN, etc.?). I tried a basic test just for fun, and I immediately got a lockdep report: mkfs.ntfs -f /dev/vdb mount /dev/vdb /mnt -t ntfs3 echo foo > /mnt/foo == WARNING: possible circular locking dependency detected 5.10.0-rc1-00275-ga34a2c322380 #33 Not tainted -- bash/160 is trying to acquire lock: 888011e68108 (>ni_lock){+.+.}-{3:3}, at: ni_lock fs/ntfs3/ntfs_fs.h:959 [inline] 888011e68108 (>ni_lock){+.+.}-{3:3}, at: ntfs_set_size+0xee/0x210 fs/ntfs3/inode.c:880 but task is already holding lock: 888011e68370 (>s_type->i_mutex_key#11){+.+.}-{3:3}, at: inode_trylock include/linux/fs.h:794 [inline] 888011e68370 (>s_type->i_mutex_key#11){+.+.}-{3:3}, at: ntfs_file_write_iter+0x1bc/0x4e0 fs/ntfs3/file.c:1040 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (>s_type->i_mutex_key#11){+.+.}-{3:3}: __lock_acquire+0x7b7/0x1060 kernel/locking/lockdep.c:4837 lock_acquire.part.0+0x13b/0x2a0 kernel/locking/lockdep.c:5442 lock_acquire+0xb4/0x1d0 kernel/locking/lockdep.c:5415 down_write+0x93/0x150 kernel/locking/rwsem.c:1531 inode_lock include/linux/fs.h:774 [inline] ntfs_set_state+0x192/0x650 fs/ntfs3/fsntfs.c:996 ntfs_create_inode+0x318/0x4590 fs/ntfs3/inode.c:1254 ntfs_atomic_open+0x350/0x450 fs/ntfs3/namei.c:521 atomic_open+0x16b/0x420 fs/namei.c:2970 lookup_open+0x7d1/0xdc0 fs/namei.c:3076 open_last_lookups+0x277/0x1040 fs/namei.c:3178 path_openat+0x161/0x310 fs/namei.c:3366 do_filp_open+0x17d/0x3b0 fs/namei.c:3396 do_sys_openat2+0x120/0x3d0 fs/open.c:1168 do_sys_open fs/open.c:1184 [inline] __do_sys_openat fs/open.c:1200 [inline] __se_sys_openat fs/open.c:1195 [inline] __x64_sys_openat+0x124/0x200 fs/open.c:1195 do_syscall_64+0x32/0x50 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x44/0xa9 -> #0 (>ni_lock){+.+.}-{3:3}: check_prev_add+0x162/0x20b0 kernel/locking/lockdep.c:2864 check_prevs_add kernel/locking/lockdep.c:2989 [inline] validate_chain+0xbff/0x1620 kernel/locking/lockdep.c:3607 __lock_acquire+0x7b7/0x1060 kernel/locking/lockdep.c:4837 lock_acquire.part.0+0x13b/0x2a0 kernel/locking/lockdep.c:5442 lock_acquire+0xb4/0x1d0 kernel/locking/lockdep.c:5415 __mutex_lock_common kernel/locking/mutex.c:956 [inline] __mutex_lock+0x142/0x12d0 kernel/locking/mutex.c:1103 mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1118 ni_lock fs/ntfs3/ntfs_fs.h:959 [inline] ntfs_set_size+0xee/0x210 fs/ntfs3/inode.c:880 ntfs_extend_ex+0x3b1/0x5c0 fs/ntfs3/file.c:466 ntfs_file_write_iter+0x244/0x4e0 fs/ntfs3/file.c:1050 call_write_iter include/linux/fs.h:1887 [inline] new_sync_write+0x348/0x6d0 fs/read_write.c:518 vfs_write+0x408/0x5b0 fs/read_write.c:605 ksys_write+0x11d/0x220 fs/read_write.c:658 __do_sys_write fs/read_write.c:670 [inline] __se_sys_write fs/read_write.c:667 [inline] __x64_sys_write+0x6e/0xb0 fs/read_write.c:667 do_syscall_64+0x32/0x50 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x44/0xa9 other info that might help us debug this: Possible unsafe locking scenario: CPU0CPU1 lock(>s_type->i_mutex_key#11); lock(>ni_lock); lock(>s_type->i_mutex_key#11); lock(>ni_lock); *** DEADLOCK *** 2 locks held by bash/160: #0: 8880084ac458 (sb_writers#11){.+.+}-{0:0}, at: file_start_write include/linux/fs.h:2759 [inline] #0: 8880084ac458 (sb_writers#11){.+.+}-{0:0}, at: vfs_write+0x3bc/0x5b0 fs/read_write.c:601 #1: 888011e68370 (>s_type->i_mutex_key#11){+.+.}-{3:3}, at: inode_trylock include/linux/fs.h:794 [inline] #1: 888011e68370 (>s_type->i_mutex_key#11){+.+.}-{3:3}, at: ntfs_file_write_iter+0x1bc/0x4e0 fs/ntfs3/file.c:1040
Hello/Hallo
-- Sir/Madam, I have access to very vital information that can be used to move a huge amount of money. I have done my homework very well and I have the machineries in place to get it done since I am still in active service. If it was possible for me to do it alone I would not have bothered contacting you. Ultimately I need an honest foreigner to play an important role in the completion of this business transaction. Send responds to this email: galvan.joh...@outlook.com Regards, John Galvan --- Sir / Madam, Ich habe Zugang zu sehr wichtigen Informationen, mit denen ich eine große Menge Geld bewegen kann. Ich habe meine Hausaufgaben sehr gut gemacht und ich habe die Maschinen, um sie zu erledigen, da ich immer noch im aktiven Dienst bin. Wenn es mir möglich gewesen wäre, es alleine zu tun, hätte ich mich nicht darum gekümmert, Sie zu kontaktieren. Letztendlich brauche ich einen ehrlichen Ausländer, der eine wichtige Rolle beim Abschluss dieses Geschäftsvorgangs spielt. Senden Sie Antworten auf diese E-Mail: galvan.joh...@outlook.com Grüße, John Galvan
Re: Using fixed LPI number for some Device ID
Hi Marc, Sorry to disturb you, Currently the LPI number is not fixed for the device. The LPI number is dynamically allocated start from 8092. For two OS which shares the ITS, One OS needs to configure the device interrupt required by another OS, and the other OS uses a fixed interrupt ID to respond the interrupt. Therefore, the LPI IRQ number of the device needed be fixed. I want to upstream this feature that allocate fixed LPI number for the device that is specified through the DTS. What is your meaning? Thanks
[PATCH] scsi: qla2xxx: add missing iounmap() on error in qlafx00_iospace_config
Add the missing iounmap() before return from qlafx00_iospace_config in the error handling case when os failed to do ioremap for ha->iobase. Signed-off-by: Qinglang Miao --- drivers/scsi/qla2xxx/qla_mr.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c index ca7306685..c6fcd47de 100644 --- a/drivers/scsi/qla2xxx/qla_mr.c +++ b/drivers/scsi/qla2xxx/qla_mr.c @@ -798,13 +798,13 @@ qlafx00_iospace_config(struct qla_hw_data *ha) ql_log_pci(ql_log_warn, ha->pdev, 0x0129, "region #2 not an MMIO resource (%s), aborting\n", pci_name(ha->pdev)); - goto iospace_error_exit; + goto ha_iobase_exit; } if (pci_resource_len(ha->pdev, 2) < BAR2_LEN_FX00) { ql_log_pci(ql_log_warn, ha->pdev, 0x012a, "Invalid PCI mem BAR2 region size (%s), aborting\n", pci_name(ha->pdev)); - goto iospace_error_exit; + goto ha_iobase_exit; } ha->iobase = @@ -812,7 +812,7 @@ qlafx00_iospace_config(struct qla_hw_data *ha) if (!ha->iobase) { ql_log_pci(ql_log_fatal, ha->pdev, 0x012b, "cannot remap MMIO (%s), aborting\n", pci_name(ha->pdev)); - goto iospace_error_exit; + goto ha_iobase_exit; } /* Determine queue resources */ @@ -824,6 +824,8 @@ qlafx00_iospace_config(struct qla_hw_data *ha) return 0; +ha_iobase_exit: + iounmap(ha->cregbase); iospace_error_exit: return -ENOMEM; } -- 2.23.0
Re: [PATCH -next v2] usb: Mark sync_all_pins() with static keyword
On 10/30/2020 6:48 PM, Zou Wei wrote: > Fix the following sparse warning: > > ./brcmstb-usb-pinmap.c:219:6: warning: symbol 'sync_all_pins' was not > declared. Should it be static? > > The sync_all_pins has only call site within brcmstb-usb-pinmap.c > Mark it static as suggested. > > Fixes: 517c4c44b323 ("usb: Add driver to allow any GPIO to be used for 7211 > USB signals") Not sure if the Fixes: tag is entirely appropriate here, but sure, why not. > Reported-by: Hulk Robot > Signed-off-by: Zou Wei Acked-by: Florian Fainelli -- Florian
Re: [PATCH] soc: mediatek: cmdq: fixup possible timeout issue
Thanks for the patch. On Thu, Oct 22, 2020 at 5:44 PM Houlong Wei wrote: > > Fixes: 576f1b4bc802 ("soc: mediatek: Add Mediatek CMDQ helper") nit: This belongs right next to the Sob line, but I guess/hope Matthias can help you fix that ,-) > > There may be possible timeout issue when lots of cmdq packets are > flushed to the same cmdq client. The necessary modifications are as > below. > 1.Adjust the timer timeout period as client->timeout_ms * client->pkt_cnt. > 2.Optimize the time to start the timer. > Reviewed-by: Nicolas Boichat > Signed-off-by: Houlong Wei > --- > drivers/soc/mediatek/mtk-cmdq-helper.c | 22 ++ > 1 file changed, 18 insertions(+), 4 deletions(-) > > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c > b/drivers/soc/mediatek/mtk-cmdq-helper.c > index dc644cfb6419..31142c193527 100644 > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c > @@ -350,7 +350,8 @@ static void cmdq_pkt_flush_async_cb(struct cmdq_cb_data > data) > del_timer(>timer); > else > mod_timer(>timer, jiffies + > - msecs_to_jiffies(client->timeout_ms)); > + msecs_to_jiffies(client->timeout_ms * > + client->pkt_cnt)); > spin_unlock_irqrestore(>lock, flags); > } > > @@ -379,9 +380,7 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt, > cmdq_async_flush_cb cb, > > if (client->timeout_ms != CMDQ_NO_TIMEOUT) { > spin_lock_irqsave(>lock, flags); > - if (client->pkt_cnt++ == 0) > - mod_timer(>timer, jiffies + > - msecs_to_jiffies(client->timeout_ms)); > + client->pkt_cnt++; > spin_unlock_irqrestore(>lock, flags); > } > > @@ -391,6 +390,21 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt, > cmdq_async_flush_cb cb, > /* We can send next packet immediately, so just call txdone. */ > mbox_client_txdone(client->chan, 0); > > + if (client->timeout_ms != CMDQ_NO_TIMEOUT) { > + spin_lock_irqsave(>lock, flags); > + /* > +* GCE HW maybe execute too quickly and the callback function > +* may be invoked earlier. If this happens, pkt_cnt is reduced > +* by 1 in cmdq_pkt_flush_async_cb(). The timer is set only if > +* pkt_cnt is greater than 0. > +*/ > + if (client->pkt_cnt > 0) > + mod_timer(>timer, jiffies + > + msecs_to_jiffies(client->timeout_ms * > + client->pkt_cnt)); > + spin_unlock_irqrestore(>lock, flags); > + } > + > return 0; > } > EXPORT_SYMBOL(cmdq_pkt_flush_async); > -- > 2.18.0
Re: [Y2038][time namespaces] Question regarding CLOCK_REALTIME support plans in Linux time namespaces
Carlos, On Fri, Oct 30 2020 at 18:19, Carlos O'Donell wrote: > On 10/30/20 4:06 PM, Thomas Gleixner wrote: >> On Fri, Oct 30 2020 at 12:58, Carlos O'Donell wrote: >>> I expect that more requests for further time isolation will happen >>> given the utility of this in containers. >> >> There was a lengthy discussion about this and the only "usecase" which >> was brought up was having different NTP servers in name spaces, i.e. the >> leap second ones and the smearing ones. > > In the non-"request for ponies" category: > > * Running legacy 32-bit applications in containers with CLOCK_REALTIME set > to some value below y2038. That's broken to begin with. That has been tried with Y2K and failed miserably. Any real application which needs access to CLOCK_REALTIME requires to have access to something which is at least close to the real time. > * Testing kernel and userspace clock handling code without needing to > run on bare-metal, VM, or other. I grant you that, but it comes with a large can of worms as it opens the door for 'request for ponies' all over the place. >> Now imagine 1000 containers each running their own NTP. Guess what the >> host does in each timer interrupt? Chasing 1000 containers and update >> their notion of CLOCK_REALTIME. In the remaining 5% CPU time the 1000 >> containers can do their computations. > > How is this different than balancing any other resource that you give > to a container/vm on a host? > > Can you enable 1000 containers running smbd/nmbd and expect to get > great IO performance? That's bogus. The kernel can control whether these daemons run or not and how much CPU time they get, as it can control whether any container application runs or not. But when it comes down to time correctness that's a different story. At the moment it allows to have a gazillion of notions of CLOCK_REALTIME then it has to guarantee the correctness for all of them no matter what. >> But even if you restrict it to a trivial offset without NTP >> capabilities, what's the semantics of that offset when the host time is >> set? > > Now you're talking about an implementation. This thread is simply > "Would we implement CLOCK_REALTIME?" Is the answer "Maybe, if we solve > all these other problems?" Maybe, if you solved all these problems which is going to be finished at the theoretical level in about 20 years from now. As I'm planning to be retired and Y2038 has passed by then, feel free to pursue that route. >>> If we have to use qemu today then that's where we're at, but again >>> I expect our use case is representative of more than just glibc. >> >> For testing purposes it might be. For real world use cases not so >> much. People tend to rely on the coordinated nature of CLOCK_TAI and >> CLOCK_REALTIME. > > Except we have two real world use cases, at the top of this email, > that could extend to a lot of software. We know legacy 32-bit > applications exist that will break with CLOCK_REALTIME past > y2038. Software exists that manipulates time and needs testing > with specific time values e.g. month crossings, day crossings, > leap year crossings, etc. Again. I agree with the testing part, but the legacy application part is wishful thinking at least. IMO it's utter nonsense. Coming back to your test coverage argument. I really don't see a problem with the requirement of having qemu installed in order to run 'make check'. If you can't ask that from your contributors, then asking me to provide you a namespace magic is just hillarious. The contributor who refuses to install qemu will also insist to run on some last century kernel which does not even know about name spaces at all. Instead of asking for ponies your time might be better spent with providing tools which just make it easy to run 'make check' with all bells and whistels. Virtualization is the right answer to the testing problem and if people really insist on running their broken legacy apps past 2038, then stick them into a VM and charge boatloads of money for that service. >>> Does checkpointing work robustly when userspace APIS use >>> CLOCK_REALTIME (directly or indirectly) in the container? >> >> AFAICT, yes. That was the conclusion over the lenghty discussion about >> time name spaces and their requirements. > > If this is the case then have we established behaviours that > happen when such processes are migrated to other systems with > different CLOCK_REALTIME clocks? Would these behaviours serve > as the basis of how CLOCK_REALTIME in a namespace would behave? > > That is to say that migrating a container to a system with a > different CLOCK_REALTIME should behave similarly to what happens > when CLOCK_REALTIME is changed locally and you have a container > with a unique CLOCK_REALTIME? Any application has to be able to deal with CLOCK_REALTIME changing under their feet no matter what. So why would migrating a container from host A to host B which have a different notion of CLOCK_REALTIME make any difference?
[PATCH -next v2] usb: Mark sync_all_pins() with static keyword
Fix the following sparse warning: ./brcmstb-usb-pinmap.c:219:6: warning: symbol 'sync_all_pins' was not declared. Should it be static? The sync_all_pins has only call site within brcmstb-usb-pinmap.c Mark it static as suggested. Fixes: 517c4c44b323 ("usb: Add driver to allow any GPIO to be used for 7211 USB signals") Reported-by: Hulk Robot Signed-off-by: Zou Wei --- v2: - Make title more precise. - Shorten the warning path. - Add period. drivers/usb/misc/brcmstb-usb-pinmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/misc/brcmstb-usb-pinmap.c b/drivers/usb/misc/brcmstb-usb-pinmap.c index 02144c3..cc9618d 100644 --- a/drivers/usb/misc/brcmstb-usb-pinmap.c +++ b/drivers/usb/misc/brcmstb-usb-pinmap.c @@ -216,7 +216,7 @@ static int parse_pins(struct device *dev, struct device_node *dn, return 0; } -void sync_all_pins(struct brcmstb_usb_pinmap_data *pdata) +static void sync_all_pins(struct brcmstb_usb_pinmap_data *pdata) { struct out_pin *pout; struct in_pin *pin; -- 2.6.2
[rcu:rcu/next] BUILD SUCCESS 76b43ef30dc30f8d5f9ac91ccd562159995f5879
defconfig nios2allyesconfig cskydefconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390 allyesconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig sparc defconfig i386defconfig mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig x86_64 randconfig-a005-20201030 x86_64 randconfig-a001-20201030 x86_64 randconfig-a002-20201030 x86_64 randconfig-a003-20201030 x86_64 randconfig-a006-20201030 x86_64 randconfig-a004-20201030 i386 randconfig-a005-20201030 i386 randconfig-a003-20201030 i386 randconfig-a002-20201030 i386 randconfig-a001-20201030 i386 randconfig-a006-20201030 i386 randconfig-a004-20201030 i386 randconfig-a011-20201030 i386 randconfig-a014-20201030 i386 randconfig-a015-20201030 i386 randconfig-a012-20201030 i386 randconfig-a013-20201030 i386 randconfig-a016-20201030 riscvnommu_k210_defconfig riscvallyesconfig riscvnommu_virt_defconfig riscv allnoconfig riscv defconfig riscv rv32_defconfig riscvallmodconfig x86_64 rhel x86_64 allyesconfig x86_64rhel-7.6-kselftests x86_64 defconfig x86_64 rhel-8.3 x86_64 kexec clang tested configs: x86_64 randconfig-a013-20201030 x86_64 randconfig-a014-20201030 x86_64 randconfig-a015-20201030 x86_64 randconfig-a016-20201030 x86_64 randconfig-a011-20201030 x86_64 randconfig-a012-20201030 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
November Equity Investment 20-20
How are you doing today I have a proposal which i think may interest you and benefit you.I will like to give you full details of this via email: gerradfinancialplann...@gmail.com Thanks. John PHIL
Re: [PATCH net-next 0/5] L2 multicast forwarding for Ocelot switch
On Thu, 29 Oct 2020 04:27:33 +0200 Vladimir Oltean wrote: > This series enables the mscc_ocelot switch to forward raw L2 (non-IP) > mdb entries as configured by the bridge driver after this patch: > > https://patchwork.ozlabs.org/project/netdev/patch/20201028233831.610076-1-vladimir.olt...@nxp.com/ Applied, thanks!
Re: [PATCH v11 4/4] bus: mhi: Add userspace client interface driver
Hi Mani, On 10/30/20 3:34 AM, Manivannan Sadhasivam wrote: Hi Hemant, On Thu, Oct 29, 2020 at 07:45:46PM -0700, Hemant Kumar wrote: This MHI client driver allows userspace clients to transfer raw data between MHI device and host using standard file operations. Driver instantiates UCI device object which is associated to device file node. UCI device object instantiates UCI channel object when device file node is opened. UCI channel object is used to manage MHI channels by calling MHI core APIs for read and write operations. MHI channels are started as part of device open(). MHI channels remain in start state until last release() is called on UCI device file node. Device file node is created with format /dev/mhi__ Currently it supports LOOPBACK channel. Signed-off-by: Hemant Kumar Thanks for continuously updating the series based on reviews, now the locking part looks a _lot_ cleaner than it used to be. I just have one query (inline) regarding the usage of refcount for uci_chan and uci_dev. Once you fix that, I think this is good to go in. Thanks for reviewing my changes. [..] +#define DEVICE_NAME "mhi" +#define MHI_UCI_DRIVER_NAME "mhi_uci" +#define MAX_UCI_MINORS 128 Prefix MHI for these. Done. + +static DEFINE_IDR(uci_idr); +static DEFINE_MUTEX(uci_drv_mutex); +static struct class *uci_dev_class; +static int uci_dev_major; + +/** + * struct uci_chan - MHI channel for a UCI device + * @udev: associated UCI device object + * @ul_wq: wait queue for writer + * @write_lock: mutex write lock for ul channel + * @dl_wq: wait queue for reader + * @read_lock: mutex read lock for dl channel + * @dl_pending_lock: spin lock for dl_pending list + * @dl_pending: list of dl buffers userspace is waiting to read + * @cur_buf: current buffer userspace is reading + * @dl_size: size of the current dl buffer userspace is reading + * @ref_count: uci_chan reference count + */ +struct uci_chan { + struct uci_dev *udev; + wait_queue_head_t ul_wq; + + /* ul channel lock to synchronize multiple writes */ I asked you to move these comments to Kdoc in previous iteration. There are multiple revisions of UCI pushed after i responded on this one. On V7 i responded to your comment :) "This was added because checkpatch --strict required to add a comment when lock is added to struct, after adding inline comment, checkpatch error was gone." i was sticking to --strict option. Considering it is best to address what --strict is complaining for. + struct mutex write_lock; + + wait_queue_head_t dl_wq; + + /* dl channel lock to synchronize multiple reads */ + struct mutex read_lock; + + /* +* protects pending list in bh context, channel release, read and +* poll +*/ + spinlock_t dl_pending_lock; + + struct list_head dl_pending; + struct uci_buf *cur_buf; + size_t dl_size; + struct kref ref_count; I'm now thinking that instead of having two reference counts for uci_chan and uci_dev, why can't you club them together and just use uci_dev's refcount to handle the channel management also. For instance in uci_open, you are incrementing the refcount for uci_dev before starting the channel and then doing the same for uci_chan in mhi_uci_dev_start_chan(). So why can't you just use a single refcount once the mhi_uci_dev_start_chan() succeeds? The UCI device is useless without a channel, isn't it? Main idea is to have the uci driver probed (uci device object is instantiated) but it is possible that device node is not opened or if it was opened before and release() was called after that. So UCI channel is not active but device node would continue to exist. Which can be opened again and channel would move to start state. So we dont want to couple mhi driver probe with starting of channels. We start channels only when it is really needed. This would allow MHI device to go to lower power state when channels are disabled. [..] + +static int mhi_queue_inbound(struct uci_dev *udev) +{ + struct mhi_device *mhi_dev = udev->mhi_dev; + struct device *dev = _dev->dev; + int nr_trbs, i, ret = -EIO; s/nr_trbs/nr_desc Done. + size_t dl_buf_size; + void *buf; + struct uci_buf *ubuf; + + /* dont queue if dl channel is not supported */ + if (!udev->mhi_dev->dl_chan) + return 0; Not returning an error? Here we dont need to return error because when open is called it would call this function and if dl_chan is not supported we still want to return success for a uci device which only supports UL channel. Keeping this check inside function looks clean so i am not adding this check in open(). [..] +static __poll_t mhi_uci_poll(struct file *file, poll_table *wait) +{ + struct uci_dev *udev = file->private_data; + struct mhi_device *mhi_dev = udev->mhi_dev; + struct device *dev = _dev->dev; + struct uci_chan *uchan = udev->uchan; +
Re: [PATCH v11 09/10] fs/ntfs3: Add NTFS3 in fs/Kconfig and fs/Makefile
Hi Konstantin, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on v5.10-rc1 next-20201030] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Konstantin-Komarov/NTFS-read-write-driver-GPL-implementation-by-Paragon-Software/20201030-230756 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 07e0887302450a62f51dba72df6afb5fabb23d1c config: h8300-randconfig-s031-20201030 (attached as .config) compiler: h8300-linux-gcc (GCC) 9.3.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # apt-get install sparse # sparse version: v0.6.3-68-g49c98aa3-dirty # https://github.com/0day-ci/linux/commit/7c34316b6c7f9af2046f8343d3b010c37340ef1d git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Konstantin-Komarov/NTFS-read-write-driver-GPL-implementation-by-Paragon-Software/20201030-230756 git checkout 7c34316b6c7f9af2046f8343d3b010c37340ef1d # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=h8300 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot "sparse warnings: (new ones prefixed by >>)" >> fs/ntfs3/frecord.c:569:33: sparse: sparse: cast to restricted __le32 >> fs/ntfs3/frecord.c:569:33: sparse: sparse: cast from restricted __le64 >> fs/ntfs3/frecord.c:569:33: sparse: sparse: cast to restricted __le32 >> fs/ntfs3/frecord.c:569:33: sparse: sparse: cast from restricted __le64 >> fs/ntfs3/frecord.c:569:33: sparse: sparse: cast to restricted __le32 >> fs/ntfs3/frecord.c:569:33: sparse: sparse: cast from restricted __le64 >> fs/ntfs3/frecord.c:569:33: sparse: sparse: cast to restricted __le32 >> fs/ntfs3/frecord.c:569:33: sparse: sparse: cast from restricted __le64 >> fs/ntfs3/frecord.c:569:33: sparse: sparse: cast to restricted __le32 >> fs/ntfs3/frecord.c:569:33: sparse: sparse: cast from restricted __le64 >> fs/ntfs3/frecord.c:569:33: sparse: sparse: cast to restricted __le32 >> fs/ntfs3/frecord.c:569:33: sparse: sparse: cast from restricted __le64 vim +569 fs/ntfs3/frecord.c cbd4257e6d85149 Konstantin Komarov 2020-10-30 532 cbd4257e6d85149 Konstantin Komarov 2020-10-30 533 /* cbd4257e6d85149 Konstantin Komarov 2020-10-30 534 * random write access to sparsed or compressed file may result to cbd4257e6d85149 Konstantin Komarov 2020-10-30 535 * not optimized packed runs. cbd4257e6d85149 Konstantin Komarov 2020-10-30 536 * Here it is the place to optimize it cbd4257e6d85149 Konstantin Komarov 2020-10-30 537 */ cbd4257e6d85149 Konstantin Komarov 2020-10-30 538 static int ni_repack(struct ntfs_inode *ni) cbd4257e6d85149 Konstantin Komarov 2020-10-30 539 { cbd4257e6d85149 Konstantin Komarov 2020-10-30 540 int err = 0; cbd4257e6d85149 Konstantin Komarov 2020-10-30 541 struct ntfs_sb_info *sbi = ni->mi.sbi; cbd4257e6d85149 Konstantin Komarov 2020-10-30 542 struct mft_inode *mi, *mi_p = NULL; cbd4257e6d85149 Konstantin Komarov 2020-10-30 543 struct ATTRIB *attr = NULL, *attr_p; cbd4257e6d85149 Konstantin Komarov 2020-10-30 544 struct ATTR_LIST_ENTRY *le = NULL, *le_p; cbd4257e6d85149 Konstantin Komarov 2020-10-30 545 CLST alloc = 0; cbd4257e6d85149 Konstantin Komarov 2020-10-30 546 u8 cluster_bits = sbi->cluster_bits; cbd4257e6d85149 Konstantin Komarov 2020-10-30 547 CLST svcn, evcn = 0, svcn_p, evcn_p, next_svcn; cbd4257e6d85149 Konstantin Komarov 2020-10-30 548 u32 roff, rs = sbi->record_size; cbd4257e6d85149 Konstantin Komarov 2020-10-30 549 struct runs_tree run; cbd4257e6d85149 Konstantin Komarov 2020-10-30 550 cbd4257e6d85149 Konstantin Komarov 2020-10-30 551 run_init(); cbd4257e6d85149 Konstantin Komarov 2020-10-30 552 cbd4257e6d85149 Konstantin Komarov 2020-10-30 553 while ((attr = ni_enum_attr_ex(ni, attr, ))) { cbd4257e6d85149 Konstantin Komarov 2020-10-30 554 if (!attr->non_res) cbd4257e6d85149 Konstantin Komarov 2020-10-30 555 continue; cbd4257e6d85149 Konstantin Komarov 2020-10-30 556 cbd4257e6d85149 Konstantin Komarov 2020-10-30 557 if (ni_load_mi(ni, le, )) { cbd4257e6d85149 Konstantin Komarov 2020-10-30 558 err = -EINVAL; cbd4257e6d85149 Konstantin Komarov 2020-10-30 559 break; cbd4257e6d85149 Konstantin Komarov 2020-10-30 560 } cbd
[PATCH -next v3] platform/x86/dell-wmi-sysman: Make some symbols static
Fix the following sparse warnings: ./passobj-attributes.c:38:23: warning: symbol 'po_is_pass_set' was not declared. Should it be static? ./passobj-attributes.c:70:23: warning: symbol 'po_current_password' was not declared. Should it be static? ./passobj-attributes.c:99:23: warning: symbol 'po_new_password' was not declared. Should it be static? ./passobj-attributes.c:103:23: warning: symbol 'po_min_pass_length' was not declared. Should it be static? ./passobj-attributes.c:107:23: warning: symbol 'po_max_pass_length' was not declared. Should it be static? ./passobj-attributes.c:116:23: warning: symbol 'po_mechanism' was not declared. Should it be static? ./passobj-attributes.c:129:23: warning: symbol 'po_role' was not declared. Should it be static? Reported-by: Hulk Robot Signed-off-by: Zou Wei --- v2: - put all of them in a way that each occupies only a single line v3: - shorten the warning paths - put the static codes a single line .../x86/dell-wmi-sysman/passobj-attributes.c| 21 +++-- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/drivers/platform/x86/dell-wmi-sysman/passobj-attributes.c b/drivers/platform/x86/dell-wmi-sysman/passobj-attributes.c index e6199fb..3abcd95 100644 --- a/drivers/platform/x86/dell-wmi-sysman/passobj-attributes.c +++ b/drivers/platform/x86/dell-wmi-sysman/passobj-attributes.c @@ -35,8 +35,7 @@ static ssize_t is_enabled_show(struct kobject *kobj, struct kobj_attribute *attr return ret; } -struct kobj_attribute po_is_pass_set = - __ATTR_RO(is_enabled); +static struct kobj_attribute po_is_pass_set = __ATTR_RO(is_enabled); static ssize_t current_password_store(struct kobject *kobj, struct kobj_attribute *attr, @@ -67,8 +66,7 @@ static ssize_t current_password_store(struct kobject *kobj, return count; } -struct kobj_attribute po_current_password = - __ATTR_WO(current_password); +static struct kobj_attribute po_current_password = __ATTR_WO(current_password); static ssize_t new_password_store(struct kobject *kobj, struct kobj_attribute *attr, @@ -96,16 +94,13 @@ static ssize_t new_password_store(struct kobject *kobj, return ret ? ret : count; } -struct kobj_attribute po_new_password = - __ATTR_WO(new_password); +static struct kobj_attribute po_new_password = __ATTR_WO(new_password); attribute_n_property_show(min_password_length, po); -struct kobj_attribute po_min_pass_length = - __ATTR_RO(min_password_length); +static struct kobj_attribute po_min_pass_length = __ATTR_RO(min_password_length); attribute_n_property_show(max_password_length, po); -struct kobj_attribute po_max_pass_length = - __ATTR_RO(max_password_length); +static struct kobj_attribute po_max_pass_length = __ATTR_RO(max_password_length); static ssize_t mechanism_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf) @@ -113,8 +108,7 @@ static ssize_t mechanism_show(struct kobject *kobj, struct kobj_attribute *attr, return sprintf(buf, "password\n"); } -struct kobj_attribute po_mechanism = - __ATTR_RO(mechanism); +static struct kobj_attribute po_mechanism = __ATTR_RO(mechanism); static ssize_t role_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf) @@ -126,8 +120,7 @@ static ssize_t role_show(struct kobject *kobj, struct kobj_attribute *attr, return -EIO; } -struct kobj_attribute po_role = - __ATTR_RO(role); +static struct kobj_attribute po_role = __ATTR_RO(role); static struct attribute *po_attrs[] = { _is_pass_set.attr, -- 2.6.2
ERROR: modpost: "__irq_set_affinity" undefined!
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 5fc6b075e165f641fbc366b58b578055762d5f8c commit: 8d58f222e85f01da0c0e1fc1e77986c86de889e2 ubsan: disable UBSAN_ALIGNMENT under COMPILE_TEST date: 6 months ago config: mips-randconfig-r024-20201030 (attached as .config) compiler: mips64-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8d58f222e85f01da0c0e1fc1e77986c86de889e2 git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git git fetch --no-tags linus master git checkout 8d58f222e85f01da0c0e1fc1e77986c86de889e2 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>, old ones prefixed by <<): >> ERROR: modpost: "__irq_set_affinity" [drivers/watchdog/octeon-wdt.ko] >> undefined! ERROR: modpost: "spurious_interrupt" [drivers/mfd/ioc3.ko] undefined! ERROR: modpost: "pci_find_host_bridge" [drivers/mfd/ioc3.ko] undefined! --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH net-next 4/5] net: mscc: ocelot: make entry_type a member of struct ocelot_multicast
On Thu, 29 Oct 2020 04:27:37 +0200 Vladimir Oltean wrote: > + mc = devm_kzalloc(ocelot->dev, sizeof(*mc), GFP_KERNEL); > + if (!mc) > + return -ENOMEM; > + > + mc->entry_type = ocelot_classify_mdb(mdb->addr); > + ether_addr_copy(mc->addr, mdb->addr); > + mc->vid = vid; > + > + pgid = ocelot_mdb_get_pgid(ocelot, mc); > > if (pgid < 0) { > dev_err(ocelot->dev, > @@ -1038,24 +1044,19 @@ int ocelot_port_mdb_add(struct ocelot *ocelot, int > port, > return -ENOSPC; > } Transitionally leaking mc here on pgid < 0
Re: [PATCH v20 04/20] mm/thp: use head for head page in lru_add_page_tail
在 2020/10/30 下午9:52, Johannes Weiner 写道: > >> From a9ee63a213f40eb4d5a69b52fbb348ff9cd7cf6c Mon Sep 17 00:00:00 2001 >> From: Alex Shi >> Date: Tue, 26 May 2020 16:49:22 +0800 >> Subject: [PATCH v21 04/20] mm/thp: use head for head page in >> lru_add_page_tail >> >> Since the first parameter is only used by head page, it's better to make >> it explicit. >> >> Signed-off-by: Alex Shi >> Reviewed-by: Kirill A. Shutemov >> Acked-by: Hugh Dickins >> Cc: Andrew Morton >> Cc: Johannes Weiner >> Cc: Matthew Wilcox >> Cc: Hugh Dickins >> Cc: linux...@kvack.org >> Cc: linux-kernel@vger.kernel.org > Acked-by: Johannes Weiner Thanks a lot! Alex
Re: [PATCH v20 02/20] mm/memcg: bail early from swap accounting if memcg disabled
在 2020/10/30 下午10:04, Johannes Weiner 写道: >>> Acked-by: Johannes Weiner >>> >>> This should go in before the previous patch that adds the WARN for it. >> Right, but than the long ops may not weird. Should I remove the ops and >> resend the whole patchset? > You mean the warning in the changelog? I think that's alright. You can > just say that you're about to remove the !page->memcg check in the > next patch because the original reasons for having it are gone, and > memcg being disabled is the only remaining exception, so this patch > makes that check explicit in preparation for the next. > > Sorry, it's all a bit of a hassle, I just wouldn't want to introduce a > known warning into the kernel between those two patches (could confuse > bisection runs, complicates partial reverts etc.) H Johannes, I see, I will exchange the 1st and 2nd patch place with above comments in commit log. I guess you could give more comments on other patches, so I am going to wait you for more comments and send v21 as a whole. :) Many thanks! Alex
[PATCH] drm: panel: simple: add missing platform_driver_unregister() in panel_simple_init
Add the missing platform_driver_unregister() before return from panel_simple_init in the error handling case when failed to register panel_simple_dsi_driver with CONFIG_DRM_MIPI_DSI enabled. Signed-off-by: Qinglang Miao --- drivers/gpu/drm/panel/panel-simple.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 2be358fb4..2966ac13c 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -4644,8 +4644,10 @@ static int __init panel_simple_init(void) if (IS_ENABLED(CONFIG_DRM_MIPI_DSI)) { err = mipi_dsi_driver_register(_simple_dsi_driver); - if (err < 0) + if (err < 0) { + platform_driver_unregister(_simple_platform_driver); return err; + } } return 0; -- 2.23.0
[PATCH] cpufreq: mediatek: add missing platform_driver_unregister() on error in mtk_cpufreq_driver_init
Add the missing platform_driver_unregister() before return from mtk_cpufreq_driver_init in the error handling case when failed to register mtk-cpufreq platform device Signed-off-by: Qinglang Miao --- drivers/cpufreq/mediatek-cpufreq.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c index 7d1212c9b..c03c76a0c 100644 --- a/drivers/cpufreq/mediatek-cpufreq.c +++ b/drivers/cpufreq/mediatek-cpufreq.c @@ -572,6 +572,7 @@ static int __init mtk_cpufreq_driver_init(void) pdev = platform_device_register_simple("mtk-cpufreq", -1, NULL, 0); if (IS_ERR(pdev)) { pr_err("failed to register mtk-cpufreq platform device\n"); + platform_driver_unregister(_cpufreq_platdrv); return PTR_ERR(pdev); } -- 2.23.0
[PATCH] serial: txx9: add missing platform_driver_unregister() on error in serial_txx9_init
Add the missing platform_driver_unregister() before return from serial_txx9_init in the error handling case when failed to register serial_txx9_pci_driver with macro ENABLE_SERIAL_TXX9_PCI defined. Signed-off-by: Qinglang Miao --- drivers/tty/serial/serial_txx9.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/serial_txx9.c b/drivers/tty/serial/serial_txx9.c index b4d89e317..3d883cd55 100644 --- a/drivers/tty/serial/serial_txx9.c +++ b/drivers/tty/serial/serial_txx9.c @@ -1280,10 +1280,13 @@ static int __init serial_txx9_init(void) #ifdef ENABLE_SERIAL_TXX9_PCI ret = pci_register_driver(_txx9_pci_driver); + if (ret) + goto unreg_platform_drv; #endif - if (ret == 0) - goto out; + return 0; + unreg_platform_drv: + platform_driver_unregister(_txx9_plat_driver); del_dev: platform_device_del(serial_txx9_plat_devs); put_dev: -- 2.23.0
Re: [PATCH net-next] net: bridge: explicitly convert between mdb entry state and port group flags
On Thu, 29 Oct 2020 01:48:15 +0200 Vladimir Oltean wrote: > When creating a new multicast port group, there is implicit conversion > between the __u8 state member of struct br_mdb_entry and the unsigned > char flags member of struct net_bridge_port_group. This implicit > conversion relies on the fact that MDB_PERMANENT is equal to > MDB_PG_FLAGS_PERMANENT. > > Let's be more explicit and convert the state to flags manually. > > Signed-off-by: Vladimir Oltean Applied, thanks!
Re: [PATCH v4 net-next] net: bridge: mcast: add support for raw L2 multicast groups
On Thu, 29 Oct 2020 01:38:31 +0200 Vladimir Oltean wrote: > From: Nikolay Aleksandrov > > Extend the bridge multicast control and data path to configure routes > for L2 (non-IP) multicast groups. > > The uapi struct br_mdb_entry union u is extended with another variant, > mac_addr, which does not change the structure size, and which is valid > when the proto field is zero. > > To be compatible with the forwarding code that is already in place, > which acts as an IGMP/MLD snooping bridge with querier capabilities, we > need to declare that for L2 MDB entries (for which there exists no such > thing as IGMP/MLD snooping/querying), that there is always a querier. > Otherwise, these entries would be flooded to all bridge ports and not > just to those that are members of the L2 multicast group. > > Needless to say, only permanent L2 multicast groups can be installed on > a bridge port. > > Signed-off-by: Nikolay Aleksandrov > Signed-off-by: Vladimir Oltean Applied, thanks!
[PATCH net-next v6 5/5] net: hdlc_fr: Add support for any Ethertype
Change the fr_rx function to make this driver support any Ethertype when receiving skbs on normal (non-Ethernet-emulating) PVC devices. (This driver is already able to handle any Ethertype when sending.) Originally in the fr_rx function, the code that parses the long (10-byte) header only recognizes a few Ethertype values and drops frames with other Ethertype values. This patch replaces this code to make fr_rx support any Ethertype. This patch also creates a new function fr_snap_parse as part of the new code. Cc: Willem de Bruijn Cc: Krzysztof Halasa Acked-by: Willem de Bruijn Signed-off-by: Xie He --- drivers/net/wan/hdlc_fr.c | 75 +-- 1 file changed, 49 insertions(+), 26 deletions(-) diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c index 98444f1d8cc3..0720f5f92caa 100644 --- a/drivers/net/wan/hdlc_fr.c +++ b/drivers/net/wan/hdlc_fr.c @@ -871,6 +871,45 @@ static int fr_lmi_recv(struct net_device *dev, struct sk_buff *skb) return 0; } +static int fr_snap_parse(struct sk_buff *skb, struct pvc_device *pvc) +{ + /* OUI 00-00-00 indicates an Ethertype follows */ + if (skb->data[0] == 0x00 && + skb->data[1] == 0x00 && + skb->data[2] == 0x00) { + if (!pvc->main) + return -1; + skb->dev = pvc->main; + skb->protocol = *(__be16 *)(skb->data + 3); /* Ethertype */ + skb_pull(skb, 5); + skb_reset_mac_header(skb); + return 0; + + /* OUI 00-80-C2 stands for the 802.1 organization */ + } else if (skb->data[0] == 0x00 && + skb->data[1] == 0x80 && + skb->data[2] == 0xC2) { + /* PID 00-07 stands for Ethernet frames without FCS */ + if (skb->data[3] == 0x00 && + skb->data[4] == 0x07) { + if (!pvc->ether) + return -1; + skb_pull(skb, 5); + if (skb->len < ETH_HLEN) + return -1; + skb->protocol = eth_type_trans(skb, pvc->ether); + return 0; + + /* PID unsupported */ + } else { + return -1; + } + + /* OUI unsupported */ + } else { + return -1; + } +} static int fr_rx(struct sk_buff *skb) { @@ -945,35 +984,19 @@ static int fr_rx(struct sk_buff *skb) skb->protocol = htons(ETH_P_IPV6); skb_reset_mac_header(skb); - } else if (skb->len > 10 && data[3] == FR_PAD && - data[4] == NLPID_SNAP && data[5] == FR_PAD) { - u16 oui = ntohs(*(__be16*)(data + 6)); - u16 pid = ntohs(*(__be16*)(data + 8)); - skb_pull(skb, 10); - - switch u32)oui) << 16) | pid) { - case ETH_P_ARP: /* routed frame with SNAP */ - case ETH_P_IPX: - case ETH_P_IP: /* a long variant */ - case ETH_P_IPV6: - if (!pvc->main) - goto rx_drop; - skb->dev = pvc->main; - skb->protocol = htons(pid); - skb_reset_mac_header(skb); - break; - - case 0x80C20007: /* bridged Ethernet frame */ - if (!pvc->ether) + } else if (data[3] == FR_PAD) { + if (skb->len < 5) + goto rx_error; + if (data[4] == NLPID_SNAP) { /* A SNAP header follows */ + skb_pull(skb, 5); + if (skb->len < 5) /* Incomplete SNAP header */ + goto rx_error; + if (fr_snap_parse(skb, pvc)) goto rx_drop; - skb->protocol = eth_type_trans(skb, pvc->ether); - break; - - default: - netdev_info(frad, "Unsupported protocol, OUI=%x PID=%x\n", - oui, pid); + } else { goto rx_drop; } + } else { netdev_info(frad, "Unsupported protocol, NLPID=%x length=%i\n", data[3], skb->len); -- 2.27.0
[PATCH net-next v6 0/5] net: hdlc_fr: Improve fr_rx and add support for any Ethertype
The main purpose of this series is the last patch. The previous 4 patches are just code clean-ups so that the last patch will not make the code too messy. The patches must be applied in sequence. The receiving code of this driver doesn't support arbitrary Ethertype values. It only recognizes a few known Ethertypes when receiving and drops skbs with other Ethertypes. However, the standard document RFC 2427 allows Frame Relay to support any Ethertype values. This series adds support for this. Change from v5: Small fix to the commit messages. Change from v4: Drop the change related to stats.rx_dropped from the 1st patch. Switch the 3rd and 4th patch. Improve the commit message of the 4th patch by stating why only a 2-byte address field is accepted. Change from v3: Split the last patch into 2 patches. Improve the commit message of the 1st patch to explicitly state that the stats.rx_dropped count is also increased after "goto rx_error". Change from v2: Small fix to the commit messages. Change from v1: Small fix to the commit messages. Xie He (5): net: hdlc_fr: Simpify fr_rx by using "goto rx_drop" to drop frames net: hdlc_fr: Change the use of "dev" in fr_rx to make the code cleaner net: hdlc_fr: Do skb_reset_mac_header for skbs received on normal PVC devices net: hdlc_fr: Improve the initial checks when we receive an skb net: hdlc_fr: Add support for any Ethertype drivers/net/wan/hdlc_fr.c | 118 +++--- 1 file changed, 72 insertions(+), 46 deletions(-) -- 2.27.0
[PATCH net-next v6 2/5] net: hdlc_fr: Change the use of "dev" in fr_rx to make the code cleaner
The eth_type_trans function is called when we receive frames carrying Ethernet frames. This function expects a non-NULL pointer as an argument, and assigns it directly to skb->dev. However, the code handling other types of frames first assigns the pointer to "dev", and then at the end checks whether the value is NULL, and if it is not NULL, assigns it to skb->dev. The two flows are different. Mixing them in this function makes the code messy. It's better that we convert the second flow to align with how eth_type_trans does things. So this patch changes the code to: first make sure the pointer is not NULL, then assign it directly to skb->dev. "dev" is no longer needed until the end where we use it to update stats. Cc: Willem de Bruijn Cc: Krzysztof Halasa Signed-off-by: Xie He --- drivers/net/wan/hdlc_fr.c | 37 - 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c index 4db0e01b96a9..71ee9b60d91b 100644 --- a/drivers/net/wan/hdlc_fr.c +++ b/drivers/net/wan/hdlc_fr.c @@ -880,7 +880,7 @@ static int fr_rx(struct sk_buff *skb) u8 *data = skb->data; u16 dlci; struct pvc_device *pvc; - struct net_device *dev = NULL; + struct net_device *dev; if (skb->len <= 4 || fh->ea1 || data[2] != FR_UI) goto rx_error; @@ -930,13 +930,17 @@ static int fr_rx(struct sk_buff *skb) } if (data[3] == NLPID_IP) { + if (!pvc->main) + goto rx_drop; skb_pull(skb, 4); /* Remove 4-byte header (hdr, UI, NLPID) */ - dev = pvc->main; + skb->dev = pvc->main; skb->protocol = htons(ETH_P_IP); } else if (data[3] == NLPID_IPV6) { + if (!pvc->main) + goto rx_drop; skb_pull(skb, 4); /* Remove 4-byte header (hdr, UI, NLPID) */ - dev = pvc->main; + skb->dev = pvc->main; skb->protocol = htons(ETH_P_IPV6); } else if (skb->len > 10 && data[3] == FR_PAD && @@ -950,13 +954,16 @@ static int fr_rx(struct sk_buff *skb) case ETH_P_IPX: case ETH_P_IP: /* a long variant */ case ETH_P_IPV6: - dev = pvc->main; + if (!pvc->main) + goto rx_drop; + skb->dev = pvc->main; skb->protocol = htons(pid); break; case 0x80C20007: /* bridged Ethernet frame */ - if ((dev = pvc->ether) != NULL) - skb->protocol = eth_type_trans(skb, dev); + if (!pvc->ether) + goto rx_drop; + skb->protocol = eth_type_trans(skb, pvc->ether); break; default: @@ -970,17 +977,13 @@ static int fr_rx(struct sk_buff *skb) goto rx_drop; } - if (dev) { - dev->stats.rx_packets++; /* PVC traffic */ - dev->stats.rx_bytes += skb->len; - if (pvc->state.becn) - dev->stats.rx_compressed++; - skb->dev = dev; - netif_rx(skb); - return NET_RX_SUCCESS; - } else { - goto rx_drop; - } + dev = skb->dev; + dev->stats.rx_packets++; /* PVC traffic */ + dev->stats.rx_bytes += skb->len; + if (pvc->state.becn) + dev->stats.rx_compressed++; + netif_rx(skb); + return NET_RX_SUCCESS; rx_error: frad->stats.rx_errors++; /* Mark error */ -- 2.27.0
[PATCH net-next v6 3/5] net: hdlc_fr: Do skb_reset_mac_header for skbs received on normal PVC devices
When an skb is received on a normal (non-Ethernet-emulating) PVC device, call skb_reset_mac_header before we pass it to upper layers. This is because normal PVC devices don't have header_ops, so any header we have would not be visible to upper layer code when sending, so the header shouldn't be visible to upper layer code when receiving, either. Cc: Willem de Bruijn Cc: Krzysztof Halasa Acked-by: Willem de Bruijn Signed-off-by: Xie He --- drivers/net/wan/hdlc_fr.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c index 71ee9b60d91b..eb83116aa9df 100644 --- a/drivers/net/wan/hdlc_fr.c +++ b/drivers/net/wan/hdlc_fr.c @@ -935,6 +935,7 @@ static int fr_rx(struct sk_buff *skb) skb_pull(skb, 4); /* Remove 4-byte header (hdr, UI, NLPID) */ skb->dev = pvc->main; skb->protocol = htons(ETH_P_IP); + skb_reset_mac_header(skb); } else if (data[3] == NLPID_IPV6) { if (!pvc->main) @@ -942,6 +943,7 @@ static int fr_rx(struct sk_buff *skb) skb_pull(skb, 4); /* Remove 4-byte header (hdr, UI, NLPID) */ skb->dev = pvc->main; skb->protocol = htons(ETH_P_IPV6); + skb_reset_mac_header(skb); } else if (skb->len > 10 && data[3] == FR_PAD && data[4] == NLPID_SNAP && data[5] == FR_PAD) { @@ -958,6 +960,7 @@ static int fr_rx(struct sk_buff *skb) goto rx_drop; skb->dev = pvc->main; skb->protocol = htons(pid); + skb_reset_mac_header(skb); break; case 0x80C20007: /* bridged Ethernet frame */ -- 2.27.0
[PATCH net-next v6 4/5] net: hdlc_fr: Improve the initial checks when we receive an skb
1. Change the skb->len check from "<= 4" to "< 4". At first we only need to ensure a 4-byte header is present. We indeed normally need the 5th byte, too, but it'd be more logical and cleaner to check its existence when we actually need it. 2. Add an fh->ea2 check to the initial checks in fr_rx. fh->ea2 == 1 means the second address byte is the final address byte. We only support the case where the address length is 2 bytes. If the address length is not 2 bytes, the control field and the protocol field would not be the 3rd and 4th byte as we assume. (Say it is 3 bytes, then the control field and the protocol field would be the 4th and 5th byte instead.) Cc: Willem de Bruijn Cc: Krzysztof Halasa Signed-off-by: Xie He --- drivers/net/wan/hdlc_fr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c index eb83116aa9df..98444f1d8cc3 100644 --- a/drivers/net/wan/hdlc_fr.c +++ b/drivers/net/wan/hdlc_fr.c @@ -882,7 +882,7 @@ static int fr_rx(struct sk_buff *skb) struct pvc_device *pvc; struct net_device *dev; - if (skb->len <= 4 || fh->ea1 || data[2] != FR_UI) + if (skb->len < 4 || fh->ea1 || !fh->ea2 || data[2] != FR_UI) goto rx_error; dlci = q922_to_dlci(skb->data); -- 2.27.0
[PATCH net-next v6 1/5] net: hdlc_fr: Simpify fr_rx by using "goto rx_drop" to drop frames
When the fr_rx function drops a received frame (because the protocol type is not supported, or because the PVC virtual device that corresponds to the DLCI number and the protocol type doesn't exist), the function frees the skb and returns. The code for freeing the skb and returning is repeated several times, this patch uses "goto rx_drop" to replace them so that the code looks cleaner. Cc: Willem de Bruijn Cc: Krzysztof Halasa Signed-off-by: Xie He --- drivers/net/wan/hdlc_fr.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c index 409e5a7ad8e2..4db0e01b96a9 100644 --- a/drivers/net/wan/hdlc_fr.c +++ b/drivers/net/wan/hdlc_fr.c @@ -904,8 +904,7 @@ static int fr_rx(struct sk_buff *skb) netdev_info(frad, "No PVC for received frame's DLCI %d\n", dlci); #endif - dev_kfree_skb_any(skb); - return NET_RX_DROP; + goto rx_drop; } if (pvc->state.fecn != fh->fecn) { @@ -963,14 +962,12 @@ static int fr_rx(struct sk_buff *skb) default: netdev_info(frad, "Unsupported protocol, OUI=%x PID=%x\n", oui, pid); - dev_kfree_skb_any(skb); - return NET_RX_DROP; + goto rx_drop; } } else { netdev_info(frad, "Unsupported protocol, NLPID=%x length=%i\n", data[3], skb->len); - dev_kfree_skb_any(skb); - return NET_RX_DROP; + goto rx_drop; } if (dev) { @@ -982,12 +979,12 @@ static int fr_rx(struct sk_buff *skb) netif_rx(skb); return NET_RX_SUCCESS; } else { - dev_kfree_skb_any(skb); - return NET_RX_DROP; + goto rx_drop; } - rx_error: +rx_error: frad->stats.rx_errors++; /* Mark error */ +rx_drop: dev_kfree_skb_any(skb); return NET_RX_DROP; } -- 2.27.0
Re: [PATCH v8 -tip 19/26] sched: Add a second-level tag for nested CGroup usecase
On Mon, Oct 19, 2020 at 6:45 PM Joel Fernandes (Google) wrote: > > +static unsigned long cpu_core_get_group_cookie(struct task_group *tg) > +{ > + unsigned long color = 0; > + > + if (!tg) > + return 0; > + > + for (; tg; tg = tg->parent) { > + if (tg->core_tag_color) { > + WARN_ON_ONCE(color); > + color = tg->core_tag_color; > + } > + > + if (tg->core_tagged) { > + unsigned long cookie = ((unsigned long)tg << 8) | > color; > + cookie &= (1UL << (sizeof(unsigned long) * 4)) - 1; > + return cookie; > + } > + } > + > + return 0; > +} I'm a bit wary of how core_task_cookie and core_group_cookie are truncated to the lower half of their bits and combined into the overall core_cookie. Now that core_group_cookie is further losing 8 bits to color, that leaves (in the case of 32 bit unsigned long) only 8 bits to uniquely identify the group contribution to the cookie. Also, I agree that 256 colors is likely adequate, but it would be nice to avoid this restriction. I'd like to propose the following alternative, which involves creating a new struct to represent the core cookie: struct core_cookie { unsigned long task_cookie; unsigned long group_cookie; unsigned long color; /* can be further extended with arbitrary fields */ struct rb_node node; refcount_t; }; struct rb_root core_cookies; /* (sorted), all active core_cookies */ seqlock_t core_cookies_lock; /* protects against removal/addition to core_cookies */ struct task_struct { ... unsigned long core_cookie; /* (struct core_cookie *) */ } A given task stores the address of a core_cookie struct in its core_cookie field. When we reconfigure a task's color/task_cookie/group_cookie, we can first look for an existing core_cookie that matches those settings, or create a new one.
[PATCH net-next v5 3/5] net: hdlc_fr: Do skb_reset_mac_header for skbs received on normal PVC devices
When an skb is received on a normal (non-Ethernet-emulating) PVC device, call skb_reset_mac_header before we pass it to upper layers. This is because normal PVC devices don't have header_ops, so any header we have would not be visible to upper layer code when sending, so the header shouldn't be visible to upper layer code when receiving, either. Cc: Willem de Bruijn Cc: Krzysztof Halasa Acked-by: Willem de Bruijn Signed-off-by: Xie He --- drivers/net/wan/hdlc_fr.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c index 71ee9b60d91b..eb83116aa9df 100644 --- a/drivers/net/wan/hdlc_fr.c +++ b/drivers/net/wan/hdlc_fr.c @@ -935,6 +935,7 @@ static int fr_rx(struct sk_buff *skb) skb_pull(skb, 4); /* Remove 4-byte header (hdr, UI, NLPID) */ skb->dev = pvc->main; skb->protocol = htons(ETH_P_IP); + skb_reset_mac_header(skb); } else if (data[3] == NLPID_IPV6) { if (!pvc->main) @@ -942,6 +943,7 @@ static int fr_rx(struct sk_buff *skb) skb_pull(skb, 4); /* Remove 4-byte header (hdr, UI, NLPID) */ skb->dev = pvc->main; skb->protocol = htons(ETH_P_IPV6); + skb_reset_mac_header(skb); } else if (skb->len > 10 && data[3] == FR_PAD && data[4] == NLPID_SNAP && data[5] == FR_PAD) { @@ -958,6 +960,7 @@ static int fr_rx(struct sk_buff *skb) goto rx_drop; skb->dev = pvc->main; skb->protocol = htons(pid); + skb_reset_mac_header(skb); break; case 0x80C20007: /* bridged Ethernet frame */ -- 2.27.0
[PATCH v5 2/2] firmware: QCOM_SCM: Allow qcom_scm driver to be loadable as a permenent module
Allow the qcom_scm driver to be loadable as a permenent module. This still uses the "depends on QCOM_SCM || !QCOM_SCM" bit to ensure that drivers that call into the qcom_scm driver are also built as modules. While not ideal in some cases its the only safe way I can find to avoid build errors without having those drivers select QCOM_SCM and have to force it on (as QCOM_SCM=n can be valid for those drivers). Cc: Catalin Marinas Cc: Will Deacon Cc: Andy Gross Cc: Bjorn Andersson Cc: Joerg Roedel Cc: Thomas Gleixner Cc: Jason Cooper Cc: Marc Zyngier Cc: Linus Walleij Cc: Vinod Koul Cc: Kalle Valo Cc: Maulik Shah Cc: Lina Iyer Cc: Saravana Kannan Cc: Todd Kjos Cc: Greg Kroah-Hartman Cc: linux-arm-...@vger.kernel.org Cc: io...@lists.linux-foundation.org Cc: linux-g...@vger.kernel.org Acked-by: Greg Kroah-Hartman Signed-off-by: John Stultz --- v3: * Fix __arm_smccc_smc build issue reported by kernel test robot v4: * Add "depends on QCOM_SCM || !QCOM_SCM" bit to ath10k config that requires it. v5: * Fix QCOM_QCM typo in Kconfig, it should be QCOM_SCM --- drivers/firmware/Kconfig| 4 ++-- drivers/firmware/Makefile | 3 ++- drivers/firmware/qcom_scm.c | 4 drivers/iommu/Kconfig | 2 ++ drivers/net/wireless/ath/ath10k/Kconfig | 1 + 5 files changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index 3315e3c215864..5e369928bc567 100644 --- a/drivers/firmware/Kconfig +++ b/drivers/firmware/Kconfig @@ -235,8 +235,8 @@ config INTEL_STRATIX10_RSU Say Y here if you want Intel RSU support. config QCOM_SCM - bool - depends on ARM || ARM64 + tristate "Qcom SCM driver" + depends on (ARM && HAVE_ARM_SMCCC) || ARM64 select RESET_CONTROLLER config QCOM_SCM_DOWNLOAD_MODE_DEFAULT diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile index 5e013b6a3692e..523173cbff335 100644 --- a/drivers/firmware/Makefile +++ b/drivers/firmware/Makefile @@ -17,7 +17,8 @@ obj-$(CONFIG_ISCSI_IBFT) += iscsi_ibft.o obj-$(CONFIG_FIRMWARE_MEMMAP) += memmap.o obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += raspberrypi.o obj-$(CONFIG_FW_CFG_SYSFS) += qemu_fw_cfg.o -obj-$(CONFIG_QCOM_SCM) += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o +obj-$(CONFIG_QCOM_SCM) += qcom-scm.o +qcom-scm-objs += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o obj-$(CONFIG_TRUSTED_FOUNDATIONS) += trusted_foundations.o obj-$(CONFIG_TURRIS_MOX_RWTM) += turris-mox-rwtm.o diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c index 7be48c1bec96d..6f431b73e617d 100644 --- a/drivers/firmware/qcom_scm.c +++ b/drivers/firmware/qcom_scm.c @@ -1280,6 +1280,7 @@ static const struct of_device_id qcom_scm_dt_match[] = { { .compatible = "qcom,scm" }, {} }; +MODULE_DEVICE_TABLE(of, qcom_scm_dt_match); static struct platform_driver qcom_scm_driver = { .driver = { @@ -1295,3 +1296,6 @@ static int __init qcom_scm_init(void) return platform_driver_register(_scm_driver); } subsys_initcall(qcom_scm_init); + +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. SCM driver"); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index 04878caf6da49..c64d7a2b65134 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -248,6 +248,7 @@ config SPAPR_TCE_IOMMU config ARM_SMMU tristate "ARM Ltd. System MMU (SMMU) Support" depends on ARM64 || ARM || (COMPILE_TEST && !GENERIC_ATOMIC64) + depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be =y select IOMMU_API select IOMMU_IO_PGTABLE_LPAE select ARM_DMA_USE_IOMMU if ARM @@ -375,6 +376,7 @@ config QCOM_IOMMU # Note: iommu drivers cannot (yet?) be built as modules bool "Qualcomm IOMMU Support" depends on ARCH_QCOM || (COMPILE_TEST && !GENERIC_ATOMIC64) + depends on QCOM_SCM=y select IOMMU_API select IOMMU_IO_PGTABLE_LPAE select ARM_DMA_USE_IOMMU diff --git a/drivers/net/wireless/ath/ath10k/Kconfig b/drivers/net/wireless/ath/ath10k/Kconfig index 40f91bc8514d8..741289e385d59 100644 --- a/drivers/net/wireless/ath/ath10k/Kconfig +++ b/drivers/net/wireless/ath/ath10k/Kconfig @@ -44,6 +44,7 @@ config ATH10K_SNOC tristate "Qualcomm ath10k SNOC support" depends on ATH10K depends on ARCH_QCOM || COMPILE_TEST + depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be =y select QCOM_QMI_HELPERS help This module adds support for integrated WCN3990 chip connected -- 2.17.1
[PATCH net-next v5 1/5] net: hdlc_fr: Simpify fr_rx by using "goto rx_drop" to drop frames
When the fr_rx function drops a received frame (because the protocol type is not supported, or because the PVC virtual device that corresponds to the DLCI number and the protocol type doesn't exist), the function frees the skb and returns. The code for freeing the skb and returning is repeated several times, this patch uses "goto rx_drop" to replace them so that the code looks cleaner. Cc: Willem de Bruijn Cc: Krzysztof Halasa Signed-off-by: Xie He --- drivers/net/wan/hdlc_fr.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c index 409e5a7ad8e2..4db0e01b96a9 100644 --- a/drivers/net/wan/hdlc_fr.c +++ b/drivers/net/wan/hdlc_fr.c @@ -904,8 +904,7 @@ static int fr_rx(struct sk_buff *skb) netdev_info(frad, "No PVC for received frame's DLCI %d\n", dlci); #endif - dev_kfree_skb_any(skb); - return NET_RX_DROP; + goto rx_drop; } if (pvc->state.fecn != fh->fecn) { @@ -963,14 +962,12 @@ static int fr_rx(struct sk_buff *skb) default: netdev_info(frad, "Unsupported protocol, OUI=%x PID=%x\n", oui, pid); - dev_kfree_skb_any(skb); - return NET_RX_DROP; + goto rx_drop; } } else { netdev_info(frad, "Unsupported protocol, NLPID=%x length=%i\n", data[3], skb->len); - dev_kfree_skb_any(skb); - return NET_RX_DROP; + goto rx_drop; } if (dev) { @@ -982,12 +979,12 @@ static int fr_rx(struct sk_buff *skb) netif_rx(skb); return NET_RX_SUCCESS; } else { - dev_kfree_skb_any(skb); - return NET_RX_DROP; + goto rx_drop; } - rx_error: +rx_error: frad->stats.rx_errors++; /* Mark error */ +rx_drop: dev_kfree_skb_any(skb); return NET_RX_DROP; } -- 2.27.0
[PATCH net-next v5 2/5] net: hdlc_fr: Change the use of "dev" in fr_rx to make the code cleaner
The eth_type_trans function is called when we receive frames carrying Ethernet frames. This function expects a non-NULL pointer as an argument, and assigns it directly to skb->dev. However, the code handling other types of frames first assigns the pointer to "dev", and then at the end checks whether the value is NULL, and if it is not NULL, assigns it to skb->dev. The two flows are different. Mixing them in this function makes the code messy. It's better that we convert the second flow to align with how eth_type_trans does things. So this patch changes the code to: first make sure the pointer is not NULL, then assign it directly to skb->dev. "dev" is no longer needed until the end where we use it to update stats. Cc: Willem de Bruijn Cc: Krzysztof Halasa Signed-off-by: Xie He --- drivers/net/wan/hdlc_fr.c | 37 - 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c index 4db0e01b96a9..71ee9b60d91b 100644 --- a/drivers/net/wan/hdlc_fr.c +++ b/drivers/net/wan/hdlc_fr.c @@ -880,7 +880,7 @@ static int fr_rx(struct sk_buff *skb) u8 *data = skb->data; u16 dlci; struct pvc_device *pvc; - struct net_device *dev = NULL; + struct net_device *dev; if (skb->len <= 4 || fh->ea1 || data[2] != FR_UI) goto rx_error; @@ -930,13 +930,17 @@ static int fr_rx(struct sk_buff *skb) } if (data[3] == NLPID_IP) { + if (!pvc->main) + goto rx_drop; skb_pull(skb, 4); /* Remove 4-byte header (hdr, UI, NLPID) */ - dev = pvc->main; + skb->dev = pvc->main; skb->protocol = htons(ETH_P_IP); } else if (data[3] == NLPID_IPV6) { + if (!pvc->main) + goto rx_drop; skb_pull(skb, 4); /* Remove 4-byte header (hdr, UI, NLPID) */ - dev = pvc->main; + skb->dev = pvc->main; skb->protocol = htons(ETH_P_IPV6); } else if (skb->len > 10 && data[3] == FR_PAD && @@ -950,13 +954,16 @@ static int fr_rx(struct sk_buff *skb) case ETH_P_IPX: case ETH_P_IP: /* a long variant */ case ETH_P_IPV6: - dev = pvc->main; + if (!pvc->main) + goto rx_drop; + skb->dev = pvc->main; skb->protocol = htons(pid); break; case 0x80C20007: /* bridged Ethernet frame */ - if ((dev = pvc->ether) != NULL) - skb->protocol = eth_type_trans(skb, dev); + if (!pvc->ether) + goto rx_drop; + skb->protocol = eth_type_trans(skb, pvc->ether); break; default: @@ -970,17 +977,13 @@ static int fr_rx(struct sk_buff *skb) goto rx_drop; } - if (dev) { - dev->stats.rx_packets++; /* PVC traffic */ - dev->stats.rx_bytes += skb->len; - if (pvc->state.becn) - dev->stats.rx_compressed++; - skb->dev = dev; - netif_rx(skb); - return NET_RX_SUCCESS; - } else { - goto rx_drop; - } + dev = skb->dev; + dev->stats.rx_packets++; /* PVC traffic */ + dev->stats.rx_bytes += skb->len; + if (pvc->state.becn) + dev->stats.rx_compressed++; + netif_rx(skb); + return NET_RX_SUCCESS; rx_error: frad->stats.rx_errors++; /* Mark error */ -- 2.27.0
[PATCH net-next v5 5/5] net: hdlc_fr: Add support for any Ethertype
Change the fr_rx function to make this driver support any Ethertype when receiving skbs on normal (non-Ethernet-emulating) PVC devices. (This driver is already able to handle any Ethertype when sending.) Originally in the fr_rx function, the code that parses the long (10-byte) header only recognizes a few Ethertype values and drops frames with other Ethertype values. This patch replaces this code to make fr_rx support any Ethertype. This patch also creates a new function fr_snap_parse as part of the new code. Cc: Willem de Bruijn Cc: Krzysztof Halasa Acked-by: Willem de Bruijn Signed-off-by: Xie He --- drivers/net/wan/hdlc_fr.c | 75 +-- 1 file changed, 49 insertions(+), 26 deletions(-) diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c index 98444f1d8cc3..0720f5f92caa 100644 --- a/drivers/net/wan/hdlc_fr.c +++ b/drivers/net/wan/hdlc_fr.c @@ -871,6 +871,45 @@ static int fr_lmi_recv(struct net_device *dev, struct sk_buff *skb) return 0; } +static int fr_snap_parse(struct sk_buff *skb, struct pvc_device *pvc) +{ + /* OUI 00-00-00 indicates an Ethertype follows */ + if (skb->data[0] == 0x00 && + skb->data[1] == 0x00 && + skb->data[2] == 0x00) { + if (!pvc->main) + return -1; + skb->dev = pvc->main; + skb->protocol = *(__be16 *)(skb->data + 3); /* Ethertype */ + skb_pull(skb, 5); + skb_reset_mac_header(skb); + return 0; + + /* OUI 00-80-C2 stands for the 802.1 organization */ + } else if (skb->data[0] == 0x00 && + skb->data[1] == 0x80 && + skb->data[2] == 0xC2) { + /* PID 00-07 stands for Ethernet frames without FCS */ + if (skb->data[3] == 0x00 && + skb->data[4] == 0x07) { + if (!pvc->ether) + return -1; + skb_pull(skb, 5); + if (skb->len < ETH_HLEN) + return -1; + skb->protocol = eth_type_trans(skb, pvc->ether); + return 0; + + /* PID unsupported */ + } else { + return -1; + } + + /* OUI unsupported */ + } else { + return -1; + } +} static int fr_rx(struct sk_buff *skb) { @@ -945,35 +984,19 @@ static int fr_rx(struct sk_buff *skb) skb->protocol = htons(ETH_P_IPV6); skb_reset_mac_header(skb); - } else if (skb->len > 10 && data[3] == FR_PAD && - data[4] == NLPID_SNAP && data[5] == FR_PAD) { - u16 oui = ntohs(*(__be16*)(data + 6)); - u16 pid = ntohs(*(__be16*)(data + 8)); - skb_pull(skb, 10); - - switch u32)oui) << 16) | pid) { - case ETH_P_ARP: /* routed frame with SNAP */ - case ETH_P_IPX: - case ETH_P_IP: /* a long variant */ - case ETH_P_IPV6: - if (!pvc->main) - goto rx_drop; - skb->dev = pvc->main; - skb->protocol = htons(pid); - skb_reset_mac_header(skb); - break; - - case 0x80C20007: /* bridged Ethernet frame */ - if (!pvc->ether) + } else if (data[3] == FR_PAD) { + if (skb->len < 5) + goto rx_error; + if (data[4] == NLPID_SNAP) { /* A SNAP header follows */ + skb_pull(skb, 5); + if (skb->len < 5) /* Incomplete SNAP header */ + goto rx_error; + if (fr_snap_parse(skb, pvc)) goto rx_drop; - skb->protocol = eth_type_trans(skb, pvc->ether); - break; - - default: - netdev_info(frad, "Unsupported protocol, OUI=%x PID=%x\n", - oui, pid); + } else { goto rx_drop; } + } else { netdev_info(frad, "Unsupported protocol, NLPID=%x length=%i\n", data[3], skb->len); -- 2.27.0
[PATCH net-next v5 0/5] net: hdlc_fr: Improve fr_rx and add support for any Ethertype
The main purpose of this series is the last patch. The previous 4 patches are just code clean-ups so that the last patch will not make the code too messy. The patches must be applied in sequence. The receiving code of this driver doesn't support arbitrary Ethertype values. It only recognizes a few known Ethertypes when receiving and drops skbs with other Ethertypes. However, the standard document RFC 2427 allows Frame Relay to support any Ethertype values. This series adds support for this. Change from v4: Drop the change related to stats.rx_dropped from the 1st patch. Switch the 3rd and 4th patch. Improve the commit message of the 4th patch by stating why only a 2-byte address field is accepted. Change from v3: Split the last patch into 2 patches. Improve the commit message of the 1st patch to explicitly state that the stats.rx_dropped count is also increased after "goto rx_error". Change from v2: Small fix to the commit messages. Change from v1: Small fix to the commit messages. Xie He (5): net: hdlc_fr: Simpify fr_rx by using "goto rx_drop" to drop frames net: hdlc_fr: Change the use of "dev" in fr_rx to make the code cleaner net: hdlc_fr: Do skb_reset_mac_header for skbs received on normal PVC devices net: hdlc_fr: Improve the initial checks when we receive an skb net: hdlc_fr: Add support for any Ethertype drivers/net/wan/hdlc_fr.c | 118 +++--- 1 file changed, 72 insertions(+), 46 deletions(-) -- 2.27.0
[PATCH net-next v5 4/5] net: hdlc_fr: Improve the initial checks when we receive an skb
1. Change the skb->len check from "<= 4" to "< 4". At first we only need to ensure a 4-byte header is present. We indeed normally need the 5th byte, too, but it'd be more logical and cleaner to check its existence when we actually need it. 2. Add an fh->ea2 check to the initial checks in fr_rx. fh->ea2 == 1 means the second address byte is the final address byte. We only support the case where the address length is 2 bytes. If the address length is not 2 bytes, the control field and the protocol field would not be the 3rd and 4th byte as we assume. (Say if it is 3 bytes, then the control field and the protocol field would be the 4th and 5th byte instead.) Cc: Willem de Bruijn Cc: Krzysztof Halasa Signed-off-by: Xie He --- drivers/net/wan/hdlc_fr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c index eb83116aa9df..98444f1d8cc3 100644 --- a/drivers/net/wan/hdlc_fr.c +++ b/drivers/net/wan/hdlc_fr.c @@ -882,7 +882,7 @@ static int fr_rx(struct sk_buff *skb) struct pvc_device *pvc; struct net_device *dev; - if (skb->len <= 4 || fh->ea1 || data[2] != FR_UI) + if (skb->len < 4 || fh->ea1 || !fh->ea2 || data[2] != FR_UI) goto rx_error; dlci = q922_to_dlci(skb->data); -- 2.27.0
[PATCH v5 1/2] pinctrl: qcom: Allow pinctrl-msm code to be loadable as a module
Tweaks to allow pinctrl-msm code to be loadable as a module. This is needed in order to support having the qcom-scm driver, which pinctrl-msm calls into, configured as a module. This requires that we tweak Kconfigs selecting PINCTRL_MSM to also depend on QCOM_SCM || QCOM_SCM=n so that we match the module setting of QCOM_SCM. Unlike the previous revision of this patch: https://lore.kernel.org/lkml/20200625001039.56174-5-john.stu...@linaro.org/ this version reworks PINCTRL_MSM to be a visible option and instead of having the various SoC specific drivers select PINCTRL_MSM, this switches those configs to depend on PINCTRL_MSM. This avoids adding the oddish looking: "depend on QCOM_SCM || QCOM_SCM=n" to every SoC specific driver, as that becomes a maintenance headache. We also add PINCTRL_MSM to the arm64 defconfig to avoid surprises as otherwise PINCTRL_MSM/IPQ* options previously enabled, will be off. Cc: Catalin Marinas Cc: Will Deacon Cc: Andy Gross Cc: Bjorn Andersson Cc: Joerg Roedel Cc: Thomas Gleixner Cc: Jason Cooper Cc: Marc Zyngier Cc: Linus Walleij Cc: Vinod Koul Cc: Kalle Valo Cc: Maulik Shah Cc: Lina Iyer Cc: Saravana Kannan Cc: Todd Kjos Cc: Greg Kroah-Hartman Cc: linux-arm-...@vger.kernel.org Cc: io...@lists.linux-foundation.org Cc: linux-g...@vger.kernel.org Signed-off-by: John Stultz --- v2: * Module description and whitespace fixes suggested by Bjorn * Added QCOM_SCM || QCOM_SCM=n bits on Kconfigs selecting PINCTRL_MSM. Reported by both Todd and Bjorn. v3: * Make sure the QCOM_SCM || QCOM_SCM=n trick is commented v4: * Rework "select PINCTRL_MSM" to "depends on PINCTRL_MSM" to consolidate the QCOM_SCM dependency. v5: * Add PINCTRL_MSM to arm64 defconfig --- arch/arm64/configs/defconfig | 1 + drivers/pinctrl/qcom/Kconfig | 49 +++--- drivers/pinctrl/qcom/pinctrl-msm.c | 2 ++ 3 files changed, 28 insertions(+), 24 deletions(-) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 17a2df6a263e8..45768828fdb8e 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -483,6 +483,7 @@ CONFIG_PINCTRL_IMX8MP=y CONFIG_PINCTRL_IMX8MQ=y CONFIG_PINCTRL_IMX8QXP=y CONFIG_PINCTRL_IMX8DXL=y +CONFIG_PINCTRL_MSM=y CONFIG_PINCTRL_IPQ8074=y CONFIG_PINCTRL_IPQ6018=y CONFIG_PINCTRL_MSM8916=y diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig index 5fe7b8aaf69d8..8bb786ed152dd 100644 --- a/drivers/pinctrl/qcom/Kconfig +++ b/drivers/pinctrl/qcom/Kconfig @@ -2,7 +2,8 @@ if (ARCH_QCOM || COMPILE_TEST) config PINCTRL_MSM - bool + tristate "Qualcomm core pin controller driver" + depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be =y select PINMUX select PINCONF select GENERIC_PINCONF @@ -13,7 +14,7 @@ config PINCTRL_MSM config PINCTRL_APQ8064 tristate "Qualcomm APQ8064 pin controller driver" depends on GPIOLIB && OF - select PINCTRL_MSM + depends on PINCTRL_MSM help This is the pinctrl, pinmux, pinconf and gpiolib driver for the Qualcomm TLMM block found in the Qualcomm APQ8064 platform. @@ -21,7 +22,7 @@ config PINCTRL_APQ8064 config PINCTRL_APQ8084 tristate "Qualcomm APQ8084 pin controller driver" depends on GPIOLIB && OF - select PINCTRL_MSM + depends on PINCTRL_MSM help This is the pinctrl, pinmux, pinconf and gpiolib driver for the Qualcomm TLMM block found in the Qualcomm APQ8084 platform. @@ -29,7 +30,7 @@ config PINCTRL_APQ8084 config PINCTRL_IPQ4019 tristate "Qualcomm IPQ4019 pin controller driver" depends on GPIOLIB && OF - select PINCTRL_MSM + depends on PINCTRL_MSM help This is the pinctrl, pinmux, pinconf and gpiolib driver for the Qualcomm TLMM block found in the Qualcomm IPQ4019 platform. @@ -37,7 +38,7 @@ config PINCTRL_IPQ4019 config PINCTRL_IPQ8064 tristate "Qualcomm IPQ8064 pin controller driver" depends on GPIOLIB && OF - select PINCTRL_MSM + depends on PINCTRL_MSM help This is the pinctrl, pinmux, pinconf and gpiolib driver for the Qualcomm TLMM block found in the Qualcomm IPQ8064 platform. @@ -45,7 +46,7 @@ config PINCTRL_IPQ8064 config PINCTRL_IPQ8074 tristate "Qualcomm Technologies, Inc. IPQ8074 pin controller driver" depends on GPIOLIB && OF - select PINCTRL_MSM + depends on PINCTRL_MSM help This is the pinctrl, pinmux, pinconf and gpiolib driver for the Qualcomm Technologies Inc. TLMM block found on the @@ -55,7 +56,7 @@ config PINCTRL_IPQ8074 config PINCTRL_IPQ6018 tristate "Qualcomm Technologies, Inc. IPQ6018 pin controller driver" depends on GPIOLIB && OF - select PINCTRL_MSM + depends on PINCTRL_MSM help This is the pinctrl, pinmux, pinconf and gpiolib driver for
[RFC][PATCH 1/2] arm-smmu-qcom: Ensure the qcom_scm driver has finished probing
Robin Murphy pointed out that if the arm-smmu driver probes before the qcom_scm driver, we may call qcom_scm_qsmmu500_wait_safe_toggle() before the __scm is initialized. Now, getting this to happen is a bit contrived, as in my efforts it required enabling asynchronous probing for both drivers, moving the firmware dts node to the end of the dtsi file, as well as forcing a long delay in the qcom_scm_probe function. With those tweaks we ran into the following crash: [2.631040] arm-smmu 1500.iommu: Stage-1: 48-bit VA -> 48-bit IPA [2.633372] Unable to handle kernel NULL pointer dereference at virtual address ... [2.633402] [] user address but active_mm is swapper [2.633409] Internal error: Oops: 9605 [#1] PREEMPT SMP [2.633415] Modules linked in: [2.633427] CPU: 5 PID: 117 Comm: kworker/u16:2 Tainted: GW 5.10.0-rc1-mainline-00025-g272a618fc36-dirty #3971 [2.633430] Hardware name: Thundercomm Dragonboard 845c (DT) [2.633448] Workqueue: events_unbound async_run_entry_fn [2.633456] pstate: 80c5 (Nzcv daif +PAN +UAO -TCO BTYPE=--) [2.633465] pc : qcom_scm_qsmmu500_wait_safe_toggle+0x78/0xb0 [2.633473] lr : qcom_smmu500_reset+0x58/0x78 [2.633476] sp : ffc0105a3b60 ... [2.633567] Call trace: [2.633572] qcom_scm_qsmmu500_wait_safe_toggle+0x78/0xb0 [2.633576] qcom_smmu500_reset+0x58/0x78 [2.633581] arm_smmu_device_reset+0x194/0x270 [2.633585] arm_smmu_device_probe+0xc94/0xeb8 [2.633592] platform_drv_probe+0x58/0xa8 [2.633597] really_probe+0xec/0x398 [2.633601] driver_probe_device+0x5c/0xb8 [2.633606] __driver_attach_async_helper+0x64/0x88 [2.633610] async_run_entry_fn+0x4c/0x118 [2.633617] process_one_work+0x20c/0x4b0 [2.633621] worker_thread+0x48/0x460 [2.633628] kthread+0x14c/0x158 [2.633634] ret_from_fork+0x10/0x18 [2.633642] Code: a9034fa0 d0007f73 29107fa0 91342273 (f9400020) To avoid this, this patch adds a check on qcom_scm_is_available() in the qcom_smmu_impl_init() function, returning -EPROBE_DEFER if its not ready. This allows the driver to try to probe again later after qcom_scm has finished probing. Cc: Robin Murphy Cc: Will Deacon Cc: Andy Gross Cc: Maulik Shah Cc: Bjorn Andersson Cc: Saravana Kannan Cc: Marc Zyngier Cc: Lina Iyer Cc: io...@lists.linux-foundation.org Cc: linux-arm-msm Reported-by: Robin Murphy Signed-off-by: John Stultz --- drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c index 66ba4870659f4..ef37ccfa82562 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c @@ -159,6 +159,10 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu) { struct qcom_smmu *qsmmu; + /* Check to make sure qcom_scm has finished probing */ + if (!qcom_scm_is_available()) + return ERR_PTR(-EPROBE_DEFER); + qsmmu = devm_kzalloc(smmu->dev, sizeof(*qsmmu), GFP_KERNEL); if (!qsmmu) return ERR_PTR(-ENOMEM); -- 2.17.1
[RFC][PATCH 2/2] iommu: Avoid crash if iommu_group is null
In trying to handle a possible driver probe ordering issue brought up by Robin Murphy, I ran across a separate null pointer crash in the iommu core in iommu_group_remove_device(): [2.732803] dwc3-qcom a6f8800.usb: failed to get usb-ddr path: -517 [2.739281] Unable to handle kernel NULL pointer dereference at virtual address 00c0 ... [2.775619] [00c0] user address but active_mm is swapper [2.782039] Internal error: Oops: 9605 [#1] PREEMPT SMP [2.787670] Modules linked in: [2.790769] CPU: 6 PID: 1 Comm: swapper/0 Tainted: GW 5.10.0-rc1-mainline-00025-g272a618fc36-dirty #3973 [2.801719] Hardware name: Thundercomm Dragonboard 845c (DT) [2.807431] pstate: 00c5 (nzcv daif +PAN +UAO -TCO BTYPE=--) [2.813508] pc : iommu_group_remove_device+0x30/0x1b0 [2.818611] lr : iommu_release_device+0x4c/0x78 [2.823189] sp : ffc01005b950 ... [2.907082] Call trace: [2.909566] iommu_group_remove_device+0x30/0x1b0 [2.914323] iommu_release_device+0x4c/0x78 [2.918559] iommu_bus_notifier+0xe8/0x108 [2.922708] blocking_notifier_call_chain+0x78/0xb8 [2.927641] device_del+0x2ac/0x3d0 [2.931177] platform_device_del.part.9+0x20/0x98 [2.935933] platform_device_unregister+0x2c/0x40 [2.940694] of_platform_device_destroy+0xd8/0xe0 [2.945450] device_for_each_child_reverse+0x58/0xb0 [2.950471] of_platform_depopulate+0x4c/0x78 [2.954886] dwc3_qcom_probe+0x93c/0xcb8 [2.958858] platform_drv_probe+0x58/0xa8 [2.962917] really_probe+0xec/0x398 [2.966531] driver_probe_device+0x5c/0xb8 [2.970677] device_driver_attach+0x74/0x98 [2.974911] __driver_attach+0x60/0xe8 [2.978700] bus_for_each_dev+0x84/0xd8 [2.982581] driver_attach+0x30/0x40 [2.986194] bus_add_driver+0x160/0x208 [2.990076] driver_register+0x64/0x110 [2.993957] __platform_driver_register+0x58/0x68 [2.998716] dwc3_qcom_driver_init+0x20/0x28 [3.003041] do_one_initcall+0x6c/0x2d0 [3.006925] kernel_init_freeable+0x214/0x268 [3.011339] kernel_init+0x18/0x118 [3.014876] ret_from_fork+0x10/0x18 [3.018495] Code: d0006a21 f9417295 91130021 910162b6 (b940c2a2) In the case above, the arm-smmu driver fails to probe with EPROBE_DEFER, and I'm guessing I'm guessing that causes iommu_group_add_device() to fail and sets the dev->iommu_group = NULL, then somehow we hit iommu_group_remove_device() and trip over the null value? I'm not really sure... Anyway, adding the null check seems to avoid the issue and the system boots fine after the arm-smmu driver later reprobed. Feedback or better ideas for a solution would be appreciated! Cc: Robin Murphy Cc: Will Deacon Cc: Andy Gross Cc: Maulik Shah Cc: Bjorn Andersson Cc: Saravana Kannan Cc: Marc Zyngier Cc: Lina Iyer Cc: io...@lists.linux-foundation.org Cc: linux-arm-msm Signed-off-by: John Stultz --- drivers/iommu/iommu.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 8c470f451a323..44639b88e77db 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -877,6 +877,10 @@ void iommu_group_remove_device(struct device *dev) struct iommu_group *group = dev->iommu_group; struct group_device *tmp_device, *device = NULL; + /* Avoid crash if iommu_group value is null */ + if (!group) + return; + dev_info(dev, "Removing from iommu group %d\n", group->id); /* Pre-notify listeners that a device is being removed. */ -- 2.17.1
Re: [PATCH v11 4/4] bus: mhi: Add userspace client interface driver
Hi Randy, On 10/29/20 10:48 PM, Randy Dunlap wrote: On 10/29/20 7:45 PM, Hemant Kumar wrote: diff --git a/drivers/bus/mhi/Kconfig b/drivers/bus/mhi/Kconfig index e841c10..476cc55 100644 --- a/drivers/bus/mhi/Kconfig +++ b/drivers/bus/mhi/Kconfig @@ -20,3 +20,16 @@ config MHI_BUS_DEBUG Enable debugfs support for use with the MHI transport. Allows reading and/or modifying some values within the MHI controller for debug and test purposes. + +config MHI_UCI + tristate "MHI UCI" + depends on MHI_BUS + help + MHI based Userspace Client Interface (UCI) driver is used for MHI-based + transferring raw data between host and device using standard file + operations from userspace. Open, read, write, and close operations + are supported by this driver. Please check mhi_uci_match_table for also poll according to the documentation. good catch, will add in next patch set. + all supported channels that are exposed to userspace. + + To compile this driver as a module, choose M here: the module will be + called mhi_uci. Thanks, Hemant -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [f2fs-dev] [PATCH v2] f2fs: move ioctl interface definitions to separated file
On 2020/10/31 0:55, Eric Biggers wrote: On Fri, Oct 30, 2020 at 03:26:10PM +0800, Chao Yu wrote: + +struct f2fs_gc_range { + u32 sync; + u64 start; + u64 len; +}; Userspace headers need to use __u32, __u64, etc. instead of u32, u64, etc. Correct. Did you try installing this header, and including it in a userspace program? Let me do more test before next version. Thanks, - Eric .
[PATCH RFC v3 0/4] x86/bus_lock: Enable bus lock detection
A bus lock [1] is acquired either through split locked access to writeback (WB) memory or by using locks to uncacheable (UC) memory (e.g. direct device assignment). This is typically >1000 cycles slower than an atomic operation within a cache line. It also disrupts performance on other cores. Although split lock can be detected by #AC trap, the trap is triggered before the instruction acquires bus lock. This makes it difficult to mitigate bus lock (e.g. throttle the user application). Some CPUs have ability to notify the kernel by an #DB trap after a user instruction acquires a bus lock and is executed. This allows the kernel to enforce user application throttling or mitigations. #DB for bus lock detect fixes issues in #AC for split lock detect: 1) It's architectural ... just need to look at one CPUID bit to know it exists 2) The IA32_DEBUGCTL MSR, which reports bus lock in #DB, is per-thread. So each process or guest can have different behavior. 3) It has support for VMM/guests (new VMEXIT codes, etc). Hardware only generates #DB for bus lock detect when CPL>0 to avoid nested #DB from multiple bus locks while the first #DB is being handled. Use the existing kernel command line option "split_lock_detect=" to handle #DB for bus lock: split_lock_detect= #AC for split lock #DB for bus lock off Do nothing Do nothing warnKernel OOPs Warn once per task and Warn once per task and and continues to run. disable future checking When both features are supported, warn in #DB fatal Kernel OOPs Send SIGBUS to user Send SIGBUS to user When both features are supported, fatal in #AC. ratelimit:N Do nothing Limit bus lock rate to N per second in the current non root user. Default split_lock_detect is "warn". [1] Chapter 8 https://software.intel.com/sites/default/files/managed/c5/15/architecture-instruction-set-extensions-programming-reference.pdf Change Log: RFC v3: - Remove DR6_RESERVED change (PeterZ). - Simplify the documentation (Randy). RFC v2: - Architecture changed based on feedback from Thomas and PeterZ. #DB is no longer generated for bus lock in ring0. - Split the one single patch into four patches. [RFC v1 can be found at: https://lore.kernel.org/lkml/1595021700-68460-1-git-send-email-fenghua...@intel.com/] Fenghua Yu (4): x86/cpufeatures: Enumerate #DB for bus lock detection x86/bus_lock: Handle warn and fatal in #DB for bus lock x86/bus_lock: Set rate limit for bus lock Documentation/admin-guide: Change doc for split_lock_detect parameter .../admin-guide/kernel-parameters.txt | 28 +++- arch/x86/include/asm/cpu.h| 10 +- arch/x86/include/asm/cpufeatures.h| 1 + arch/x86/include/asm/msr-index.h | 1 + arch/x86/include/uapi/asm/debugreg.h | 1 + arch/x86/kernel/cpu/common.c | 2 +- arch/x86/kernel/cpu/intel.c | 145 +++--- arch/x86/kernel/traps.c | 7 + include/linux/sched/user.h| 4 +- kernel/user.c | 7 + 10 files changed, 176 insertions(+), 30 deletions(-) -- 2.29.1
Re: [PATCH] stop_machine: Mark functions as notrace
On Fri, 30 Oct 2020 14:47:56 -0700 Atish Patra wrote: > > Look at arm64, they __kprobes flag and I guess it would also prevent > > ftrace call site. > > > > Are you sure about that ? __kprobes puts the code in .kprobes.text section > which is under whitelist sections in recordmcount.pl & recordmcount.c. Correct, ftrace can trace functions marked with __kprobes. That said, the instruction you are looking at here, is in a file that is blacklisted from recordmcount. CFLAGS_REMOVE_insn.o = $(CC_FLAGS_FTRACE) All ftrace flags are removed from the compiling of insn.c, and every function in that file will not be traced. -- Steve
[PATCH RFC v3 2/4] x86/bus_lock: Handle warn and fatal in #DB for bus lock
#DB for bus lock is enabled by bus lock detection bit 2 in DEBUGCTL MSR while #AC for split lock is enabled by split lock detection bit 29 in TEST_CTRL MSR. Delivery of #DB for bus lock in userspace clears DR6[11]. To avoid confusion in identifying #DB, #DB handler sets the bit to 1 before returning to the interrupted task. Use the existing kernel command line option "split_lock_detect=" to handle #DB for bus lock: split_lock_detect= #AC for split lock #DB for bus lock off Do nothing Do nothing warnKernel OOPs Warn once per task and Warn once per task and and continues to run. disable future checking When both features are supported, warn in #DB fatal Kernel OOPs Send SIGBUS to user Send SIGBUS to user When both features are supported, fatal in #AC. Default option is "warn". Hardware only generates #DB for bus lock detect when CPL>0 to avoid nested #DB from multiple bus locks while the first #DB is being handled. So no need to handle #DB for bus lock detected in the kernel. Signed-off-by: Fenghua Yu Reviewed-by: Tony Luck --- Change Log: RFC v3: - Remove DR6_RESERVED change (PeterZ). arch/x86/include/asm/cpu.h | 10 ++- arch/x86/include/asm/msr-index.h | 1 + arch/x86/include/uapi/asm/debugreg.h | 1 + arch/x86/kernel/cpu/common.c | 2 +- arch/x86/kernel/cpu/intel.c | 114 ++- arch/x86/kernel/traps.c | 7 ++ 6 files changed, 113 insertions(+), 22 deletions(-) diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h index da78ccbd493b..e74de9fccc0b 100644 --- a/arch/x86/include/asm/cpu.h +++ b/arch/x86/include/asm/cpu.h @@ -41,12 +41,13 @@ unsigned int x86_family(unsigned int sig); unsigned int x86_model(unsigned int sig); unsigned int x86_stepping(unsigned int sig); #ifdef CONFIG_CPU_SUP_INTEL -extern void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c); +extern void __init sld_setup(struct cpuinfo_x86 *c); extern void switch_to_sld(unsigned long tifn); extern bool handle_user_split_lock(struct pt_regs *regs, long error_code); extern bool handle_guest_split_lock(unsigned long ip); +extern bool handle_bus_lock(struct pt_regs *regs); #else -static inline void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c) {} +static inline void __init sld_setup(struct cpuinfo_x86 *c) {} static inline void switch_to_sld(unsigned long tifn) {} static inline bool handle_user_split_lock(struct pt_regs *regs, long error_code) { @@ -57,6 +58,11 @@ static inline bool handle_guest_split_lock(unsigned long ip) { return false; } + +static inline bool handle_bus_lock(struct pt_regs *regs) +{ + return false; +} #endif #ifdef CONFIG_IA32_FEAT_CTL void init_ia32_feat_ctl(struct cpuinfo_x86 *c); diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h index 972a34d93505..62f7534e0855 100644 --- a/arch/x86/include/asm/msr-index.h +++ b/arch/x86/include/asm/msr-index.h @@ -264,6 +264,7 @@ #define DEBUGCTLMSR_LBR(1UL << 0) /* last branch recording */ #define DEBUGCTLMSR_BTF_SHIFT 1 #define DEBUGCTLMSR_BTF(1UL << 1) /* single-step on branches */ +#define DEBUGCTLMSR_BUS_LOCK_DETECT(1UL << 2) /* Bus lock detect */ #define DEBUGCTLMSR_TR (1UL << 6) #define DEBUGCTLMSR_BTS(1UL << 7) #define DEBUGCTLMSR_BTINT (1UL << 8) diff --git a/arch/x86/include/uapi/asm/debugreg.h b/arch/x86/include/uapi/asm/debugreg.h index d95d080b30e3..0007ba077c0c 100644 --- a/arch/x86/include/uapi/asm/debugreg.h +++ b/arch/x86/include/uapi/asm/debugreg.h @@ -24,6 +24,7 @@ #define DR_TRAP3 (0x8) /* db3 */ #define DR_TRAP_BITS (DR_TRAP0|DR_TRAP1|DR_TRAP2|DR_TRAP3) +#define DR_BUS_LOCK(0x800) /* bus_lock */ #define DR_STEP(0x4000)/* single-step */ #define DR_SWITCH (0x8000)/* task switch */ diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index 35ad8480c464..92705ac301ec 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -1327,7 +1327,7 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c) cpu_set_bug_bits(c); - cpu_set_core_cap_bits(c); + sld_setup(c); fpu__init_system(c); diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index 59a1e3ce3f14..3aa57199484b 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -44,11 +44,15 @@ enum split_lock_detect_state { /* * Default to sld_off because most systems do not support split lock detection - * split_lock_setup() will switch this to sld_warn on systems
[PATCH RFC v3 3/4] x86/bus_lock: Set rate limit for bus lock
To enforce user application throttling or mitigations, extend the existing split lock detect kernel parameter: split_lock_detect=ratelimit:N It limits bus lock rate to N per second for non root users. Signed-off-by: Fenghua Yu Reviewed-by: Tony Luck --- arch/x86/kernel/cpu/intel.c | 37 - include/linux/sched/user.h | 4 +++- kernel/user.c | 7 +++ 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index 3aa57199484b..6dcc7e404f8b 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -10,6 +10,9 @@ #include #include #include +#include +#include +#include #include #include @@ -40,6 +43,7 @@ enum split_lock_detect_state { sld_off = 0, sld_warn, sld_fatal, + sld_ratelimit, }; /* @@ -998,13 +1002,25 @@ static const struct { { "off",sld_off }, { "warn", sld_warn }, { "fatal", sld_fatal }, + { "ratelimit:", sld_ratelimit }, }; static inline bool match_option(const char *arg, int arglen, const char *opt) { - int len = strlen(opt); - return len == arglen && !strncmp(arg, opt, len); + int len = strlen(opt), ratelimit; + + if (strncmp(arg, opt, len)) + return false; + + if (sscanf(arg, "ratelimit:%d", ) == 1 && ratelimit > 0 && + ratelimit_bl <= HZ / 2) { + ratelimit_bl = ratelimit; + + return true; + } + + return len == arglen; } static bool split_lock_verify_msr(bool on) @@ -1084,8 +1100,8 @@ static void sld_update_msr(bool on) static void split_lock_init(void) { - /* If supported, #DB for bus lock will handle warn. */ - if (bld && sld_state == sld_warn) + /* If supported, #DB for bus lock will handle warn and ratelimit. */ + if (bld && (sld_state == sld_warn || sld_state == sld_ratelimit)) return; if (cpu_model_supports_sld) @@ -1142,7 +1158,8 @@ static void bus_lock_init(void) bool handle_user_split_lock(struct pt_regs *regs, long error_code) { - if ((regs->flags & X86_EFLAGS_AC) || !sld || sld_state == sld_fatal) + if ((regs->flags & X86_EFLAGS_AC) || !sld || sld_state == sld_fatal || + sld_state == sld_ratelimit) return false; split_lock_warn(regs->ip); return true; @@ -1156,6 +1173,11 @@ bool handle_bus_lock(struct pt_regs *regs) pr_warn_ratelimited("#DB: %s/%d took a bus_lock trap at address: 0x%lx\n", current->comm, current->pid, regs->ip); + if (sld_state == sld_ratelimit) { + while (!__ratelimit(_current_user()->ratelimit_bl)) + msleep(1000 / ratelimit_bl); + } + return true; } @@ -1251,6 +1273,11 @@ static void sld_state_show(void) else pr_info("#DB: sending SIGBUS on user-space bus_locks\n"); break; + + case sld_ratelimit: + if (bld) + pr_info("#DB: setting rate limit to %d/sec per user on non root user-space bus_locks\n", ratelimit_bl); + break; } } diff --git a/include/linux/sched/user.h b/include/linux/sched/user.h index a8ec3b6093fc..79f95002a123 100644 --- a/include/linux/sched/user.h +++ b/include/linux/sched/user.h @@ -40,8 +40,9 @@ struct user_struct { atomic_t nr_watches;/* The number of watches this user currently has */ #endif - /* Miscellaneous per-user rate limit */ + /* Miscellaneous per-user rate limits */ struct ratelimit_state ratelimit; + struct ratelimit_state ratelimit_bl; }; extern int uids_sysfs_init(void); @@ -51,6 +52,7 @@ extern struct user_struct *find_user(kuid_t); extern struct user_struct root_user; #define INIT_USER (_user) +extern int ratelimit_bl; /* per-UID process charging. */ extern struct user_struct * alloc_uid(kuid_t); diff --git a/kernel/user.c b/kernel/user.c index b1635d94a1f2..023dad617625 100644 --- a/kernel/user.c +++ b/kernel/user.c @@ -103,6 +103,7 @@ struct user_struct root_user = { .locked_shm = 0, .uid= GLOBAL_ROOT_UID, .ratelimit = RATELIMIT_STATE_INIT(root_user.ratelimit, 0, 0), + .ratelimit_bl = RATELIMIT_STATE_INIT(root_user.ratelimit_bl, 0, 0), }; /* @@ -172,6 +173,9 @@ void free_uid(struct user_struct *up) free_user(up, flags); } +/* Architectures (e.g. X86) may set this for rate limited bus locks. */ +int ratelimit_bl; + struct user_struct *alloc_uid(kuid_t uid) { struct hlist_head *hashent = uidhashentry(uid); @@ -190,6 +194,9 @@ struct user_struct *alloc_uid(kuid_t uid) refcount_set(>__count, 1); ratelimit_state_init(>ratelimit, HZ, 100); ratelimit_set_flags(>ratelimit,
[PATCH RFC v3 1/4] x86/cpufeatures: Enumerate #DB for bus lock detection
A bus lock is acquired either through split locked access to writeback (WB) memory or by using locks to uncacheable (UC) memory (e.g. direct device assignment). This is typically >1000 cycles slower than an atomic operation within a cache line. It also disrupts performance on other cores. Some CPUs have ability to notify the kernel by an #DB trap after a user instruction acquires a bus lock and is executed. This allows the kernel to enforce user application throttling or mitigations. The CPU feature flag to be shown in /proc/cpuinfo will be "bus_lock_detect". Signed-off-by: Fenghua Yu Reviewed-by: Tony Luck --- arch/x86/include/asm/cpufeatures.h | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h index dad350d42ecf..f375d9cb8123 100644 --- a/arch/x86/include/asm/cpufeatures.h +++ b/arch/x86/include/asm/cpufeatures.h @@ -352,6 +352,7 @@ #define X86_FEATURE_AVX512_VPOPCNTDQ (16*32+14) /* POPCNT for vectors of DW/QW */ #define X86_FEATURE_LA57 (16*32+16) /* 5-level page tables */ #define X86_FEATURE_RDPID (16*32+22) /* RDPID instruction */ +#define X86_FEATURE_BUS_LOCK_DETECT(16*32+24) /* Bus Lock detect */ #define X86_FEATURE_CLDEMOTE (16*32+25) /* CLDEMOTE instruction */ #define X86_FEATURE_MOVDIRI(16*32+27) /* MOVDIRI instruction */ #define X86_FEATURE_MOVDIR64B (16*32+28) /* MOVDIR64B instruction */ -- 2.29.1
[PATCH RFC v3 4/4] Documentation/admin-guide: Change doc for split_lock_detect parameter
Since #DB for bus lock detect changes the split_lock_detect parameter, update the documentation for the changes. Signed-off-by: Fenghua Yu Reviewed-by: Tony Luck --- Change Log: - Simplify the documentation (Randy). .../admin-guide/kernel-parameters.txt | 28 +++ 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 526d65d8573a..ee419ce659f5 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -5044,27 +5044,45 @@ spia_peddr= split_lock_detect= - [X86] Enable split lock detection + [X86] Enable split lock detection or bus lock detection When enabled (and if hardware support is present), atomic instructions that access data across cache line - boundaries will result in an alignment check exception. + boundaries will result in an alignment check exception + for split lock detection or a debug exception for + bus lock detection. off - not enabled warn- the kernel will emit rate limited warnings about applications triggering the #AC - exception. This mode is the default on CPUs - that supports split lock detection. + exception or the #DB exception. This mode is + the default on CPUs that supports split lock + detection or bus lock detection. Default + behavior is from #DB if both features are + enabled in hardware. fatal - the kernel will send SIGBUS to applications - that trigger the #AC exception. + that trigger the #AC exception or the #DB + exception. Default behavior is from #AC + if both features are enabled in hardware. + + ratelimit:N - + Set rate limit to N bus locks per second + for bus lock detection. 0 < N <= HZ/2 and + N is approximate. Only applied to non root + user. + + N/A for split lock detection. If an #AC exception is hit in the kernel or in firmware (i.e. not while executing in user mode) the kernel will oops in either "warn" or "fatal" mode. + #DB exception for bus lock is triggered only when + CPL > 0. + srbds= [X86,INTEL] Control the Special Register Buffer Data Sampling (SRBDS) mitigation. -- 2.29.1
Re: [PATCH v2 net 0/5] net: ipa: minor bug fixes
On Thu, 29 Oct 2020 11:50:52 -0500 Alex Elder wrote: > On 10/29/20 11:11 AM, Jakub Kicinski wrote: > > On Wed, 28 Oct 2020 14:41:43 -0500 Alex Elder wrote: > >> This series fixes several bugs. They are minor, in that the code > >> currently works on supported platforms even without these patches > >> applied, but they're bugs nevertheless and should be fixed. > > > > By which you mean "it seems to work just fine most of the time" or "the > > current code does not exercise this paths/functionally these bugs don't > > matter for current platforms". > > The latter, although for patch 3 I'm not 100% sure. > > Case by case: > Patch 1: >It works. I inquired what the consequence of passing this >wrong buffer pointer was, and for the way we are using IPA >it seems it's fine--the memory pointer we were assigning is >not used, so it's OK. But we're assigning the wrong pointer. > Patch 2: >It works. Even though the bit field is 1 bit wide (not two) >we never actually write a value greater than 1, so we don't >cause a problem. But the definition is incorrect. > Patch 3: >It works, but on the SDM845 we should be assigning the endpoints >to use resource group 1 (they are 0 by default). The way we >currently use this upstream we don't have other endpoints >competing for resources, so I think this is fine. SC7180 we >will assign endpoints to resource group 0, which is the default. > Patch 4: >It works. This is like patch 2; we define the number of these >things incorrectly, but the way we currently use them we never >exceed the limit in a broken way. > Patch 5: >It works. The maximum number of supported groups is even, >and if a (smaller) odd number are used the remainder are >programmed with 0, which is appropriate for undefined >fields. > > If you have any concerns about back-porting these fixes I > think I'm comfortable posting them for net-next instead. > I debated that before sending them out. Please request that > if it's what you think would be best. Looks like these patches apply cleanly to net-next, so I put them there. Thanks!
Re: [PATCH net-next v4] net: dec: tulip: de2104x: Add shutdown handler to stop NIC
On Wed, 28 Oct 2020 10:21:25 -0700 Moritz Fischer wrote: > The driver does not implement a shutdown handler which leads to issues > when using kexec in certain scenarios. The NIC keeps on fetching > descriptors which gets flagged by the IOMMU with errors like this: > > DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 > DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 > DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 > DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 > DMAR: DMAR:[DMA read] Request device [5e:00.0]fault addr f000 > > Signed-off-by: Moritz Fischer Applied, thanks!
Re: [PATCH v2 5/5] firmware: QCOM_SCM: Allow qcom_scm driver to be loadable as a permenent module
On Fri, Oct 30, 2020 at 7:12 AM Robin Murphy wrote: > On 2020-10-30 01:02, John Stultz wrote: > > On Wed, Oct 28, 2020 at 7:51 AM Robin Murphy wrote: > >> Hmm, perhaps I'm missing something here, but even if the config options > >> *do* line up, what prevents arm-smmu probing before qcom-scm and > >> dereferencing NULL in qcom_scm_qsmmu500_wait_safe_toggle() before __scm > >> is initialised? > > > > Oh man, this spun me on a "wait, but how does it all work!" trip. :) > > > > So in the non-module case, the qcom_scm driver is a subsys_initcall > > and the arm-smmu is a module_platform_driver, so the ordering works > > out. > > > > In the module case, the arm-smmu code isn't loaded until the qcom_scm > > driver finishes probing due to the symbol dependency handling. > > My point is that module load != driver probe. AFAICS you could disable > drivers_autoprobe, load both modules, bind the SMMU to its driver first, > and boom! > > > To double check this, I added a big msleep at the top of the > > qcom_scm_probe to try to open the race window you described, but the > > arm_smmu_device_probe() doesn't run until after qcom_scm_probe > > completes. > > I don't think asynchronous probing is enabled by default, so indeed I > would expect that to still happen to work ;) > > > So at least as a built in / built in, or a module/module case its ok. > > And in the case where arm-smmu is a module and qcom_scm is built in > > that's ok too. > > In the built-in case, I'm sure it happens to work out similarly because > the order of nodes in the DTB tends to be the order in which devices are > autoprobed. Again, async probe might be enough to break things; manually > binding drivers definitely should; moving the firmware node to the end > of the DTB probably would as well. So, with modules, I turned on async probing for the two drivers, as well as moved the firmware node to the end of the dtb, and couldn't manage to trip it up even with a 6 second delay in the qcom_scm probe function. It was only when doing the same with everything built in that I did manage to trigger the issue. So yes, you're right! And it is an issue more easily tripped with everything built in statically (and not really connected to this patch series). > > Its just the case my patch is trying to prevent is where arm-smmu is > > built in, but qcom_scm is a module that it can't work (due to build > > errors in missing symbols, or if we tried to use function pointers to > > plug in the qcom_scm - the lack of initialization ordering). > > > > Hopefully that addresses your concern? Let me know if I'm still > > missing something. > > What I was dancing around is that the SCM API (and/or its users) appears > to need a general way to tell whether SCM is ready or not, because the > initialisation ordering problem is there anyway. Neither Kconfig nor the > module loader can solve that. Hrm... There is qcom_scm_is_available(). I tried adding a check for that in qcom_smmu_impl_init() and return EPROBE_DEFER if it fails, but I then hit a separate crash (tripping in iommu_group_remove_device on a null dev->iommu_group value). But after adding a check for that, it seems to be working... I'll try to spin up a separate patch here for that in a second. thanks -john
Re: [PATCH] fix for potential NULL pointer dereference with bare lan743x
On Thu, 29 Oct 2020 03:28:45 +0300 Sergej Bauer wrote: > This is just a minor fix which prevents a kernel NULL pointer > dereference when using phy-less lan743x. > > Signed-off-by: Sergej Bauer I take you mean when the device is down netdev->phydev will be NULL? > diff --git a/drivers/net/ethernet/microchip/lan743x_ethtool.c > b/drivers/net/ethernet/microchip/lan743x_ethtool.c > index dcde496da7fb..354d72d550f2 100644 > --- a/drivers/net/ethernet/microchip/lan743x_ethtool.c > +++ b/drivers/net/ethernet/microchip/lan743x_ethtool.c > @@ -793,6 +795,9 @@ static int lan743x_ethtool_set_wol(struct net_device > *netdev, > { > struct lan743x_adapter *adapter = netdev_priv(netdev); > > + if (!netdev->phydev) > + return -EIO; Does it make sense to just skip the phy_ethtool_set_wol() call instead? Also doesn't the wol configuration of the PHY get lost across an netdev up/down cycle in this driver? Should it be re-applied after phy is connected back? > adapter->wolopts = 0; > if (wol->wolopts & WAKE_UCAST) > adapter->wolopts |= WAKE_UCAST;
Re: [PATCH bpf-next 1/5] bpf: Implement task local storage
On Fri, Oct 30, 2020 at 4:07 AM KP Singh wrote: > > " > > On Fri, Oct 30, 2020 at 12:28 AM Song Liu wrote: > > > > On Wed, Oct 28, 2020 at 9:17 AM KP Singh wrote: > > > > > > From: KP Singh > > > > > > Similar to bpf_local_storage for sockets and inodes add local storage > > > for task_struct. > > > > > > The life-cycle of storage is managed with the life-cycle of the > > > task_struct. i.e. the storage is destroyed along with the owning task > > > with a callback to the bpf_task_storage_free from the task_free LSM > > > hook. > > > > It looks like task local storage is tightly coupled to LSM. As we discussed, > > it will be great to use task local storage in tracing programs. Would you > > like to enable it from the beginning? Alternatively, I guess we can also do > > follow-up patches. > > > > I would prefer if we do it in follow-up patches. > > > > > > > The BPF LSM allocates an __rcu pointer to the bpf_local_storage in > > > the security blob which are now stackable and can co-exist with other > > > LSMs. > > > > > > The userspace map operations can be done by using a pid fd as a key > > > passed to the lookup, update and delete operations. > > > > While testing task local storage, I noticed a limitation of pid fd: > > > > /* Currently, the process identified by > > * @pid must be a thread-group leader. This restriction currently exists > > * for all aspects of pidfds including pidfd creation (CLONE_PIDFD cannot > > * be used with CLONE_THREAD) and pidfd polling (only supports thread group > > * leaders). > > */ > > > > This could be a problem for some use cases. How about we try to remove > > this restriction (maybe with a new flag to pidfd_open) as part of this set? > > I would appreciate it if we could also do this in a follow-up patch. > > I do see that there is a comment in fork.c: > > "CLONE_THREAD is blocked until someone really needs it." > > But I don't understand the requirements well enough and would thus prefer > to do this in a follow-up series. Sounds good. Let's work on these in follow-up patches. Thanks, Song
Re: [PATCH v2 0/3] gpiolib: cdev: allow edge event timestamps to be configured as REALTIME
On Fri, Oct 30, 2020 at 03:52:24PM +0100, Bartosz Golaszewski wrote: > On Fri, Oct 30, 2020 at 3:49 PM Bartosz Golaszewski > wrote: > > > > On Thu, Oct 29, 2020 at 12:22 AM Kent Gibson wrote: > > > > > > On Wed, Oct 28, 2020 at 05:01:49PM +0100, Linus Walleij wrote: > > > > On Thu, Oct 15, 2020 at 1:12 AM Kent Gibson > > > > wrote: > > > > > > > > > This patch set adds the option to select CLOCK_REALTIME as the source > > > > > clock for line events. > > > > > > > > > > The first patch is the core of the change, while the remaining two > > > > > update > > > > > the GPIO tools to make use of the new option. > > > > > > > > > > Changes for v2: > > > > > - change line_event_timestamp() return to u64 to avoid clipping to > > > > > 32bits > > > > >on 32bit platforms. > > > > > - fix the line spacing after line_event_timestamp() > > > > > > > > Where are we standing with this patch set? Good to go so > > > > I should just try to merge it? > > > > > > > > > > I'm fine with it, especially now that I've tested it on 32bit platforms > > > as well as 64bit. > > > > > > Bart was ok with v1, and I doubt the changes for v2 would negatively > > > impact that, though I did overlook adding his review tag. > > > > > > Cheers, > > > Kent. > > > > > > > Yours, > > > > Linus Walleij > > > > I'll take it through my tree then. > > > > Bartosz > > The series no longer applies on top of v5.10-rc1. Could you rebase and resend? > Nuts, it relies on my doc tidy-up series that Linus has pulled into fixes, and so will likely go into v5.10-rc2?? Specifically it is based over/conflicts with: 2cc522d3931b gpio: uapi: kernel-doc formatting improvements If I rebase it onto devel then you will get a conflict when those merge. Is that what you want? Cheers, Kent.
[PATCH] perf: increase size of buf in perf_evsel__hists_browse()
Making perf with gcc-9.1.1 generates the following warning: CC ui/browsers/hists.o ui/browsers/hists.c: In function 'perf_evsel__hists_browse': ui/browsers/hists.c:3078:61: error: '%d' directive output may be \ truncated writing between 1 and 11 bytes into a region of size \ between 2 and 12 [-Werror=format-truncation=] 3078 | "Max event group index to sort is %d (index from 0 to %d)", | ^~ ui/browsers/hists.c:3078:7: note: directive argument in the range [-2147483648, 8] 3078 | "Max event group index to sort is %d (index from 0 to %d)", | ^~ In file included from /usr/include/stdio.h:937, from ui/browsers/hists.c:5: IOW, the string in line 3078 might be too long for buf[] of 64 bytes. Fix this by increasing the size of buf[] to 128. Fixes: dbddf1747441 ("perf report/top TUI: Support hotkeys to let user select any event for sorting") Cc: stable # v5.7+ Cc: Jin Yao Cc: Jiri Olsa Cc: Arnaldo Carvalho de Melo Cc: Arnaldo Carvalho de Melo Signed-off-by: Song Liu --- tools/perf/ui/browsers/hists.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c index a07626f072087..b0e1880cf992b 100644 --- a/tools/perf/ui/browsers/hists.c +++ b/tools/perf/ui/browsers/hists.c @@ -2963,7 +2963,7 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events, struct popup_action actions[MAX_OPTIONS]; int nr_options = 0; int key = -1; - char buf[64]; + char buf[128]; int delay_secs = hbt ? hbt->refresh : 0; #define HIST_BROWSER_HELP_COMMON \ -- 2.24.1
Re: [patch V2 00/18] mm/highmem: Preemptible variant of kmap_atomic & friends
On Sat, Oct 31 2020 at 00:26, Thomas Gleixner wrote: > On Fri, Oct 30 2020 at 15:46, Linus Torvalds wrote: >> On Fri, Oct 30, 2020 at 3:26 PM Thomas Gleixner wrote: >> To me, your patch series has two big advantages: >> >> - more common code >> >> - kmap_local() becomes more of a no-op >> >> and the last thing we want is to expand on kmap. > > Happy to go with that. > > While trying to document the mess, I just stumbled over the abuse of > kmap_atomic_prot() in > > drivers/gpu/drm/ttm/ttm_bo_util.c: dst = kmap_atomic_prot(d, prot); > drivers/gpu/drm/ttm/ttm_bo_util.c: src = kmap_atomic_prot(s, prot); > drivers/gpu/drm/vmwgfx/vmwgfx_blit.c: > kmap_atomic_prot(d->dst_pages[dst_page], > drivers/gpu/drm/vmwgfx/vmwgfx_blit.c: > kmap_atomic_prot(d->src_pages[src_page], > > For !HIGHMEM kmap_atomic_prot() just ignores the 'prot' argument and > returns the page address. > > Moar patches to be written ... Sigh! Or not. This is actually correct by some definition of correct. For the non highmem case pgprot is set via the set_memory_*() functions and this just handles the highmem case. Comments are overrrated...
[PATCH v2 4/6] dt-bindings: arm: sunxi: add Elimo bindings
Document board compatible names for Elimo Engineering Impetus and Initium Signed-off-by: Matteo Scordino --- Documentation/devicetree/bindings/arm/sunxi.yaml | 13 + 1 file changed, 13 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml index 0f23133672a3..ef2ce3bd2bed 100644 --- a/Documentation/devicetree/bindings/arm/sunxi.yaml +++ b/Documentation/devicetree/bindings/arm/sunxi.yaml @@ -201,6 +201,19 @@ properties: - const: dserve,dsrv9703c - const: allwinner,sun4i-a10 + - description: Elimo Engineering Impetus SoM +items: + - const: elimo,impetus + - const: sochip,s3 + - const: allwinner,sun8i-v3 + + - description: Elimo Engineering Initium +items: + - const: elimo,initium + - const: elimo,impetus + - const: sochip,s3 + - const: allwinner,sun8i-v3 + - description: Empire Electronix D709 Tablet items: - const: empire-electronix,d709 -- 2.20.1
[PATCH v2 2/6] ARM: dts: sun8i: V3/S3: Add UART1 pin definitions to the V3/S3 dtsi
The Allwinner V3 and S3 can use PG6/7 as RX/TX for UART1. Since no other functions are assigned to those pins, they are a convenient choice for a debugging or application UART. This is specific to V3/S3 as the V3s's non-BGA package did not have those pins. Signed-off-by: Matteo Scordino --- arch/arm/boot/dts/sun8i-v3.dtsi | 5 + 1 file changed, 5 insertions(+) diff --git a/arch/arm/boot/dts/sun8i-v3.dtsi b/arch/arm/boot/dts/sun8i-v3.dtsi index ca4672ed2e02..c279e13583ba 100644 --- a/arch/arm/boot/dts/sun8i-v3.dtsi +++ b/arch/arm/boot/dts/sun8i-v3.dtsi @@ -24,4 +24,9 @@ { compatible = "allwinner,sun8i-v3-pinctrl"; + + uart1_pg_pins: uart1-pg-pins { + pins = "PG6", "PG7"; + function = "uart1"; + }; }; -- 2.20.1
[PATCH v2 0/6] Elimo Impetus and Initium support - rework
Hi, I applied the suggestions from the discussion that stemmed from V1, i.e.: - using "sochip,s3", "allwinner,sun8i-v3" in the compatible string - add a patch to do the same for the Pine64 PineCube - alphabetic order in the dt-binding file - match the dt-binding file with the actual dts files Matteo Scordino (6): dt-bindings: vendors: add Elimo Engineering vendor prefix ARM: dts: sun8i: V3/S3: Add UART1 pin definitions to the V3/S3 dtsi ARM: dts: sun8i: s3: Add dtsi for the Elimo Impetus SoM dt-bindings: arm: sunxi: add Elimo bindings ARM: dts: sun8i: s3: Add dts for the Elimo Initium SBC ARM: dts: sunxi: align pinecube compatible property to other S3 boards .../devicetree/bindings/arm/sunxi.yaml| 13 + .../devicetree/bindings/vendor-prefixes.yaml | 2 + arch/arm/boot/dts/Makefile| 1 + arch/arm/boot/dts/sun8i-s3-elimo-impetus.dtsi | 51 +++ arch/arm/boot/dts/sun8i-s3-elimo-initium.dts | 28 ++ arch/arm/boot/dts/sun8i-s3-pinecube.dts | 2 +- arch/arm/boot/dts/sun8i-v3.dtsi | 5 ++ 7 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 arch/arm/boot/dts/sun8i-s3-elimo-impetus.dtsi create mode 100644 arch/arm/boot/dts/sun8i-s3-elimo-initium.dts -- 2.20.1
[PATCH v2 3/6] ARM: dts: sun8i: s3: Add dtsi for the Elimo Impetus SoM
The Elimo Engineering Impetus is an Open Source Hardware System-on-Module based on the SoChip S3 SoC. It is meant for integration into carrier boards or, more generally, larger designs, and uses an M2 connector to facilitate that. Interfaces on the M.2/NGFF 42mm connector: WiFi IEEE 802. 11abgn (on-module Realtek) Bluetooth 4.2/BLE (on-module Realtek) RGB LCD Interface (on-module connector) MIPI Camera Interface (on-module connector) IEEE 802. 3u Ethernet MAC (external connecto) USB2.0 (Host, Device, OTG) (external connector) Audio Line In/Out (external connector) Signed-off-by: Matteo Scordino --- arch/arm/boot/dts/sun8i-s3-elimo-impetus.dtsi | 51 +++ 1 file changed, 51 insertions(+) create mode 100644 arch/arm/boot/dts/sun8i-s3-elimo-impetus.dtsi diff --git a/arch/arm/boot/dts/sun8i-s3-elimo-impetus.dtsi b/arch/arm/boot/dts/sun8i-s3-elimo-impetus.dtsi new file mode 100644 index ..f219188fc9ba --- /dev/null +++ b/arch/arm/boot/dts/sun8i-s3-elimo-impetus.dtsi @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/* + * Copyright (C) 2020 Matteo Scordino + */ + +/dts-v1/; +#include "sun8i-v3.dtsi" +#include "sunxi-common-regulators.dtsi" + +/ { + model = "Elimo Impetus SoM"; + compatible = "elimo,impetus", "sochip,s3", "allwinner,sun8i-v3"; + + aliases { + serial0 = + serial1 = + }; + + chosen { + stdout-path = "serial0:115200n8"; + }; +}; + + { + broken-cd; + bus-width = <4>; + vmmc-supply = <_vcc3v3>; + status = "okay"; +}; + + { + pinctrl-0 = <_pb_pins>; + pinctrl-names = "default"; + status = "okay"; +}; + + { + pinctrl-0 = <_pg_pins>; + pinctrl-names = "default"; + status = "okay"; +}; + +_otg { + dr_mode = "otg"; + status = "okay"; +}; + + { + usb0_id_det-gpio = < 5 6 GPIO_ACTIVE_HIGH>; + status = "okay"; +}; -- 2.20.1
[PATCH v2 1/6] dt-bindings: vendors: add Elimo Engineering vendor prefix
Add elimo as vendor prefix for dt bindings, since we are adding a dtsi for a SoM and a dts for an SBC Signed-off-by: Matteo Scordino --- Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index 2735be1a8470..b877a3516277 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -319,6 +319,8 @@ patternProperties: description: Elgin S/A. "^elida,.*": description: Shenzhen Elida Technology Co., Ltd. + "^elimo,.*": +description: Elimo Engineering Ltd. "^embest,.*": description: Shenzhen Embest Technology Co., Ltd. "^emlid,.*": -- 2.20.1
[PATCH v2 6/6] ARM: dts: sunxi: align pinecube compatible property to other S3 boards
The compatible string in the Pine64 Pincube dts diverges from the ones used in other S3 based boards, like the LicheePi and the Elimo Impetus and Initium. Discussion on LKML decided the PineCube should align to the others. Signed-off-by: Matteo Scordino --- arch/arm/boot/dts/sun8i-s3-pinecube.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/sun8i-s3-pinecube.dts b/arch/arm/boot/dts/sun8i-s3-pinecube.dts index 9bab6b7f4014..4aa0ee897a0a 100644 --- a/arch/arm/boot/dts/sun8i-s3-pinecube.dts +++ b/arch/arm/boot/dts/sun8i-s3-pinecube.dts @@ -10,7 +10,7 @@ / { model = "PineCube IP Camera"; - compatible = "pine64,pinecube", "allwinner,sun8i-s3"; + compatible = "pine64,pinecube", "sochip,s3", "allwinner,sun8i-v3"; aliases { serial0 = -- 2.20.1
[PATCH v2 5/6] ARM: dts: sun8i: s3: Add dts for the Elimo Initium SBC
The Elimo Engineering Initium is an Open Source Hardware Single Board Computer based on the Elimo Impetus SoM. It is meant as the first development platform for the Impetus, providing convenient access to the peripherals on the Impetus. It provides: USB-C power input UART-to-USB bridge on the USB-C connector, connected to UART1 USB-A connector for USB2.0 (Host, Device, OTG) Audio Line In/Out Pin header to access all signals on the M2 connector of the SoM Signed-off-by: Matteo Scordino --- arch/arm/boot/dts/Makefile | 1 + arch/arm/boot/dts/sun8i-s3-elimo-initium.dts | 28 2 files changed, 29 insertions(+) create mode 100644 arch/arm/boot/dts/sun8i-s3-elimo-initium.dts diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index 4f0adfead547..dcfb8d39c267 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -1210,6 +1210,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \ sun8i-r40-bananapi-m2-ultra.dtb \ sun8i-s3-lichee-zero-plus.dtb \ sun8i-s3-pinecube.dtb \ + sun8i-s3-elimo-initium.dtb \ sun8i-t3-cqa3t-bv3.dtb \ sun8i-v3s-licheepi-zero.dtb \ sun8i-v3s-licheepi-zero-dock.dtb \ diff --git a/arch/arm/boot/dts/sun8i-s3-elimo-initium.dts b/arch/arm/boot/dts/sun8i-s3-elimo-initium.dts new file mode 100644 index ..7677ddc07bf9 --- /dev/null +++ b/arch/arm/boot/dts/sun8i-s3-elimo-initium.dts @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/* + * Copyright (C) 2020 Matteo Scordino + */ + +/dts-v1/; +#include "sun8i-s3-elimo-impetus.dtsi" + +/ { + model = "Elimo Initium"; + compatible = "elimo,initium", "elimo,impetus", "sochip,s3", +"allwinner,sun8i-v3"; + + aliases { + serial0 = + serial1 = + }; + + chosen { + stdout-path = "serial0:115200n8"; + }; +}; + + { + phy-handle = <_mii_phy>; + phy-mode = "mii"; + status = "okay"; +}; -- 2.20.1
Re: [PATCH v4 2/3] bindings: pm8941-misc: Add support for VBUS detection
On Fri, Oct 30, 2020 at 08:36:12AM -0500, Rob Herring wrote: > On Wed, Oct 28, 2020 at 12:18:53AM -0700, Guru Das Srinagesh wrote: > > Add compatible string that adds support for reporting VBUS detection > > status that can be detected via a dedicated PMIC pin. > > > > Signed-off-by: Anirudh Ghayal > > Signed-off-by: Guru Das Srinagesh > > --- > > Documentation/devicetree/bindings/extcon/qcom,pm8941-misc.yaml | 7 ++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/extcon/qcom,pm8941-misc.yaml > > b/Documentation/devicetree/bindings/extcon/qcom,pm8941-misc.yaml > > index e8eea83..15e3749 100644 > > --- a/Documentation/devicetree/bindings/extcon/qcom,pm8941-misc.yaml > > +++ b/Documentation/devicetree/bindings/extcon/qcom,pm8941-misc.yaml > > @@ -15,18 +15,23 @@ description: | > > > > properties: > >compatible: > > +minItems: 1 > > items: > >- const: qcom,pm8941-misc > > + - const: qcom,pmd-vbus-det > > Do you really need another compatible here? Just detect this by having a > 2nd interrupt. Agreed; will drop this bit in the next patchset. > > > > >reg: > > maxItems: 1 > > > >interrupts: > > -maxItems: 1 > > +minItems: 1 > > +maxItems: 2 > > > >interrupt-names: > > +minItems: 1 > > items: > >- const: usb_id > > + - const: usb_vbus > > > > required: > >- compatible > > -- > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > > a Linux Foundation Collaborative Project > >
Re: [PATCH net-next 00/19] net: phy: add support for shared interrupts (part 1)
> > - Every PHY driver gains a .handle_interrupt() implementation that, for > > the most part, would look like below: > > > > irq_status = phy_read(phydev, INTR_STATUS); > > if (irq_status < 0) { > > phy_error(phydev); > > return IRQ_NONE; > > } > > > > if (irq_status == 0) > > Here I have a concern, bits may be set even if the respective interrupt > source isn't enabled. Therefore we may falsely blame a device to have > triggered the interrupt. irq_status should be masked with the actually > enabled irq source bits. Hi Heiner I would say that is a driver implementation detail, for each driver to handle how it needs to handle it. I've seen some hardware where the interrupt status is already masked with the interrupt enabled bits. I've soon other hardware where it is not. For example code, what is listed above is O.K. The real implementation in a driver need knowledge of the hardware. Andrew
Re: [PATCH 1/3] x86/resctrl: Remove superfluous kernfs_get() calls to prevent refcount leak
On Fri, Oct 30, 2020 at 2:45 PM Xiaochen Shen wrote: > > Willem reported growing of kernfs_node_cache entries in slabtop when > repeatedly creating and removing resctrl subdirectories as well as when > repeatedly mounting and unmounting resctrl filesystem. > > On resource group (control as well as monitoring) creation via a mkdir > an extra kernfs_node reference is obtained to ensure that the rdtgroup > structure remains accessible for the rdtgroup_kn_unlock() calls where it > is removed on deletion. The kernfs_node reference count is dropped by > kernfs_put() in rdtgroup_kn_unlock(). > > With the above explaining the need for one kernfs_get()/kernfs_put() > pair in resctrl there are more places where a kernfs_node reference is > obtained without a corresponding release. The excessive amount of > reference count on kernfs nodes will never be dropped to 0 and the > kernfs nodes will never be freed in the call paths of rmdir and umount. > It leads to reference count leak and kernfs_node_cache memory leak. > > Remove the superfluous kernfs_get() calls and expand the existing > comments surrounding the remaining kernfs_get()/kernfs_put() pair that > remains in use. > > Superfluous kernfs_get() calls are removed from two areas: > > (1) In call paths of mount and mkdir, when kernfs nodes for "info", > "mon_groups" and "mon_data" directories and sub-directories are > created, the reference count of newly created kernfs node is set to 1. > But after kernfs_create_dir() returns, superfluous kernfs_get() are > called to take an additional reference. > > (2) kernfs_get() calls in rmdir call paths. > > Cc: sta...@vger.kernel.org > Fixes: 17eafd076291 ("x86/intel_rdt: Split resource group removal in two") > Fixes: 4af4a88e0c92 ("x86/intel_rdt/cqm: Add mount,umount support") > Fixes: f3cbeacaa06e ("x86/intel_rdt/cqm: Add rmdir support") > Fixes: d89b7379015f ("x86/intel_rdt/cqm: Add mon_data") > Fixes: c7d9aac61311 ("x86/intel_rdt/cqm: Add mkdir support for RDT > monitoring") > Fixes: 5dc1d5c6bac2 ("x86/intel_rdt: Simplify info and base file lists") > Fixes: 60cf5e101fd4 ("x86/intel_rdt: Add mkdir to resctrl file system") > Fixes: 4e978d06dedb ("x86/intel_rdt: Add "info" files to resctrl file system") > Reported-by: Willem de Bruijn > Signed-off-by: Xiaochen Shen > Reviewed-by: Reinette Chatre Tested-by: Willem de Bruijn This addresses both kernfs_node_cache slab leaks I previously observed. Thanks for fixing these! for i in {1..10}; do mount -t resctrl resctrl /sys/fs/resctrl; umount /sys/fs/resctrl; done for i in {1..10}; do mkdir /sys/fs/resctrl/task1; rmdir /sys/fs/resctrl/task1; done