[PATCH v4] mips: Do not include hi and lo in clobber list for R6
From: Romain Naour >From [1] "GCC 10 (PR 91233) won't silently allow registers that are not architecturally available to be present in the clobber list anymore, resulting in build failure for mips*r6 targets in form of: ... .../sysdep.h:146:2: error: the register ‘lo’ cannot be clobbered in ‘asm’ for the current target 146 | __asm__ volatile ( \ | ^~~ This is because base R6 ISA doesn't define hi and lo registers w/o DSP extension. This patch provides the alternative clobber list for r6 targets that won't include those registers." Since kernel 5.4 and mips support for generic vDSO [2], the kernel fail to build for mips r6 cpus with gcc 10 for the same reason as glibc. [1] https://sourceware.org/git/?p=glibc.git;a=commit;h=020b2a97bb15f807c0482f0faee2184ed05bcad8 [2] '24640f233b46 ("mips: Add support for generic vDSO")' Signed-off-by: Romain Naour Signed-off-by: Sudip Mukherjee --- v4: [sudip] added macro VDSO_SYSCALL_CLOBBERS and fix checkpatch errors with commit message. v3 Avoid duplicate code (Maciej W. Rozycki) v2 use MIPS_ISA_REV instead of __mips_isa_rev (Alexander Lobakin) I have reused the original patch by Romain and have retained his s-o-b and author name as he is the original author of this patch. I have just added the macro. Build tested with gcc-10.3.1 and gcc-9.3.0. arch/mips/include/asm/vdso/gettimeofday.h | 26 ++- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/arch/mips/include/asm/vdso/gettimeofday.h b/arch/mips/include/asm/vdso/gettimeofday.h index 2203e2d0ae2a..44a45f3fa4b0 100644 --- a/arch/mips/include/asm/vdso/gettimeofday.h +++ b/arch/mips/include/asm/vdso/gettimeofday.h @@ -20,6 +20,12 @@ #define VDSO_HAS_CLOCK_GETRES 1 +#if MIPS_ISA_REV < 6 +#define VDSO_SYSCALL_CLOBBERS "hi", "lo", +#else +#define VDSO_SYSCALL_CLOBBERS +#endif + static __always_inline long gettimeofday_fallback( struct __kernel_old_timeval *_tv, struct timezone *_tz) @@ -35,7 +41,9 @@ static __always_inline long gettimeofday_fallback( : "=r" (ret), "=r" (error) : "r" (tv), "r" (tz), "r" (nr) : "$1", "$3", "$8", "$9", "$10", "$11", "$12", "$13", - "$14", "$15", "$24", "$25", "hi", "lo", "memory"); + "$14", "$15", "$24", "$25", + VDSO_SYSCALL_CLOBBERS + "memory"); return error ? -ret : ret; } @@ -59,7 +67,9 @@ static __always_inline long clock_gettime_fallback( : "=r" (ret), "=r" (error) : "r" (clkid), "r" (ts), "r" (nr) : "$1", "$3", "$8", "$9", "$10", "$11", "$12", "$13", - "$14", "$15", "$24", "$25", "hi", "lo", "memory"); + "$14", "$15", "$24", "$25", + VDSO_SYSCALL_CLOBBERS + "memory"); return error ? -ret : ret; } @@ -83,7 +93,9 @@ static __always_inline int clock_getres_fallback( : "=r" (ret), "=r" (error) : "r" (clkid), "r" (ts), "r" (nr) : "$1", "$3", "$8", "$9", "$10", "$11", "$12", "$13", - "$14", "$15", "$24", "$25", "hi", "lo", "memory"); + "$14", "$15", "$24", "$25", + VDSO_SYSCALL_CLOBBERS + "memory"); return error ? -ret : ret; } @@ -105,7 +117,9 @@ static __always_inline long clock_gettime32_fallback( : "=r" (ret), "=r" (error) : "r" (clkid), "r" (ts), "r" (nr) : "$1", "$3", "$8", "$9", "$10", "$11", "$12", "$13", - "$14", "$15", "$24", "$25", "hi", "lo", "memory"); + "$14", "$15", "$24", "$25", + VDSO_SYSCALL_CLOBBERS + "memory"); return error ? -ret : ret; } @@ -125,7 +139,9 @@ static __always_inline int clock_getres32_fallback( : "=r" (ret), "=r" (error) : "r" (clkid), "r" (ts), "r" (nr) : "$1", "$3", "$8", "$9", "$10", "$11", "$12", "$13", - "$14", "$15", "$24", "$25", "hi", "lo", "memory"); + "$14", "$15", "$24", "$25", + VDSO_SYSCALL_CLOBBERS + "memory"); return error ? -ret : ret; } -- 2.30.2
Re: [PATCH 5.10 000/103] 5.10.32-rc1 review
Hi Greg, On Mon, Apr 19, 2021 at 03:05:11PM +0200, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 5.10.32 release. > There are 103 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Wed, 21 Apr 2021 13:05:09 +. > Anything received after that time might be too late. Build test: mips (gcc version 10.3.1 20210419): 63 configs -> no new failure arm (gcc version 10.3.1 20210419): 105 configs -> no new failure x86_64 (gcc version 10.2.1 20210110): 2 configs -> no failure Boot test: x86_64: Booted on my test laptop. No regression. x86_64: Booted on qemu. No regression. arm: Booted on rpi3b. No regression. Tested-by: Sudip Mukherjee -- Regards Sudip
Re: [PATCH 5.4 00/73] 5.4.114-rc1 review
Hi Greg, On Mon, Apr 19, 2021 at 03:05:51PM +0200, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 5.4.114 release. > There are 73 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Wed, 21 Apr 2021 13:05:09 +. > Anything received after that time might be too late. Build test: mips (gcc version 10.3.1 20210416): 65 configs -> no new failure arm (gcc version 10.3.1 20210416): 107 configs -> no new failure x86_64 (gcc version 10.2.1 20210110): 2 configs -> no failure Boot test: x86_64: Booted on my test laptop. No regression. x86_64: Booted on qemu. No regression. arm: Booted on rpi3b. No regression. Tested-by: Sudip Mukherjee -- Regards Sudip
Re: build failure of malta_qemu_32r6_defconfig
Hi Thomas, On Fri, Apr 9, 2021 at 1:17 PM Thomas Bogendoerfer wrote: > > On Thu, Apr 08, 2021 at 09:42:11AM +0800, YunQiang Su wrote: > > Sudip Mukherjee 于2021年4月8日周四 上午2:26写道: > > > > > > Hi Thomas, > > > > > > I was building v5.10.28 with malta_qemu_32r6_defconfig and noticed that > > > it fails to build, so tried next-20210407 to see if it has been fixed. > > > But linux-next also has the issue with gcc-10. > > > > > > The error is: > > > > > > ./arch/mips/include/asm/vdso/gettimeofday.h: In function > > > '__vdso_clock_gettime': > > > ./arch/mips/include/asm/vdso/gettimeofday.h:103:2: error: the register > > > 'lo' cannot be clobbered in 'asm' for the current target > > > 103 | asm volatile( > > > | ^~~ > > > > this operation try to save lo and hi register, while they are not > > exisiting on r6. > > We are working on figure out a patch for it. > > looks like there is already a patch in patchwork, which just needs > a workup: > > https://patchwork.kernel.org/project/linux-mips/patch/20200801154401.4177009-1-romain.na...@gmail.com/ Looks like there has been no response to it since last 8 months. Do you want me to respin it and send a proper patch? -- Regards Sudip
[PATCH] media: sp887x: drop unneeded assignment
The pointer 'mem' was initialized to 'fw->data' but immediately after that it was assigned 'fw->data + 10'. Lets remove the extra assignement and initialize the pointer to the address its going to use. Signed-off-by: Sudip Mukherjee --- drivers/media/dvb-frontends/sp887x.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/media/dvb-frontends/sp887x.c b/drivers/media/dvb-frontends/sp887x.c index c89a91a3daf4..146e7f2dd3c5 100644 --- a/drivers/media/dvb-frontends/sp887x.c +++ b/drivers/media/dvb-frontends/sp887x.c @@ -140,7 +140,7 @@ static int sp887x_initial_setup (struct dvb_frontend* fe, const struct firmware u8 buf [BLOCKSIZE + 2]; int i; int fw_size = fw->size; - const unsigned char *mem = fw->data; + const unsigned char *mem = fw->data + 10; dprintk("%s\n", __func__); @@ -148,8 +148,6 @@ static int sp887x_initial_setup (struct dvb_frontend* fe, const struct firmware if (fw_size < FW_SIZE + 10) return -ENODEV; - mem = fw->data + 10; - /* soft reset */ sp887x_writereg(state, 0xf1a, 0x000); -- 2.30.2
Re: [PATCH 5.10 00/25] 5.10.31-rc1 review
Hi Greg, On Thu, Apr 15, 2021 at 04:47:54PM +0200, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 5.10.31 release. > There are 25 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Sat, 17 Apr 2021 14:44:01 +. > Anything received after that time might be too late. Build test: mips (gcc version 10.3.1 20210416): 63 configs -> no new failure arm (gcc version 10.3.1 20210416): 105 configs -> no new failure x86_64 (gcc version 10.2.1 20210110): 2 configs -> no failure Boot test: x86_64: Booted on my test laptop. No regression. x86_64: Booted on qemu. No regression. arm: Booted on rpi3b. No regression. Tested-by: Sudip Mukherjee -- Regards Sudip
Re: [PATCH 5.4 00/18] 5.4.113-rc1 review
Hi Greg, On Thu, Apr 15, 2021 at 04:47:53PM +0200, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 5.4.113 release. > There are 18 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Sat, 17 Apr 2021 14:44:01 +. > Anything received after that time might be too late. Build test: mips (gcc version 10.3.1 20210416): 65 configs -> no new failure arm (gcc version 10.3.1 20210416): 107 configs -> no new failure x86_64 (gcc version 10.2.1 20210110): 2 configs -> no failure Boot test: x86_64: Booted on my test laptop. No regression. x86_64: Booted on qemu. No regression. arm: Booted on rpi3b. No regression. Tested-by: Sudip Mukherjee -- Regards Sudip
Re: [PATCH 4.19 00/13] 4.19.188-rc1 review
Hi Greg, On Thu, Apr 15, 2021 at 04:47:49PM +0200, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 4.19.188 release. > There are 13 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Sat, 17 Apr 2021 14:44:01 +. > Anything received after that time might be too late. Build test: mips (gcc version 10.3.1 20210416): 63 configs -> no new failure arm (gcc version 10.3.1 20210416): 116 configs -> no new failure x86_64 (gcc version 10.2.1 20210110): 2 configs -> no failure Boot test: x86_64: Booted on my test laptop. No regression. x86_64: Booted on qemu. No regression. arm: Booted on rpi3b. No regression. Tested-by: Sudip Mukherjee -- Regards Sudip
Re: [PATCH 5.10 000/188] 5.10.30-rc1 review
Hi Greg, On Mon, Apr 12, 2021 at 10:38:34AM +0200, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 5.10.30 release. > There are 188 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Wed, 14 Apr 2021 08:39:44 +. > Anything received after that time might be too late. Build test: (gcc version 10.2.1 20210406) mips: 63 configs -> no new failure arm: 105 configs -> no new failure x86_64: 2 configs -> no failure Boot test: x86_64: Booted on my test laptop. No regression. Tested-by: Sudip Mukherjee -- Regards Sudip
Re: [PATCH 4.19 00/66] 4.19.187-rc1 review
Hi Greg, On Mon, Apr 12, 2021 at 10:40:06AM +0200, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 4.19.187 release. > There are 66 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Wed, 14 Apr 2021 08:39:44 +. > Anything received after that time might be too late. Boot test: x86_64: Booted on my test laptop. No regression. Tested-by: Sudip Mukherjee -- Regards Sudip
Re: [PATCH 5.4 000/111] 5.4.112-rc1 review
Hi Greg, On Mon, Apr 12, 2021 at 10:39:38AM +0200, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 5.4.112 release. > There are 111 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Wed, 14 Apr 2021 08:39:44 +. > Anything received after that time might be too late. Build test: (gcc version 10.2.1 20210406) mips: 65 configs -> no new failure arm: 107 configs -> no new failure x86_64: 2 configs -> no failure Boot test: x86_64: Booted on my test laptop. No regression. Tested-by: Sudip Mukherjee -- Regards Sudip
Re: [PATCH 4.19 52/66] net: sched: bump refcount for new action in ACT replace mode
Hi Greg, On Mon, Apr 12, 2021 at 10:40:58AM +0200, Greg Kroah-Hartman wrote: > From: Kumar Kartikeya Dwivedi > > commit 6855e8213e06efcaf7c02a15e12b1ae64b9a7149 upstream. This has been reverted upstream by: 4ba86128ba07 ("Revert "net: sched: bump refcount for new action in ACT replace mode"") -- Regards Sudip
Re: [PATCH 5.4 097/111] net: sched: bump refcount for new action in ACT replace mode
Hi Greg, On Mon, Apr 12, 2021 at 10:41:15AM +0200, Greg Kroah-Hartman wrote: > From: Kumar Kartikeya Dwivedi > > commit 6855e8213e06efcaf7c02a15e12b1ae64b9a7149 upstream. This has been reverted upstream by: 4ba86128ba07 ("Revert "net: sched: bump refcount for new action in ACT replace mode"") -- Regards Sudip
Re: [PATCH 5.10 00/41] 5.10.29-rc1 review
Hi Greg, On Fri, Apr 09, 2021 at 11:53:22AM +0200, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 5.10.29 release. > There are 41 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Sun, 11 Apr 2021 09:52:52 +. > Anything received after that time might be too late. Build test: mips: 63 configs -> no new failure arm: 105 configs -> no new failure x86_64: 2 configs -> no failure Boot test: x86_64: Booted on my test laptop. No regression. Tested-by: Sudip Mukherjee -- Regards Sudip
Re: [PATCH 5.4 00/23] 5.4.111-rc1 review
Hi Greg, On Fri, Apr 09, 2021 at 11:53:30AM +0200, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 5.4.111 release. > There are 23 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Sun, 11 Apr 2021 09:52:52 +. > Anything received after that time might be too late. Build test: mips: 65 configs -> no new failure arm: 107 configs -> no new failure x86_64: 2 configs -> no failure Boot test: x86_64: Booted on my test laptop. No regression. Tested-by: Sudip Mukherjee -- Regards Sudip
Re: [PATCH 5.10 38/41] bpf, x86: Validate computation of branch displacements for x86-64
Hi Greg, On Fri, Apr 09, 2021 at 11:54:00AM +0200, Greg Kroah-Hartman wrote: > From: Piotr Krysiuk > > commit e4d4d456436bfb2fe412ee2cd489f7658449b098 upstream. And, this one also. I am unable to find this SHA in Linus's tree. -- Regards Sudip
Re: [PATCH 5.10 39/41] bpf, x86: Validate computation of branch displacements for x86-32
Hi Greg, On Fri, Apr 09, 2021 at 11:54:01AM +0200, Greg Kroah-Hartman wrote: > From: Piotr Krysiuk > > commit 26f55a59dc65ff77cd1c4b37991e26497fc68049 upstream. I am not finding this in Linus's tree and even not seeing this change in master branch also. Am I missing something? -- Regards Sudip
build failure of malta_qemu_32r6_defconfig
Hi Thomas, I was building v5.10.28 with malta_qemu_32r6_defconfig and noticed that it fails to build, so tried next-20210407 to see if it has been fixed. But linux-next also has the issue with gcc-10. The error is: ./arch/mips/include/asm/vdso/gettimeofday.h: In function '__vdso_clock_gettime': ./arch/mips/include/asm/vdso/gettimeofday.h:103:2: error: the register 'lo' cannot be clobbered in 'asm' for the current target 103 | asm volatile( | ^~~ ./arch/mips/include/asm/vdso/gettimeofday.h: In function '__vdso_gettimeofday': ./arch/mips/include/asm/vdso/gettimeofday.h:33:2: error: the register 'lo' cannot be clobbered in 'asm' for the current target 33 | asm volatile( | ^~~ ./arch/mips/include/asm/vdso/gettimeofday.h: In function '__vdso_clock_getres': ./arch/mips/include/asm/vdso/gettimeofday.h:123:2: error: the register 'lo' cannot be clobbered in 'asm' for the current target 123 | asm volatile( | ^~~ ./arch/mips/include/asm/vdso/gettimeofday.h: In function '__vdso_clock_gettime64': ./arch/mips/include/asm/vdso/gettimeofday.h:57:2: error: the register 'lo' cannot be clobbered in 'asm' for the current target 57 | asm volatile( | ^~~ Any idea how to fix this? Please ignore if it has been reported before. -- Regards Sudip
Re: [PATCH 5.10 000/126] 5.10.28-rc1 review
Hi Greg, On Mon, Apr 5, 2021 at 10:09 AM Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 5.10.28 release. > There are 126 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Wed, 07 Apr 2021 08:50:09 +. > Anything received after that time might be too late. Booted on my test laptop. No regression. Tested-by: Sudip Mukherjee -- Regards Sudip
Re: [PATCH v2 1/3] parport: Introduce module_parport_driver() helper macro
On Mon, Mar 1, 2021 at 12:12 PM Andy Shevchenko wrote: > > On Sun, Feb 28, 2021 at 11:27:10PM +, Sudip Mukherjee wrote: > > On Fri, Feb 26, 2021 at 07:03:09PM +0200, Andy Shevchenko wrote: > > > On Tue, Feb 16, 2021 at 01:07:39PM +0200, Andy Shevchenko wrote: > > > > Introduce module_parport_driver() helper macro to reduce boilerplate > > > > in the existing and new code. > > > > > > Sudip, any comments on this? > > > > Sorry for the delay in reply. > > lgtm. I think there are few more drivers which can also use this new helper. > > Will you like to do them also? > > Yes, that's the (slow going) plan. > > > Acked-by: Sudip Mukherjee > > Thanks! I considered that you take it thru parport tree. Do you have something > else in mind? I dont have a separate parport tree (parport is not under active development to have a separate tree). I send everything parport related to Greg and then he adds it to his tree. I guess in this case if Greg agrees it can go via Mark's tree as the series will also have SPI related patches. -- Regards Sudip
Re: [PATCH v2 1/3] parport: Introduce module_parport_driver() helper macro
Hi Andy, On Fri, Feb 26, 2021 at 07:03:09PM +0200, Andy Shevchenko wrote: > On Tue, Feb 16, 2021 at 01:07:39PM +0200, Andy Shevchenko wrote: > > Introduce module_parport_driver() helper macro to reduce boilerplate > > in the existing and new code. > > Sudip, any comments on this? Sorry for the delay in reply. lgtm. I think there are few more drivers which can also use this new helper. Will you like to do them also? Acked-by: Sudip Mukherjee -- Regards Sudip
Re: [PATCH] Documentation: process: Correct numbering
Hi Milan, On Tue, Dec 15, 2020 at 07:50:35PM +, Milan Lakhani wrote: > Renumber the steps in submit-checklist.rst as some numbers were skipped. > > Signed-off-by: Milan Lakhani Maybe you can also add: Fixes: 72deb455b5ec ("block: remove CONFIG_LBDAF") But I am confused about why you have added Greg and staging list instead of 'linux-...@vger.kernel.org'. -- Regards Sudip
Re: [linux-safety] [PATCH] PCI: tegra: Use PTR_ERR_OR_ZERO
On 16/11/2020 17:01, Thierry Reding wrote: > On Mon, Nov 16, 2020 at 04:54:07PM +0000, Sudip Mukherjee wrote: >> Coccinelle suggested using PTR_ERR_OR_ZERO() and looking at the code, >> we can use PTR_ERR_OR_ZERO() instead of checking IS_ERR() and then >> doing 'return 0'. >> >> Signed-off-by: Sudip Mukherjee >> --- >> drivers/pci/controller/pci-tegra.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) > > This has been proposed multiple times in the past and Bjorn and I have > agreed every time that this is not an improvement, so sorry, but NAK. Thanks Thierry and Neil. I have now added a blacklist script in our CI so "PTR_ERR_OR_ZERO" will not be flagged for anything in drivers/pci/* anymore in our testing. -- Regards Sudip
Re: linux-next: Tree for Nov 20
Hi, On Fri, Nov 20, 2020 at 5:59 AM Stephen Rothwell wrote: > > Hi all, > > Changes since 20201119: mips allmodconfig fails for next-20201120 with the error: /home/sudip/linux/drivers/video/fbdev/udlfb.c: In function 'dlfb_ops_mmap': /home/sudip/linux/drivers/video/fbdev/udlfb.c:343:52: error: 'PAGE_SHARED' undeclared (first use in this function) 343 | if (remap_pfn_range(vma, start, page, PAGE_SIZE, PAGE_SHARED)) Which has been caused by 0df162e1377a ("MIPS: mm: Clean up setup of protection map") which removed "PAGE_SHARED". -- Regards Sudip
Re: [PATCH 5.4 140/203] MIPS: PCI: remember nasid changed by set interrupt affinity
Hi Greg, On Fri, Jan 17, 2020 at 12:17:37AM +0100, Greg Kroah-Hartman wrote: > From: Thomas Bogendoerfer > > commit 37640adbefd66491cb8083a438f7bf366ac09bc7 upstream. > > > --- > arch/mips/pci/pci-xtalk-bridge.c |5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > --- a/arch/mips/pci/pci-xtalk-bridge.c > +++ b/arch/mips/pci/pci-xtalk-bridge.c > @@ -279,16 +279,15 @@ static int bridge_set_affinity(struct ir > struct bridge_irq_chip_data *data = d->chip_data; > int bit = d->parent_data->hwirq; > int pin = d->hwirq; > - nasid_t nasid; > int ret, cpu; > > ret = irq_chip_set_affinity_parent(d, mask, force); > if (ret >= 0) { > cpu = cpumask_first_and(mask, cpu_online_mask); > - nasid = COMPACT_TO_NASID_NODEID(cpu_to_node(cpu)); > + data->nnasid = COMPACT_TO_NASID_NODEID(cpu_to_node(cpu)); This will be 'data->nasid' and its causing mips builds to fail. -- Regards Sudip
[PATCH] rtc: pcf8523: Use PTR_ERR_OR_ZERO
Coccinelle suggested using PTR_ERR_OR_ZERO() and looking at the code, we can use PTR_ERR_OR_ZERO() instead of checking IS_ERR() and then doing 'return 0'. Signed-off-by: Sudip Mukherjee --- drivers/rtc/rtc-pcf8523.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/rtc/rtc-pcf8523.c b/drivers/rtc/rtc-pcf8523.c index 57d351dfe272..94f182599438 100644 --- a/drivers/rtc/rtc-pcf8523.c +++ b/drivers/rtc/rtc-pcf8523.c @@ -358,10 +358,8 @@ static int pcf8523_probe(struct i2c_client *client, rtc = devm_rtc_device_register(&client->dev, DRIVER_NAME, &pcf8523_rtc_ops, THIS_MODULE); - if (IS_ERR(rtc)) - return PTR_ERR(rtc); - return 0; + return PTR_ERR_OR_ZERO(rtc); } static const struct i2c_device_id pcf8523_id[] = { -- 2.11.0
[PATCH] PCI: tegra: Use PTR_ERR_OR_ZERO
Coccinelle suggested using PTR_ERR_OR_ZERO() and looking at the code, we can use PTR_ERR_OR_ZERO() instead of checking IS_ERR() and then doing 'return 0'. Signed-off-by: Sudip Mukherjee --- drivers/pci/controller/pci-tegra.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c index 8fcabed7c6a6..4c52b2d58645 100644 --- a/drivers/pci/controller/pci-tegra.c +++ b/drivers/pci/controller/pci-tegra.c @@ -1308,10 +1308,8 @@ static int tegra_pcie_resets_get(struct tegra_pcie *pcie) return PTR_ERR(pcie->afi_rst); pcie->pcie_xrst = devm_reset_control_get_exclusive(dev, "pcie_x"); - if (IS_ERR(pcie->pcie_xrst)) - return PTR_ERR(pcie->pcie_xrst); - return 0; + return PTR_ERR_OR_ZERO(pcie->pcie_xrst); } static int tegra_pcie_phys_get_legacy(struct tegra_pcie *pcie) -- 2.11.0
[PATCH] PCI: dwc: kirin: Use PTR_ERR_OR_ZERO
Coccinelle suggested using PTR_ERR_OR_ZERO() and looking at the code, we can use PTR_ERR_OR_ZERO() instead of checking IS_ERR() and then doing 'return 0'. Signed-off-by: Sudip Mukherjee --- drivers/pci/controller/dwc/pcie-kirin.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c index d0a6a2dee6f5..cf1379a8b950 100644 --- a/drivers/pci/controller/dwc/pcie-kirin.c +++ b/drivers/pci/controller/dwc/pcie-kirin.c @@ -138,10 +138,8 @@ static long kirin_pcie_get_clk(struct kirin_pcie *kirin_pcie, return PTR_ERR(kirin_pcie->apb_sys_clk); kirin_pcie->pcie_aclk = devm_clk_get(dev, "pcie_aclk"); - if (IS_ERR(kirin_pcie->pcie_aclk)) - return PTR_ERR(kirin_pcie->pcie_aclk); - return 0; + return PTR_ERR_OR_ZERO(kirin_pcie->pcie_aclk); } static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie, @@ -169,10 +167,8 @@ static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie, kirin_pcie->sysctrl = syscon_regmap_lookup_by_compatible("hisilicon,hi3660-sctrl"); - if (IS_ERR(kirin_pcie->sysctrl)) - return PTR_ERR(kirin_pcie->sysctrl); - return 0; + return PTR_ERR_OR_ZERO(kirin_pcie->sysctrl); } static int kirin_pcie_phy_init(struct kirin_pcie *kirin_pcie) -- 2.11.0
[PATCH] PCI: dwc/meson: Use PTR_ERR_OR_ZERO
Coccinelle suggested using PTR_ERR_OR_ZERO() and looking at the code, we can use PTR_ERR_OR_ZERO() instead of checking IS_ERR() and then doing 'return 0'. Signed-off-by: Sudip Mukherjee --- drivers/pci/controller/dwc/pci-meson.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c index 1913dc2c8fa0..f4261f5aceb1 100644 --- a/drivers/pci/controller/dwc/pci-meson.c +++ b/drivers/pci/controller/dwc/pci-meson.c @@ -115,10 +115,8 @@ static int meson_pcie_get_mems(struct platform_device *pdev, return PTR_ERR(pci->dbi_base); mp->cfg_base = devm_platform_ioremap_resource_byname(pdev, "cfg"); - if (IS_ERR(mp->cfg_base)) - return PTR_ERR(mp->cfg_base); - return 0; + return PTR_ERR_OR_ZERO(mp->cfg_base); } static int meson_pcie_power_on(struct meson_pcie *mp) @@ -208,10 +206,8 @@ static int meson_pcie_probe_clocks(struct meson_pcie *mp) return PTR_ERR(res->general_clk); res->clk = meson_pcie_probe_clock(dev, "pclk", 0); - if (IS_ERR(res->clk)) - return PTR_ERR(res->clk); - return 0; + return PTR_ERR_OR_ZERO(res->clk); } static inline u32 meson_cfg_readl(struct meson_pcie *mp, u32 reg) -- 2.11.0
[PATCH] mm: fix build failure with xtensa
If CONFIG_ZSMALLOC is enabled with xtensa then the build fails with: mm/zsmalloc.c:43:10: fatal error: asm/sparsemem.h: No such file or directory Disable CONFIG_ZSMALLOC for xtensa as xtensa arch has not defined sparsemem.h. Signed-off-by: Sudip Mukherjee --- Build failed with next-20201110. Build log at https://travis-ci.org/github/sudipm-mukherjee/linux-next/jobs/742793855#L13375 mm/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/Kconfig b/mm/Kconfig index e8587f6bf29a..5b9426ba5e6a 100644 --- a/mm/Kconfig +++ b/mm/Kconfig @@ -698,7 +698,7 @@ config Z3FOLD config ZSMALLOC tristate "Memory allocator for compressed pages" - depends on MMU + depends on (MMU && !XTENSA) help zsmalloc is a slab-based memory allocator designed to store compressed RAM pages. zsmalloc uses virtual memory mapping -- 2.11.0
Re: [PATCH RESEND] driver core: export device_is_bound() to fix build failure
Hi Aisheng, On Mon, Nov 9, 2020 at 10:57 AM Aisheng Dong wrote: > > Hi Greg, > > > From: Greg Kroah-Hartman > > Sent: Monday, November 9, 2020 6:37 PM > > Subject: Re: [PATCH RESEND] driver core: export device_is_bound() to fix > > build > > failure > > > > On Mon, Nov 09, 2020 at 10:14:46AM +, Sudip Mukherjee wrote: > > > Hi Greg, > > > > > > On Sun, Nov 8, 2020 at 8:23 AM Greg Kroah-Hartman > > > wrote: > > > > > > > > On Sat, Nov 07, 2020 at 10:47:27PM +, Sudip Mukherjee wrote: > > > > > When CONFIG_MXC_CLK_SCU is configured as 'm' the build fails as it > > > > > is unable to find device_is_bound(). The error being: > > > > > ERROR: modpost: "device_is_bound" [drivers/clk/imx/clk-imx-scu.ko] > > > > > undefined! > > > > > > > > > > Export the symbol so that the module finds it. > > > > > > > > > probe() should never call this function as it makes no sense at all at that > > point in > > time. The driver should be fixed. > > Would you suggest if any other API we can use to allow the driver to know > whether > another device has been probed? > > For imx scu driver in question, it has a special requirement that it depends > on scu power domain > driver. However, there're a huge number (200+) of power domains for each > device clock, we can't define > them all in DT for a single clock controller node. > > That's why we wanted to use device_is_bound() before to check if scu power > domain is ready or not to > support defer probe. iiuc, you are waiting for "fsl,scu-pd" to be registered. I think you might be able to use bus_for_each_dev() to check if the device has registered with the bus or not. It will be on the bus only after bind was successful. The bus will be "platform_bus_type". But I am sure Greg can give you better suggestion than this. :) -- Regards Sudip
Re: [PATCH RESEND] driver core: export device_is_bound() to fix build failure
Hi Greg, On Sun, Nov 8, 2020 at 8:23 AM Greg Kroah-Hartman wrote: > > On Sat, Nov 07, 2020 at 10:47:27PM +, Sudip Mukherjee wrote: > > When CONFIG_MXC_CLK_SCU is configured as 'm' the build fails as it > > is unable to find device_is_bound(). The error being: > > ERROR: modpost: "device_is_bound" [drivers/clk/imx/clk-imx-scu.ko] > > undefined! > > > > Export the symbol so that the module finds it. > > > > Fixes: 77d8f3068c63 ("clk: imx: scu: add two cells binding support") > > Signed-off-by: Sudip Mukherjee > > --- > > > > resending with the Fixes: tag. > > > > drivers/base/dd.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/base/dd.c b/drivers/base/dd.c > > index 148e81969e04..a796a57e5efb 100644 > > --- a/drivers/base/dd.c > > +++ b/drivers/base/dd.c > > @@ -353,6 +353,7 @@ bool device_is_bound(struct device *dev) > > { > > return dev->p && klist_node_attached(&dev->p->knode_driver); > > } > > +EXPORT_SYMBOL(device_is_bound); > > EXPORT_SYMBOL_GPL() please, like all the other exports in this file. > > Also, wait, no, don't call this, are you sure you are calling it in a > race-free way? And what branch/tree is the above commit in? I have not checked fully but since it is being called from probe() I assume the lock will be held at that time. This is from linux-next and the original commit is in "for-next" branch of git://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git repo. I came across this problem while doing my daily allmodconfig builds. The build log is at https://travis-ci.com/github/sudipm-mukherjee/linux-test/jobs/429382451#L24431 -- Regards Sudip
[PATCH RESEND] driver core: export device_is_bound() to fix build failure
When CONFIG_MXC_CLK_SCU is configured as 'm' the build fails as it is unable to find device_is_bound(). The error being: ERROR: modpost: "device_is_bound" [drivers/clk/imx/clk-imx-scu.ko] undefined! Export the symbol so that the module finds it. Fixes: 77d8f3068c63 ("clk: imx: scu: add two cells binding support") Signed-off-by: Sudip Mukherjee --- resending with the Fixes: tag. drivers/base/dd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 148e81969e04..a796a57e5efb 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -353,6 +353,7 @@ bool device_is_bound(struct device *dev) { return dev->p && klist_node_attached(&dev->p->knode_driver); } +EXPORT_SYMBOL(device_is_bound); static void driver_bound(struct device *dev) { -- 2.11.0
[PATCH] driver core: export device_is_bound() to fix build failure
When CONFIG_MXC_CLK_SCU is configured as 'm' the build fails as it is unable to find device_is_bound(). The error being: ERROR: modpost: "device_is_bound" [drivers/clk/imx/clk-imx-scu.ko] undefined! Export the symbol so that the module finds it. Signed-off-by: Sudip Mukherjee --- drivers/base/dd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 148e81969e04..a796a57e5efb 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -353,6 +353,7 @@ bool device_is_bound(struct device *dev) { return dev->p && klist_node_attached(&dev->p->knode_driver); } +EXPORT_SYMBOL(device_is_bound); static void driver_bound(struct device *dev) { -- 2.11.0
Re: [linux-safety] [PATCH] taskstats: remove unneeded dead assignment
Hi Lukas, On 06/11/2020 10:31, Lukas Bulwahn wrote: > > > On Fri, 6 Nov 2020, Sudip Mukherjee wrote: > >> Hi Lukas, >> > > I did not try but I bet (a beverage of your choice) that the object code > remains the same also for your suggested patch. Try to disprove my claim > and possibly earn yourself a beverage when we meet... Lets decide which beverage.. ;-) Using gcc-7.2.0 for MIPS: original:- ab81d3305d578c2568fbc73aad2f9e61 kernel/taskstats.o After your change:- ab81d3305d578c2568fbc73aad2f9e61 kernel/taskstats.o After my change:- 0acae2c8d72abd3e15bf805fccdca711 kernel/taskstats.o -- Regards Sudip
Re: [linux-safety] [PATCH] taskstats: remove unneeded dead assignment
Hi Lukas, On 06/11/2020 06:22, Lukas Bulwahn wrote: > make clang-analyzer on x86_64 defconfig caught my attention with: > > kernel/taskstats.c:120:2: warning: Value stored to 'rc' is never read \ > [clang-analyzer-deadcode.DeadStores] > rc = 0; > ^ > > Commit d94a041519f3 ("taskstats: free skb, avoid returns in > send_cpu_listeners") made send_cpu_listeners() not return a value and > hence, the rc variable remained only to be used within the loop where > it is always assigned before read and it does not need any other > initialisation. > > So, simply remove this unneeded dead initializing assignment. Might be better to remove 'rc' completely as it is only used for the if condition now. diff --git a/kernel/taskstats.c b/kernel/taskstats.c index a2802b6ff4bb..63541f1ae04a 100644 --- a/kernel/taskstats.c +++ b/kernel/taskstats.c @@ -113,11 +113,10 @@ static void send_cpu_listeners(struct sk_buff *skb, struct listener *s, *tmp; struct sk_buff *skb_next, *skb_cur = skb; void *reply = genlmsg_data(genlhdr); - int rc, delcount = 0; + int delcount = 0; genlmsg_end(skb, reply); - rc = 0; down_read(&listeners->sem); list_for_each_entry(s, &listeners->list, list) { skb_next = NULL; @@ -126,8 +125,8 @@ static void send_cpu_listeners(struct sk_buff *skb, if (!skb_next) break; } - rc = genlmsg_unicast(&init_net, skb_cur, s->pid); - if (rc == -ECONNREFUSED) { + if (genlmsg_unicast(&init_net, skb_cur, s->pid) == + -ECONNREFUSED) { s->valid = 0; delcount++; } -- Regards Sudip
[PATCH] ASoC: mediatek: mt8192: Fix build failure
A build of arm64 allmodconfig with next-20201105 fails with the error: ERROR: modpost: "mt8192_afe_gpio_request" undefined! ERROR: modpost: "mt8192_afe_gpio_init" undefined! Export the symbols so that mt8192-mt6359-rt1015-rt5682.ko finds it. Signed-off-by: Sudip Mukherjee --- build log at: https://travis-ci.com/github/sudipm-mukherjee/linux-test/jobs/428486008 sound/soc/mediatek/mt8192/mt8192-afe-gpio.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sound/soc/mediatek/mt8192/mt8192-afe-gpio.c b/sound/soc/mediatek/mt8192/mt8192-afe-gpio.c index ea000888c9e8..fbbe9ed9adb3 100644 --- a/sound/soc/mediatek/mt8192/mt8192-afe-gpio.c +++ b/sound/soc/mediatek/mt8192/mt8192-afe-gpio.c @@ -160,6 +160,7 @@ int mt8192_afe_gpio_init(struct device *dev) return 0; } +EXPORT_SYMBOL(mt8192_afe_gpio_init); static int mt8192_afe_gpio_adda_dl(struct device *dev, bool enable) { @@ -304,3 +305,4 @@ int mt8192_afe_gpio_request(struct device *dev, bool enable, return 0; } +EXPORT_SYMBOL(mt8192_afe_gpio_request); -- 2.11.0
[PATCH] misc: hisi_hikey_usb: use PTR_ERR_OR_ZERO
Coccinelle suggested using PTR_ERR_OR_ZERO() and looking at the code, we can use PTR_ERR_OR_ZERO() instead of checking IS_ERR() and then doing 'return 0'. Signed-off-by: Sudip Mukherjee --- drivers/misc/hisi_hikey_usb.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/misc/hisi_hikey_usb.c b/drivers/misc/hisi_hikey_usb.c index cc93569e601c..989d7d129469 100644 --- a/drivers/misc/hisi_hikey_usb.c +++ b/drivers/misc/hisi_hikey_usb.c @@ -168,10 +168,7 @@ static int hisi_hikey_usb_parse_kirin970(struct platform_device *pdev, hisi_hikey_usb->reset = devm_gpiod_get(&pdev->dev, "hub_reset_en_gpio", GPIOD_OUT_HIGH); - if (IS_ERR(hisi_hikey_usb->reset)) - return PTR_ERR(hisi_hikey_usb->reset); - - return 0; + return PTR_ERR_OR_ZERO(hisi_hikey_usb->reset); } static int hisi_hikey_usb_probe(struct platform_device *pdev) -- 2.11.0
Re: [PATCH] libtraceevent: install html files
Hi Steve, On Mon, Oct 19, 2020 at 2:42 PM Steven Rostedt wrote: > > On Sun, 18 Oct 2020 22:19:12 +0100 > Sudip Mukherjee wrote: > > > Only the man pages were installed using "make install". Add rules to > > install html files also. > > > > Signed-off-by: Sudip Mukherjee > > --- > > tools/lib/traceevent/Documentation/Makefile | 14 -- > > Thanks Sudip, > > Although, to apply it to the libtraceevent.git repo, I had to strip off the > "tools/lib/traceevent/" from the file paths. I was actually confused about which repo is to be followed for development purposes. I assumed all patches will land here and you will then mirror them to libtraceevent.git repo when you decide to make a release. I can send you a patch for libtraceevent.git repo if you want. -- Regards Sudip
[PATCH] libtraceevent: install html files
Only the man pages were installed using "make install". Add rules to install html files also. Signed-off-by: Sudip Mukherjee --- tools/lib/traceevent/Documentation/Makefile | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tools/lib/traceevent/Documentation/Makefile b/tools/lib/traceevent/Documentation/Makefile index aa72ab96c3c1..dd3d62b17140 100644 --- a/tools/lib/traceevent/Documentation/Makefile +++ b/tools/lib/traceevent/Documentation/Makefile @@ -147,7 +147,7 @@ html: $(MAN_HTML) $(MAN_HTML) $(DOC_MAN3): asciidoc.conf -install: install-man +install: install-man install-html check-man-tools: ifdef missing_tools @@ -161,12 +161,22 @@ do-install-man: man install-man: check-man-tools man do-install-man -uninstall: uninstall-man +do-install-html: html + $(call QUIET_INSTALL, Documentation-html) \ + $(INSTALL) -d -m 755 $(DESTDIR)$(htmldir); \ + $(INSTALL) -m 644 $(OUTPUT)*.html $(DESTDIR)$(htmldir); + +install-html: check-man-tools html do-install-html + +uninstall: uninstall-man uninstall-html uninstall-man: $(call QUIET_UNINST, Documentation-man) \ $(Q)$(RM) $(addprefix $(DESTDIR)$(man3dir)/,$(DOC_MAN3)) +uninstall-html: + $(call QUIET_UNINST, Documentation-html) \ + $(Q)$(RM) $(addprefix $(DESTDIR)$(htmldir)/,$(MAN_HTML)) ifdef missing_tools DO_INSTALL_MAN = $(warning Please install $(missing_tools) to have the man pages installed) -- 2.11.0
Re: [ANNOUNCE] libtraceevent.git
Hi Steve, On Tue, Oct 13, 2020 at 2:02 PM Steven Rostedt wrote: > > On Tue, 13 Oct 2020 11:06:16 +0800 > Zamir SUN wrote: > > > On Tue, Oct 13, 2020 at 3:17 AM Steven Rostedt wrote: > > > > > So, for me, there is no more issue for Fedora packaging. > > > > So should I just add that one patch and tag it? Just a thought, if you see https://repology.org/project/linux-tools/versions then you will notice that libtracevent has been packaged by the distros with a version of v5.x+, and I will have the same problem for Debian also. Do you think it makes sense to start with a version of v6.x when you tag it? If that is not possible then we will have to use epoch like we did for libbpf. -- Regards Sudip
Re: [linux-safety] [PATCH] usb: host: ehci-sched: add comment about find_tt() not returning error
Hi Lukas, On Tue, Oct 13, 2020 at 6:37 AM Lukas Bulwahn wrote: > > > > On Tue, 13 Oct 2020, Greg Kroah-Hartman wrote: > > > On Mon, Oct 12, 2020 at 08:25:30PM +0200, Lukas Bulwahn wrote: > > > > > > > > > On Mon, 12 Oct 2020, Greg Kroah-Hartman wrote: > > > > > > > On Mon, Oct 12, 2020 at 05:10:21PM +0200, Lukas Bulwahn wrote: > > > > > And for the static analysis finding, we need to find a way to ignore > > > > > this > > > > > finding without simply ignoring all findings or new findings that just > > > > > look very similar to the original finding, but which are valid. > > > > > > > > Why not fix the things that it finds that are actually issues? If there > > are no actual issues found, then perhaps you should use a better tool? :) > > > > Completely agree. That is why I was against adding comments here and > elsewhere just to have the "good feeling of doing something" after the > tool reported a warning and we spend some time understanding the code to > conclude that we now understand the code better than the tool. I think you are missing the point here. I sent the comment not because of any tool, I sent it because the code there was not that simple like other drivers and at a first glance its not apparent why there are no error checks. And, afaik, the only purpose of comments is to make the source code easier to understand. -- Regards Sudip
Re: [ANNOUNCE] libtraceevent.git
Hi Steve, On Mon, Oct 12, 2020 at 8:17 PM Steven Rostedt wrote: > > > [ Removing the powertop mailing list because it's rejecting everything ] > > On Mon, 12 Oct 2020 11:41:20 -0700 > Tony Jones wrote: > > > On Mon, Oct 12, 2020 at 11:19:50AM -0400, Steven Rostedt wrote: > > > > > Once it's shown that it works for all the package maintainers, I will tag > > > it which should create the tarballs automatically on the above link. > > Works for me. > Can you see if this patch fixes your current issue? > > -- Steve > > diff --git a/Documentation/Makefile b/Documentation/Makefile > index edb8623..3a981be 100644 > --- a/Documentation/Makefile > +++ b/Documentation/Makefile > @@ -157,7 +157,7 @@ endif > do-install-man: man > $(call QUIET_INSTALL, Documentation-man) \ > $(INSTALL) -d -m 755 $(DESTDIR)$(man3dir); \ > - $(INSTALL) -m 644 $(DOC_MAN3) $(DESTDIR)$(man3dir); > + $(INSTALL) -m 644 $(OUTPUT)*.3 $(DESTDIR)$(man3dir); > > install-man: check-man-tools man do-install-man > I faced the same problem and this patch worked for me. -- Regards Sudip
Re: [ANNOUNCE] libtraceevent.git
On Mon, Oct 12, 2020 at 4:19 PM Steven Rostedt wrote: > > On Mon, 12 Oct 2020 12:12:08 +0200 > Jiri Olsa wrote: > > > On Wed, Oct 07, 2020 at 01:07:50PM -0400, Steven Rostedt wrote: > > > I split out tools/lib/traceevent from the kernel tree using "git subtree", > > > which recreates all the commits of a directory and makes that directory a > > > stand alone. I then updated the Makefiles, and copied over some of the > > > header files used to build the library. I pushed this up to: > > > > > > https://git.kernel.org/pub/scm/libs/libtrace/libtraceevent.git/ > > > > > > My hope is that this will now be the source of all updates to the > > > libtraceevent library that can be used as a stand alone package that both > > > perf and tracecmd can use. I would also like powertop and rasdaemon to use > > > this as well. > > > > hi, > > I'm adding this as fedora package, is there a source arhive somewhere > > in git.kernel.org for libtraceevent that spec could download? > > > > Hi Jiri! > > Once it's shown that it works for all the package maintainers, I will tag > it which should create the tarballs automatically on the above link. But I > wanted to fix all the packaging bugs before doing so. I hope this doesn't > make it into a catch-22. Where you can't package till there's a source > tarball, but I can't make a source tarball until I know you can package > it ;-) For Debian I have raised https://bugs.debian.org/971976 but I will package it locally today just to check there is no packaging bugs for our packaging. -- Regards Sudip
Re: [linux-safety] [PATCH] usb: host: ehci-sched: add comment about find_tt() not returning error
Hi Lukas, On Mon, Oct 12, 2020 at 3:11 PM Lukas Bulwahn wrote: > > > > On Sun, 11 Oct 2020, Sudip Mukherjee wrote: > > > Add a comment explaining why find_tt() will not return error even though > > find_tt() is checking for NULL and other errors. > > > > Signed-off-by: Sudip Mukherjee > > I get the intent of the comment but there is a large risk that somebody > could remove the find_tt() call before the calling the function and there > is no chance to then see later that the comment is actually wrong. Removing the find_tt() will mean a major rework of the code. > > I guess you want to link this comment here to a code line above and > request anyone touching the line above to reconsider the comment then. > > But there is no 'concept' for such kind of requests to changes and > comments. > > So, the comment is true now, but might be true or wrong later. If it is wrong later due to some code change then I guess someone will need to send a patch to correct the comment. > > I am wondering if such comment deserves to be included if we cannot check > its validity later... I am failing to understand why will you not be able to check its validity later. You just need to read the code to check it. > > I would prefer a simple tool that could check that your basic assumption > on the code is valid and if it the basic assumption is still valid, > just shut up any stupid tool that simply does not get that these calls > here cannot return any error. > > We encountered this issue because of clang analyzer complaining, but it is > clear that it is a false positive of that (smart but) incomplete tool. I dont think it is a false positive. The error return value is not checked and that is true. Only that it is not applicable because of the way the coding is done. > > Do you intend to add comment for all false positives from all tools or are > we going to find a better solution than that? I think tools will always give you some false positives and you will need to read the code to know if its false positive or not. I dont think there is any tool yet which will only give true positives. -- Regards Sudip
[PATCH] e1000: drop unneeded assignment in e1000_set_itr()
The variable 'current_itr' is assigned to 0 before jumping to 'set_itr_now' but it has not been used after the jump. So, remove the unneeded assignement. Signed-off-by: Sudip Mukherjee --- drivers/net/ethernet/intel/e1000/e1000_main.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c index 5e28cf4fa2cd..042de276e632 100644 --- a/drivers/net/ethernet/intel/e1000/e1000_main.c +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c @@ -2632,7 +2632,6 @@ static void e1000_set_itr(struct e1000_adapter *adapter) /* for non-gigabit speeds, just fix the interrupt rate at 4000 */ if (unlikely(adapter->link_speed != SPEED_1000)) { - current_itr = 0; new_itr = 4000; goto set_itr_now; } -- 2.11.0
[PATCH] usb: host: ehci-sched: add comment about find_tt() not returning error
Add a comment explaining why find_tt() will not return error even though find_tt() is checking for NULL and other errors. Signed-off-by: Sudip Mukherjee --- drivers/usb/host/ehci-sched.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c index 6dfb242f9a4b..0f85aa9b2fb1 100644 --- a/drivers/usb/host/ehci-sched.c +++ b/drivers/usb/host/ehci-sched.c @@ -244,6 +244,12 @@ static void reserve_release_intr_bandwidth(struct ehci_hcd *ehci, /* FS/LS bus bandwidth */ if (tt_usecs) { + /* +* find_tt() will not return any error here as we have +* already called find_tt() before calling this function +* and checked for any error return. The previous call +* would have created the data structure. +*/ tt = find_tt(qh->ps.udev); if (sign > 0) list_add_tail(&qh->ps.ps_list, &tt->ps_list); @@ -1337,6 +1343,12 @@ static void reserve_release_iso_bandwidth(struct ehci_hcd *ehci, } } + /* +* find_tt() will not return any error here as we have +* already called find_tt() before calling this function +* and checked for any error return. The previous call +* would have created the data structure. +*/ tt = find_tt(stream->ps.udev); if (sign > 0) list_add_tail(&stream->ps.ps_list, &tt->ps_list); -- 2.11.0
Re: [PATCH] char: ppdev: check if ioctl argument is present and valid
On Sat, Oct 10, 2020 at 1:08 AM Sudip Mukherjee wrote: > > On Fri, Oct 9, 2020 at 5:57 AM Greg KH wrote: > > > > On Thu, Oct 08, 2020 at 11:57:13PM +0530, Harshal Chaudhari wrote: > > > Checking the argument passed to the ioctl is valid > > > or not. if not then return -EINVAL. > > > > Along the the comments that Arnd made, this is not the correct value to > > be returning from an ioctl when you don't pass in the correct command. > > > > And it doesn't match what your patch says, please be consistent. > > > > And do you have this device to be able to test your changes? > > I will test this tomorrow. But from an initial look, its going to > break ppdev. There are few ioctls which don't need any arguments. No, sorry. I missed the check for _IOC_NONE. Tested on a desktop which has a parallel port with a very basic test code of open->claim->write->release->close and it still works. -- Regards Sudip
Re: [PATCH] char: ppdev: check if ioctl argument is present and valid
On Fri, Oct 9, 2020 at 5:57 AM Greg KH wrote: > > On Thu, Oct 08, 2020 at 11:57:13PM +0530, Harshal Chaudhari wrote: > > Checking the argument passed to the ioctl is valid > > or not. if not then return -EINVAL. > > Along the the comments that Arnd made, this is not the correct value to > be returning from an ioctl when you don't pass in the correct command. > > And it doesn't match what your patch says, please be consistent. > > And do you have this device to be able to test your changes? I will test this tomorrow. But from an initial look, its going to break ppdev. There are few ioctls which don't need any arguments. -- Regards Sudip
[PATCH v2] kernel/sysctl.c: drop unneeded assignment in proc_do_large_bitmap()
The variable 'first' is assigned 0 inside the while loop in the if block but it is not used in the if block and is only used in the else block. So, remove the unneeded assignment and move the variable in the else block. Signed-off-by: Sudip Mukherjee --- v1: only had the removal of assignment The resultant binary stayed same after this change. Verified with md5sum which remained same with and without this change. $ md5sum kernel/sysctl.o 77e8b8f3cd9da4446e7f117115c8ba84 kernel/sysctl.o kernel/sysctl.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/kernel/sysctl.c b/kernel/sysctl.c index ce75c67572b9..cc274a431d91 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -1423,7 +1423,6 @@ int proc_do_large_bitmap(struct ctl_table *table, int write, void *buffer, size_t *lenp, loff_t *ppos) { int err = 0; - bool first = 1; size_t left = *lenp; unsigned long bitmap_len = table->maxlen; unsigned long *bitmap = *(unsigned long **) table->data; @@ -1508,12 +1507,12 @@ int proc_do_large_bitmap(struct ctl_table *table, int write, } bitmap_set(tmp_bitmap, val_a, val_b - val_a + 1); - first = 0; proc_skip_char(&p, &left, '\n'); } left += skipped; } else { unsigned long bit_a, bit_b = 0; + bool first = 1; while (left) { bit_a = find_next_bit(bitmap, bitmap_len, bit_b); -- 2.11.0
[PATCH] usb: host: ehci-sched: avoid possible NULL dereference
find_tt() can return NULL or the error value in ERR_PTR() and dereferencing the return value without checking for the error can lead to a possible dereference of NULL pointer or ERR_PTR(). Signed-off-by: Sudip Mukherjee --- drivers/usb/host/ehci-sched.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c index 6dfb242f9a4b..f3fd7e9fe6b2 100644 --- a/drivers/usb/host/ehci-sched.c +++ b/drivers/usb/host/ehci-sched.c @@ -245,6 +245,8 @@ static void reserve_release_intr_bandwidth(struct ehci_hcd *ehci, /* FS/LS bus bandwidth */ if (tt_usecs) { tt = find_tt(qh->ps.udev); + if (IS_ERR_OR_NULL(tt)) + return; if (sign > 0) list_add_tail(&qh->ps.ps_list, &tt->ps_list); else @@ -1338,6 +1340,8 @@ static void reserve_release_iso_bandwidth(struct ehci_hcd *ehci, } tt = find_tt(stream->ps.udev); + if (IS_ERR_OR_NULL(tt)) + return; if (sign > 0) list_add_tail(&stream->ps.ps_list, &tt->ps_list); else -- 2.11.0
[PATCH] kernel/relay.c: drop unneeded initialization
The variable 'consumed' is initialized with the consumed count but immediately after that the consumed count is updated and assigned to 'consumed' again thus overwriting the previous value. So, drop the unneeded initialization. Signed-off-by: Sudip Mukherjee --- The resultant binary stayed same after this change. Verified with md5sum which remained same with and without this change. $ md5sum kernel/relay.o 20854215a46e241520576a5d3c523073 kernel/relay.o kernel/relay.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/relay.c b/kernel/relay.c index fb4e0c530c08..b08d936d5fa7 100644 --- a/kernel/relay.c +++ b/kernel/relay.c @@ -1002,7 +1002,7 @@ static int relay_file_read_avail(struct rchan_buf *buf) size_t subbuf_size = buf->chan->subbuf_size; size_t n_subbufs = buf->chan->n_subbufs; size_t produced = buf->subbufs_produced; - size_t consumed = buf->subbufs_consumed; + size_t consumed; relay_file_read_consume(buf, 0, 0); -- 2.11.0
[PATCH] kernel/sysctl.c: drop unneeded assignment in proc_do_large_bitmap()
The variable 'first' is assigned 0 inside the while loop in the if block but it is not used in the if block and is only used in the else block. So, remove the unneeded assignment. Signed-off-by: Sudip Mukherjee --- The resultant binary stayed same after this change. Verified with md5sum which remained same with and without this change. $ md5sum kernel/sysctl.o e9e97adbfd3f0b32f83dd35d100c0e4e kernel/sysctl.o kernel/sysctl.c | 1 - 1 file changed, 1 deletion(-) diff --git a/kernel/sysctl.c b/kernel/sysctl.c index ce75c67572b9..b51ebfd1ba6e 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -1508,7 +1508,6 @@ int proc_do_large_bitmap(struct ctl_table *table, int write, } bitmap_set(tmp_bitmap, val_a, val_b - val_a + 1); - first = 0; proc_skip_char(&p, &left, '\n'); } left += skipped; -- 2.11.0
[PATCH] proc: remove a pointless assignment
The variable 'env_start' has only been used for the if condition before this assignment and is never read after this. So, remove the assignement. Signed-off-by: Sudip Mukherjee --- fs/proc/base.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/proc/base.c b/fs/proc/base.c index aa69c35d904c..238925ff3a80 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -279,7 +279,7 @@ static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf, * only when it overflows into the environment area. */ if (env_start != arg_end || env_end < env_start) - env_start = env_end = arg_end; + env_end = arg_end; len = env_end - arg_start; /* We're not going to care if "*ppos" has high bits set */ -- 2.11.0
[PATCH] tracing: remove a pointless assignment
The variable 'len' has been assigned a value but is not used after that. So, remove the assignement. Signed-off-by: Sudip Mukherjee --- kernel/trace/trace.c | 1 - 1 file changed, 1 deletion(-) diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index d3c75aab44ad..e13cb3baeff0 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -6662,7 +6662,6 @@ tracing_mark_write(struct file *filp, const char __user *ubuf, written = -EFAULT; } else written = cnt; - len = cnt; if (tr->trace_marker_file && !list_empty(&tr->trace_marker_file->triggers)) { /* do not add \n before testing triggers, but add \0 */ -- 2.11.0
[PATCH] nfs: use 'break' instead of 'fallthrough'
The fallthrough attribute should only be used in a switch statement, after a preceding statement and before a logically succeeding case label, or user-defined label. But here, it is used in the last 'switch-case' with no other 'case' where it can fallthrough. So, use break instead of fallthrough. Fixes: cf65e49f89f2 ("nfs: Convert to use the preferred fallthrough macro") Signed-off-by: Sudip Mukherjee --- fs/nfs/super.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/nfs/super.c b/fs/nfs/super.c index d20326ee0475..eb2401079b04 100644 --- a/fs/nfs/super.c +++ b/fs/nfs/super.c @@ -889,7 +889,7 @@ static struct nfs_server *nfs_try_mount_request(struct fs_context *fc) default: if (rpcauth_get_gssinfo(flavor, &info) != 0) continue; - fallthrough; + break; } dfprintk(MOUNT, "NFS: attempting to use auth flavor %u\n", flavor); ctx->selected_flavor = flavor; -- 2.11.0
[PATCH] mmc: block: remove unused variable
The use of 'status' was removed but the variable itself was not removed and thus adding a build warning. Fixes: 05224f7e4975 ("mmc: block: Add CMD13 polling for MMC IOCTLS with R1B response") Signed-off-by: Sudip Mukherjee --- drivers/mmc/core/block.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index ee1fd7df4ec8..95b41c0891d0 100644 --- a/drivers/mmc/core/block.c +++ b/drivers/mmc/core/block.c @@ -497,7 +497,6 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md, struct scatterlist sg; int err; unsigned int target_part; - u32 status = 0; if (!card || !md || !idata) return -EINVAL; -- 2.11.0
[PATCH] tty: serial: samsung: rename to fix build warning
The build of arm allmodconfig gives a warning: warning: same module names found: drivers/tty/serial/samsung.ko drivers/mtd/nand/onenand/samsung.ko Rename drivers/tty/serial/samsung.c to drivers/tty/serial/samsung_tty.c to fix the warning. Signed-off-by: Sudip Mukherjee --- drivers/tty/serial/Makefile | 2 +- drivers/tty/serial/{samsung.c => samsung_tty.c} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename drivers/tty/serial/{samsung.c => samsung_tty.c} (100%) diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile index 863f47056539..d056ee6cca33 100644 --- a/drivers/tty/serial/Makefile +++ b/drivers/tty/serial/Makefile @@ -30,7 +30,7 @@ obj-$(CONFIG_SERIAL_PXA_NON8250) += pxa.o obj-$(CONFIG_SERIAL_PNX8XXX) += pnx8xxx_uart.o obj-$(CONFIG_SERIAL_SA1100) += sa1100.o obj-$(CONFIG_SERIAL_BCM63XX) += bcm63xx_uart.o -obj-$(CONFIG_SERIAL_SAMSUNG) += samsung.o +obj-$(CONFIG_SERIAL_SAMSUNG) += samsung_tty.o obj-$(CONFIG_SERIAL_MAX3100) += max3100.o obj-$(CONFIG_SERIAL_MAX310X) += max310x.o obj-$(CONFIG_SERIAL_IP22_ZILOG) += ip22zilog.o diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung_tty.c similarity index 100% rename from drivers/tty/serial/samsung.c rename to drivers/tty/serial/samsung_tty.c -- 2.11.0
[PATCH] omapfb: reduce stack usage
The build of xtensa allmodconfig is giving a warning of: In function 'dsi_dump_dsidev_irqs': warning: the frame size of 1120 bytes is larger than 1024 bytes Allocate the memory for 'struct dsi_irq_stats' dynamically instead of assigning it in stack. Signed-off-by: Sudip Mukherjee --- drivers/video/fbdev/omap2/omapfb/dss/dsi.c | 24 ++-- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dsi.c b/drivers/video/fbdev/omap2/omapfb/dss/dsi.c index d620376216e1..43402467bf40 100644 --- a/drivers/video/fbdev/omap2/omapfb/dss/dsi.c +++ b/drivers/video/fbdev/omap2/omapfb/dss/dsi.c @@ -1536,22 +1536,25 @@ static void dsi_dump_dsidev_irqs(struct platform_device *dsidev, { struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev); unsigned long flags; - struct dsi_irq_stats stats; + struct dsi_irq_stats *stats; + stats = kmalloc(sizeof(*stats), GFP_KERNEL); + if (!stats) + return; spin_lock_irqsave(&dsi->irq_stats_lock, flags); - stats = dsi->irq_stats; + memcpy(stats, &dsi->irq_stats, sizeof(*stats)); memset(&dsi->irq_stats, 0, sizeof(dsi->irq_stats)); dsi->irq_stats.last_reset = jiffies; spin_unlock_irqrestore(&dsi->irq_stats_lock, flags); seq_printf(s, "period %u ms\n", - jiffies_to_msecs(jiffies - stats.last_reset)); + jiffies_to_msecs(jiffies - stats->last_reset)); - seq_printf(s, "irqs %d\n", stats.irq_count); + seq_printf(s, "irqs %d\n", stats->irq_count); #define PIS(x) \ - seq_printf(s, "%-20s %10d\n", #x, stats.dsi_irqs[ffs(DSI_IRQ_##x)-1]); + seq_printf(s, "%-20s %10d\n", #x, stats->dsi_irqs[ffs(DSI_IRQ_##x)-1]); seq_printf(s, "-- DSI%d interrupts --\n", dsi->module_id + 1); PIS(VC0); @@ -1575,10 +1578,10 @@ static void dsi_dump_dsidev_irqs(struct platform_device *dsidev, #define PIS(x) \ seq_printf(s, "%-20s %10d %10d %10d %10d\n", #x, \ - stats.vc_irqs[0][ffs(DSI_VC_IRQ_##x)-1], \ - stats.vc_irqs[1][ffs(DSI_VC_IRQ_##x)-1], \ - stats.vc_irqs[2][ffs(DSI_VC_IRQ_##x)-1], \ - stats.vc_irqs[3][ffs(DSI_VC_IRQ_##x)-1]); + stats->vc_irqs[0][ffs(DSI_VC_IRQ_##x)-1], \ + stats->vc_irqs[1][ffs(DSI_VC_IRQ_##x)-1], \ + stats->vc_irqs[2][ffs(DSI_VC_IRQ_##x)-1], \ + stats->vc_irqs[3][ffs(DSI_VC_IRQ_##x)-1]); seq_printf(s, "-- VC interrupts --\n"); PIS(CS); @@ -1594,7 +1597,7 @@ static void dsi_dump_dsidev_irqs(struct platform_device *dsidev, #define PIS(x) \ seq_printf(s, "%-20s %10d\n", #x, \ - stats.cio_irqs[ffs(DSI_CIO_IRQ_##x)-1]); + stats->cio_irqs[ffs(DSI_CIO_IRQ_##x)-1]); seq_printf(s, "-- CIO interrupts --\n"); PIS(ERRSYNCESC1); @@ -1618,6 +1621,7 @@ static void dsi_dump_dsidev_irqs(struct platform_device *dsidev, PIS(ULPSACTIVENOT_ALL0); PIS(ULPSACTIVENOT_ALL1); #undef PIS + kfree(stats); } static void dsi1_dump_irqs(struct seq_file *s) -- 2.11.0
[PATCH v2] tty: rocket: reduce stack usage
The build of xtensa allmodconfig gives warning of: In function 'get_ports.isra.0': warning: the frame size of 1040 bytes is larger than 1024 bytes Signed-off-by: Sudip Mukherjee --- v2: check faliure of kzalloc drivers/tty/rocket.c | 32 +++- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/drivers/tty/rocket.c b/drivers/tty/rocket.c index 5ba6816ebf81..fbaa4ec85560 100644 --- a/drivers/tty/rocket.c +++ b/drivers/tty/rocket.c @@ -1222,22 +1222,28 @@ static int set_config(struct tty_struct *tty, struct r_port *info, */ static int get_ports(struct r_port *info, struct rocket_ports __user *retports) { - struct rocket_ports tmp; - int board; + struct rocket_ports *tmp; + int board, ret = 0; - memset(&tmp, 0, sizeof (tmp)); - tmp.tty_major = rocket_driver->major; + tmp = kzalloc(sizeof(*tmp), GFP_KERNEL); + if (!tmp) + return -ENOMEM; + + tmp->tty_major = rocket_driver->major; for (board = 0; board < 4; board++) { - tmp.rocketModel[board].model = rocketModel[board].model; - strcpy(tmp.rocketModel[board].modelString, rocketModel[board].modelString); - tmp.rocketModel[board].numPorts = rocketModel[board].numPorts; - tmp.rocketModel[board].loadrm2 = rocketModel[board].loadrm2; - tmp.rocketModel[board].startingPortNumber = rocketModel[board].startingPortNumber; - } - if (copy_to_user(retports, &tmp, sizeof (*retports))) - return -EFAULT; - return 0; + tmp->rocketModel[board].model = rocketModel[board].model; + strcpy(tmp->rocketModel[board].modelString, + rocketModel[board].modelString); + tmp->rocketModel[board].numPorts = rocketModel[board].numPorts; + tmp->rocketModel[board].loadrm2 = rocketModel[board].loadrm2; + tmp->rocketModel[board].startingPortNumber = + rocketModel[board].startingPortNumber; + } + if (copy_to_user(retports, tmp, sizeof(*retports))) + ret = -EFAULT; + kfree(tmp); + return ret; } static int reset_rm2(struct r_port *info, void __user *arg) -- 2.11.0
[PATCH] tty: rocket: reduce stack usage
The build of xtensa allmodconfig gives warning of: In function 'get_ports.isra.0': warning: the frame size of 1040 bytes is larger than 1024 bytes Allocate memory for 'struct rocket_ports' dynamically to reduce the stack usage, as an added benifit we can remove the memset by using kzalloc while allocating memory. Signed-off-by: Sudip Mukherjee --- drivers/tty/rocket.c | 29 - 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/drivers/tty/rocket.c b/drivers/tty/rocket.c index 5ba6816ebf81..cc1424b9a1e5 100644 --- a/drivers/tty/rocket.c +++ b/drivers/tty/rocket.c @@ -1222,22 +1222,25 @@ static int set_config(struct tty_struct *tty, struct r_port *info, */ static int get_ports(struct r_port *info, struct rocket_ports __user *retports) { - struct rocket_ports tmp; - int board; + struct rocket_ports *tmp; + int board, ret = 0; - memset(&tmp, 0, sizeof (tmp)); - tmp.tty_major = rocket_driver->major; + tmp = kzalloc(sizeof(*tmp), GFP_KERNEL); + tmp->tty_major = rocket_driver->major; for (board = 0; board < 4; board++) { - tmp.rocketModel[board].model = rocketModel[board].model; - strcpy(tmp.rocketModel[board].modelString, rocketModel[board].modelString); - tmp.rocketModel[board].numPorts = rocketModel[board].numPorts; - tmp.rocketModel[board].loadrm2 = rocketModel[board].loadrm2; - tmp.rocketModel[board].startingPortNumber = rocketModel[board].startingPortNumber; - } - if (copy_to_user(retports, &tmp, sizeof (*retports))) - return -EFAULT; - return 0; + tmp->rocketModel[board].model = rocketModel[board].model; + strcpy(tmp->rocketModel[board].modelString, + rocketModel[board].modelString); + tmp->rocketModel[board].numPorts = rocketModel[board].numPorts; + tmp->rocketModel[board].loadrm2 = rocketModel[board].loadrm2; + tmp->rocketModel[board].startingPortNumber = + rocketModel[board].startingPortNumber; + } + if (copy_to_user(retports, tmp, sizeof(*retports))) + ret = -EFAULT; + kfree(tmp); + return ret; } static int reset_rm2(struct r_port *info, void __user *arg) -- 2.11.0
[PATCH] staging: rtl8723bs: reduce stack usage of rtw_cfg80211_unlink_bss
The build of xtensa allmodconfig gives warning of: In function 'rtw_cfg80211_unlink_bss': warning: the frame size of 1136 bytes is larger than 1024 bytes Instead of having 'select_network' structure as a variable use it as a pointer. Signed-off-by: Sudip Mukherjee --- drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c index 59ea4fce9a08..a25c535b6b4f 100644 --- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c +++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c @@ -1410,16 +1410,17 @@ void rtw_cfg80211_unlink_bss(struct adapter *padapter, struct wlan_network *pnet struct wireless_dev *pwdev = padapter->rtw_wdev; struct wiphy *wiphy = pwdev->wiphy; struct cfg80211_bss *bss = NULL; - struct wlan_bssid_ex select_network = pnetwork->network; + struct wlan_bssid_ex *select_network = &pnetwork->network; bss = cfg80211_get_bss(wiphy, NULL/*notify_channel*/, - select_network.MacAddress, select_network.Ssid.Ssid, - select_network.Ssid.SsidLength, 0/*WLAN_CAPABILITY_ESS*/, + select_network->MacAddress, select_network->Ssid.Ssid, + select_network->Ssid.SsidLength, 0/*WLAN_CAPABILITY_ESS*/, 0/*WLAN_CAPABILITY_ESS*/); if (bss) { cfg80211_unlink_bss(wiphy, bss); - DBG_8192C("%s(): cfg80211_unlink %s!! () ", __func__, select_network.Ssid.Ssid); + DBG_8192C("%s(): cfg80211_unlink %s!! () ", __func__, + select_network->Ssid.Ssid); cfg80211_put_bss(padapter->rtw_wdev->wiphy, bss); } } -- 2.11.0
[PATCH] staging: rtl8723bs: reduce stack usage of cfg80211_rtw_scan
The build of xtensa allmodconfig gives warning of: In function 'cfg80211_rtw_scan': warning: the frame size of 1040 bytes is larger than 1024 bytes Allocate memory for ssid dynamically to reduce the stack usage, as an added benifit we can remove the memset by using kzalloc while allocating memory. Signed-off-by: Sudip Mukherjee --- drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c index 8555f52ceb7c..59ea4fce9a08 100644 --- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c +++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c @@ -1512,7 +1512,7 @@ static int cfg80211_rtw_scan(struct wiphy *wiphy int i; u8 _status = false; int ret = 0; - struct ndis_802_11_ssid ssid[RTW_SSID_SCAN_AMOUNT]; + struct ndis_802_11_ssid *ssid = NULL; struct rtw_ieee80211_channel ch[RTW_CHANNEL_SCAN_AMOUNT]; u8 survey_times =3; u8 survey_times_for_one_ch =6; @@ -1603,7 +1603,13 @@ static int cfg80211_rtw_scan(struct wiphy *wiphy goto check_need_indicate_scan_done; } - memset(ssid, 0, sizeof(struct ndis_802_11_ssid)*RTW_SSID_SCAN_AMOUNT); + ssid = kzalloc(RTW_SSID_SCAN_AMOUNT * sizeof(struct ndis_802_11_ssid), + GFP_KERNEL); + if (!ssid) { + ret = -ENOMEM; + goto check_need_indicate_scan_done; + } + /* parsing request ssids, n_ssids */ for (i = 0; i < request->n_ssids && i < RTW_SSID_SCAN_AMOUNT; i++) { #ifdef DEBUG_CFG80211 @@ -1647,6 +1653,7 @@ static int cfg80211_rtw_scan(struct wiphy *wiphy } check_need_indicate_scan_done: + kfree(ssid); if (need_indicate_scan_done) { rtw_cfg80211_surveydone_event_callback(padapter); -- 2.11.0
Re: [PATCH] Input: walkera0701 - Fix possible NULL pointer dereference in walkera0701_detach
On Tue, Apr 23, 2019 at 10:56:37PM +0800, Yue Haibing wrote: > From: YueHaibing > > KASAN report this: > > static void walkera0701_detach(struct parport *port) > { > struct walkera_dev *w = &w_dev; > > - if (!w->pardevice || w->parport->number != port->number) > + if (!w->parport) It doesn't look correct. This way the detach function will never know the port number to which it is attached, and as a result it will try to do detach() with all the ports in the system. Please check the attached patch and maybe (if possible) ask Hulk Robot to verify if it fixes the problem. -- Regards Sudip >From 0338a89a873e7df57707852402f90bb0d6626f12 Mon Sep 17 00:00:00 2001 From: Sudip Mukherjee Date: Wed, 16 Oct 2019 16:08:43 +0100 Subject: [PATCH] Input: walkera0701 - Fix possible NULL pointer dereference If walkera0701_attach() fails and input_dev is made NULL then we are unregistering the pardevice but it still has the pointer to the dev which has now been released. And as a result in the walkera0701_detach() it will now try to do input_unregister_device() with a NULL pointer. We should mark the pardevice as NULL when it is unregistered. Reported-by: Hulk Robot Reported-by: Yue Haibing Fixes: 221bcb24c653 ("Input: walkera0701 - use parallel port device model") Cc: sta...@vger.kernel.org # v4.4+ Signed-off-by: Sudip Mukherjee --- drivers/input/joystick/walkera0701.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/input/joystick/walkera0701.c b/drivers/input/joystick/walkera0701.c index 56abc8c6c763..d8ae1329bf00 100644 --- a/drivers/input/joystick/walkera0701.c +++ b/drivers/input/joystick/walkera0701.c @@ -275,6 +275,7 @@ static void walkera0701_attach(struct parport *pp) input_free_device(w->input_dev); err_unregister_device: parport_unregister_device(w->pardevice); + w->pardevice = NULL; } static void walkera0701_detach(struct parport *port) -- 2.11.0
[PATCH 1/4] parport: daisy: avoid hardcoded name
The daisy device name is hardcoded, define it in the header file and use it in the code. Signed-off-by: Sudip Mukherjee --- drivers/parport/probe.c | 2 +- include/linux/parport.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/parport/probe.c b/drivers/parport/probe.c index e035174ba205..4e2bfeb6b4a6 100644 --- a/drivers/parport/probe.c +++ b/drivers/parport/probe.c @@ -257,7 +257,7 @@ static ssize_t parport_read_device_id (struct parport *port, char *buffer, ssize_t parport_device_id (int devnum, char *buffer, size_t count) { ssize_t retval = -ENXIO; - struct pardevice *dev = parport_open (devnum, "Device ID probe"); + struct pardevice *dev = parport_open(devnum, daisy_dev_name); if (!dev) return -ENXIO; diff --git a/include/linux/parport.h b/include/linux/parport.h index 397607a0c0eb..13932ce8b37b 100644 --- a/include/linux/parport.h +++ b/include/linux/parport.h @@ -460,6 +460,7 @@ extern size_t parport_ieee1284_epp_read_addr (struct parport *, void *, size_t, int); /* IEEE1284.3 functions */ +#define daisy_dev_name "Device ID probe" extern int parport_daisy_init (struct parport *port); extern void parport_daisy_fini (struct parport *port); extern struct pardevice *parport_open (int devnum, const char *name); -- 2.11.0
[PATCH 2/4] parport: do not check portlist when using device-model
We do not need to maintain a list of ports when we are using the device-model. The base layer is going to maintain the list for us and we can get the list of ports just using bus_for_each_dev(). Signed-off-by: Sudip Mukherjee --- drivers/parport/share.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/parport/share.c b/drivers/parport/share.c index 7b4ee33c1935..96538b7975e5 100644 --- a/drivers/parport/share.c +++ b/drivers/parport/share.c @@ -266,9 +266,6 @@ static int port_check(struct device *dev, void *dev_drv) int __parport_register_driver(struct parport_driver *drv, struct module *owner, const char *mod_name) { - if (list_empty(&portlist)) - get_lowlevel_driver(); - if (drv->devmodel) { /* using device model */ int ret; @@ -292,6 +289,8 @@ int __parport_register_driver(struct parport_driver *drv, struct module *owner, drv->devmodel = false; + if (list_empty(&portlist)) + get_lowlevel_driver(); mutex_lock(®istration_lock); list_for_each_entry(port, &portlist, list) drv->attach(port); -- 2.11.0
[PATCH 3/4] parport: load lowlevel driver if ports not found
Usually all the distro will load the parport low level driver as part of their initialization. But we can get into a situation where all the parallel port drivers are built as module and we unload all the modules at a later time. Then if we just do "modprobe parport" it will only load the parport module and will not load the low level driver which will actually register the ports. So, check the bus if there is any parport registered, if not, load the low level driver. We can get into the above situation with all distro but only Suse has setup the alias for "parport_lowlevel" and so it only works in Suse. Users of Debian based distro will need to load the lowlevel module manually. Signed-off-by: Sudip Mukherjee --- drivers/parport/share.c | 21 + 1 file changed, 21 insertions(+) diff --git a/drivers/parport/share.c b/drivers/parport/share.c index 96538b7975e5..d6920ebeabcd 100644 --- a/drivers/parport/share.c +++ b/drivers/parport/share.c @@ -230,6 +230,18 @@ static int port_check(struct device *dev, void *dev_drv) return 0; } +/* + * Iterates through all the devices connected to the bus and return 1 + * if the device is a parallel port. + */ + +static int port_detect(struct device *dev, void *dev_drv) +{ + if (is_parport(dev)) + return 1; + return 0; +} + /** * parport_register_driver - register a parallel port device driver * @drv: structure describing the driver @@ -279,6 +291,15 @@ int __parport_register_driver(struct parport_driver *drv, struct module *owner, if (ret) return ret; + /* +* check if bus has any parallel port registered, if +* none is found then load the lowlevel driver. +*/ + ret = bus_for_each_dev(&parport_bus_type, NULL, NULL, + port_detect); + if (!ret) + get_lowlevel_driver(); + mutex_lock(®istration_lock); if (drv->match_port) bus_for_each_dev(&parport_bus_type, NULL, drv, -- 2.11.0
[PATCH 4/4] parport: daisy: use new parport device model
Modify parport daisy driver to use the new parallel port device model. Last attempt was '1aec4211204d ("parport: daisy: use new parport device model")' which failed as daisy was also trying to load the low level driver and that resulted in a deadlock. Cc: Michal Kubecek Cc: Steven Rostedt (VMware) Signed-off-by: Sudip Mukherjee --- Steven, Michal, Can you please test this series in your test environment and verify that I am not breaking anything this time. drivers/parport/daisy.c | 40 +++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/drivers/parport/daisy.c b/drivers/parport/daisy.c index 5484a46dafda..95b5c3363582 100644 --- a/drivers/parport/daisy.c +++ b/drivers/parport/daisy.c @@ -45,6 +45,7 @@ static struct daisydev { static DEFINE_SPINLOCK(topology_lock); static int numdevs; +static bool daisy_init_done; /* Forward-declaration of lower-level functions. */ static int mux_present(struct parport *port); @@ -87,6 +88,24 @@ static struct parport *clone_parport(struct parport *real, int muxport) return extra; } +static int daisy_drv_probe(struct pardevice *par_dev) +{ + struct device_driver *drv = par_dev->dev.driver; + + if (strcmp(drv->name, "daisy_drv")) + return -ENODEV; + if (strcmp(par_dev->name, daisy_dev_name)) + return -ENODEV; + + return 0; +} + +static struct parport_driver daisy_driver = { + .name = "daisy_drv", + .probe = daisy_drv_probe, + .devmodel = true, +}; + /* Discover the IEEE1284.3 topology on a port -- muxes and daisy chains. * Return value is number of devices actually detected. */ int parport_daisy_init(struct parport *port) @@ -98,6 +117,23 @@ int parport_daisy_init(struct parport *port) int i; int last_try = 0; + if (!daisy_init_done) { + /* +* flag should be marked true first as +* parport_register_driver() might try to load the low +* level driver which will lead to announcing new ports +* and which will again come back here at +* parport_daisy_init() +*/ + daisy_init_done = true; + i = parport_register_driver(&daisy_driver); + if (i) { + pr_err("daisy registration failed\n"); + daisy_init_done = false; + return i; + } + } + again: /* Because this is called before any other devices exist, * we don't have to claim exclusive access. */ @@ -213,10 +249,12 @@ void parport_daisy_fini(struct parport *port) struct pardevice *parport_open(int devnum, const char *name) { struct daisydev *p = topology; + struct pardev_cb par_cb; struct parport *port; struct pardevice *dev; int daisy; + memset(&par_cb, 0, sizeof(par_cb)); spin_lock(&topology_lock); while (p && p->devnum != devnum) p = p->next; @@ -230,7 +268,7 @@ struct pardevice *parport_open(int devnum, const char *name) port = parport_get_port(p->port); spin_unlock(&topology_lock); - dev = parport_register_device(port, name, NULL, NULL, NULL, 0, NULL); + dev = parport_register_dev_model(port, name, &par_cb, devnum); parport_put_port(port); if (!dev) return NULL; -- 2.11.0
[PATCH 1/3] parport: Add missing newline at end of file
From: Geert Uytterhoeven "git diff" says: \ No newline at end of file after modifying the file. Signed-off-by: Geert Uytterhoeven Signed-off-by: Sudip Mukherjee --- drivers/parport/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/parport/Makefile b/drivers/parport/Makefile index 6fa41f8173b6..022c566c0f32 100644 --- a/drivers/parport/Makefile +++ b/drivers/parport/Makefile @@ -19,4 +19,4 @@ obj-$(CONFIG_PARPORT_ATARI) += parport_atari.o obj-$(CONFIG_PARPORT_SUNBPP) += parport_sunbpp.o obj-$(CONFIG_PARPORT_GSC) += parport_gsc.o obj-$(CONFIG_PARPORT_AX88796) += parport_ax88796.o -obj-$(CONFIG_PARPORT_IP32) += parport_ip32.o \ No newline at end of file +obj-$(CONFIG_PARPORT_IP32) += parport_ip32.o -- 2.11.0
[PATCH 2/3] ppdev: add header include guard
From: Masahiro Yamada Add a header include guard just in case. Signed-off-by: Masahiro Yamada Signed-off-by: Sudip Mukherjee --- include/uapi/linux/ppdev.h | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/include/uapi/linux/ppdev.h b/include/uapi/linux/ppdev.h index 8fe3c64d149e..eb895b83f2bd 100644 --- a/include/uapi/linux/ppdev.h +++ b/include/uapi/linux/ppdev.h @@ -15,6 +15,9 @@ * Added PPGETMODES/PPGETMODE/PPGETPHASE, Fred Barnes , 03/01/2001 */ +#ifndef _UAPI_LINUX_PPDEV_H +#define _UAPI_LINUX_PPDEV_H + #define PP_IOCTL 'p' /* Set mode for read/write (e.g. IEEE1284_MODE_EPP) */ @@ -97,4 +100,4 @@ struct ppdev_frob_struct { /* only masks user-visible flags */ #define PP_FLAGMASK(PP_FASTWRITE | PP_FASTREAD | PP_W91284PIC) - +#endif /* _UAPI_LINUX_PPDEV_H */ -- 2.11.0
[PATCH 3/3] parport: parport_serial: Use dev_get_drvdata
From: Chuhong Yuan Instead of using to_pci_dev + pci_get_drvdata, use dev_get_drvdata to make code simpler. Signed-off-by: Chuhong Yuan Signed-off-by: Sudip Mukherjee --- drivers/parport/parport_serial.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/parport/parport_serial.c b/drivers/parport/parport_serial.c index 461fd8a24278..b11f5d238eda 100644 --- a/drivers/parport/parport_serial.c +++ b/drivers/parport/parport_serial.c @@ -660,8 +660,7 @@ static void parport_serial_pci_remove(struct pci_dev *dev) static int __maybe_unused parport_serial_pci_suspend(struct device *dev) { - struct pci_dev *pdev = to_pci_dev(dev); - struct parport_serial_private *priv = pci_get_drvdata(pdev); + struct parport_serial_private *priv = dev_get_drvdata(dev); if (priv->serial) pciserial_suspend_ports(priv->serial); @@ -672,8 +671,7 @@ static int __maybe_unused parport_serial_pci_suspend(struct device *dev) static int __maybe_unused parport_serial_pci_resume(struct device *dev) { - struct pci_dev *pdev = to_pci_dev(dev); - struct parport_serial_private *priv = pci_get_drvdata(pdev); + struct parport_serial_private *priv = dev_get_drvdata(dev); if (priv->serial) pciserial_resume_ports(priv->serial); -- 2.11.0
Re: regression with napi/softirq ?
On Thu, Jul 18, 2019 at 4:08 PM Eric Dumazet wrote: > > > > On 7/18/19 2:55 PM, Sudip Mukherjee wrote: > > > Thanks Eric. But there is no improvement in delay between > > softirq_raise and softirq_entry with this change. > > But moving to a later kernel (linus master branch? ) like Thomas has > > said in the other mail might be difficult atm. I can definitely > > move to v4.14.133 if that helps. Thomas ? > > If you are tracking max latency then I guess you have to tweak > SOFTIRQ_NOW_MASK > to include NET_RX_SOFTIRQ > > The patch I gave earlier would only lower the probability of events, not > completely get rid of them. > > > > diff --git a/kernel/softirq.c b/kernel/softirq.c > index > 0427a86743a46b7e1891f7b6c1ff585a8a1695f5..302046dd8d7e6740e466c422954f22565fe19e69 > 100644 > --- a/kernel/softirq.c > +++ b/kernel/softirq.c > @@ -81,7 +81,7 @@ static void wakeup_softirqd(void) > * right now. Let ksoftirqd handle this at its own rate, to get fairness, > * unless we're doing some of the synchronous softirqs. > */ > -#define SOFTIRQ_NOW_MASK ((1 << HI_SOFTIRQ) | (1 << TASKLET_SOFTIRQ)) > +#define SOFTIRQ_NOW_MASK ((1 << HI_SOFTIRQ) | (1 << TASKLET_SOFTIRQ) | (1 << > NET_RX_SOFTIRQ)) > static bool ksoftirqd_running(unsigned long pending) > { > struct task_struct *tsk = __this_cpu_read(ksoftirqd); Thanks Eric, this looks better than the hack that tglx gave. :) Though the hack was good for testing. But my original problem was a drop is network packets and till now I was thinking that the delay in processing the softirq is causing that. But with the hack tglx has given the latency has decreased but my problem is still there. So, I am looking into it again now. Thanks again for the patch. -- Regards Sudip
Re: regression with napi/softirq ?
On Thu, Jul 18, 2019 at 12:42 PM Eric Dumazet wrote: > > > > On 7/18/19 1:18 PM, Sudip Mukherjee wrote: > > Hi Eric, > > > > On Thu, Jul 18, 2019 at 7:58 AM Eric Dumazet wrote: > >> > >> > >> > >> On 7/17/19 11:52 PM, Thomas Gleixner wrote: > >>> Sudip, > >>> > >>> On Wed, 17 Jul 2019, Sudip Mukherjee wrote: > >>>> On Wed, Jul 17, 2019 at 9:53 PM Thomas Gleixner > >>>> wrote: > >>>>> You can hack ksoftirq_running() to return always false to avoid this, > >>>>> but > >>>>> that might cause application starvation and a huge packet buffer backlog > >>>>> when the amount of incoming packets makes the CPU do nothing else than > >>>>> softirq processing. > >>>> > >>>> I tried that now, it is better but still not as good as v3.8 > >>>> Now I am getting 375.9usec as the maximum time between raising the > >>>> softirq > >>>> and it starting to execute and packet drops still there. > >>>> > >>>> And just a thought, do you think there should be a CONFIG_ option for > >>>> this feature of ksoftirqd_running() so that it can be disabled if needed > >>>> by users like us? > >>> > >>> If at all then a sysctl to allow runtime control. > >>> > > > > > > > >> > >> ksoftirqd might be spuriously scheduled from tx path, when > >> __qdisc_run() also reacts to need_resched(). > >> > >> By raising NET_TX while we are processing NET_RX (say we send a TCP ACK > >> packet > >> in response to incoming packet), we force __do_softirq() to perform > >> another loop, but before doing an other round, it will also check > >> need_resched() > >> and eventually call wakeup_softirqd() > >> > >> I wonder if following patch makes any difference. > >> > >> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c > >> index > >> 11c03cf4aa74b44663c74e0e3284140b0c75d9c4..ab736e974396394ae6ba409868aaea56a50ad57b > >> 100644 > >> --- a/net/sched/sch_generic.c > >> +++ b/net/sched/sch_generic.c > >> @@ -377,6 +377,8 @@ void __qdisc_run(struct Qdisc *q) > >> int packets; > >> > >> while (qdisc_restart(q, &packets)) { > >> + if (qdisc_is_empty(q)) > >> + break; > > > > unfortunately its v4.14.55 and qdisc_is_empty() is not yet introduced. > > And I can not backport 28cff537ef2e ("net: sched: add empty status > > flag for NOLOCK qdisc") > > also as TCQ_F_NOLOCK is there. :( > > > > On old kernels, you can simply use > > static inline bool qdisc_is_empty(struct Qdisc *q) > { > return !qdisc_qlen(q); > } > Thanks Eric. But there is no improvement in delay between softirq_raise and softirq_entry with this change. But moving to a later kernel (linus master branch? ) like Thomas has said in the other mail might be difficult atm. I can definitely move to v4.14.133 if that helps. Thomas ? -- Regards Sudip
Re: regression with napi/softirq ?
Hi Eric, On Thu, Jul 18, 2019 at 7:58 AM Eric Dumazet wrote: > > > > On 7/17/19 11:52 PM, Thomas Gleixner wrote: > > Sudip, > > > > On Wed, 17 Jul 2019, Sudip Mukherjee wrote: > >> On Wed, Jul 17, 2019 at 9:53 PM Thomas Gleixner wrote: > >>> You can hack ksoftirq_running() to return always false to avoid this, but > >>> that might cause application starvation and a huge packet buffer backlog > >>> when the amount of incoming packets makes the CPU do nothing else than > >>> softirq processing. > >> > >> I tried that now, it is better but still not as good as v3.8 > >> Now I am getting 375.9usec as the maximum time between raising the softirq > >> and it starting to execute and packet drops still there. > >> > >> And just a thought, do you think there should be a CONFIG_ option for > >> this feature of ksoftirqd_running() so that it can be disabled if needed > >> by users like us? > > > > If at all then a sysctl to allow runtime control. > > > > ksoftirqd might be spuriously scheduled from tx path, when > __qdisc_run() also reacts to need_resched(). > > By raising NET_TX while we are processing NET_RX (say we send a TCP ACK packet > in response to incoming packet), we force __do_softirq() to perform > another loop, but before doing an other round, it will also check > need_resched() > and eventually call wakeup_softirqd() > > I wonder if following patch makes any difference. > > diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c > index > 11c03cf4aa74b44663c74e0e3284140b0c75d9c4..ab736e974396394ae6ba409868aaea56a50ad57b > 100644 > --- a/net/sched/sch_generic.c > +++ b/net/sched/sch_generic.c > @@ -377,6 +377,8 @@ void __qdisc_run(struct Qdisc *q) > int packets; > > while (qdisc_restart(q, &packets)) { > + if (qdisc_is_empty(q)) > + break; unfortunately its v4.14.55 and qdisc_is_empty() is not yet introduced. And I can not backport 28cff537ef2e ("net: sched: add empty status flag for NOLOCK qdisc") also as TCQ_F_NOLOCK is there. :( -- Regards Sudip
Re: regression with napi/softirq ?
Hi, On Wed, Jul 17, 2019 at 9:53 PM Thomas Gleixner wrote: > > On Wed, 17 Jul 2019, Sudip Mukherjee wrote: > > I am using v4.14.55 on an Intel Atom based board and I am seeing network > > packet drops frequently on wireshark logs. After lots of debugging it > > seems that when this happens softirq is taking huge time to start after > > it has been raised. This is a small snippet from ftrace: > > > ><...>-2110 [001] dNH1 466.634916: irq_handler_entry: irq=126 > > name=eth0-TxRx-0 > ><...>-2110 [001] dNH1 466.634917: softirq_raise: vec=3 > > [action=NET_RX] > ><...>-2110 [001] dNH1 466.634918: irq_handler_exit: irq=126 > > ret=handled > > ksoftirqd/1-15[001] ..s. 466.635826: softirq_entry: vec=3 > > [action=NET_RX] > > ksoftirqd/1-15[001] ..s. 466.635852: softirq_exit: vec=3 > > [action=NET_RX] > > ksoftirqd/1-15[001] d.H. 466.635856: irq_handler_entry: irq=126 > > name=eth0-TxRx-0 > > ksoftirqd/1-15[001] d.H. 466.635857: softirq_raise: vec=3 > > [action=NET_RX] > > ksoftirqd/1-15[001] d.H. 466.635858: irq_handler_exit: irq=126 > > ret=handled > > ksoftirqd/1-15[001] ..s. 466.635860: softirq_entry: vec=3 > > [action=NET_RX] > > ksoftirqd/1-15[001] ..s. 466.635863: softirq_exit: vec=3 > > [action=NET_RX] > > > > So, softirq was raised at 466.634917 but it started at 466.635826 almost > > 909 usec after it was raised. > > This is a situation where the network softirq decided to delegate softirq > processing to ksoftirqd. That happens when too much work is available while > processing softirqs on return from interrupt. > > That means that softirq processing happens under scheduler control. So if > there are other runnable tasks on the same CPU ksoftirqd can be delayed > until their time slice expired. As a consequence ksoftirqd might not be > able to catch up with the incoming packet flood and the NIC starts to drop. Yes, and I see in the ftrace that there are many other userspace processes getting scheduled in that time. > > You can hack ksoftirq_running() to return always false to avoid this, but > that might cause application starvation and a huge packet buffer backlog > when the amount of incoming packets makes the CPU do nothing else than > softirq processing. I tried that now, it is better but still not as good as v3.8 Now I am getting 375.9usec as the maximum time between raising the softirq and it starting to execute and packet drops still there. And just a thought, do you think there should be a CONFIG_ option for this feature of ksoftirqd_running() so that it can be disabled if needed by users like us? Can you please think of anything else that might have changed which I still need to change to make the time comparable to v3.8.. -- Regards Sudip
regression with napi/softirq ?
Hi All, I am using v4.14.55 on an Intel Atom based board and I am seeing network packet drops frequently on wireshark logs. After lots of debugging it seems that when this happens softirq is taking huge time to start after it has been raised. This is a small snippet from ftrace: <...>-2110 [001] dNH1 466.634916: irq_handler_entry: irq=126 name=eth0-TxRx-0 <...>-2110 [001] dNH1 466.634917: softirq_raise: vec=3 [action=NET_RX] <...>-2110 [001] dNH1 466.634918: irq_handler_exit: irq=126 ret=handled ksoftirqd/1-15[001] ..s. 466.635826: softirq_entry: vec=3 [action=NET_RX] ksoftirqd/1-15[001] ..s. 466.635852: softirq_exit: vec=3 [action=NET_RX] ksoftirqd/1-15[001] d.H. 466.635856: irq_handler_entry: irq=126 name=eth0-TxRx-0 ksoftirqd/1-15[001] d.H. 466.635857: softirq_raise: vec=3 [action=NET_RX] ksoftirqd/1-15[001] d.H. 466.635858: irq_handler_exit: irq=126 ret=handled ksoftirqd/1-15[001] ..s. 466.635860: softirq_entry: vec=3 [action=NET_RX] ksoftirqd/1-15[001] ..s. 466.635863: softirq_exit: vec=3 [action=NET_RX] So, softirq was raised at 466.634917 but it started at 466.635826 almost 909 usec after it was raised. If I move back to v4.4 kernel I still see similar behaviour but the maximum delay I get is in the range of 500usec. But if I move back to v3.8 kernel I can see there is no packet loss and the maximum delay between softirq_raise and irq_handler_entry is 103usec. Is this a known issue? Will really appreciate your help in this problem. -- Regards Sudip
Re: [PATCH 4.14 68/92] appletalk: Fix use-after-free in atalk_proc_exit
Hi Greg, On Thu, Apr 18, 2019 at 7:25 PM Greg Kroah-Hartman wrote: > > [ Upstream commit 6377f787aeb945cae7abbb6474798de129e1f3ac ] This has been fixed by 27da0d2ef998 ("appletalk: Fix compile regression"). -- Regards Sudip
[PATCH] parport: ieee1284: mark expected switch fall-through
From: "Gustavo A. R. Silva" In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. This patch fixes the following warning: drivers/parport/ieee1284.c: In function ‘parport_read’: drivers/parport/ieee1284.c:722:6: warning: this statement may fall through [-Wimplicit-fallthrough=] if (parport_negotiate (port, IEEE1284_MODE_NIBBLE)) { ^ drivers/parport/ieee1284.c:726:2: note: here case IEEE1284_MODE_NIBBLE: ^~~~ Warning level 3 was used: -Wimplicit-fallthrough=3 Notice that, in this particular case, the code comment is modified in accordance with what GCC is expecting to find. This patch is part of the ongoing efforts to enable -Wimplicit-fallthrough. Signed-off-by: Gustavo A. R. Silva Signed-off-by: Sudip Mukherjee --- drivers/parport/ieee1284.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/parport/ieee1284.c b/drivers/parport/ieee1284.c index f12b9da69255..90fb73575495 100644 --- a/drivers/parport/ieee1284.c +++ b/drivers/parport/ieee1284.c @@ -722,7 +722,7 @@ ssize_t parport_read (struct parport *port, void *buffer, size_t len) if (parport_negotiate (port, IEEE1284_MODE_NIBBLE)) { return -EIO; } - /* fall through to NIBBLE */ + /* fall through - to NIBBLE */ case IEEE1284_MODE_NIBBLE: DPRINTK (KERN_DEBUG "%s: Using nibble mode\n", port->name); fn = port->ops->nibble_read_data; -- 2.11.0
Re: [PATCH] parport: daisy: do not try to load lowlevel driver
Hi Linus, On Mon, Mar 25, 2019 at 9:49 PM Linus Torvalds wrote: > > On Mon, Mar 25, 2019 at 2:13 PM Sudip Mukherjee > wrote: > > > > We do not need to search for ports and bind the initial list of ports > > to daisy driver as daisy driver is always the first driver to use the > > new found parport and we know when the parport bus is registering the > > list of parport will always be empty. So, proceed with the daisy_drv > > registration even if the list of parport is empty. > > This is completely hacky and senseless. I admit its hacky. :( > > The problem as far as I can tell is that the daisy driver shouldn't > register early at all, and shouldn't be a subsys initcall. It should > just be a driver initcall, and happen naturally after the parport > subsystem has been initialized. yes, but the daisy driver has to be the first one to register even before any of the ports have actually registered and then it will try to load the lowlevel driver. In x86, parport_pc will register the port and then announce it. When the port is announced daisy_driver should check that port and this happens even before the port is added to the list. So, we will always be in this situation that when daisy driver registers the port list is empty. > > This patch just makes the code even *less* understandable. > > I'm going to revert that problematic commit. > > If you can get it working without that incorrect and senseless tie-in > of the daisy driver registration with the regular partport init > sequence, we can revisit this. I will find a better way to do this. -- Regards Sudip
[PATCH] parport: daisy: do not try to load lowlevel driver
Some distros like Suse has an alias for "parport_lowlevel" and that alias points to "parport_pc". Now when the parport bus registers, it also initialises the daisy driver as the daisy driver is needed to check the port when the port is first found. Due to the new device model daisy driver will now try to find the parallel ports while trying to register its driver so that it can bind with them. Now, since daisy driver is loaded while parport bus is initialising the list of parport is still empty and it tries to load the lowlevel driver, which has an alias set to parport_pc, now causes a deadlock. We do not need to search for ports and bind the initial list of ports to daisy driver as daisy driver is always the first driver to use the new found parport and we know when the parport bus is registering the list of parport will always be empty. So, proceed with the daisy_drv registration even if the list of parport is empty. Fixes: 1aec4211204d ("parport: daisy: use new parport device model") Reported-by: Michal Kubecek Reported-by: Steven Rostedt Tested-by: Michal Kubecek Cc: Linus Torvalds Signed-off-by: Sudip Mukherjee --- drivers/parport/share.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/parport/share.c b/drivers/parport/share.c index 0171b8dbcdcd..f87948fbfc34 100644 --- a/drivers/parport/share.c +++ b/drivers/parport/share.c @@ -274,7 +274,7 @@ static int port_check(struct device *dev, void *dev_drv) int __parport_register_driver(struct parport_driver *drv, struct module *owner, const char *mod_name) { - if (list_empty(&portlist)) + if (list_empty(&portlist) && strcmp(drv->name, "daisy_drv")) get_lowlevel_driver(); if (drv->devmodel) { -- 2.11.0
Re: [REGRESSION] failed to boot: commit 1aec4211204d parport: daisy: use new parport device model
Hi Greg, Linus, On Mon, Mar 25, 2019 at 8:12 PM Greg Kroah-Hartman wrote: > > On Mon, Mar 25, 2019 at 11:04:49AM -0700, Linus Torvalds wrote: > > On Mon, Mar 25, 2019 at 8:36 AM Steven Rostedt wrote: > > > > > > I bisected it down to this commit: > > > > > > aec4211204d ("parport: daisy: use new parport device model") > > > > I was confused, because no such commit exists. > > > > But it turns out you have the right commit ID in your subject line, > > and you just dropped the initial '1' . > > > > Anyway, that commit does look odd - why is the daisy_drv_init() done > > by parport_bus_init() rather than just as a regular module init? And > > as a result, now when daisy_drv_init() blocks, that blocks > > parport_bus_init(). I dunno. It seems to not really have a good reason > > for it. > > > > I think it should just be reverted unless Sudip can come up with a > > trivial fix. Greg? > > I recommend just reverting it for now. The parport is "odd" in how it > has had to be converted over time to the driver model, which is why I > thought this patch was ok. Looks like it wasn't, sorry. It was reported by Michal from Suse and has now been fixed and he has tested and confirmed it is working. It is only happening on those systems/distro who has an alias set for "parport_lowlevel". Will send out the patch shortly. -- Regards Sudip
Re: regression (bisected): "modprobe parport_pc" hangs in current mainline
On Mon, Mar 25, 2019 at 7:30 AM Michal Kubecek wrote: > > On Sun, Mar 24, 2019 at 07:38:38PM +, Sudip Mukherjee wrote: > > And I was able to reproduce the problem using a vm and Suse Tumblewood with > > next-20190322. Can you please try the attached patch and test on your vm and > > machine and check if it fixes the problem. > > > > -- > > Regards > > Sudip > > > diff --git a/drivers/parport/share.c b/drivers/parport/share.c > > index 0171b8dbcdcd..f87948fbfc34 100644 > > --- a/drivers/parport/share.c > > +++ b/drivers/parport/share.c > > @@ -274,7 +274,7 @@ static int port_check(struct device *dev, void *dev_drv) > > int __parport_register_driver(struct parport_driver *drv, struct module > > *owner, > > const char *mod_name) > > { > > - if (list_empty(&portlist)) > > + if (list_empty(&portlist) && strcmp(drv->name, "daisy_drv")) > > get_lowlevel_driver(); > > > > if (drv->devmodel) { > > Yes, with this patch (on top of v5.1-rc2), both physical machine and VM > let the module(s) load cleanly even with the alias line restored. > > Tested-by: Michal Kubecek Thanks Michal. I will add it to my queue with your Tested-by. btw, I think I liked using Suse. :) -- Regards Sudip
Re: regression (bisected): "modprobe parport_pc" hangs in current mainline
Hi Michal, On Fri, Mar 22, 2019 at 07:13:23AM +0100, Michal Kubecek wrote: > On Thursday, 21 March 2019 23:43 Sudip Mukherjee wrote: > > HI Michal, > > > > On Wed, Mar 20, 2019 at 9:18 PM Michal Kubecek wrote: > > > On Wed, Mar 20, 2019 at 09:30:59AM +, Sudip Mukherjee wrote: > > Can you please check in your VM or machine what do you have the alias > > as? It should be either in "/etc/modprobe.conf" or some conf file in > > "/etc/modprobe.d" folder. > > You are right, this is in /etc/modprobe.d/00-system which is part of > suse-module-tools package: And I was able to reproduce the problem using a vm and Suse Tumblewood with next-20190322. Can you please try the attached patch and test on your vm and machine and check if it fixes the problem. -- Regards Sudip diff --git a/drivers/parport/share.c b/drivers/parport/share.c index 0171b8dbcdcd..f87948fbfc34 100644 --- a/drivers/parport/share.c +++ b/drivers/parport/share.c @@ -274,7 +274,7 @@ static int port_check(struct device *dev, void *dev_drv) int __parport_register_driver(struct parport_driver *drv, struct module *owner, const char *mod_name) { - if (list_empty(&portlist)) + if (list_empty(&portlist) && strcmp(drv->name, "daisy_drv")) get_lowlevel_driver(); if (drv->devmodel) {
Re: [PATCH] parport: ieee1284: mark expected switch fall-through
Hi Greg, On Wed, Mar 20, 2019 at 8:12 PM Greg KH wrote: > > On Wed, Mar 20, 2019 at 03:06:04PM -0500, Gustavo A. R. Silva wrote: > > Greg, > > > > Can you take this, please? > > Will do, give me a week or so to catch up... I will send it to you with the another parport regression fix I have. Will send you by next week. -- Regards Sudip
Re: regression (bisected): "modprobe parport_pc" hangs in current mainline
HI Michal, On Wed, Mar 20, 2019 at 9:18 PM Michal Kubecek wrote: > > On Wed, Mar 20, 2019 at 09:30:59AM +, Sudip Mukherjee wrote: > > Sorry, I didn't get the chance to look at it yet and have kept it > > pending for this weekend. But just had a quick look and I was > > wondering if the machine on which you are trying the modprobe has an > > actual parallel port or the machine is not having any parallel port. > > And also will you be able to send me a dmesg please. > > Attaching dmesg output from a virtual machine which doesn't seem to have > a (virtual) parallel port. This part: > > [ 63.962283] parport_pc 00:05: reported by Plug and Play ACPI > [ 63.962469] parport0: PC-style at 0x378, irq 7 [PCSPP,TRISTATE] > [ 64.061723] ppdev: user-space parallel port driver > > was after I manually killed "/sbin/modprobe -q -- parport_lowlevel" which > was started during boot. Thanks for testing. I am unable to reproduce the problem in VM or in machine, with or without parallel port. But from your logs it looks like you have an alias set for "parport_lowlevel". When parport module is being loaded if it does not find any port in its list, it will try to load "parport_lowlevel" and that is where you are getting the deadlock. "parport_lowlevel" is not a real module, but instead should be an alias pointing to some real module. I tried by setting an alias of parport_lowlevel" as parport_pc but still could not get the problem. Can you please check in your VM or machine what do you have the alias as? It should be either in "/etc/modprobe.conf" or some conf file in "/etc/modprobe.d" folder. And also, will you be able to test a debug patch on your VM? -- Regards Sudip
Re: regression (bisected): "modprobe parport_pc" hangs in current mainline
Hi Michal, On Sun, Mar 17, 2019 at 6:05 PM Michal Kubecek wrote: > > On Sun, Mar 17, 2019 at 05:01:37PM +, Sudip Mukherjee wrote: > > On Wed, Mar 13, 2019 at 6:45 AM Michal Kubecek wrote: > > > I encountered a regression in current (post-5.0) mainline kernel which I > > > bisected to commit 1aec4211204d ("parport: daisy: use new parport device > > > model"). Running "modprobe parport_pc" hangs up: > > > > Can you please send me your .config so that I can test it from my side. > > Attaching two versions: config-full.gz is the real life config from the > machine where I found the issue and config-mini.gz is a minimized config > I was using while bisecting the issue. (I made a mistake and thought > that I have seen the issue with snapshot before both parport commits so > that I ran a full bisect instead of simply checking the two parport > commits which came in the merge window.) Sorry, I didn't get the chance to look at it yet and have kept it pending for this weekend. But just had a quick look and I was wondering if the machine on which you are trying the modprobe has an actual parallel port or the machine is not having any parallel port. And also will you be able to send me a dmesg please. -- Regards Sudip
Re: regression (bisected): "modprobe parport_pc" hangs in current mainline
HI Michal, On Wed, Mar 13, 2019 at 6:45 AM Michal Kubecek wrote: > > Hello, > > I encountered a regression in current (post-5.0) mainline kernel which I > bisected to commit 1aec4211204d ("parport: daisy: use new parport device > model"). Running "modprobe parport_pc" hangs up: Can you please send me your .config so that I can test it from my side. > > tweed:~ # ps ax | grep modprobe > 1206 pts/0D+ 0:00 modprobe parport_pc > 1209 ?S 0:00 /sbin/modprobe -q -- parport_lowlevel > 1211 pts/1S+ 0:00 grep modprobe > tweed:~ # cat /proc/1206/stack > [<0>] call_usermodehelper_exec+0xc7/0x140 > [<0>] __request_module+0x1a1/0x430 > [<0>] __parport_register_driver+0x142/0x150 [parport] And also, modprobe is trying to load a dependent module, so it will be great if you can also do a "lsmod" before doing the modprobe and I can check what is going wrong. -- Regards Sudip
Re: [PATCH V3] parport_serial.c change for fix hanging problem.
On Tue, Feb 19, 2019 at 9:23 AM saumah wrote: > > parport_serial.c change for fix hanging problem while suspend machine. > parport_seria.c support the same vendor id and device id definition as sunix > multi-I/O card driver, when the sunix multi-io borad driver is > installed,there are two drivers to support the same sunix multi-io card,it > will cause operation error. > Please use checkpatch to check your patch. > Signed-off-by: saumah > --- > parport_serial.c | 180 +-- > 1 file changed, 1 insertion(+), 179 deletions(-) > > diff --git a/parport_serial.c b/parport_serial.c > index ae9e01e..f54d1d3 100644 > --- a/parport_serial.c > +++ b/parport_serial.c This doesnot apply. git is showing error: error: parport_serial.c: No such file or directory > @@ -38,29 +38,10 @@ enum parport_pc_pci_cards { > siig_2p1s_20x, > > /* each element directly indexed from enum list, above */ > @@ -127,29 +108,10 @@ static struct parport_pc_pci cards[] = { > /* siig_2p1s_20x */ { 2, { { 1, 2 }, { 3, 4 }, } }, > /* siig_1s1p_20x */ { 1, { { 1, 2 }, } }, > /* siig_2s1p_20x */ { 1, { { 2, 3 }, } }, > - /* timedia_4078a */ { 1, { { 2, -1 }, } }, You are removing support for the parallel port of these boards. Can you please point me to the driver with which it conflicts. -- Regards Sudip
Re: [PATCH 4.14 04/62] uapi/if_ether.h: prevent redefinition of struct ethhdr
Hi Greg, On Mon, Feb 18, 2019 at 1:56 PM Greg Kroah-Hartman wrote: > > 4.14-stable review patch. If anyone has any objections, please let me know. > > -- > > commit 6926e041a8920c8ec27e4e155efa760aa01551fd upstream. This has been fixed upstream by da360299b673 ("uapi/if_ether.h: move __UAPI_DEF_ETHHDR libc define") -- Regards Sudip
[PATCH v2] parport: daisy: use new parport device model
Modify parport daisy driver to use the new parallel port device model. Signed-off-by: Sudip Mukherjee --- v2: hide ifdef in the header file drivers/parport/daisy.c | 32 +++- drivers/parport/probe.c | 2 +- drivers/parport/share.c | 10 +- include/linux/parport.h | 13 + 4 files changed, 54 insertions(+), 3 deletions(-) diff --git a/drivers/parport/daisy.c b/drivers/parport/daisy.c index 5484a46dafda..56dd83a45e55 100644 --- a/drivers/parport/daisy.c +++ b/drivers/parport/daisy.c @@ -213,10 +213,12 @@ void parport_daisy_fini(struct parport *port) struct pardevice *parport_open(int devnum, const char *name) { struct daisydev *p = topology; + struct pardev_cb par_cb; struct parport *port; struct pardevice *dev; int daisy; + memset(&par_cb, 0, sizeof(par_cb)); spin_lock(&topology_lock); while (p && p->devnum != devnum) p = p->next; @@ -230,7 +232,7 @@ struct pardevice *parport_open(int devnum, const char *name) port = parport_get_port(p->port); spin_unlock(&topology_lock); - dev = parport_register_device(port, name, NULL, NULL, NULL, 0, NULL); + dev = parport_register_dev_model(port, name, &par_cb, devnum); parport_put_port(port); if (!dev) return NULL; @@ -480,3 +482,31 @@ static int assign_addrs(struct parport *port) kfree(deviceid); return detected; } + +static int daisy_drv_probe(struct pardevice *par_dev) +{ + struct device_driver *drv = par_dev->dev.driver; + + if (strcmp(drv->name, "daisy_drv")) + return -ENODEV; + if (strcmp(par_dev->name, daisy_dev_name)) + return -ENODEV; + + return 0; +} + +static struct parport_driver daisy_driver = { + .name = "daisy_drv", + .probe = daisy_drv_probe, + .devmodel = true, +}; + +int daisy_drv_init(void) +{ + return parport_register_driver(&daisy_driver); +} + +void daisy_drv_exit(void) +{ + parport_unregister_driver(&daisy_driver); +} diff --git a/drivers/parport/probe.c b/drivers/parport/probe.c index e035174ba205..e5e6a463a941 100644 --- a/drivers/parport/probe.c +++ b/drivers/parport/probe.c @@ -257,7 +257,7 @@ static ssize_t parport_read_device_id (struct parport *port, char *buffer, ssize_t parport_device_id (int devnum, char *buffer, size_t count) { ssize_t retval = -ENXIO; - struct pardevice *dev = parport_open (devnum, "Device ID probe"); + struct pardevice *dev = parport_open(devnum, daisy_dev_name); if (!dev) return -ENXIO; diff --git a/drivers/parport/share.c b/drivers/parport/share.c index 5dc53d420ca8..0171b8dbcdcd 100644 --- a/drivers/parport/share.c +++ b/drivers/parport/share.c @@ -137,11 +137,19 @@ static struct bus_type parport_bus_type = { int parport_bus_init(void) { - return bus_register(&parport_bus_type); + int retval; + + retval = bus_register(&parport_bus_type); + if (retval) + return retval; + daisy_drv_init(); + + return 0; } void parport_bus_exit(void) { + daisy_drv_exit(); bus_unregister(&parport_bus_type); } diff --git a/include/linux/parport.h b/include/linux/parport.h index 397607a0c0eb..f41f1d041e2c 100644 --- a/include/linux/parport.h +++ b/include/linux/parport.h @@ -460,6 +460,7 @@ extern size_t parport_ieee1284_epp_read_addr (struct parport *, void *, size_t, int); /* IEEE1284.3 functions */ +#define daisy_dev_name "Device ID probe" extern int parport_daisy_init (struct parport *port); extern void parport_daisy_fini (struct parport *port); extern struct pardevice *parport_open (int devnum, const char *name); @@ -468,6 +469,18 @@ extern ssize_t parport_device_id (int devnum, char *buffer, size_t len); extern void parport_daisy_deselect_all (struct parport *port); extern int parport_daisy_select (struct parport *port, int daisy, int mode); +#ifdef CONFIG_PARPORT_1284 +extern int daisy_drv_init(void); +extern void daisy_drv_exit(void); +#else +static inline int daisy_drv_init(void) +{ + return 0; +} + +static inline void daisy_drv_exit(void) {} +#endif + /* Lowlevel drivers _can_ call this support function to handle irqs. */ static inline void parport_generic_irq(struct parport *port) { -- 2.11.0
[PATCH 1/2] parport: daisy: use new parport device model
Modify parport daisy driver to use the new parallel port device model. Signed-off-by: Sudip Mukherjee --- drivers/parport/daisy.c | 32 +++- drivers/parport/probe.c | 2 +- drivers/parport/share.c | 14 +- include/linux/parport.h | 3 +++ 4 files changed, 48 insertions(+), 3 deletions(-) diff --git a/drivers/parport/daisy.c b/drivers/parport/daisy.c index 5484a46dafda..56dd83a45e55 100644 --- a/drivers/parport/daisy.c +++ b/drivers/parport/daisy.c @@ -213,10 +213,12 @@ void parport_daisy_fini(struct parport *port) struct pardevice *parport_open(int devnum, const char *name) { struct daisydev *p = topology; + struct pardev_cb par_cb; struct parport *port; struct pardevice *dev; int daisy; + memset(&par_cb, 0, sizeof(par_cb)); spin_lock(&topology_lock); while (p && p->devnum != devnum) p = p->next; @@ -230,7 +232,7 @@ struct pardevice *parport_open(int devnum, const char *name) port = parport_get_port(p->port); spin_unlock(&topology_lock); - dev = parport_register_device(port, name, NULL, NULL, NULL, 0, NULL); + dev = parport_register_dev_model(port, name, &par_cb, devnum); parport_put_port(port); if (!dev) return NULL; @@ -480,3 +482,31 @@ static int assign_addrs(struct parport *port) kfree(deviceid); return detected; } + +static int daisy_drv_probe(struct pardevice *par_dev) +{ + struct device_driver *drv = par_dev->dev.driver; + + if (strcmp(drv->name, "daisy_drv")) + return -ENODEV; + if (strcmp(par_dev->name, daisy_dev_name)) + return -ENODEV; + + return 0; +} + +static struct parport_driver daisy_driver = { + .name = "daisy_drv", + .probe = daisy_drv_probe, + .devmodel = true, +}; + +int daisy_drv_init(void) +{ + return parport_register_driver(&daisy_driver); +} + +void daisy_drv_exit(void) +{ + parport_unregister_driver(&daisy_driver); +} diff --git a/drivers/parport/probe.c b/drivers/parport/probe.c index e035174ba205..e5e6a463a941 100644 --- a/drivers/parport/probe.c +++ b/drivers/parport/probe.c @@ -257,7 +257,7 @@ static ssize_t parport_read_device_id (struct parport *port, char *buffer, ssize_t parport_device_id (int devnum, char *buffer, size_t count) { ssize_t retval = -ENXIO; - struct pardevice *dev = parport_open (devnum, "Device ID probe"); + struct pardevice *dev = parport_open(devnum, daisy_dev_name); if (!dev) return -ENXIO; diff --git a/drivers/parport/share.c b/drivers/parport/share.c index 5dc53d420ca8..b89560d53a35 100644 --- a/drivers/parport/share.c +++ b/drivers/parport/share.c @@ -137,11 +137,23 @@ static struct bus_type parport_bus_type = { int parport_bus_init(void) { - return bus_register(&parport_bus_type); + int retval; + + retval = bus_register(&parport_bus_type); + if (retval) + return retval; +#ifdef CONFIG_PARPORT_1284 + daisy_drv_init(); +#endif + + return 0; } void parport_bus_exit(void) { +#ifdef CONFIG_PARPORT_1284 + daisy_drv_exit(); +#endif bus_unregister(&parport_bus_type); } diff --git a/include/linux/parport.h b/include/linux/parport.h index 397607a0c0eb..e0992953ff2b 100644 --- a/include/linux/parport.h +++ b/include/linux/parport.h @@ -460,6 +460,7 @@ extern size_t parport_ieee1284_epp_read_addr (struct parport *, void *, size_t, int); /* IEEE1284.3 functions */ +#define daisy_dev_name "Device ID probe" extern int parport_daisy_init (struct parport *port); extern void parport_daisy_fini (struct parport *port); extern struct pardevice *parport_open (int devnum, const char *name); @@ -467,6 +468,8 @@ extern void parport_close (struct pardevice *dev); extern ssize_t parport_device_id (int devnum, char *buffer, size_t len); extern void parport_daisy_deselect_all (struct parport *port); extern int parport_daisy_select (struct parport *port, int daisy, int mode); +extern int daisy_drv_init(void); +extern void daisy_drv_exit(void); /* Lowlevel drivers _can_ call this support function to handle irqs. */ static inline void parport_generic_irq(struct parport *port) -- 2.11.0
[PATCH 2/2] parport_pc: fix find_superio io compare code, should use equal test.
From: QiaoChong In the original code before 181bf1e815a2 the loop was continuing until it finds the first matching superios[i].io and p->base. But after 181bf1e815a2 the logic changed and the loop now returns the pointer to the first mismatched array element which is then used in get_superio_dma() and get_superio_irq() and thus returning the wrong value. Fix the condition so that it now returns the correct pointer. Fixes: 181bf1e815a2 ("parport_pc: clean up the modified while loops using for") Cc: Alan Cox Cc: sta...@vger.kernel.org Signed-off-by: QiaoChong Signed-off-by: Sudip Mukherjee [rewrite the commit message] --- drivers/parport/parport_pc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/parport/parport_pc.c b/drivers/parport/parport_pc.c index 9c8249f74479..6296dbb83d47 100644 --- a/drivers/parport/parport_pc.c +++ b/drivers/parport/parport_pc.c @@ -1377,7 +1377,7 @@ static struct superio_struct *find_superio(struct parport *p) { int i; for (i = 0; i < NR_SUPERIOS; i++) - if (superios[i].io != p->base) + if (superios[i].io == p->base) return &superios[i]; return NULL; } -- 2.11.0
Re: [PATCH] parport_pc: fix find_superio io compare code, should use equal test.
Hi QiaoChong, On Tue, Jan 22, 2019 at 3:04 PM qiaochong wrote: > > From: QiaoChong > > git blame drivers/parport/parport_pc.c > > 181bf1e815a2a (Alan Cox 2009-06-11 13:08:10 +0100 1376) > static struct superio_struct *find_superio(struct parport *p) > ^1da177e4c3f4 (Linus Torvalds2005-04-16 15:20:36 -0700 1377) { > 181bf1e815a2a (Alan Cox 2009-06-11 13:08:10 +0100 1378) > int i; > 181bf1e815a2a (Alan Cox 2009-06-11 13:08:10 +0100 1379) > for (i = 0; i < NR_SUPERIOS; i++) > 181bf1e815a2a (Alan Cox 2009-06-11 13:08:10 +0100 1380) > if (superios[i].io != p->base) > 181bf1e815a2a (Alan Cox 2009-06-11 13:08:10 +0100 1381) > return &superios[i]; > 181bf1e815a2a (Alan Cox 2009-06-11 13:08:10 +0100 1382) > return NULL; > 181bf1e815a2a (Alan Cox 2009-06-11 13:08:10 +0100 1383) } > 73e0d48b8c28f (Michael Buesch2009-06-11 13:06:31 +0100 1384) > > git log -1 -p 181bf1e815a2a > > -static int get_superio_dma(struct parport *p) > +static struct superio_struct *find_superio(struct parport *p) > { > - int i = 0; > + int i; > + for (i = 0; i < NR_SUPERIOS; i++) > + if (superios[i].io != p->base) > + return &superios[i]; > + return NULL; > +} > > - while ((i < NR_SUPERIOS) && (superios[i].io != p->base)) > - i++; > - if (i != NR_SUPERIOS) > - return superios[i].dma; > > the code before 181bf1e815a2a also mean superio[i].io == p->base, fixup it. So, this means the Super-IO chips irq and dma has not worked since 2009. :( Can you please resend this with a proper commit message.. -- Regards Sudip
Re: [PATCH 4.14 024/105] bnx2x: Remove configured vlans as part of unload sequence.
Hi Greg, On Fri, Jan 11, 2019 at 2:33 PM Greg Kroah-Hartman wrote: > > 4.14-stable review patch. If anyone has any objections, please let me know. > > -- > > [ Upstream commit 04f05230c5c13b1384f66f5186a68d7499e34622 ] This was fixed upstream by 38355a5f9a22 ("bnx2x: Fix NULL pointer dereference in bnx2x_del_all_vlans() on some hw") -- Regards Sudip
Re: [PATCH 4.14 058/105] tools: fix cross-compile var clobbering
Hi Greg, On Fri, Jan 11, 2019 at 2:34 PM Greg Kroah-Hartman wrote: > > 4.14-stable review patch. If anyone has any objections, please let me know. > > -- > > From: Martin Kelly > > commit 7ed1c1901fe52e6c5828deb155920b44b0adabb1 upstream. This was fixed upstream by 755396163148 ("tools: power/acpi, revert to LD = gcc") -- Regards Sudip
Re: [PATCH 4.14 090/101] MIPS: c-r4k: Add r4k_blast_scache_node for Loongson-3
On Tue, Jan 8, 2019 at 7:38 AM Greg Kroah-Hartman wrote: > > On Mon, Jan 07, 2019 at 09:17:22PM +, Sudip Mukherjee wrote: > > Hi Greg, > > > > On Mon, Jan 7, 2019 at 1:14 PM Greg Kroah-Hartman > > wrote: > > > > > > 4.14-stable review patch. If anyone has any objections, please let me > > > know. > > > > > > -- > > > > > > From: Huacai Chen > > > > > > commit bb53fdf395eed103f85061bfff3b116cee123895 upstream. > > > > This has been fixed by 66a4059ba72c ("MIPS: Only include mmzone.h when > > CONFIG_NEED_MULTIPLE_NODES=y") > > That commit is already in the queues for 4.14, 4.19, and 4.20, so all > should be fine, right? I am not seeing it in 4.14.92-rc3. Am I missing something? -- Regards Sudip
Re: [PATCH 4.14 090/101] MIPS: c-r4k: Add r4k_blast_scache_node for Loongson-3
Hi Greg, On Mon, Jan 7, 2019 at 1:14 PM Greg Kroah-Hartman wrote: > > 4.14-stable review patch. If anyone has any objections, please let me know. > > -- > > From: Huacai Chen > > commit bb53fdf395eed103f85061bfff3b116cee123895 upstream. This has been fixed by 66a4059ba72c ("MIPS: Only include mmzone.h when CONFIG_NEED_MULTIPLE_NODES=y") -- Regards Sudip
Re: [PATCH 4.14 069/101] spi: bcm2835: Fix race on DMA termination
HI Greg, On Mon, Jan 7, 2019 at 1:15 PM Greg Kroah-Hartman wrote: > > 4.14-stable review patch. If anyone has any objections, please let me know. > > -- > > From: Lukas Wunner > > commit e82b0b3828451c1cd331d9f304c6078fcd43b62e upstream. This has been fixed later by 29bdedfd9cf4 ("spi: bcm2835: Unbreak the build of esoteric configs") -- Regards Sudip
Re: [PATCH 4.14 002/101] net: core: Fix Spectre v1 vulnerability
Hi Greg, On Mon, Jan 7, 2019 at 1:00 PM Greg Kroah-Hartman wrote: > > 4.14-stable review patch. If anyone has any objections, please let me know. > > -- > > From: "Gustavo A. R. Silva" > > [ Upstream commit 50d5258634aee2e62832aa086d2fb0de00e72b91 ] This has been reverted upstream by f2ab95814103 ("net: Revert recent Spectre-v1 patches.") -- Regards Sudip
Re: [PATCH 4.14 001/101] phonet: af_phonet: Fix Spectre v1 vulnerability
Hi Greg, On Mon, Jan 7, 2019 at 1:16 PM Greg Kroah-Hartman wrote: > > 4.14-stable review patch. If anyone has any objections, please let me know. > > -- > > From: "Gustavo A. R. Silva" > > [ Upstream commit d686026b1e6ed4ea27d630d8f54f9a694db088b2 ] This has been reverted upstream by f2ab95814103 ("net: Revert recent Spectre-v1 patches.") -- Regards Sudip