Re: [PATCH 23/79] libperf: Make libperf.a part of the build
On Mon, Jul 22, 2019 at 09:39:24AM -0300, Arnaldo Carvalho de Melo wrote: > Em Sun, Jul 21, 2019 at 01:24:10PM +0200, Jiri Olsa escreveu: > > Adding empty libperf.a under toos/perf/lib and > > linking it with perf. > > So, I noticed you created a subdirectory under tools/perf/, while I > first thought you would have it in tools/lib/perf/, why not move it > there? yea I started like this, and then there was only one reason I could think of ;-) I mentioned it in the cover letter: - move under tools/lib after the interface is more stable-ish if you insist to move it there now, would it be ok to make it under separate patch afterwards? it would safe me zillions of small changes ;-) thanks, jirka > > - Arnaldo > > > It can also be built separately with: > > > > $ cd tools/perf/lib && make > > CC core.o > > LD libperf-in.o > > AR libperf.a > > LINK libperf.so > > > > Link: http://lkml.kernel.org/n/tip-lzrlfu3hutepbeqyntjks...@git.kernel.org > > Signed-off-by: Jiri Olsa > > --- > > tools/perf/Makefile.config | 1 + > > tools/perf/Makefile.perf | 30 > > tools/perf/lib/Build | 1 + > > tools/perf/lib/Makefile| 74 ++ > > tools/perf/lib/core.c | 0 > > 5 files changed, 92 insertions(+), 14 deletions(-) > > create mode 100644 tools/perf/lib/Build > > create mode 100644 tools/perf/lib/Makefile > > create mode 100644 tools/perf/lib/core.c > > > > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config > > index 89ac5a1f1550..e4988f49ea79 100644 > > --- a/tools/perf/Makefile.config > > +++ b/tools/perf/Makefile.config > > @@ -277,6 +277,7 @@ ifeq ($(DEBUG),0) > >endif > > endif > > > > +INC_FLAGS += -I$(src-perf)/lib/include > > INC_FLAGS += -I$(src-perf)/util/include > > INC_FLAGS += -I$(src-perf)/arch/$(SRCARCH)/include > > INC_FLAGS += -I$(srctree)/tools/include/uapi > > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf > > index 0fffd2bb6cd9..6e7e7d44ffac 100644 > > --- a/tools/perf/Makefile.perf > > +++ b/tools/perf/Makefile.perf > > @@ -224,6 +224,7 @@ LIB_DIR = $(srctree)/tools/lib/api/ > > TRACE_EVENT_DIR = $(srctree)/tools/lib/traceevent/ > > BPF_DIR = $(srctree)/tools/lib/bpf/ > > SUBCMD_DIR = $(srctree)/tools/lib/subcmd/ > > +LIBPERF_DIR = $(srctree)/tools/perf/lib/ > > > > # Set FEATURE_TESTS to 'all' so all possible feature checkers are executed. > > # Without this setting the output feature dump file misses some features, > > for > > @@ -272,6 +273,7 @@ ifneq ($(OUTPUT),) > >TE_PATH=$(OUTPUT) > >BPF_PATH=$(OUTPUT) > >SUBCMD_PATH=$(OUTPUT) > > + LIBPERF_PATH=$(OUTPUT) > > ifneq ($(subdir),) > >API_PATH=$(OUTPUT)/../lib/api/ > > else > > @@ -282,6 +284,7 @@ else > >API_PATH=$(LIB_DIR) > >BPF_PATH=$(BPF_DIR) > >SUBCMD_PATH=$(SUBCMD_DIR) > > + LIBPERF_PATH=$(LIBPERF_DIR) > > endif > > > > LIBTRACEEVENT = $(TE_PATH)libtraceevent.a > > @@ -303,6 +306,8 @@ LIBBPF = $(BPF_PATH)libbpf.a > > > > LIBSUBCMD = $(SUBCMD_PATH)libsubcmd.a > > > > +LIBPERF = $(LIBPERF_PATH)libperf.a > > + > > # python extension build directories > > PYTHON_EXTBUILD := $(OUTPUT)python_ext_build/ > > PYTHON_EXTBUILD_LIB := $(PYTHON_EXTBUILD)lib/ > > @@ -348,9 +353,7 @@ endif > > > > export PERL_PATH > > > > -LIBPERF_A=$(OUTPUT)libperf.a > > - > > -PERFLIBS = $(LIBAPI) $(LIBTRACEEVENT) $(LIBSUBCMD) > > +PERFLIBS = $(LIBAPI) $(LIBTRACEEVENT) $(LIBSUBCMD) $(LIBPERF) > > ifndef NO_LIBBPF > >PERFLIBS += $(LIBBPF) > > endif > > @@ -583,8 +586,6 @@ JEVENTS_IN:= $(OUTPUT)pmu-events/jevents-in.o > > > > PMU_EVENTS_IN := $(OUTPUT)pmu-events/pmu-events-in.o > > > > -LIBPERF_IN := $(OUTPUT)libperf-in.o > > - > > export JEVENTS > > > > build := -f $(srctree)/tools/build/Makefile.build dir=. obj > > @@ -601,12 +602,9 @@ $(JEVENTS): $(JEVENTS_IN) > > $(PMU_EVENTS_IN): $(JEVENTS) FORCE > > $(Q)$(MAKE) -f $(srctree)/tools/build/Makefile.build dir=pmu-events > > obj=pmu-events > > > > -$(LIBPERF_IN): prepare FORCE > > - $(Q)$(MAKE) $(build)=libperf > > - > > -$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(PMU_EVENTS_IN) $(LIBPERF_IN) > > $(LIBTRACEEVENT_DYNAMIC_LIST) > > +$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(PMU_EVENTS_IN) > > $(LIBTRACEEVENT_DYNAMIC_LIST) > > $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) > > $(LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS) \ > > - $(PERF_IN) $(PMU_EVENTS_IN) $(LIBPERF_IN) $(LIBS) -o $@ > > + $(PERF_IN) $(PMU_EVENTS_IN) $(LIBS) -o $@ > > > > $(GTK_IN): FORCE > > $(Q)$(MAKE) $(build)=gtk > > @@ -727,9 +725,6 @@ endif > > > > $(patsubst perf-%,%.o,$(PROGRAMS)): $(wildcard */*.h) > > > > -$(LIBPERF_A): $(LIBPERF_IN) > > - $(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(LIBPERF_IN) $(LIB_OBJS) > > - > > LIBTRACEEVENT_FLAGS += plugin_dir=$(plugindir_SQ) > > 'EXTRA_CFLAGS=$(EXTRA_CFLAGS)' 'LDFLAGS=$(LDFLAGS)' > > > > $(LIBTRACEEVENT): FO
Re: RFC: call_rcu_outstanding (was Re: WARNING in __mmdrop)
On Mon, Jul 22, 2019 at 11:47:24AM -0400, Michael S. Tsirkin wrote: > On Mon, Jul 22, 2019 at 11:14:39AM -0400, Joel Fernandes wrote: > > [snip] > > > > Would it make sense to have call_rcu() check to see if there are many > > > > outstanding requests on this CPU and if so process them before > > > > returning? > > > > That would ensure that frequent callers usually ended up doing their > > > > own processing. > > > > Other than what Paul already mentioned about deadlocks, I am not sure if > > this > > would even work for all cases since call_rcu() has to wait for a grace > > period. > > > > So, if the number of outstanding requests are higher than a certain amount, > > then you *still* have to wait for some RCU configurations for the grace > > period duration and cannot just execute the callback in-line. Did I miss > > something? > > > > Can waiting in-line for a grace period duration be tolerated in the vhost > > case? > > > > thanks, > > > > - Joel > > No, but it has many other ways to recover (try again later, drop a > packet, use a slower copy to/from user). True enough! And your idea of taking recovery action based on the number of callbacks seems like a good one while we are getting RCU's callback scheduling improved. By the way, was this a real problem that you could make happen on real hardware? If not, I would suggest just letting RCU get improved over the next couple of releases. If it is something that you actually made happen, please let me know what (if anything) you need from me for your callback-counting EBUSY scheme. Thanx, Paul
Re: [for-next][PATCH 12/16] kprobes: Initialize kprobes at postcore_initcall
On Mon, Jul 22, 2019 at 11:00:10AM -0400, Steven Rostedt wrote: > On Mon, 22 Jul 2019 13:42:02 +0100 > Catalin Marinas wrote: > > > > I agree with Masami, that the selftest actually caught a bug here. I > > > think the arch code may need to change as the purpose of Masami's > > > changes was to enable kprobes earlier in boot. The selftest failing > > > means that an early kprobe will fail too. > > > > I just got back from holiday and catching up with emails. Do I still > > need to merge the arm64 fix making the debug initialisation a > > core_initcall()? > > > > Can we actually get kprobes invoked before init_kprobes() has been > > called? > > Bah, I can't remember everything that we discussed. I thought there was > some more patches that obsoleted my work around. Everything has been > pushed to Linus, can you see if the upstream still has issues for you? Upstream is fine since you reverted the postcore_initcall(init_kprobes) change and used subsys_initcall() instead. So I don't think we need any arch changes. -- Catalin
Re: Driver has suspect GRO implementation, TCP performance may be compromised.
Dear Eric,
Sorry for the late reply.
On 5/29/19 6:00 PM, Eric Dumazet wrote:
> On Wed, May 29, 2019 at 7:49 AM Paul Menzel wrote:
>> On 05/28/19 19:18, Eric Dumazet wrote:
>>> On 5/28/19 8:42 AM, Paul Menzel wrote:
>>
Occasionally, Linux outputs the message below on the workstation Dell
OptiPlex 5040 MT.
TCP: net00: Driver has suspect GRO implementation, TCP performance may
be compromised.
Linux 4.14.55 and Linux 5.2-rc2 show the message, and the WWW also
gives some hits [1][2].
```
$ sudo ethtool -i net00
driver: e1000e
version: 3.2.6-k
firmware-version: 0.8-4
expansion-rom-version:
bus-info: :00:1f.6
supports-statistics: yes
supports-test: yes
supports-eeprom-access: yes
supports-register-dump: yes
supports-priv-flags: no
```
Can the driver e1000e be improved?
Any idea, what triggers this, as I do not see it every boot? Download
of big files?
>>> Maybe the driver/NIC can receive frames bigger than MTU, although this
>>> would be strange.
>>>
>>> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
>>> index
>>> c61edd023b352123e2a77465782e0d32689e96b0..cb0194f66125bcba427e6e7e3cacf0c93040ef61
>>> 100644
>>> --- a/net/ipv4/tcp_input.c
>>> +++ b/net/ipv4/tcp_input.c
>>> @@ -150,8 +150,10 @@ static void tcp_gro_dev_warn(struct sock *sk, const
>>> struct sk_buff *skb,
>>> rcu_read_lock();
>>> dev = dev_get_by_index_rcu(sock_net(sk), skb->skb_iif);
>>> if (!dev || len >= dev->mtu)
>>> - pr_warn("%s: Driver has suspect GRO implementation,
>>> TCP performance may be compromised.\n",
>>> - dev ? dev->name : "Unknown driver");
>>> + pr_warn("%s: Driver has suspect GRO implementation,
>>> TCP performance may be compromised."
>>> + " len %u mtu %u\n",
>>> + dev ? dev->name : "Unknown driver",
>>> + len, dev ? dev->mtu : 0);
>>> rcu_read_unlock();
>>> }
>>> }
>>
>> I applied your patch on commit 9fb67d643 (Merge tag 'pinctrl-v5.2-2' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl):
>>
>> [ 5507.291769] TCP: net00: Driver has suspect GRO implementation, TCP
>> performance may be compromised. len 1856 mtu 1500
>
>
> The 'GRO' in the warning can be probably ignored, since this NIC does
> not implement its own GRO.
>
> You can confirm this with this debug patch:
>
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c
> b/drivers/net/ethernet/intel/e1000e/netdev.c
> index
> 0e09bede42a2bd2c912366a68863a52a22def8ee..014a43ce77e09664bda0568dd118064b006acd67
> 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -561,6 +561,9 @@ static void e1000_receive_skb(struct e1000_adapter
> *adapter,
> if (staterr & E1000_RXD_STAT_VP)
> __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), tag);
>
> + if (skb->len > netdev->mtu)
> + pr_err_ratelimited("received packet bigger (%u) than
> MTU (%u)\n",
> + skb->len, netdev->mtu);
> napi_gro_receive(&adapter->napi, skb);
> }
With this patch applied, I unfortunately could not trigger the condition
anymore. No idea why. Or is that expected?
(As a side note, plain Linux 5.2.2 still shows the warning.)
Kind regards,
Paul
smime.p7s
Description: S/MIME Cryptographic Signature
Re: RFC: call_rcu_outstanding (was Re: WARNING in __mmdrop)
On Mon, Jul 22, 2019 at 08:52:35AM -0700, Paul E. McKenney wrote: > So why then is there a problem? I'm not sure there is a real problem, I thought Michael was just asking how to design with RCU in the case where the user controls the kfree_rcu?? Sounds like the answer is "don't worry about it" ? Thanks, Jason
Re: About threaded interrupt handler CPU affinity
On 22/07/2019 16:34, Thomas Gleixner wrote:
John,
Hi Thomas,
On Mon, 22 Jul 2019, John Garry wrote:
On 22/07/2019 15:41, Marc Zyngier wrote:
On 22/07/2019 15:14, John Garry wrote:
I have a question on commit cbf866a6 ("genirq: Let irq thread follow
the effective hard irq affinity"), if you could kindly check:
Here we set the thread affinity to be the same as the hard interrupt
affinity. For an arm64 system with GIC ITS, this will be a single CPU,
the lowest in the interrupt affinity mask. So, in this case, effectively
the thread will be bound to a single CPU. I think APIC is the same for
this.
The commit message describes the problem that we solve here is that the
thread may become affine to a different CPU to the hard interrupt - does
it mean that the thread CPU mask could not cover that of the hard
interrupt? I couldn't follow the reason.
Assume a 4 CPU system. If the interrupt affinity is on CPU0-1, you could
end up with the effective interrupt affinity on CPU0 (which would be
typical of the ITS), and the thread running on CPU1. Not great.
Sure, not great. But the thread can possibly still run on CPU0.
Sure. It could, but it's up to the scheduler to decide. In general it's the
right thing to run the threaded handler on the CPU which handles the
interrupt.
I'd agree.
With single CPU affinity thats surely a limitation.
We have experimented with fixing the thread mask to be the same as the
interrupt mask (we're using managed interrupts), like before, and get a
significant performance boost at high IO datarates on our storage
controller - like ~11%.
My understanding is that this patch does exactly that. Does it result in
a regression?
Not in the strictest sense for us, I don't know about others. Currently we use
tasklets, and we find that the CPUs servicing the interrupts (and hence
tasklets) are heavily loaded. We experience the same for when experimenting
with threaded interrupt handlers - which would be as expected.
But, when we make the change as mentioned, our IOPS goes from ~3M -> 3.4M.
So your interrupt is affined to more than one CPU, but due to the ITS
limitation the effective affinity is a single CPU, which in turn restricts
the thread handler affinity to the same single CPU.
Even though this is an ITS limitation, the same thing is effectively
done for x86 APIC as policy, right? I'm referring to commit fdba46ffb4c2
("x86/apic: Get rid of multi CPU affinity")
If you lift that
restriction and let it be affine to the full affinity set of the interrupt
then you get better performance, right?
Right
Probably because the other CPU(s)
in the affinity set are less loaded than the one which handles the hard
interrupt.
I will look to get some figures for CPU loading to back this up.
This is heavily use case dependent I assume, so making this a general
change is perhaps not a good idea, but we could surely make this optional.
That sounds reasonable. So would the idea be to enable this optionally
at the request threaded irq call?
Thanks,
John
Thanks,
tglx
.
Re: [v5.3-rc1 regression] Hitting a kernel BUG() when trying to load a module on DaVinci SoC
+++ Bartosz Golaszewski [22/07/19 14:12 +0200]:
Hi,
with v5.3-rc1 I'm hitting the following BUG() when trying to load the
gpio-backlight module:
kernel BUG at kernel/module.c:1919!
Internal error: Oops - BUG: 0 [#1] PREEMPT ARM
Modules linked in:
CPU: 0 PID: 1 Comm: systemd Tainted: GW
5.2.0-rc2-5-g7dabaa5ce05a #19
Hardware name: DaVinci DA850/OMAP-L138/AM18x EVM
PC is at frob_text.constprop.16+0x2c/0x34
LR is at load_module+0x1888/0x21b4
pc : []lr : []psr: 2013
sp : c6837e58 ip : c6b4fa80 fp : bf00574c
r10: c0601008 r9 : bf005740 r8 : c0493e38
r7 : c00807f8 r6 : r5 : 0001 r4 : c6837f38
r3 : 0fff r2 : bf00 r1 : 4b80 r0 : bf005818
Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
Control: 0005317f Table: c6b78000 DAC: 0051
Process systemd (pid: 1, stack limit = 0x(ptrval))
Stack: (0xc6837e58 to 0xc6838000)
7e40: bf005740 c6492a90
7e60: 0001 c6bf5788 0003 c6837f38 bf0058c0 bf0058a8 bf00a495 c0493e38
7e80: c05368c0 0001 c0580030 c0574b28
7ea0: 6e72656b 6c65
7ec0:
7ee0: aefb2bb8 7fff c0601008 0004
7f00: b6cee714 c00091e4 c6836000 005bc668 c00847b0 7fff
7f20: 0003 b2d0 c884e000 b2d0 c8852dcb c88533a0
7f40: c884e000 b2d0 c8858cb8 c8858b34 c88568d4 58c0 6190 23b8
7f60: 6713 23a8 0024 0025 0019
7f80: 001d 0011 aefb2bb8
7fa0: 017b c0009000 0004 b6cee714
7fc0: 017b 0001 005bc668
7fe0: be907b00 be907af0 b6ce66b0 b6c3fac0 6010 0004
[] (frob_text.constprop.16) from []
(load_module+0x1888/0x21b4)
[] (load_module) from [] (sys_finit_module+0xbc/0xdc)
[] (sys_finit_module) from [] (ret_fast_syscall+0x0/0x50)
Exception stack(0xc6837fa8 to 0xc6837ff0)
7fa0: 0004 b6cee714
7fc0: 017b 0001 005bc668
7fe0: be907b00 be907af0 b6ce66b0 b6c3fac0
Code: e1a01621 e1a2 eafe4531 e7f001f2 (e7f001f2)
---[ end trace 2cbefb0005882c52 ]---
Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b
---[ end Kernel panic - not syncing: Attempted to kill init!
exitcode=0x000b ]---
I bisected it to commit 06bd260e836d ("modules: fix BUG when load
module with rodata=n") with commit 7dabaa5ce05a ("modules: fix compile
error if don't have strict module rwx") on top to make it build.
Let me know if you need me to provide more info.
Best regards,
Bartosz Golaszewski
Hi Bartosz,
Thanks for reporting this, I was able to reproduce this on qemu.
This is due to hitting the BUG_ON() in frob_text() due to layout->base
and/or layout->text_size not being page-aligned. These values are
always page-aligned when CONFIG_STRICT_MODULE_RWX=y, but in commit
2eef1399a86 ("modules: fix BUG when load module with rodata=n"), the
frob_text()+set_memory_x() calls got moved *outside* of the
STRICT_MODULE_RWX block since some arches (like x86 and arm64)
allocate non-executable module memory via module_alloc(), so naturally
the module text needed to be made executable at a later stage of
load_module(), regardless of whether STRICT_MODULE_RWX is set or not.
In your case, you must've had CONFIG_STRICT_MODULE_RWX=n and so we
were calling frob_text() with non-page-aligned values, triggering the
BUG_ON().
In any case, could you please try and see if the following patch fixes
the issue for you?
diff --git a/kernel/module.c b/kernel/module.c
index 5933395af9a0..cd8df51d 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -64,14 +64,9 @@
/*
* Modules' sections will be aligned on page boundaries
- * to ensure complete separation of code and data, but
- * only when CONFIG_STRICT_MODULE_RWX=y
+ * to ensure complete separation of code and data
*/
-#ifdef CONFIG_STRICT_MODULE_RWX
# define debug_align(X) ALIGN(X, PAGE_SIZE)
-#else
-# define debug_align(X) (X)
-#endif
/* If this is set, the section belongs in the init part of the module */
#define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1))
Re: next-20190722, imx25: Oops when loading a module
+++ Martin Kaiser [22/07/19 12:13 +0200]: Dear all, I run next-20190722 on an arm imx25 system and came across an issue that might be worth reporting. I am no sure to whom, though. Please let me know if I got that wrong. Loading a module, no matter which one, causes a segfault and a dump such as [root@host ]# insmod /mnt/kernel/iio/potentiometer/max5432.ko [ 63.043683] Internal error: Oops - undefined instruction: 0 [#1] ARM [ 63.050123] Modules linked in: [ 63.053266] CPU: 0 PID: 170 Comm: insmod Tainted: GW 5.3.0-rc1-next-20190722+ #3104 [ 63.062344] Hardware name: Freescale i.MX25 (Device Tree Support) [ 63.068529] PC is at frob_text.constprop.15+0x2c/0x40 [ 63.073639] LR is at load_module+0x10dc/0x125c [ 63.078115] pc : []lr : []psr: 0013 [ 63.084407] sp : d3303e30 ip : d3303e40 fp : d3303e3c [ 63.089654] r10: r9 : d3303e98 r8 : 0018 [ 63.094903] r7 : 0001 r6 : bf0006cc r5 : d3303f20 r4 : bf0006c0 [ 63.101454] r3 : bf00 r2 : 1800 r1 : 0180 r0 : bf0007a0 [ 63.108013] Flags: nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [ 63.115176] Control: 0005317f Table: 9333 DAC: 0051 [ 63.120961] Process insmod (pid: 170, stack limit = 0x90da5324) [ 63.126917] Stack: (0xd3303e30 to 0xd3304000) [ 63.131318] 3e20: d3303f0c d3303e40 c0071f8c c0072fe0 [ 63.139546] 3e40: bf0006c0 d3309920 2cc0 0002 0002 d3309900 [ 63.147776] 3e60: 001a77e2 bf00289b bf0008a0 c067c9c8 025f c07df990 d3303ecc d3303e88 [ 63.155999] 3e80: c00d7688 c00d689c [ 63.164225] 3ea0: 6e72656b 6c65 [ 63.172453] 3ec0: ba520d2b [ 63.180681] 3ee0: c00721a8 1aed 0120abad c0883028 d4c17aed 001a77e2 d3302000 [ 63.188912] 3f00: d3303fa4 d3303f10 c0072270 c0070ec0 c0034324 c0677478 d32f1640 0051 [ 63.197142] 3f20: d4c16788 d4c16a80 d4c16000 1aed d4c16dd8 d4c16cd5 d4c177f0 08a0 [ 63.205368] 3f40: 0950 0850 09b4 0840 001b [ 63.213590] 3f60: 001c 0011 000d 0009 ba520d2b c01030f4 [ 63.221819] 3f80: 1aed 0080 c00091e4 d3302000 d3303fa8 [ 63.230046] 3fa0: c0009000 c007211c 012090c0 1aed 001a77e2 [ 63.238273] 3fc0: 1aed 0080 0001 be90de0c 001a77e2 [ 63.246499] 3fe0: be90dac0 be90dab0 0001ec34 9b30 6010 012090c0 [ 63.254694] Backtrace: [ 63.257232] [] (frob_text.constprop.15) from [] (load_module+0x10dc/0x125c) [ 63.265999] [] (load_module) from [] (sys_init_module+0x164/0x194) [ 63.273970] r10:d3302000 r9:001a77e2 r8:d4c17aed r7:c0883028 r6: r5:0120abad [ 63.281823] r4:1aed [ 63.284410] [] (sys_init_module) from [] (ret_fast_syscall+0x0/0x50) [ 63.292529] Exception stack(0xd3303fa8 to 0xd3303ff0) [ 63.297624] 3fa0: 012090c0 1aed 001a77e2 [ 63.305851] 3fc0: 1aed 0080 0001 be90de0c 001a77e2 [ 63.314067] 3fe0: be90dac0 be90dab0 0001ec34 9b30 [ 63.319170] r10: r9:d3302000 r8:c00091e4 r7:0080 r6:1aed r5: [ 63.327021] r4: [ 63.329603] Code: 1a02 e5901008 e1b02a01 0a00 (e7f001f2) [ 63.335742] ---[ end trace c38bbcd6af0938a2 ]--- Segmentation fault [root@host ]# It seems that this is realated to strict module rwx. The config below crashes: CONFIG_ARCH_HAS_STRICT_MODULE_RWX=y # CONFIG_STRICT_MODULE_RWX is not set If I enable CONFIG_STRICT_MODULE_RWX, modules can be loaded and unloaded without problems. Best regards, Martin Hi Martin, Thank you for reporting this. Could you please try the patch I posted here: https://lore.kernel.org/lkml/20190722161006.GA4297@linux-8ccs/ And let me know if that fixes the issue for you? Thanks, Jessica
Re: RFC: call_rcu_outstanding (was Re: WARNING in __mmdrop)
On Mon, Jul 22, 2019 at 08:55:34AM -0700, Paul E. McKenney wrote: > On Mon, Jul 22, 2019 at 11:47:24AM -0400, Michael S. Tsirkin wrote: > > On Mon, Jul 22, 2019 at 11:14:39AM -0400, Joel Fernandes wrote: > > > [snip] > > > > > Would it make sense to have call_rcu() check to see if there are many > > > > > outstanding requests on this CPU and if so process them before > > > > > returning? > > > > > That would ensure that frequent callers usually ended up doing their > > > > > own processing. > > > > > > Other than what Paul already mentioned about deadlocks, I am not sure if > > > this > > > would even work for all cases since call_rcu() has to wait for a grace > > > period. > > > > > > So, if the number of outstanding requests are higher than a certain > > > amount, > > > then you *still* have to wait for some RCU configurations for the grace > > > period duration and cannot just execute the callback in-line. Did I miss > > > something? > > > > > > Can waiting in-line for a grace period duration be tolerated in the vhost > > > case? > > > > > > thanks, > > > > > > - Joel > > > > No, but it has many other ways to recover (try again later, drop a > > packet, use a slower copy to/from user). > > True enough! And your idea of taking recovery action based on the number > of callbacks seems like a good one while we are getting RCU's callback > scheduling improved. > > By the way, was this a real problem that you could make happen on real > hardware? > If not, I would suggest just letting RCU get improved over > the next couple of releases. So basically use kfree_rcu but add a comment saying e.g. "WARNING: in the future callers of kfree_rcu might need to check that not too many callbacks get queued. In that case, we can disable the optimization, or recover in some other way. Watch this space." > If it is something that you actually made happen, please let me know > what (if anything) you need from me for your callback-counting EBUSY > scheme. > > Thanx, Paul If you mean kfree_rcu causing OOM then no, it's all theoretical. If you mean synchronize_rcu stalling to the point where guest will OOPs, then yes, that's not too hard to trigger.
[PATCH] ACPI/IORT: Rename arm_smmu_v3_set_proximity() 'node' local variable
Commit 36a2ba07757d ("ACPI/IORT: Reject platform device creation on NUMA
node mapping failure") introduced a local variable 'node' in
arm_smmu_v3_set_proximity() that shadows the struct acpi_iort_node
pointer function parameter.
Execution was unaffected but it is prone to errors and can lead
to subtle bugs.
Rename the local variable to prevent any issue.
Reported-by: Will Deacon
Signed-off-by: Lorenzo Pieralisi
Cc: Will Deacon
Cc: Hanjun Guo
Cc: Sudeep Holla
Cc: Catalin Marinas
Cc: Robin Murphy
Cc: Kefeng Wang
---
drivers/acpi/arm64/iort.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index d4551e33fa71..15dbfd657d82 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1256,12 +1256,12 @@ static int __init arm_smmu_v3_set_proximity(struct
device *dev,
smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
if (smmu->flags & ACPI_IORT_SMMU_V3_PXM_VALID) {
- int node = acpi_map_pxm_to_node(smmu->pxm);
+ int dev_node = acpi_map_pxm_to_node(smmu->pxm);
- if (node != NUMA_NO_NODE && !node_online(node))
+ if (dev_node != NUMA_NO_NODE && !node_online(dev_node))
return -EINVAL;
- set_dev_node(dev, node);
+ set_dev_node(dev, dev_node);
pr_info("SMMU-v3[%llx] Mapped to Proximity domain %d\n",
smmu->base_address,
smmu->pxm);
--
2.21.0
[PATCH RESEND] scsi: megaraid_sas: fix panic on loading firmware crashdump
While loading fw crashdump in function fw_crash_buffer_show(), left bytes in one dma chunk was not checked, if copying size over it, overflow access will cause kernel panic. Signed-off-by: Junxiao Bi --- drivers/scsi/megaraid/megaraid_sas_base.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c index 80ab9700f1de..3eef0858fa8e 100644 --- a/drivers/scsi/megaraid/megaraid_sas_base.c +++ b/drivers/scsi/megaraid/megaraid_sas_base.c @@ -3153,6 +3153,7 @@ fw_crash_buffer_show(struct device *cdev, (struct megasas_instance *) shost->hostdata; u32 size; unsigned long dmachunk = CRASH_DMA_BUF_SIZE; + unsigned long chunk_left_bytes; unsigned long src_addr; unsigned long flags; u32 buff_offset; @@ -3176,6 +3177,8 @@ fw_crash_buffer_show(struct device *cdev, } size = (instance->fw_crash_buffer_size * dmachunk) - buff_offset; + chunk_left_bytes = dmachunk - (buff_offset % dmachunk); + size = (size > chunk_left_bytes) ? chunk_left_bytes : size; size = (size >= PAGE_SIZE) ? (PAGE_SIZE - 1) : size; src_addr = (unsigned long)instance->crash_buf[buff_offset / dmachunk] + -- 2.17.1
Re: RFC: call_rcu_outstanding (was Re: WARNING in __mmdrop)
On Mon, Jul 22, 2019 at 01:04:48PM -0300, Jason Gunthorpe wrote: > On Mon, Jul 22, 2019 at 08:52:35AM -0700, Paul E. McKenney wrote: > > So why then is there a problem? > > I'm not sure there is a real problem, I thought Michael was just > asking how to design with RCU in the case where the user controls the > kfree_rcu?? Right it's all based on documentation saying we should worry :) > Sounds like the answer is "don't worry about it" ? > > Thanks, > Jason
Re: RFC: call_rcu_outstanding (was Re: WARNING in __mmdrop)
On Mon, Jul 22, 2019 at 01:04:48PM -0300, Jason Gunthorpe wrote: > On Mon, Jul 22, 2019 at 08:52:35AM -0700, Paul E. McKenney wrote: > > So why then is there a problem? > > I'm not sure there is a real problem, I thought Michael was just > asking how to design with RCU in the case where the user controls the > kfree_rcu?? > > Sounds like the answer is "don't worry about it" ? Unless you can force failures, you should be good. And either way, improvements to RCU's handling of this sort of situation are in the works. And rcutorture has gained tests of this stuff in the last year or so as well, see its "fwd_progress" module parameter and the related code. Thanx, Paul
Re: [PATCH 1/2] drivers: qcom: rpmh-rsc: simplify TCS locking
On Fri, Jul 19 2019 at 12:20 -0600, Stephen Boyd wrote:
Quoting Lina Iyer (2019-07-01 08:29:06)
From: "Raju P.L.S.S.S.N"
tcs->lock was introduced to serialize access with in TCS group. But
even without tcs->lock, drv->lock is serving the same purpose. So
use a single drv->lock.
Isn't the downside now that we're going to be serializing access to the
different TCSes when two are being written in parallel or waited on? I
thought that was the whole point of splitting the lock into a TCS lock
and a general "driver" lock that protects the global driver state vs.
the specific TCS state.
Yes but we were holding the drv->lock as well as tcs->lock for the most
critical of the path anyways (writing to TCS). The added complexity
doesn't seem to help reduce the latency that it expected to reduce.
Other optimizations include -
- Remove locking around clear_bit() in IRQ handler. clear_bit() is
atomic.
- Remove redundant read of TCS registers.
- Use spin_lock instead of _irq variants as the locks are not held
in interrupt context.
Can you please split this patch up into 3 or 4 different patches? I'm
not sure why any of these patches are marked with Fixes either. It's an
optimization patch, not a fix patch, unless the optimization is really
large somehow?
Okay. I will try that.
Fixes: 658628 ("drivers: qcom: rpmh-rsc: add RPMH controller for QCOM
SoCs")
Signed-off-by: Raju P.L.S.S.S.N
Signed-off-by: Lina Iyer
---
drivers/soc/qcom/rpmh-internal.h | 2 --
drivers/soc/qcom/rpmh-rsc.c | 37 +++-
drivers/soc/qcom/rpmh.c | 20 +++--
3 files changed, 21 insertions(+), 38 deletions(-)
diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
index a767991c..969d5030860e 100644
--- a/drivers/soc/qcom/rpmh-internal.h
+++ b/drivers/soc/qcom/rpmh-internal.h
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index e278fc11fe5c..92461311aef3 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -93,8 +93,7 @@ static void write_tcs_reg_sync(struct rsc_drv *drv, int reg,
int tcs_id,
static bool tcs_is_free(struct rsc_drv *drv, int tcs_id)
{
- return !test_bit(tcs_id, drv->tcs_in_use) &&
- read_tcs_reg(drv, RSC_DRV_STATUS, tcs_id, 0);
+ return !test_bit(tcs_id, drv->tcs_in_use);
This can be a diffedjusted rent patch. Why is reading the tcs register
redundant? Please put that information in the commit text.
The tcs_in_use, is adjusted along with the DRV_STS and reading the
tcs_in_use should be enough.
Thanks for your review Stephen.
--Lina
Re: [PATCH V6 01/21] irqchip: tegra: Do not disable COP IRQ during suspend
On 7/22/19 3:57 AM, Dmitry Osipenko wrote:
22.07.2019 13:13, Marc Zyngier пишет:
On 22/07/2019 10:54, Dmitry Osipenko wrote:
21.07.2019 22:40, Sowjanya Komatineni пишет:
Tegra210 platforms use sc7 entry firmware to program Tegra LP0/SC7 entry
sequence and sc7 entry firmware is run from COP/BPMP-Lite.
So, COP/BPMP-Lite still need IRQ function to finish SC7 suspend sequence
for Tegra210.
This patch has fix for leaving the COP IRQ enabled for Tegra210 during
interrupt controller suspend operation.
Acked-by: Thierry Reding
Signed-off-by: Sowjanya Komatineni
---
drivers/irqchip/irq-tegra.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/irqchip/irq-tegra.c b/drivers/irqchip/irq-tegra.c
index e1f771c72fc4..851f88cef508 100644
--- a/drivers/irqchip/irq-tegra.c
+++ b/drivers/irqchip/irq-tegra.c
@@ -44,6 +44,7 @@ static unsigned int num_ictlrs;
struct tegra_ictlr_soc {
unsigned int num_ictlrs;
+ bool supports_sc7;
};
static const struct tegra_ictlr_soc tegra20_ictlr_soc = {
@@ -56,6 +57,7 @@ static const struct tegra_ictlr_soc tegra30_ictlr_soc = {
static const struct tegra_ictlr_soc tegra210_ictlr_soc = {
.num_ictlrs = 6,
+ .supports_sc7 = true,
};
static const struct of_device_id ictlr_matches[] = {
@@ -67,6 +69,7 @@ static const struct of_device_id ictlr_matches[] = {
struct tegra_ictlr_info {
void __iomem *base[TEGRA_MAX_NUM_ICTLRS];
+ const struct tegra_ictlr_soc *soc;
#ifdef CONFIG_PM_SLEEP
u32 cop_ier[TEGRA_MAX_NUM_ICTLRS];
u32 cop_iep[TEGRA_MAX_NUM_ICTLRS];
@@ -147,8 +150,20 @@ static int tegra_ictlr_suspend(void)
lic->cop_ier[i] = readl_relaxed(ictlr + ICTLR_COP_IER);
lic->cop_iep[i] = readl_relaxed(ictlr + ICTLR_COP_IEP_CLASS);
- /* Disable COP interrupts */
- writel_relaxed(~0ul, ictlr + ICTLR_COP_IER_CLR);
+ /*
+* AVP/COP/BPMP-Lite is the Tegra boot processor.
+*
+* Tegra210 system suspend flow uses sc7entry firmware which
+* is executed by COP/BPMP and it includes disabling COP IRQ,
+* clamping CPU rail, turning off VDD_CPU, and preparing the
+* system to go to SC7/LP0.
+*
+* COP/BPMP wakes up when COP IRQ is triggered and runs
+* sc7entry-firmware. So need to keep COP interrupt enabled.
+*/
+ if (!lic->soc->supports_sc7)
+ /* Disable COP interrupts if SC7 is not supported */
All Tegra SoCs support SC7, hence the 'supports_sc7' and the comment
doesn't sound correct to me. Something like 'firmware_sc7' should suit
better here.
If what you're saying is true, then the whole patch is wrong, and the
SC7 property should come from DT.
It should be safe to assume that all of existing Tegra210 devices use
the firmware for SC7, hence I wouldn't say that the patch is entirely
wrong. To me it's not entirely correct.
Yes, all existing Tegra210 platforms uses sc7 entry firmware for SC7 and
AVP/COP IRQ need to be kept enabled as during suspend ATF triggers IRQ
to COP for SC7 entry fw execution.
+ writel_relaxed(~0ul, ictlr + ICTLR_COP_IER_CLR);
Secondly, I'm also not sure why COP interrupts need to be disabled for
pre-T210 at all, since COP is unused. This looks to me like it was
cut-n-pasted from downstream kernel without a good reason and could be
simply removed.
Please verify that this is actually the case. Tegra-2 definitely needed
some level of poking, and I'm not keen on changing anything there until
you (or someone else) has verified it on actual HW (see e307cc8941fc).
Tested on Tegra20 and Tegra30, LP1 suspend-resume works perfectly fine
with all COP bits removed from the driver.
AFAIK, the reason why downstream needed that disabling is that it uses
proprietary firmware which is running on the COP and that firmware is
usually a BLOB audio/video DEC-ENC driver which doesn't cleanup
interrupts after itself. That firmware is not applicable for the
upstream kernel, hence there is no need to care about it.
Joseph, can you please shed some light here?
SC7 entry flow uses 3rd party ATF (arm-trusted FW) blob which is the one that
actually loads SC7 entry firmware and triggers IRQ to AVP/COP which causes COP
to wakeup and run SC7 entry FW.
So when SC7 support is enabled, IRQ need to be kept enabled and when SC7 FW
starts execution, it will disable COP IRQ.
[GIT pull] sched/urgent for 5.3-rc2
Linus, please pull the latest sched-urgent-for-linus git tree from: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git sched-urgent-for-linus up to: b8d3349803ba: sched/rt, Kconfig: Unbreak def/oldconfig with CONFIG_PREEMPT=y The PREEMPT_RT stub config renamed PREEMPT to PREEMPT_LL and defined PREEMPT outside of the menu and made it selectable by both PREEMPT_LL and PREEMPT_RT. Stupid me missed that 114 defconfigs select CONFIG_PREEMPT which obviously can't work anymore. oldconfig builds are affected as well, but it's more obvious as the user gets asked. [old]defconfig silently fixes it up and selects PREEMPT_NONE. Unbreak it by undoing the rename and adding a intermediate config symbol which is selected by both PREEMPT and PREEMPT_RT. That requires to chase down a few #ifdefs, but it's better than tweaking 114 defconfigs and annoying users. Sorry for the inconveniance. Thanks, tglx --> Thomas Gleixner (1): sched/rt, Kconfig: Unbreak def/oldconfig with CONFIG_PREEMPT=y kernel/Kconfig.preempt | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kernel/Kconfig.preempt b/kernel/Kconfig.preempt index fc020c09b7e8..deff97217496 100644 --- a/kernel/Kconfig.preempt +++ b/kernel/Kconfig.preempt @@ -35,10 +35,10 @@ config PREEMPT_VOLUNTARY Select this if you are building a kernel for a desktop system. -config PREEMPT_LL +config PREEMPT bool "Preemptible Kernel (Low-Latency Desktop)" depends on !ARCH_NO_PREEMPT - select PREEMPT + select PREEMPTION select UNINLINE_SPIN_UNLOCK if !ARCH_INLINE_SPIN_UNLOCK help This option reduces the latency of the kernel by making @@ -58,7 +58,7 @@ config PREEMPT_LL config PREEMPT_RT bool "Fully Preemptible Kernel (Real-Time)" depends on EXPERT && ARCH_SUPPORTS_RT - select PREEMPT + select PREEMPTION help This option turns the kernel into a real-time kernel by replacing various locking primitives (spinlocks, rwlocks, etc.) with @@ -77,6 +77,6 @@ endchoice config PREEMPT_COUNT bool -config PREEMPT +config PREEMPTION bool select PREEMPT_COUNT
Re: [RFC 00/79] perf tools: Initial libperf separation
This is really helpful and thank you for taking the initiative! I've no real feedback other than to offer support. We've been looking in this direction to avoid memory overhead in: https://github.com/google/perf_data_converter Some use cases: 1) streaming protobuf rather than perf.data files, 2) using libperf inside of a runtime with JIT for a symbolization story that can reduce the storage/memory overhead of the current 'perf record' and 'perf inject -j'. Something similar is done here: https://github.com/jvm-profiling-tools/async-profiler/blob/936a9fea8dafc5d0674860d229a1b3d86d295e2f/src/perfEvents_linux.cpp 3) self profiling with dwarf based call graphs, to avoid stack samples being visible outside of the process which could be a security concern. Most of our tooling is C++ rather than C, and we run into issues like tools/include/linux/list.h using 'new' as a variable name. The duplication of header files in tools/, the importance of -I precedence and the use of -include have been other build system gotchas - principally because there are so many files with exactly the same name. I don't know if in the reorganization into a library this can be simplified. Thanks again, Ian Rogers Google On Sun, Jul 21, 2019 at 4:25 AM Jiri Olsa wrote: > > hi, > we have long term goal to separate some of the perf functionality > into library. This patchset is initial effort on separating some > of the interface. > > Currently only the basic counting interface is exported, it allows > to: > - create cpu/threads maps > - create evlist/evsel objects > - add evsel objects into evlist > - open/close evlist/evsel objects > - enable/disable events > - read evsel counts > > The initial effort was to have total separation of the objects > from perf code, but it showed not to be a good way. The amount > of changed code was too big with high chance for regressions, > mainly because of the code embedding one of the above objects > statically. > > We took the other approach of sharing the objects/struct details > within the perf and libperf code. This way we can keep perf > functionality without any major changes and the libperf users > are still separated from the object/struct details. We can move > to total libperf's objects separation gradually in future. > > You can check current interface/functionality in examples under: > tools/perf/lib/Documentation/tutorial > > or check tests in here: > $ cd tools/perf/lib && make tests > LINK test-cpumap-a > LINK test-threadmap-a > LINK test-evlist-a > LINK test-evsel-a > LINK test-cpumap-so > LINK test-threadmap-so > LINK test-evlist-so > LINK test-evsel-so > running static: > - running test-cpumap.c...OK > - running test-threadmap.c...OK > - running test-evlist.c...OK > - running test-evsel.c...OK > running dynamic: > - running test-cpumap.c...OK > - running test-threadmap.c...OK > - running test-evlist.c...OK > - running test-evsel.c...OK > > The upcoming changes in the near future: > - move parse_events interface in, so we have the event parsing > interface in the library together with the all events from > tools/perf/pmu-events/arch > - add sampling interface with event mmap support and all the > sampling events objects > - add user mmap interface to read counters > - more documentation and tutorial ;-) > - move under tools/lib after the interface is more stable-ish > > Big kudos to BPF guys, because most of the infrastructure is > 'borrowed' from libbpf library.. ;-) > > It's also available in here: > git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git > perf/lib > > I tested so far on x86, I still need to run it through s390/ppc/arm. > > throughts? thanks, > jirka > > > Cc: Daniel Borkmann > Cc: Alexei Starovoitov > Cc: Kan Liang > Cc: Song Liu > Cc: Steven Rostedt > Cc: Stephane Eranian > --- > Jiri Olsa (79): > perf tools: Move loaded out of struct perf_counts_values > perf tools: Rename struct cpu_map to struct perf_cpu_map > perf tools: Rename struct thread_map to struct perf_thread_map > perf tools: Rename struct perf_evsel to struct evsel > perf tools: Rename struct perf_evlist to struct evlist > perf tools: Rename perf_evsel__init to evsel__init > perf tools: Rename perf_evlist__init to evlist__init > perf tools: Rename perf_evlist__new to evlist__new > perf tools: Rename perf_evlist__delete to evlist__delete > perf tools: Rename perf_evsel__delete to evsel__delete > perf tools: Rename perf_evsel__new to evsel__new > perf tools: Rename perf_evlist__add to evlist__add > perf tools: Rename perf_evlist__remove to evlist__remove > perf tools: Rename perf_evsel__open to evsel__open > perf tools: Rename perf_evsel__enable to evsel__enable > perf tools: Rename perf_evsel__disable to evsel__disable > perf tools: Rename perf_evsel__apply_filte
Re: RFC: call_rcu_outstanding (was Re: WARNING in __mmdrop)
On Mon, Jul 22, 2019 at 12:13:40PM -0400, Michael S. Tsirkin wrote: > On Mon, Jul 22, 2019 at 08:55:34AM -0700, Paul E. McKenney wrote: > > On Mon, Jul 22, 2019 at 11:47:24AM -0400, Michael S. Tsirkin wrote: > > > On Mon, Jul 22, 2019 at 11:14:39AM -0400, Joel Fernandes wrote: > > > > [snip] > > > > > > Would it make sense to have call_rcu() check to see if there are > > > > > > many > > > > > > outstanding requests on this CPU and if so process them before > > > > > > returning? > > > > > > That would ensure that frequent callers usually ended up doing their > > > > > > own processing. > > > > > > > > Other than what Paul already mentioned about deadlocks, I am not sure > > > > if this > > > > would even work for all cases since call_rcu() has to wait for a grace > > > > period. > > > > > > > > So, if the number of outstanding requests are higher than a certain > > > > amount, > > > > then you *still* have to wait for some RCU configurations for the grace > > > > period duration and cannot just execute the callback in-line. Did I miss > > > > something? > > > > > > > > Can waiting in-line for a grace period duration be tolerated in the > > > > vhost case? > > > > > > > > thanks, > > > > > > > > - Joel > > > > > > No, but it has many other ways to recover (try again later, drop a > > > packet, use a slower copy to/from user). > > > > True enough! And your idea of taking recovery action based on the number > > of callbacks seems like a good one while we are getting RCU's callback > > scheduling improved. > > > > By the way, was this a real problem that you could make happen on real > > hardware? > > > If not, I would suggest just letting RCU get improved over > > the next couple of releases. > > So basically use kfree_rcu but add a comment saying e.g. "WARNING: > in the future callers of kfree_rcu might need to check that > not too many callbacks get queued. In that case, we can > disable the optimization, or recover in some other way. > Watch this space." That sounds fair. > > If it is something that you actually made happen, please let me know > > what (if anything) you need from me for your callback-counting EBUSY > > scheme. > > > > Thanx, Paul > > If you mean kfree_rcu causing OOM then no, it's all theoretical. > If you mean synchronize_rcu stalling to the point where guest will OOPs, > then yes, that's not too hard to trigger. Is synchronize_rcu() being stalled by the userspace loop that is invoking your ioctl that does kfree_rcu()? Or instead by the resulting callback invocation? Thanx, Paul
[PATCH] ACPI/IORT: Fix off-by-one check in iort_dev_find_its_id()
Static analysis identified that index comparison against ITS entries in
iort_dev_find_its_id() is off by one.
Update the comparison condition and clarify the resulting error
message.
Fixes: 4bf2efd26d76 ("ACPI: Add new IORT functions to support MSI domain
handling")
Link: https://lore.kernel.org/linux-arm-kernel/20190613065410.GB16334@mwanda/
Reported-by: Dan Carpenter
Signed-off-by: Lorenzo Pieralisi
Cc: Dan Carpenter
Cc: Will Deacon
Cc: Hanjun Guo
Cc: Sudeep Holla
Cc: Catalin Marinas
Cc: Robin Murphy
---
drivers/acpi/arm64/iort.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 15dbfd657d82..5a7551d060f2 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -611,8 +611,8 @@ static int iort_dev_find_its_id(struct device *dev, u32
req_id,
/* Move to ITS specific data */
its = (struct acpi_iort_its_group *)node->node_data;
- if (idx > its->its_count) {
- dev_err(dev, "requested ITS ID index [%d] is greater than
available [%d]\n",
+ if (idx >= its->its_count) {
+ dev_err(dev, "requested ITS ID index [%d] overruns ITS entries
[%d]\n",
idx, its->its_count);
return -ENXIO;
}
--
2.21.0
Re: [GIT PULL] pidfd fixes
On Mon, Jul 22, 2019 at 7:26 AM Christian Brauner wrote:
>
> This contains a fix for pidfd polling. It ensures that the task's exit
> state is visible to all waiters:
Hmm.
I've pulled this, but the exit_state thing has been very fragile
before, and I'm not entirely happy with how this just changes where it
is set. I guess the movement here is all inside the tasklist_lock, so
it's not that big of a deal, but still..
I would *really* like Oleg to take a look.
Also, and the primary reason I write this email is that this basically
makes the "EXIT_ZOMBIE / EXIT_DEAD" state handling look all kinds of
crazy. You set it to EXIT_ZOMBIE potentially _twice_. Whaa?
So if we set EXIT_ZOMBIE early, then I think we should change the
EXIT_DEAD case too. IOW, do something like this on top:
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -734,9 +734,10 @@ static void exit_notify(struct task_struct
*tsk, int group_dead)
autoreap = true;
}
- tsk->exit_state = autoreap ? EXIT_DEAD : EXIT_ZOMBIE;
- if (tsk->exit_state == EXIT_DEAD)
+ if (autoreap) {
+ tsk->exit_state = EXIT_DEAD;
list_add(&tsk->ptrace_entry, &dead);
+ }
/* mt-exec, de_thread() is waiting for group leader */
if (unlikely(tsk->signal->notify_count < 0))
where now the logic becomes "ok, we turned into a zombie above, and if
we autoreap this thread then we turn the zombie into a fully dead
thread".
Because currently we end up having "first turn it into a zombie", then
"set it to zombie or dead depending on autoreap" and then "if we
turned it into dead, move it to the dead list".
That just feels confused to me. Comments?
Linus
Re: [PATCH 1/4][V3] spi: Add optional stall delay between cs_change transfers
On Mon, Jul 22, 2019 at 03:47:44PM +0300, Alexandru Ardelean wrote: > Some devices like the ADIS16460 IMU require a longer period between > transfers, i.e. between when the CS is de-asserted and re-asserted. The > default value of 10us is not enough. This change makes the delay > configurable for when the next CS change goes active, allowing the default > to remain 10us is case it is unspecified. For the third time: | This looks like cs_change_delay. > #define SPI_NBITS_QUAD 0x04 /* 4bits transfer */ > u8 bits_per_word; > u8 word_delay_usecs; > + u8 cs_change_delay; > u16 delay_usecs; > u32 speed_hz; > u16 word_delay; This patch doesn't apply and even if it did it won't compile because you are trying to add a field with the same name as an existing one. signature.asc Description: PGP signature
Re: RFC: call_rcu_outstanding (was Re: WARNING in __mmdrop)
On Mon, Jul 22, 2019 at 09:25:51AM -0700, Paul E. McKenney wrote: > On Mon, Jul 22, 2019 at 12:13:40PM -0400, Michael S. Tsirkin wrote: > > On Mon, Jul 22, 2019 at 08:55:34AM -0700, Paul E. McKenney wrote: > > > On Mon, Jul 22, 2019 at 11:47:24AM -0400, Michael S. Tsirkin wrote: > > > > On Mon, Jul 22, 2019 at 11:14:39AM -0400, Joel Fernandes wrote: > > > > > [snip] > > > > > > > Would it make sense to have call_rcu() check to see if there are > > > > > > > many > > > > > > > outstanding requests on this CPU and if so process them before > > > > > > > returning? > > > > > > > That would ensure that frequent callers usually ended up doing > > > > > > > their > > > > > > > own processing. > > > > > > > > > > Other than what Paul already mentioned about deadlocks, I am not sure > > > > > if this > > > > > would even work for all cases since call_rcu() has to wait for a grace > > > > > period. > > > > > > > > > > So, if the number of outstanding requests are higher than a certain > > > > > amount, > > > > > then you *still* have to wait for some RCU configurations for the > > > > > grace > > > > > period duration and cannot just execute the callback in-line. Did I > > > > > miss > > > > > something? > > > > > > > > > > Can waiting in-line for a grace period duration be tolerated in the > > > > > vhost case? > > > > > > > > > > thanks, > > > > > > > > > > - Joel > > > > > > > > No, but it has many other ways to recover (try again later, drop a > > > > packet, use a slower copy to/from user). > > > > > > True enough! And your idea of taking recovery action based on the number > > > of callbacks seems like a good one while we are getting RCU's callback > > > scheduling improved. > > > > > > By the way, was this a real problem that you could make happen on real > > > hardware? > > > > > If not, I would suggest just letting RCU get improved over > > > the next couple of releases. > > > > So basically use kfree_rcu but add a comment saying e.g. "WARNING: > > in the future callers of kfree_rcu might need to check that > > not too many callbacks get queued. In that case, we can > > disable the optimization, or recover in some other way. > > Watch this space." > > That sounds fair. > > > > If it is something that you actually made happen, please let me know > > > what (if anything) you need from me for your callback-counting EBUSY > > > scheme. > > > > > > Thanx, Paul > > > > If you mean kfree_rcu causing OOM then no, it's all theoretical. > > If you mean synchronize_rcu stalling to the point where guest will OOPs, > > then yes, that's not too hard to trigger. > > Is synchronize_rcu() being stalled by the userspace loop that is invoking > your ioctl that does kfree_rcu()? Or instead by the resulting callback > invocation? > > Thanx, Paul Sorry, let me clarify. We currently have synchronize_rcu in a userspace loop. I have a patch replacing that with kfree_rcu. This isn't the first time synchronize_rcu is stalling a VM for a long while so I didn't investigate further. -- MST
[PATCH 2/2] HID: core: only warn once of oversize hid report
From: Joshua Clayton
On HP spectre x360 convertible the message:
hid-sensor-hub 001F:8087:0AC2.0002: hid_field_extract() called with n (192) >
32! (kworker/1:2)
is continually printed many times per second, crowding out all other kernel logs
Protect dmesg by printing the warning only one time.
Signed-off-by: Joshua Clayton
---
drivers/hid/hid-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 210b81a56e1a..7afd0422b280 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1311,7 +1311,7 @@ u32 hid_field_extract(const struct hid_device *hid, u8
*report,
unsigned offset, unsigned n)
{
if (n > 32) {
- hid_warn(hid, "hid_field_extract() called with n (%d) > 32!
(%s)\n",
+ hid_warn_once(hid, "hid_field_extract() called with n (%d) >
32! (%s)\n",
n, current->comm);
n = 32;
}
--
2.21.0
[PATCH 1/2] HID: core: Add hid printk_once macros
From: Joshua Clayton
Make available printk_once variants to hid_warn() etc
Signed-off-by: Joshua Clayton
---
include/linux/hid.h | 19 +++
1 file changed, 19 insertions(+)
diff --git a/include/linux/hid.h b/include/linux/hid.h
index d770ab1a0479..5b239712c902 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1179,4 +1179,23 @@ do {
\
#define hid_dbg(hid, fmt, arg...) \
dev_dbg(&(hid)->dev, fmt, ##arg)
+#define hid_level_once(level, hid, fmt, arg...)\
+ dev_level_once(level, &(hid)->dev, fmt, ##arg)
+#define hid_emerg_once(hid, fmt, arg...) \
+ dev_emerg_once(&(hid)->dev, fmt, ##arg)
+#define hid_crit_once(hid, fmt, arg...)\
+ dev_crit_once(&(hid)->dev, fmt, ##arg)
+#define hid_alert_once(hid, fmt, arg...) \
+ dev_alert_once(&(hid)->dev, fmt, ##arg)
+#define hid_err_once(hid, fmt, arg...) \
+ dev_err_once(&(hid)->dev, fmt, ##arg)
+#define hid_notice_once(hid, fmt, arg...) \
+ dev_notice_once(&(hid)->dev, fmt, ##arg)
+#define hid_warn_once(hid, fmt, arg...)\
+ dev_warn_once(&(hid)->dev, fmt, ##arg)
+#define hid_info_once(hid, fmt, arg...)\
+ dev_info_once(&(hid)->dev, fmt, ##arg)
+#define hid_dbg_once(hid, fmt, arg...) \
+ dev_dbg_once(&(hid)->dev, fmt, ##arg)
+
#endif
--
2.21.0
Re: [GIT PULL] pidfd fixes
On Mon, Jul 22, 2019 at 09:27:08AM -0700, Linus Torvalds wrote:
> On Mon, Jul 22, 2019 at 7:26 AM Christian Brauner
> wrote:
> >
> > This contains a fix for pidfd polling. It ensures that the task's exit
> > state is visible to all waiters:
>
> Hmm.
>
> I've pulled this, but the exit_state thing has been very fragile
> before, and I'm not entirely happy with how this just changes where it
> is set. I guess the movement here is all inside the tasklist_lock, so
> it's not that big of a deal, but still..
>
> I would *really* like Oleg to take a look.
Oh, sorry. Oleg did take a look. See:
https://lore.kernel.org/lkml/20190718150057.gb13...@redhat.com/
https://lore.kernel.org/lkml/20190719161404.ga24...@redhat.com/
>
> Also, and the primary reason I write this email is that this basically
> makes the "EXIT_ZOMBIE / EXIT_DEAD" state handling look all kinds of
> crazy. You set it to EXIT_ZOMBIE potentially _twice_. Whaa?
>
> So if we set EXIT_ZOMBIE early, then I think we should change the
> EXIT_DEAD case too. IOW, do something like this on top:
>
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -734,9 +734,10 @@ static void exit_notify(struct task_struct
> *tsk, int group_dead)
> autoreap = true;
> }
>
> - tsk->exit_state = autoreap ? EXIT_DEAD : EXIT_ZOMBIE;
> - if (tsk->exit_state == EXIT_DEAD)
> + if (autoreap) {
> + tsk->exit_state = EXIT_DEAD;
> list_add(&tsk->ptrace_entry, &dead);
> + }
>
> /* mt-exec, de_thread() is waiting for group leader */
> if (unlikely(tsk->signal->notify_count < 0))
>
> where now the logic becomes "ok, we turned into a zombie above, and if
> we autoreap this thread then we turn the zombie into a fully dead
> thread".
>
> Because currently we end up having "first turn it into a zombie", then
> "set it to zombie or dead depending on autoreap" and then "if we
> turned it into dead, move it to the dead list".
>
> That just feels confused to me. Comments?
Agreed. But that codepath is so core-kernel that I really felt more
comfortable just doing the absolut minimal thing so that when things
bite us we see it right away.
There's no harm in sending a cleanup for this later I think, when we
haven't hit any weirdness with the current change. Does that sound ok?
Christian
Re: [PATCH] watchdog:alim1535_wdt: Fix data race in ali_settimer() concerning ali_timeout_bits variable.
On Mon, Jul 22, 2019 at 07:35:03AM -0700, Mark Balantzyan wrote:
> Hello all,
>
> I had previously submitted 2 patches attempting to fix the data race
> condition in alim1535_wdt.c as part of my work with individuals on the
> Linux Driver Verification project. I am including the original patch
> description I provided, below, along with revised patch. Thank you.
>
> (PATCH DESCRIPTION AND PATCH BELOW)
>
This is not how the description is supposed to look like; the above would
end up in the commit log. Please check the kernel documentation on how
to write subject lines and patch descriptions.
> In drivers/watchdog/ailm1535_wdt.c, there is the potential for a data race
> due to parallel call stacks as follows: Thread 1 calls a file operation
> callback, denoted *ali_fops* in the .c file, which in turn results in calls
> to ali_write() followed by ali_start(), which has the line
>
> val |= (1 << 25) | ali_timeout_bits;
>
> surrounded by a spin_lock and spin_unlock. This is crucial because Thread 2
The "surrounded by spinlock" does not refer to the line above, but to
pci_read_config_dword() followed by pci_write_config_dword(), which
needs to be protected. The described race condition around ali_timeout_bits
[ali_start() vs. ali_settimer()] does not exist.
The only race condition in the driver is 'ali_timeout_bits' vs. 'timeout'.
It is theoretically possible that those two get out of sync, ie that
ali_timeout_bits does not reflect the value of timeout. This can happen
if one of the threads is interrupted after setting 'ali_timeout_bits' but
before updating 'timeout'.
> can access "ali_timeout_bits" then when it calls ali_ioctl(), which calls
> ali_settimer() having the lines (else if (t < 60) ali_timeout_bits = t|(1
> << 6);, lines 112-113, etc.)
>
There is no need to be that detailed. It is sufficient to explain that
there is a race condition when updating 'ali_timeout_bits' and 'timeout'
(or maybe use my explanation above).
> (Revised) patch adds spinlocking around "ali_timeout_bits" in
> ali_settimer() should "ali_ioctl()" be called in a concurrent thread (at
> any time).
> ---
> drivers/watchdog/alim1535_wdt.c | 12
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/watchdog/alim1535_wdt.c
> b/drivers/watchdog/alim1535_wdt.c
> index 60f0c2e..1260e9e 100644
> --- a/drivers/watchdog/alim1535_wdt.c
> +++ b/drivers/watchdog/alim1535_wdt.c
> @@ -106,19 +106,23 @@ static void ali_keepalive(void)
> */
>
> static int ali_settimer(int t)
> -{
> - if (t < 0)
> +{ spin_lock(&ali_lock);
> + if (t < 0) {
> + spin_unlock(&ali_unlock);
> return -EINVAL;
> + }
> else if (t < 60)
> ali_timeout_bits = t|(1 << 6);
> else if (t < 3600)
> ali_timeout_bits = (t / 60)|(1 << 7);
> else if (t < 18000)
> ali_timeout_bits = (t / 300)|(1 << 6)|(1 << 7);
> - else
> + else {
> + spin_unlock(&ali_lock);
> return -EINVAL;
Please use goto for error exits, as suggested in the coding style document.
> -
> + }
> timeout = t;
> + spin_unlock(&ali_lock);
> return 0;
> }
Formatting is completely messed up.
>
> --
> 2.15.1
> Signed-off-by: Mark Balantzyan
Wrong location for Signed-off-by:.
Guenter
Re: [GIT PULL for v5.3-rc1] media updates
The pull request you sent on Mon, 22 Jul 2019 08:58:06 -0300: > git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media > tags/media/v5.3-2 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/c92f0380673bd295c9ac73030a17c16b9df3e702 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [GIT PULL] pidfd fixes
The pull request you sent on Mon, 22 Jul 2019 16:22:41 +0200: > g...@gitolite.kernel.org:pub/scm/linux/kernel/git/brauner/linux > tags/for-linus-20190722 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/44b912cd0b5596c5ae8ae857bd1d5ff83ed5 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [GIT] Networking
The pull request you sent on Sun, 21 Jul 2019 21:13:21 -0700 (PDT): > git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git refs/heads/master has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/83768245a3b158b96d33012b22ab01d193afb2da Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [GIT pull] sched/urgent for 5.3-rc2
The pull request you sent on Mon, 22 Jul 2019 18:23:38 +0200 (CEST): > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git > sched-urgent-for-linus has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/7b5cf701ea9c395c792e2a7e3b7caf4c68b87721 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
5.3-rc1 panic in dma_direct_max_mapping_size
only got a partial panic, but when I threw 5.3-rc1 on a linode vm, it hit this: bus_add_driver+0x1a9/0x1c0 ? scsi_init_sysctl+0x22/0x22 driver_register+0x6b/0xa6 ? scsi_init_sysctl+0x22/0x22 init+0x86/0xcc do_one_initcall+0x69/0x334 kernel_init_freeable+0x367/0x3ff ? rest_init+0x247/0x247 kernel_init+0xa/0xf9 ret_from_fork+0x3a/0x50 CR2: ---[ end trace 2967cd16f7b1a303 ]--- RIP: 0010:dma_direct_max_mapping_size+0x21/0x71 Code: 0f b6 c0 c3 0f 1f 44 00 00 0f 1f 44 00 00 55 53 48 89 fb e8 21 0e 00 00 84 c0 74 2c 48 8b 83 20 03 00 00 48 8b ab 30 03 00 00 <48> 8b 00 48 85 c0 75 20 48 89 df e8 ff f3 ff ff 48 39 e8 77 2c 83 RSP: 0018:b58f00013ae8 EFLAGS: 00010202 RAX: RBX: a35ff8914ac8 RCX: b58f00013a1c RDX: a35ff81d4658 RSI: 007e RDI: a35ff8914ac8 RBP: R08: a35ff81d4cc0 R09: a35ff82e3bc8 R10: R11: R12: a35ff8914ac8 R13: R14: a35ff826c160 R15: FS: () GS:a35ffba0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: CR3: 00012d220001 CR4: 003606f0 DR0: DR1: DR2: DR3: DR6: fffe0ff0 DR7: 0400 Kernel panic - not syncing: Attempted to kill init! exitcode=0x0009 Kernel Offset: 0x1b00 from 0x8100 (relocation range: 0x8000-0xbfff) Will try and get some more debug info this evening if it isn't obvious from the above. Dave
[PATCH] arm64: dts: qcom: msm8998: Node ordering, address cleanups
DT nodes should be ordered by address, then node name, and finally label.
The msm8998 dtsi does not follow this, so clean it up by reordering the
nodes. While we are at it, extend the addresses to be fully 32-bits wide
so that ordering is easy to determine when adding new nodes. Also, two
or so nodes had the wrong address value in their node name (did not match
the reg property), so fix those up as well.
Hopefully going forward, things can be maintained so that a cleanup like
this is not needed.
Signed-off-by: Jeffrey Hugo
---
arch/arm64/boot/dts/qcom/msm8998.dtsi | 254 +-
1 file changed, 127 insertions(+), 127 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi
b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index c13ed7aeb1e0..4b66a1c588f8 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -787,14 +787,22 @@
ranges = <0 0 0 0x>;
compatible = "simple-bus";
- rpm_msg_ram: memory@68000 {
+ gcc: clock-controller@10 {
+ compatible = "qcom,gcc-msm8998";
+ #clock-cells = <1>;
+ #reset-cells = <1>;
+ #power-domain-cells = <1>;
+ reg = <0x0010 0xb>;
+ };
+
+ rpm_msg_ram: memory@778000 {
compatible = "qcom,rpm-msg-ram";
- reg = <0x778000 0x7000>;
+ reg = <0x00778000 0x7000>;
};
qfprom: qfprom@78 {
compatible = "qcom,qfprom";
- reg = <0x78 0x621c>;
+ reg = <0x0078 0x621c>;
#address-cells = <1>;
#size-cells = <1>;
@@ -804,47 +812,10 @@
};
};
- gcc: clock-controller@10 {
- compatible = "qcom,gcc-msm8998";
- #clock-cells = <1>;
- #reset-cells = <1>;
- #power-domain-cells = <1>;
- reg = <0x10 0xb>;
- };
-
- tlmm: pinctrl@340 {
- compatible = "qcom,msm8998-pinctrl";
- reg = <0x340 0xc0>;
- interrupts = ;
- gpio-controller;
- #gpio-cells = <0x2>;
- interrupt-controller;
- #interrupt-cells = <0x2>;
- };
-
- spmi_bus: spmi@800f000 {
- compatible = "qcom,spmi-pmic-arb";
- reg = <0x800f000 0x1000>,
- <0x840 0x100>,
- <0x940 0x100>,
- <0xa40 0x22>,
- <0x800a000 0x3000>;
- reg-names = "core", "chnls", "obsrvr", "intr", "cnfg";
- interrupt-names = "periph_irq";
- interrupts = ;
- qcom,ee = <0>;
- qcom,channel = <0>;
- #address-cells = <2>;
- #size-cells = <0>;
- interrupt-controller;
- #interrupt-cells = <4>;
- cell-index = <0>;
- };
-
tsens0: thermal@10ab000 {
compatible = "qcom,msm8998-tsens", "qcom,tsens-v2";
- reg = <0x10ab000 0x1000>, /* TM */
- <0x10aa000 0x1000>; /* SROT */
+ reg = <0x010ab000 0x1000>, /* TM */
+ <0x010aa000 0x1000>; /* SROT */
#qcom,sensors = <14>;
#thermal-sensor-cells = <1>;
@@ -852,8 +823,8 @@
tsens1: thermal@10ae000 {
compatible = "qcom,msm8998-tsens", "qcom,tsens-v2";
- reg = <0x10ae000 0x1000>, /* TM */
- <0x10ad000 0x1000>; /* SROT */
+ reg = <0x010ae000 0x1000>, /* TM */
+ <0x010ad000 0x1000>; /* SROT */
#qcom,sensors = <8>;
#thermal-sensor-cells = <1>;
@@ -943,16 +914,107 @@
};
};
+ ufshc: ufshc@1da4000 {
+ compatible = "qcom,msm8998-ufshc", "qcom,ufshc",
"jedec,ufs-2.0";
+ reg = <0x01da4000 0x2500>;
+ interrupts = ;
+ phys = <&ufsphy_lanes>;
+ phy-names = "ufsphy";
+ lanes-per-direction = <2>;
+ power-domains = <&gcc UFS_GDSC>;
+ #reset-cells = <1>;
+
+
[PATCH] power: sysfs: Remove wakeup_abort_count attribute.
wakeup_abort_count and wakeup_count sysfs entries print the
same (wakeup_count) attribute. This patch removes the duplicate
wakeup_abort_count sysfs entry.
Signed-off-by: Ravi Chandra Sadineni
---
Documentation/ABI/testing/sysfs-devices-power | 25 ++-
drivers/base/power/sysfs.c| 19 --
2 files changed, 7 insertions(+), 37 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-devices-power
b/Documentation/ABI/testing/sysfs-devices-power
index 80a00f7b6667..1ca04b4f0489 100644
--- a/Documentation/ABI/testing/sysfs-devices-power
+++ b/Documentation/ABI/testing/sysfs-devices-power
@@ -81,12 +81,13 @@ What: /sys/devices/.../power/wakeup_count
Date: September 2010
Contact: Rafael J. Wysocki
Description:
- The /sys/devices/.../wakeup_count attribute contains the number
- of signaled wakeup events associated with the device. This
- attribute is read-only. If the device is not capable to wake up
- the system from sleep states, this attribute is not present.
- If the device is not enabled to wake up the system from sleep
- states, this attribute is empty.
+ The /sys/devices/.../wakeup_count attribute contains the
+ number of times the processing of a wakeup event associated with
+ the device might have aborted system transition into a sleep
+ state in progress. This attribute is read-only. If the device
+ is not capable to wake up the system from sleep states, this
+ attribute is not present. If the device is not enabled to wake
+ up the system from sleep states, this attribute is empty.
What: /sys/devices/.../power/wakeup_active_count
Date: September 2010
@@ -100,18 +101,6 @@ Description:
the device is not enabled to wake up the system from sleep
states, this attribute is empty.
-What: /sys/devices/.../power/wakeup_abort_count
-Date: February 2012
-Contact: Rafael J. Wysocki
-Description:
- The /sys/devices/.../wakeup_abort_count attribute contains the
- number of times the processing of a wakeup event associated with
- the device might have aborted system transition into a sleep
- state in progress. This attribute is read-only. If the device
- is not capable to wake up the system from sleep states, this
- attribute is not present. If the device is not enabled to wake
- up the system from sleep states, this attribute is empty.
-
What: /sys/devices/.../power/wakeup_expire_count
Date: February 2012
Contact: Rafael J. Wysocki
diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
index 1b9c281cbe41..f42044d9711c 100644
--- a/drivers/base/power/sysfs.c
+++ b/drivers/base/power/sysfs.c
@@ -375,24 +375,6 @@ static ssize_t wakeup_active_count_show(struct device *dev,
static DEVICE_ATTR_RO(wakeup_active_count);
-static ssize_t wakeup_abort_count_show(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- unsigned long count = 0;
- bool enabled = false;
-
- spin_lock_irq(&dev->power.lock);
- if (dev->power.wakeup) {
- count = dev->power.wakeup->wakeup_count;
- enabled = true;
- }
- spin_unlock_irq(&dev->power.lock);
- return enabled ? sprintf(buf, "%lu\n", count) : sprintf(buf, "\n");
-}
-
-static DEVICE_ATTR_RO(wakeup_abort_count);
-
static ssize_t wakeup_expire_count_show(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -580,7 +562,6 @@ static struct attribute *wakeup_attrs[] = {
&dev_attr_wakeup.attr,
&dev_attr_wakeup_count.attr,
&dev_attr_wakeup_active_count.attr,
- &dev_attr_wakeup_abort_count.attr,
&dev_attr_wakeup_expire_count.attr,
&dev_attr_wakeup_active.attr,
&dev_attr_wakeup_total_time_ms.attr,
--
2.20.1
Re: [PATCH] lib: zstd: Make ZSTD_compressBlock_greedy_extDict static
On Wed, Jul 17, 2019 at 05:18:52PM +0800, YueHaibing wrote:
> Fix sparse warnings:
>
> lib/zstd/compress.c:2252:6: warning:
> symbol 'ZSTD_compressBlock_greedy_extDict' was not declared. Should it be
> static?
> lib/zstd/compress.c:2982:14: warning:
> symbol 'ZSTD_createCStream_advanced' was not declared. Should it be static?
>
> Reported-by: Hulk Robot
> Signed-off-by: YueHaibing
Reviewed-by: Kees Cook
-Kees
> ---
> lib/zstd/compress.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/lib/zstd/compress.c b/lib/zstd/compress.c
> index 5e0b67003e55..651d686c00b6 100644
> --- a/lib/zstd/compress.c
> +++ b/lib/zstd/compress.c
> @@ -2249,7 +2249,11 @@ void ZSTD_compressBlock_lazy_extDict_generic(ZSTD_CCtx
> *ctx, const void *src, si
> }
> }
>
> -void ZSTD_compressBlock_greedy_extDict(ZSTD_CCtx *ctx, const void *src,
> size_t srcSize) { ZSTD_compressBlock_lazy_extDict_generic(ctx, src, srcSize,
> 0, 0); }
> +static void ZSTD_compressBlock_greedy_extDict(ZSTD_CCtx *ctx, const void
> *src,
> + size_t srcSize)
> +{
> + ZSTD_compressBlock_lazy_extDict_generic(ctx, src, srcSize, 0, 0);
> +}
>
> static void ZSTD_compressBlock_lazy_extDict(ZSTD_CCtx *ctx, const void *src,
> size_t srcSize)
> {
> @@ -2979,7 +2983,7 @@ size_t
> ZSTD_CStreamWorkspaceBound(ZSTD_compressionParameters cParams)
> return ZSTD_CCtxWorkspaceBound(cParams) +
> ZSTD_ALIGN(sizeof(ZSTD_CStream)) + ZSTD_ALIGN(inBuffSize) +
> ZSTD_ALIGN(outBuffSize);
> }
>
> -ZSTD_CStream *ZSTD_createCStream_advanced(ZSTD_customMem customMem)
> +static ZSTD_CStream *ZSTD_createCStream_advanced(ZSTD_customMem customMem)
> {
> ZSTD_CStream *zcs;
>
> --
> 2.20.1
>
>
--
Kees Cook
Re: drm/exynos: scaler: Reset hardware before starting the operation (bug report)
Hi,
On 2019-07-05 18:09, Colin Ian King wrote:
> Static analysis on today's linux-next has found a potential error in the
> following commit:
>
> commit 280e54c9f614c88292685383cf2d65057586e9fb
> Author: Andrzej Pietrasiewicz
> Date: Thu Jun 7 13:06:08 2018 +0200
>
> drm/exynos: scaler: Reset hardware before starting the operation
>
> In the following code the retry counter does not appear to be
> decremented, so potentially the loop could get stuck forever if the H/W
> does not change state:
>
> static inline int scaler_reset(struct scaler_context *scaler)
> {
> int retry = SCALER_RESET_WAIT_RETRIES;
>
> scaler_write(SCALER_CFG_SOFT_RESET, SCALER_CFG);
> do {
> cpu_relax();
> } while (retry > 1 &&
> scaler_read(SCALER_CFG) & SCALER_CFG_SOFT_RESET);
>
> do {
> cpu_relax();
> scaler_write(1, SCALER_INT_EN);
> } while (retry > 0 && scaler_read(SCALER_INT_EN) != 1);
>
> return retry ? 0 : -EIO;
> }
>
> Maybe I'm missing something here subtle.
Right. Indeed there is missing decrementation of the 'retry' variable. I
suggest to add it to both loops and reset retry value to
SCALER_RESET_WAIT_RETRIES between them.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Re: [PATCH v4 5/8] clk: sunxi-ng: v3s: add Allwinner V3 support
On Sat, 13 Jul 2019 11:46:31 +0800, Icenowy Zheng wrote: > Allwinner V3 has the same main die with V3s, but with more pins wired. > There's a I2S bus on V3 that is not available on V3s. > > Add the V3-only peripheral's clocks and reset to the V3s CCU driver, > bound to a new V3 compatible string. The driver name is not changed > because it's part of the device tree binding (the header file name). > > Signed-off-by: Icenowy Zheng > --- > Changes in v4: > - Add the missing MMC2 clock slices. > > No changes in v3/v2. > > drivers/clk/sunxi-ng/ccu-sun8i-v3s.c | 228 +- > drivers/clk/sunxi-ng/ccu-sun8i-v3s.h | 2 +- > include/dt-bindings/clock/sun8i-v3s-ccu.h | 4 + > include/dt-bindings/reset/sun8i-v3s-ccu.h | 3 + > 4 files changed, 234 insertions(+), 3 deletions(-) > Please add Acked-by/Reviewed-by tags when posting new versions. However, there's no need to repost patches *only* to add the tags. The upstream maintainer will do that for acks received on the version they apply. If a tag was not added on purpose, please state why and what changed.
Re: [PATCH 1/2] HID: core: Add hid printk_once macros
On Mon, 2019-07-22 at 10:36 -0600, stillcompil...@gmail.com wrote:
> From: Joshua Clayton
>
> Make available printk_once variants to hid_warn() etc
>
> Signed-off-by: Joshua Clayton
This seems OK, but I suggest a slightly different style:
> diff --git a/include/linux/hid.h b/include/linux/hid.h
[]
> @@ -1179,4 +1179,23 @@ do {
> \
> #define hid_dbg(hid, fmt, arg...)\
> dev_dbg(&(hid)->dev, fmt, ##arg)
>
> +#define hid_level_once(level, hid, fmt, arg...) \
> + dev_level_once(level, &(hid)->dev, fmt, ##arg)
This one is probably not useful in actual code.
> +#define hid_emerg_once(hid, fmt, arg...) \
> + dev_emerg_once(&(hid)->dev, fmt, ##arg)
Even though I introduced those macros originally,
it's now a more common style to use:
#define hid_emerg_once(hid, fmt, ...) \
dev_emerg_once(&(hid)->dev, fmt, ##__VA_ARGS__)
etc...
And trivially:
hid_printk, hid_emerg, hid_crit, and hid_alert aren't
used at all and could all be removed.
I'm not sure there is a use case for any of them.
Perhaps:
---
include/linux/hid.h | 39 +--
1 file changed, 21 insertions(+), 18 deletions(-)
diff --git a/include/linux/hid.h b/include/linux/hid.h
index d770ab1a0479..5d2c4b63954f 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1160,23 +1160,26 @@ do {
\
printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
} while (0)
-#define hid_printk(level, hid, fmt, arg...)\
- dev_printk(level, &(hid)->dev, fmt, ##arg)
-#define hid_emerg(hid, fmt, arg...)\
- dev_emerg(&(hid)->dev, fmt, ##arg)
-#define hid_crit(hid, fmt, arg...) \
- dev_crit(&(hid)->dev, fmt, ##arg)
-#define hid_alert(hid, fmt, arg...)\
- dev_alert(&(hid)->dev, fmt, ##arg)
-#define hid_err(hid, fmt, arg...) \
- dev_err(&(hid)->dev, fmt, ##arg)
-#define hid_notice(hid, fmt, arg...) \
- dev_notice(&(hid)->dev, fmt, ##arg)
-#define hid_warn(hid, fmt, arg...) \
- dev_warn(&(hid)->dev, fmt, ##arg)
-#define hid_info(hid, fmt, arg...) \
- dev_info(&(hid)->dev, fmt, ##arg)
-#define hid_dbg(hid, fmt, arg...) \
- dev_dbg(&(hid)->dev, fmt, ##arg)
+#define hid_err(hid, fmt, ...) \
+ dev_err(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_notice(hid, fmt, ...) \
+ dev_notice(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_warn(hid, fmt, ...)
\
+ dev_warn(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_info(hid, fmt, ...)
\
+ dev_info(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_dbg(hid, fmt, ...) \
+ dev_dbg(&(hid)->dev, fmt, ##__VA_ARGS__)
+
+#define hid_err_once(hid, fmt, ...)\
+ dev_err_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_notice_once(hid, fmt, ...) \
+ dev_notice_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_warn_once(hid, fmt, ...) \
+ dev_warn_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_info_once(hid, fmt, ...) \
+ dev_info_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_dbg_once(hid, fmt, ...)\
+ dev_dbg_once(&(hid)->dev, fmt, ##__VA_ARGS__)
#endif
Re: [PATCH v3 08/11] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall
In addition, it seems that svm_page_enc_status_hc() accepts 'gpa',
'npages', 'enc' directly from the guest, and so these can take
arbitrary values. A very large 'npages' could lead to an int overflow
in 'gfn_end = gfn_start + npages', making gfn_end < gfn_start. This
could an OOB access in the bitmap. Concrete example: gfn_start = 2,
npages = -1, gfn_end = 2+(-1) = 1, sev_resize_page_enc_bitmap
allocates a bitmap for a single page (new_size=1), __bitmap_set access
offset gfn_end - gfn_start = -1.
On Sun, Jul 21, 2019 at 1:57 PM David Rientjes wrote:
>
> On Wed, 10 Jul 2019, Singh, Brijesh wrote:
>
> > diff --git a/Documentation/virtual/kvm/hypercalls.txt
> > b/Documentation/virtual/kvm/hypercalls.txt
> > index da24c138c8d1..94f0611f4d88 100644
> > --- a/Documentation/virtual/kvm/hypercalls.txt
> > +++ b/Documentation/virtual/kvm/hypercalls.txt
> > @@ -141,3 +141,17 @@ a0 corresponds to the APIC ID in the third argument
> > (a2), bit 1
> > corresponds to the APIC ID a2+1, and so on.
> >
> > Returns the number of CPUs to which the IPIs were delivered successfully.
> > +
> > +7. KVM_HC_PAGE_ENC_STATUS
> > +-
> > +Architecture: x86
> > +Status: active
> > +Purpose: Notify the encryption status changes in guest page table (SEV
> > guest)
> > +
> > +a0: the guest physical address of the start page
> > +a1: the number of pages
> > +a2: encryption attribute
> > +
> > + Where:
> > + * 1: Encryption attribute is set
> > + * 0: Encryption attribute is cleared
> > diff --git a/arch/x86/include/asm/kvm_host.h
> > b/arch/x86/include/asm/kvm_host.h
> > index 26d1eb83f72a..b463a81dc176 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -1199,6 +1199,8 @@ struct kvm_x86_ops {
> > uint16_t (*nested_get_evmcs_version)(struct kvm_vcpu *vcpu);
> >
> > bool (*need_emulation_on_page_fault)(struct kvm_vcpu *vcpu);
> > + int (*page_enc_status_hc)(struct kvm *kvm, unsigned long gpa,
> > + unsigned long sz, unsigned long mode);
> > };
> >
> > struct kvm_arch_async_pf {
> > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> > index 3089942f6630..431718309359 100644
> > --- a/arch/x86/kvm/svm.c
> > +++ b/arch/x86/kvm/svm.c
> > @@ -135,6 +135,8 @@ struct kvm_sev_info {
> > int fd; /* SEV device fd */
> > unsigned long pages_locked; /* Number of pages locked */
> > struct list_head regions_list; /* List of registered regions */
> > + unsigned long *page_enc_bmap;
> > + unsigned long page_enc_bmap_size;
> > };
> >
> > struct kvm_svm {
> > @@ -1910,6 +1912,8 @@ static void sev_vm_destroy(struct kvm *kvm)
> >
> > sev_unbind_asid(kvm, sev->handle);
> > sev_asid_free(kvm);
> > +
> > + kvfree(sev->page_enc_bmap);
> > }
> >
> > static void avic_vm_destroy(struct kvm *kvm)
>
> Adding Cfir who flagged this kvfree().
>
> Other freeing of sev->page_enc_bmap in this patch also set
> sev->page_enc_bmap_size to 0 and neither set sev->page_enc_bmap to NULL
> after freeing it.
>
> For extra safety, is it possible to sev->page_enc_bmap = NULL anytime the
> bitmap is kvfreed?
>
> > @@ -2084,6 +2088,7 @@ static void avic_set_running(struct kvm_vcpu *vcpu,
> > bool is_run)
> >
> > static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> > {
> > + struct kvm_sev_info *sev = &to_kvm_svm(vcpu->kvm)->sev_info;
> > struct vcpu_svm *svm = to_svm(vcpu);
> > u32 dummy;
> > u32 eax = 1;
> > @@ -2105,6 +2110,12 @@ static void svm_vcpu_reset(struct kvm_vcpu *vcpu,
> > bool init_event)
> >
> > if (kvm_vcpu_apicv_active(vcpu) && !init_event)
> > avic_update_vapic_bar(svm, APIC_DEFAULT_PHYS_BASE);
> > +
> > + /* reset the page encryption bitmap */
> > + if (sev_guest(vcpu->kvm)) {
> > + kvfree(sev->page_enc_bmap);
> > + sev->page_enc_bmap_size = 0;
> > + }
> > }
> >
> > static int avic_init_vcpu(struct vcpu_svm *svm)
>
> What is protecting sev->page_enc_bmap and sev->page_enc_bmap_size in calls
> to svm_vcpu_reset()?
Re: [PATCH v2 6/7] dt-bindings: net: realtek: Add property to configure LED mode
On Wed, Jul 10, 2019 at 09:55:12AM -0600, Rob Herring wrote: > On Wed, Jul 3, 2019 at 5:23 PM Matthias Kaehlcke wrote: > > > > Hi Florian, > > > > On Wed, Jul 03, 2019 at 02:37:47PM -0700, Florian Fainelli wrote: > > > On 7/3/19 12:37 PM, Matthias Kaehlcke wrote: > > > > The LED behavior of some Realtek PHYs is configurable. Add the > > > > property 'realtek,led-modes' to specify the configuration of the > > > > LEDs. > > > > > > > > Signed-off-by: Matthias Kaehlcke > > > > --- > > > > Changes in v2: > > > > - patch added to the series > > > > --- > > > > .../devicetree/bindings/net/realtek.txt | 9 + > > > > include/dt-bindings/net/realtek.h | 17 + > > > > 2 files changed, 26 insertions(+) > > > > create mode 100644 include/dt-bindings/net/realtek.h > > > > > > > > diff --git a/Documentation/devicetree/bindings/net/realtek.txt > > > > b/Documentation/devicetree/bindings/net/realtek.txt > > > > index 71d386c78269..40b0d6f9ee21 100644 > > > > --- a/Documentation/devicetree/bindings/net/realtek.txt > > > > +++ b/Documentation/devicetree/bindings/net/realtek.txt > > > > @@ -9,6 +9,12 @@ Optional properties: > > > > > > > > SSC is only available on some Realtek PHYs (e.g. RTL8211E). > > > > > > > > +- realtek,led-modes: LED mode configuration. > > > > + > > > > + A 0..3 element vector, with each element configuring the operating > > > > + mode of an LED. Omitted LEDs are turned off. Allowed values are > > > > + defined in "include/dt-bindings/net/realtek.h". > > > > > > This should probably be made more general and we should define LED modes > > > that makes sense regardless of the PHY device, introduce a set of > > > generic functions for validating and then add new function pointer for > > > setting the LED configuration to the PHY driver. This would allow to be > > > more future proof where each PHY driver could expose standard LEDs class > > > devices to user-space, and it would also allow facilities like: ethtool > > > -p to plug into that. > > > > > > Right now, each driver invents its own way of configuring LEDs, that > > > does not scale, and there is not really a good reason for that other > > > than reviewing drivers in isolation and therefore making it harder to > > > extract the commonality. Yes, I realize that since you are the latest > > > person submitting something in that area, you are being selected :) > > I agree. > > > I see the merit of your proposal to come up with a generic mechanism > > to configure Ethernet LEDs, however I can't justify spending much of > > my work time on this. If it is deemed useful I'm happy to send another > > version of the current patchset that addresses the reviewer's comments, > > but if the implementation of a generic LED configuration interface is > > a requirement I will have to abandon at least the LED configuration > > part of this series. > > Can you at least define a common binding for this. Maybe that's just > removing 'realtek'. While the kernel side can evolve to a common > infrastructure, the DT bindings can't. I'm working on a generic binding. I wonder what is the best process for reviewing/landing it, I'm doubting between two options: a) only post the binding doc and the generic PHY code that reads the configuration from the DT. Post Realtek patches once the binding/generic code has been acked. pros: no churn from Realtek specific patches cons: initially no (real) user of the new binding b) post generic and Realtek changes together pros: the binding has a user initially cons: churn from Realtek specific patches I can do either, depending on what maintainers/reviewers prefer. I'm slightly inclined towards a)
Re: [PATCH] [RESEND v2] structleak: disable STRUCTLEAK_BYREF in combination with KASAN_STACK
On Mon, Jul 22, 2019 at 5:23 PM Alexander Potapenko wrote: > On Mon, Jul 22, 2019 at 4:26 PM Arnd Bergmann wrote: > > On Mon, Jul 22, 2019 at 3:43 PM Alexander Potapenko > > wrote: > > > On Mon, Jul 22, 2019 at 1:41 PM Arnd Bergmann wrote: > > Doesn't that just limit the usefulness of KASAN, as you no longer catch > > actual accesses to unintialized variables that KASAN is designed to find? > KASAN (unlike KMSAN) doesn't detect accesses to uninitialized variables. > It can of course detect bugs induced by e.g. treating an uninitialized > variable as a pointer or an array index. > Depending on the pattern used to initialize those variables, this can > indeed mask some bugs, but OTOH others will become more reproducible. > > I'm not really sure KASAN+CONFIG_INIT_STACK_ALL is a problem right > now, will need to take a look. See below for the slightly trimmed warning output of a 32-bit ARM allmodconfig build with KASAN_STACK enabled. In the default allmodconfig build (no KASAN_STACK, with CONFIG_INIT_STACK_ALL) I see no warnings. With KASAN_STACK, some really bad ones start to appear, such as drivers/gpu/drm/panel/panel-sitronix-st7789v.c:197:12: 14944 bytes 'st7789v_prepare' drivers/usb/misc/sisusbvga/sisusb.c:1878:12: 12480 bytes 'sisusb_init_gfxcore'. drivers/gpu/drm/omapdrm/displays/panel-lgphilips-lb035q02.c:108:12: 7904 bytes 'lb035q02_connect' drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c:163:13: 9600 bytes 'td028ttec1_panel_enable' drivers/media/dvb-frontends/stv0367.c:995:12: 7488 bytes 'stv0367ter_algo' drivers/media/i2c/mt9t112.c:670:12: 7584 bytes 'mt9t112_init_camera' drivers/usb/misc/sisusbvga/sisusb.c:1750:13: 8096 bytes 'sisusb_set_default_mode' In most functions, the frame sizes stay the same or go up 32 bytes. In a few files, the warnings change drastically, and but it goes both ways, smaller or larger with init_stack_all added in, presumably as clang makes different inlining decisions, the worst ones are now drivers/gpu/drm/panel/panel-sitronix-st7789v.c:197:12: 14944 bytes 'st7789v_prepare' drivers/media/i2c/mt9t112.c:670:12: 10080 bytes 'mt9t112_init_camera' drivers/usb/misc/sisusbvga/sisusb.c:1878:12: 11072 bytes 'sisusb_init_gfxcore' drivers/firmware/broadcom/bcm47xx_sprom.c:187:13: 9152 bytes 'bcm47xx_sprom_fill_auto' drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c:163:13: 9600 bytes 'td028ttec1_panel_enable' drivers/usb/misc/sisusbvga/sisusb.c:1750:13: 8096 bytes 'sisusb_set_default_mode' See below for the full list. Overall, I'd say there is probably no harm in allowing CONFIG_INIT_STACK_ALL in combination with KASAN_STACK when building with clang, as KASAN_STACK by itself is the actual problem. With gcc, CONFIG_INIT_STACK_ALL+KASAN_STACK is dangerous, while KASAN_STACK by itself is not a problem any more on recent gcc versions. Arnd --- allmodconfig-arm+kasan-stack 2019-07-22 18:57:29.970466686 +0200 +++ allmodconfig-arm+kasan_stack+init_stack_all 2019-07-22 18:57:31.434493204 +0200 @@ -1,262 +1,260 @@ crypto/asymmetric_keys/asym_tpm.c:720:12: 1376 bytes 'tpm_key_eds_op' crypto/crypto_user_stat.c:298:5: 1920 bytes 'crypto_reportstat' -drivers/block/drbd/drbd_receiver.c:921:12: 1632 bytes 'conn_connect' +drivers/block/drbd/drbd_receiver.c:921:12: 1664 bytes 'conn_connect' drivers/block/loop.c:1569:12: 1824 bytes 'lo_ioctl' drivers/cdrom/cdrom.c:1149:5: 1600 bytes 'cdrom_open' drivers/crypto/ccree/cc_cipher.c:383:12: 1536 bytes 'cc_cipher_setkey' -drivers/firmware/broadcom/bcm47xx_sprom.c:187:13: 3424 bytes 'bcm47xx_sprom_fill_auto' +drivers/firmware/broadcom/bcm47xx_sprom.c:187:13: 9152 bytes 'bcm47xx_sprom_fill_auto' +drivers/firmware/broadcom/bcm47xx_sprom.c:408:13: 1536 bytes 'bcm47xx_fill_sprom_path_r4589' drivers/fpga/machxo2-spi.c:186:12: 1952 bytes 'machxo2_write_init' drivers/fpga/machxo2-spi.c:286:12: 1856 bytes 'machxo2_write_complete' drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:291:6: 1376 bytes 'amdgpu_atombios_get_connector_info_from_object_table' drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:2993:6: 1312 bytes 'bw_calcs' +drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:1361:17: 1312 bytes 'link_create' drivers/gpu/drm/amd/amdgpu/../display/dc/dce112/dce112_resource.c:1328:23: 1568 bytes 'dce112_create_resource_pool' drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c:1553:6: 2112 bytes 'mod_color_calculate_regamma_params' drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/vega10_hwmgr.c:2932:12: 1504 bytes 'vega10_enable_dpm_tasks' -drivers/gpu/drm/amd/amdgpu/../powerplay/smumgr/vegam_smumgr.c:1923:12: 1504 bytes 'vegam_init_smc_table' +drivers/gpu/drm/amd/amdgpu/../powerplay/smumgr/vegam_smumgr.c:1923:12: 1536 bytes 'vegam_init_smc_table' drivers/gpu/drm/bridge/parade-ps8622.c:352:13: 1632 bytes 'ps8622_pre_enable' -drivers/gpu/drm/i2c/tda998x_drv.c:1419:13: 1376 bytes 'tda998x_bridge_mode_set' -drivers/gpu/drm/i2c/tda998x_drv.c:2058:1: 1312 bytes 'tda998x_probe' drivers/gpu/drm/nouveau/
Re: [5.2 REGRESSION] Generic vDSO breaks seccomp-enabled userspace on i386
On Fri, Jul 19, 2019 at 01:40:13PM -0400, Andy Lutomirski wrote:
> > On Jul 19, 2019, at 1:03 PM, Sean Christopherson
> > wrote:
> >
> > The generic vDSO implementation, starting with commit
> >
> > 7ac870747988 ("x86/vdso: Switch to generic vDSO implementation")
> >
> > breaks seccomp-enabled userspace on 32-bit x86 (i386) kernels. Prior to
> > the generic implementation, the x86 vDSO used identical code for both
> > x86_64 and i386 kernels, which worked because it did all calcuations using
> > structs with naturally sized variables, i.e. didn't use __kernel_timespec.
> >
> > The generic vDSO does its internal calculations using __kernel_timespec,
> > which in turn requires the i386 fallback syscall to use the 64-bit
> > variation, __NR_clock_gettime64.
>
> This is basically doomed to break eventually, right?
Just so I'm understanding: the vDSO change introduced code to make an
actual syscall on i386, which for most seccomp filters would be rejected?
> I’ve occasionally considered adding a concept of “seccomp aliases”. The idea
> is that, if a filter returns anything other than ALLOW, we re-run it with a
> different nr that we dig out it a small list of such cases. This would be
> limited to cases where the new syscall does the same thing with the same
> arguments.
Would that help here? The kernel just sees this a direct syscall. I
guess it could whitelist it by checking the return address?
> I want this for restart_syscall: I want to renumber it.
Oh man, don't get me started on restart_syscall. Some architectures make
it invisible to seccomp and others don't. ugh.
--
Kees Cook
Re: [PATCH v2] powerpc: slightly improve cache helpers
On Sun, Jul 21, 2019 at 11:19 PM Segher Boessenkool
wrote:
>
> On Sun, Jul 21, 2019 at 07:41:40PM -0700, Nathan Chancellor wrote:
> > Hi Segher,
> >
> > On Sun, Jul 21, 2019 at 01:01:50PM -0500, Segher Boessenkool wrote:
> > > On Sun, Jul 21, 2019 at 12:58:46AM -0700, Nathan Chancellor wrote:
> > > > 017c clear_user_page:
> > > > 17c: 94 21 ff f0 stwu 1, -16(1)
> > > > 180: 38 80 00 80 li 4, 128
> > > > 184: 38 63 ff e0 addi 3, 3, -32
> > > > 188: 7c 89 03 a6 mtctr 4
> > > > 18c: 38 81 00 0f addi 4, 1, 15
> > > > 190: 8c c3 00 20 lbzu 6, 32(3)
> > > > 194: 98 c1 00 0f stb 6, 15(1)
> > > > 198: 7c 00 27 ec dcbz 0, 4
> > > > 19c: 42 00 ff f4 bdnz .+65524
> > >
> > > Uh, yeah, well, I have no idea what clang tried here, but that won't
> > > work. It's copying a byte from each target cache line to the stack,
> > > and then does clears the cache line containing that byte on the stack.
> > >
> > > I *guess* this is about "Z" and not about "%y", but you'll have to ask
> > > the clang people.
> > >
> > > Or it may be that they do not treat inline asm operands as lvalues
> > > properly? That rings some bells. Yeah that looks like it.
>
> The code is
> __asm__ __volatile__ ("dcbz %y0" : : "Z"(*(u8 *)addr) : "memory");
>
> so yeah it looks like clang took that *(u8 *)addr as rvalue, and
> stored that in stack, and then used *that* as memory.
What's the %y modifier supposed to mean here? addr is in the list of
inputs, so what's wrong with using it as an rvalue?
>
> Maybe clang simply does not not to treat "Z" the same as "m"? (And "Y"
> and "Q" and "es" and a whole bunch of "w*", what about those?)
--
Thanks,
~Nick Desaulniers
Re: [PATCH 12/12] closures: fix a race on wakeup from closure_sync
On Thu, Jul 18, 2019 at 03:46:46PM +0800, Coly Li wrote:
> On 2019/7/16 6:47 下午, Coly Li wrote:
> > Hi Kent,
> >
> > On 2019/6/11 3:14 上午, Kent Overstreet wrote:
> >> Signed-off-by: Kent Overstreet
> > Acked-by: Coly Li
> >
> > And also I receive report for suspicious closure race condition in
> > bcache, and people ask for having this patch into Linux v5.3.
> >
> > So before this patch gets merged into upstream, I plan to rebase it to
> > drivers/md/bcache/closure.c at this moment. Of cause the author is you.
> >
> > When lib/closure.c merged into upstream, I will rebase all closure usage
> > from bcache to use lib/closure.{c,h}.
>
> Hi Kent,
>
> The race bug reporter replies me that the closure race bug is very rare
> to reproduce, after applying the patch and testing, they are not sure
> whether their closure race problem is fixed or not.
>
> And I notice rcu_read_lock()/rcu_read_unlock() is used here, but it is
> not clear to me what is the functionality of the rcu read lock in
> closure_sync_fn(). I believe you have reason to use the rcu stuffs here,
> could you please provide some hints to help me to understand the change
> better ?
The race was when a thread using closure_sync() notices cl->s->done == 1 before
the thread calling closure_put() calls wake_up_process(). Then, it's possible
for that thread to return and exit just before wake_up_process() is called - so
we're trying to wake up a process that no longer exists.
rcu_read_lock() is sufficient to protect against this, as there's an rcu barrier
somewhere in the process teardown path.
Re: [PATCH 2/2] HID: core: only warn once of oversize hid report
On Mon, 2019-07-22 at 10:36 -0600, stillcompil...@gmail.com wrote:
> On HP spectre x360 convertible the message:
> hid-sensor-hub 001F:8087:0AC2.0002: hid_field_extract() called with n (192) >
> 32! (kworker/1:2)
> is continually printed many times per second, crowding out all other kernel
> logs
> Protect dmesg by printing the warning only one time.
[]
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
[]
> @@ -1311,7 +1311,7 @@ u32 hid_field_extract(const struct hid_device *hid, u8
> *report,
> unsigned offset, unsigned n)
> {
> if (n > 32) {
> - hid_warn(hid, "hid_field_extract() called with n (%d) > 32!
> (%s)\n",
> + hid_warn_once(hid, "hid_field_extract() called with n (%d) >
> 32! (%s)\n",
>n, current->comm);
> n = 32;
> }
Is this papering over an actual defect somewhere else?
Trivially, this could use "%s: ...", __func__, ...
Re: [PATCH] [RESEND v2] structleak: disable STRUCTLEAK_BYREF in combination with KASAN_STACK
On Mon, Jul 22, 2019 at 01:41:20PM +0200, Arnd Bergmann wrote:
> The combination of KASAN_STACK and GCC_PLUGIN_STRUCTLEAK_BYREF
> leads to much larger kernel stack usage, as seen from the warnings
> about functions that now exceed the 2048 byte limit:
>
> drivers/media/i2c/tvp5150.c:253:1: error: the frame size of 3936 bytes is
> larger than 2048 bytes
> drivers/media/tuners/r820t.c:1327:1: error: the frame size of 2816 bytes is
> larger than 2048 bytes
> drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:16552:1: error:
> the frame size of 3144 bytes is larger than 2048 bytes
> [-Werror=frame-larger-than=]
> fs/ocfs2/aops.c:1892:1: error: the frame size of 2088 bytes is larger than
> 2048 bytes
> fs/ocfs2/dlm/dlmrecovery.c:737:1: error: the frame size of 2088 bytes is
> larger than 2048 bytes
> fs/ocfs2/namei.c:1677:1: error: the frame size of 2584 bytes is larger than
> 2048 bytes
> fs/ocfs2/super.c:1186:1: error: the frame size of 2640 bytes is larger than
> 2048 bytes
> fs/ocfs2/xattr.c:3678:1: error: the frame size of 2176 bytes is larger than
> 2048 bytes
> net/bluetooth/l2cap_core.c:7056:1: error: the frame size of 2144 bytes is
> larger than 2048 bytes [-Werror=frame-larger-than=]
> net/bluetooth/l2cap_core.c: In function 'l2cap_recv_frame':
> net/bridge/br_netlink.c:1505:1: error: the frame size of 2448 bytes is larger
> than 2048 bytes
> net/ieee802154/nl802154.c:548:1: error: the frame size of 2232 bytes is
> larger than 2048 bytes
> net/wireless/nl80211.c:1726:1: error: the frame size of 2224 bytes is larger
> than 2048 bytes
> net/wireless/nl80211.c:2357:1: error: the frame size of 4584 bytes is larger
> than 2048 bytes
> net/wireless/nl80211.c:5108:1: error: the frame size of 2760 bytes is larger
> than 2048 bytes
> net/wireless/nl80211.c:6472:1: error: the frame size of 2112 bytes is larger
> than 2048 bytes
>
> The structleak plugin was previously disabled for CONFIG_COMPILE_TEST,
> but meant we missed some bugs, so this time we should address them.
>
> The frame size warnings are distracting, and risking a kernel stack
> overflow is generally not beneficial to performance, so it may be best
> to disallow that particular combination. This can be done by turning
> off either one. I picked the dependency in GCC_PLUGIN_STRUCTLEAK_BYREF
> and GCC_PLUGIN_STRUCTLEAK_BYREF_ALL, as this option is designed to
> make uninitialized stack usage less harmful when enabled on its own,
> but it also prevents KASAN from detecting those cases in which it was
> in fact needed.
>
> KASAN_STACK is currently implied by KASAN on gcc, but could be made a
> user selectable option if we want to allow combining (non-stack) KASAN
> with GCC_PLUGIN_STRUCTLEAK_BYREF.
>
> Note that it would be possible to specifically address the files that
> print the warning, but presumably the overall stack usage is still
> significantly higher than in other configurations, so this would not
> address the full problem.
>
> I could not test this with CONFIG_INIT_STACK_ALL, which may or may not
> suffer from a similar problem.
>
> Fixes: 81a56f6dcd20 ("gcc-plugins: structleak: Generalize to all variable
> types")
> Acked-by: Kees Cook
> Link: https://lore.kernel.org/lkml/20190628123819.2785504-1-a...@arndb.de/
> Signed-off-by: Arnd Bergmann
> ---
> [v2] do it for both GCC_PLUGIN_STRUCTLEAK_BYREF and
> GCC_PLUGIN_STRUCTLEAK_BYREF_ALL.
>
> Andrew, can you pick this up in -mm? It looks like nobody else
> wanted it in their trees even though there was agreement on the
> patch itself.
Andrew, if you don't want it, I can take it via my gcc-plugins tree?
-Kees
> ---
> security/Kconfig.hardening | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
> index a1ffe2eb4d5f..af4c979b38ee 100644
> --- a/security/Kconfig.hardening
> +++ b/security/Kconfig.hardening
> @@ -61,6 +61,7 @@ choice
> config GCC_PLUGIN_STRUCTLEAK_BYREF
> bool "zero-init structs passed by reference (strong)"
> depends on GCC_PLUGINS
> + depends on !(KASAN && KASAN_STACK=1)
> select GCC_PLUGIN_STRUCTLEAK
> help
> Zero-initialize any structures on the stack that may
> @@ -70,9 +71,15 @@ choice
> exposures, like CVE-2017-1000410:
> https://git.kernel.org/linus/06e7e776ca4d3654
>
> + As a side-effect, this keeps a lot of variables on the
> + stack that can otherwise be optimized out, so combining
> + this with CONFIG_KASAN_STACK can lead to a stack overflow
> + and is disallowed.
> +
> config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
> bool "zero-init anything passed by reference (very strong)"
> depends on GCC_PLUGINS
> + depends on !(KASAN && KASAN_STACK=1)
> select GCC_PLUGIN_STRUCTLEAK
> help
> Zero-initialize any
[PATCH 3/8] media: dvb-frontends: mn88472: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/media/dvb-frontends/mn88472.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/media/dvb-frontends/mn88472.c
b/drivers/media/dvb-frontends/mn88472.c
index 731b44b9b74c..ef8094aa97df 100644
--- a/drivers/media/dvb-frontends/mn88472.c
+++ b/drivers/media/dvb-frontends/mn88472.c
@@ -612,9 +612,9 @@ static int mn88472_probe(struct i2c_client *client,
* Also, register bank 2 do not support sequential I/O. Only single
* register write or read is allowed to that bank.
*/
- dev->client[1] = i2c_new_dummy(client->adapter, 0x1a);
- if (!dev->client[1]) {
- ret = -ENODEV;
+ dev->client[1] = i2c_new_dummy_device(client->adapter, 0x1a);
+ if (IS_ERR(dev->client[1])) {
+ ret = PTR_ERR(dev->client[1]);
dev_err(&client->dev, "I2C registration failed\n");
if (ret)
goto err_regmap_0_regmap_exit;
@@ -626,9 +626,9 @@ static int mn88472_probe(struct i2c_client *client,
}
i2c_set_clientdata(dev->client[1], dev);
- dev->client[2] = i2c_new_dummy(client->adapter, 0x1c);
- if (!dev->client[2]) {
- ret = -ENODEV;
+ dev->client[2] = i2c_new_dummy_device(client->adapter, 0x1c);
+ if (IS_ERR(dev->client[2])) {
+ ret = PTR_ERR(dev->client[2]);
dev_err(&client->dev, "2nd I2C registration failed\n");
if (ret)
goto err_regmap_1_regmap_exit;
--
2.20.1
[PATCH 7/8] media: i2c: adv7511-v4l2: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/media/i2c/adv7511-v4l2.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/media/i2c/adv7511-v4l2.c b/drivers/media/i2c/adv7511-v4l2.c
index 2ad6bdf1a9fc..7db94267bcab 100644
--- a/drivers/media/i2c/adv7511-v4l2.c
+++ b/drivers/media/i2c/adv7511-v4l2.c
@@ -1872,11 +1872,11 @@ static int adv7511_probe(struct i2c_client *client,
const struct i2c_device_id *
goto err_entity;
}
- state->i2c_edid = i2c_new_dummy(client->adapter,
+ state->i2c_edid = i2c_new_dummy_device(client->adapter,
state->i2c_edid_addr >> 1);
- if (state->i2c_edid == NULL) {
+ if (IS_ERR(state->i2c_edid)) {
v4l2_err(sd, "failed to register edid i2c client\n");
- err = -ENOMEM;
+ err = PTR_ERR(state->i2c_edid);
goto err_entity;
}
@@ -1889,11 +1889,11 @@ static int adv7511_probe(struct i2c_client *client,
const struct i2c_device_id *
}
if (state->pdata.cec_clk) {
- state->i2c_cec = i2c_new_dummy(client->adapter,
+ state->i2c_cec = i2c_new_dummy_device(client->adapter,
state->i2c_cec_addr >> 1);
- if (state->i2c_cec == NULL) {
+ if (IS_ERR(state->i2c_cec)) {
v4l2_err(sd, "failed to register cec i2c client\n");
- err = -ENOMEM;
+ err = PTR_ERR(state->i2c_cec);
goto err_unreg_edid;
}
adv7511_wr(sd, 0xe2, 0x00); /* power up cec section */
@@ -1901,10 +1901,10 @@ static int adv7511_probe(struct i2c_client *client,
const struct i2c_device_id *
adv7511_wr(sd, 0xe2, 0x01); /* power down cec section */
}
- state->i2c_pktmem = i2c_new_dummy(client->adapter,
state->i2c_pktmem_addr >> 1);
- if (state->i2c_pktmem == NULL) {
+ state->i2c_pktmem = i2c_new_dummy_device(client->adapter,
state->i2c_pktmem_addr >> 1);
+ if (IS_ERR(state->i2c_pktmem)) {
v4l2_err(sd, "failed to register pktmem i2c client\n");
- err = -ENOMEM;
+ err = PTR_ERR(state->i2c_pktmem);
goto err_unreg_cec;
}
--
2.20.1
[PATCH] i2c: replace i2c_new_secondary_device with an ERR_PTR variant
In the general move to have i2c_new_*_device functions which return
ERR_PTR instead of NULL, this patch converts i2c_new_secondary_device().
There are only few users, so this patch converts the I2C core and all
users in one go. The function gets renamed to i2c_new_ancillary_device()
so out-of-tree users will get a build failure to understand they need to
adapt their error checking code.
Signed-off-by: Wolfram Sang
---
Kindly asking for acks/revs/tests from people knowing the modified
drivers.
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 18 +-
drivers/i2c/i2c-core-base.c | 10 +-
drivers/media/i2c/adv748x/adv748x-core.c | 6 +++---
drivers/media/i2c/adv7604.c | 10 +-
include/linux/i2c.h | 2 +-
5 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index f6d2681f6927..9e13e466e72c 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -981,10 +981,10 @@ static int adv7511_init_cec_regmap(struct adv7511 *adv)
{
int ret;
- adv->i2c_cec = i2c_new_secondary_device(adv->i2c_main, "cec",
+ adv->i2c_cec = i2c_new_ancillary_device(adv->i2c_main, "cec",
ADV7511_CEC_I2C_ADDR_DEFAULT);
- if (!adv->i2c_cec)
- return -EINVAL;
+ if (IS_ERR(adv->i2c_cec))
+ return PTR_ERR(adv->i2c_cec);
i2c_set_clientdata(adv->i2c_cec, adv);
adv->regmap_cec = devm_regmap_init_i2c(adv->i2c_cec,
@@ -1165,20 +1165,20 @@ static int adv7511_probe(struct i2c_client *i2c, const
struct i2c_device_id *id)
adv7511_packet_disable(adv7511, 0x);
- adv7511->i2c_edid = i2c_new_secondary_device(i2c, "edid",
+ adv7511->i2c_edid = i2c_new_ancillary_device(i2c, "edid",
ADV7511_EDID_I2C_ADDR_DEFAULT);
- if (!adv7511->i2c_edid) {
- ret = -EINVAL;
+ if (IS_ERR(adv7511->i2c_edid)) {
+ ret = PTR_ERR(adv7511->i2c_edid);
goto uninit_regulators;
}
regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR,
adv7511->i2c_edid->addr << 1);
- adv7511->i2c_packet = i2c_new_secondary_device(i2c, "packet",
+ adv7511->i2c_packet = i2c_new_ancillary_device(i2c, "packet",
ADV7511_PACKET_I2C_ADDR_DEFAULT);
- if (!adv7511->i2c_packet) {
- ret = -EINVAL;
+ if (IS_ERR(adv7511->i2c_packet)) {
+ ret = PTR_ERR(adv7511->i2c_packet);
goto err_i2c_unregister_edid;
}
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index f26ed495d384..76cb91e064b8 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -966,7 +966,7 @@ struct i2c_client *devm_i2c_new_dummy_device(struct device
*dev,
EXPORT_SYMBOL_GPL(devm_i2c_new_dummy_device);
/**
- * i2c_new_secondary_device - Helper to get the instantiated secondary address
+ * i2c_new_ancillary_device - Helper to get the instantiated secondary address
* and create the associated device
* @client: Handle to the primary client
* @name: Handle to specify which secondary address to get
@@ -985,9 +985,9 @@ EXPORT_SYMBOL_GPL(devm_i2c_new_dummy_device);
* cell whose "reg-names" value matches the slave name.
*
* This returns the new i2c client, which should be saved for later use with
- * i2c_unregister_device(); or NULL to indicate an error.
+ * i2c_unregister_device(); or an ERR_PTR to describe the error.
*/
-struct i2c_client *i2c_new_secondary_device(struct i2c_client *client,
+struct i2c_client *i2c_new_ancillary_device(struct i2c_client *client,
const char *name,
u16 default_addr)
{
@@ -1002,9 +1002,9 @@ struct i2c_client *i2c_new_secondary_device(struct
i2c_client *client,
}
dev_dbg(&client->adapter->dev, "Address for %s : 0x%x\n", name, addr);
- return i2c_new_dummy(client->adapter, addr);
+ return i2c_new_dummy_device(client->adapter, addr);
}
-EXPORT_SYMBOL_GPL(i2c_new_secondary_device);
+EXPORT_SYMBOL_GPL(i2c_new_ancillary_device);
/* - */
diff --git a/drivers/media/i2c/adv748x/adv748x-core.c
b/drivers/media/i2c/adv748x/adv748x-core.c
index f57cd77a32fa..2567de2b0037 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -183,14 +183,14 @@ static int adv748x_initialise_clients(struct
adv748x_state *state)
int ret;
for (i = ADV748X_PAGE_DPLL; i < ADV748X_PAGE_MAX; ++i) {
- state->i2c_clients[i] = i2c_new_secondary_device(
+
[PATCH 1/8] media: dvb-frontends: cxd2820r_core: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/media/dvb-frontends/cxd2820r_core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/dvb-frontends/cxd2820r_core.c
b/drivers/media/dvb-frontends/cxd2820r_core.c
index 1f006f8e8cc2..5280ba4085e4 100644
--- a/drivers/media/dvb-frontends/cxd2820r_core.c
+++ b/drivers/media/dvb-frontends/cxd2820r_core.c
@@ -632,9 +632,9 @@ static int cxd2820r_probe(struct i2c_client *client,
* one dummy I2C client in in order to get own I2C client for each
* register bank.
*/
- priv->client[1] = i2c_new_dummy(client->adapter, client->addr | (1 <<
1));
- if (!priv->client[1]) {
- ret = -ENODEV;
+ priv->client[1] = i2c_new_dummy_device(client->adapter, client->addr |
(1 << 1));
+ if (IS_ERR(priv->client[1])) {
+ ret = PTR_ERR(priv->client[1]);
dev_err(&client->dev, "I2C registration failed\n");
if (ret)
goto err_regmap_0_regmap_exit;
--
2.20.1
[PATCH 1/3] iio: light: cm36651: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/iio/light/cm36651.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/light/cm36651.c b/drivers/iio/light/cm36651.c
index 7702c2bcbcfa..1019d625adb1 100644
--- a/drivers/iio/light/cm36651.c
+++ b/drivers/iio/light/cm36651.c
@@ -646,18 +646,18 @@ static int cm36651_probe(struct i2c_client *client,
i2c_set_clientdata(client, indio_dev);
cm36651->client = client;
- cm36651->ps_client = i2c_new_dummy(client->adapter,
+ cm36651->ps_client = i2c_new_dummy_device(client->adapter,
CM36651_I2C_ADDR_PS);
- if (!cm36651->ps_client) {
+ if (IS_ERR(cm36651->ps_client)) {
dev_err(&client->dev, "%s: new i2c device failed\n", __func__);
- ret = -ENODEV;
+ ret = PTR_ERR(cm36651->ps_client);
goto error_disable_reg;
}
- cm36651->ara_client = i2c_new_dummy(client->adapter, CM36651_ARA);
- if (!cm36651->ara_client) {
+ cm36651->ara_client = i2c_new_dummy_device(client->adapter,
CM36651_ARA);
+ if (IS_ERR(cm36651->ara_client)) {
dev_err(&client->dev, "%s: new i2c device failed\n", __func__);
- ret = -ENODEV;
+ ret = PTR_ERR(cm36651->ara_client);
goto error_i2c_unregister_ps;
}
--
2.20.1
[PATCH 07/14] mfd: max77693: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/mfd/max77693.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
index 901d99d65924..596ed85cab3b 100644
--- a/drivers/mfd/max77693.c
+++ b/drivers/mfd/max77693.c
@@ -183,17 +183,17 @@ static int max77693_i2c_probe(struct i2c_client *i2c,
} else
dev_info(max77693->dev, "device ID: 0x%x\n", reg_data);
- max77693->i2c_muic = i2c_new_dummy(i2c->adapter, I2C_ADDR_MUIC);
- if (!max77693->i2c_muic) {
+ max77693->i2c_muic = i2c_new_dummy_device(i2c->adapter, I2C_ADDR_MUIC);
+ if (IS_ERR(max77693->i2c_muic)) {
dev_err(max77693->dev, "Failed to allocate I2C device for
MUIC\n");
- return -ENODEV;
+ return PTR_ERR(max77693->i2c_muic);
}
i2c_set_clientdata(max77693->i2c_muic, max77693);
- max77693->i2c_haptic = i2c_new_dummy(i2c->adapter, I2C_ADDR_HAPTIC);
- if (!max77693->i2c_haptic) {
+ max77693->i2c_haptic = i2c_new_dummy_device(i2c->adapter,
I2C_ADDR_HAPTIC);
+ if (IS_ERR(max77693->i2c_haptic)) {
dev_err(max77693->dev, "Failed to allocate I2C device for
Haptic\n");
- ret = -ENODEV;
+ ret = PTR_ERR(max77693->i2c_haptic);
goto err_i2c_haptic;
}
i2c_set_clientdata(max77693->i2c_haptic, max77693);
--
2.20.1
[PATCH 0/2] media: ir-kbd-i2c: fix potential OOPS & minor cleanup
From: Wolfram Sang This series is part of a tree-wide movement to replace the I2C API call 'i2c_new_dummy' which returns NULL with its new counterpart returning an ERRPTR. It was manually converted and only build tested (by me and buildbot). A small cleanup was added while working on this driver. Looking for someone to ack/rev/test this series. The series is based on v5.3-rc1. A branch (with some more stuff included) can be found here: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/i2c/new_dummy Thanks! Wolfram Sang (2): media: ir-kbd-i2c: prevent potential NULL pointer access media: ir-kbd-i2c: remove outdated comments drivers/media/i2c/ir-kbd-i2c.c | 17 + 1 file changed, 5 insertions(+), 12 deletions(-) -- 2.20.1
[PATCH 2/2] media: ir-kbd-i2c: remove outdated comments
The "free memory" comment is obsolete since 2013 and the other ones
explain the obvious. Just remove the comments.
Signed-off-by: Wolfram Sang
---
drivers/media/i2c/ir-kbd-i2c.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/media/i2c/ir-kbd-i2c.c b/drivers/media/i2c/ir-kbd-i2c.c
index f46717052efc..30fde0e025c9 100644
--- a/drivers/media/i2c/ir-kbd-i2c.c
+++ b/drivers/media/i2c/ir-kbd-i2c.c
@@ -916,13 +916,9 @@ static int ir_remove(struct i2c_client *client)
{
struct IR_i2c *ir = i2c_get_clientdata(client);
- /* kill outstanding polls */
cancel_delayed_work_sync(&ir->work);
-
- /* unregister device */
rc_unregister_device(ir->rc);
- /* free memory */
return 0;
}
--
2.20.1
[PATCH 09/14] mfd: max8907: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/mfd/max8907.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mfd/max8907.c b/drivers/mfd/max8907.c
index cc01f706cb32..d44baafd9d14 100644
--- a/drivers/mfd/max8907.c
+++ b/drivers/mfd/max8907.c
@@ -214,9 +214,9 @@ static int max8907_i2c_probe(struct i2c_client *i2c,
goto err_regmap_gen;
}
- max8907->i2c_rtc = i2c_new_dummy(i2c->adapter, MAX8907_RTC_I2C_ADDR);
- if (!max8907->i2c_rtc) {
- ret = -ENOMEM;
+ max8907->i2c_rtc = i2c_new_dummy_device(i2c->adapter,
MAX8907_RTC_I2C_ADDR);
+ if (IS_ERR(max8907->i2c_rtc)) {
+ ret = PTR_ERR(max8907->i2c_rtc);
goto err_dummy_rtc;
}
i2c_set_clientdata(max8907->i2c_rtc, max8907);
--
2.20.1
[PATCH 08/14] mfd: max77843: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/mfd/max77843.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mfd/max77843.c b/drivers/mfd/max77843.c
index 25cbb2242b26..209ee24d9ce1 100644
--- a/drivers/mfd/max77843.c
+++ b/drivers/mfd/max77843.c
@@ -70,11 +70,11 @@ static int max77843_chg_init(struct max77693_dev *max77843)
{
int ret;
- max77843->i2c_chg = i2c_new_dummy(max77843->i2c->adapter, I2C_ADDR_CHG);
- if (!max77843->i2c_chg) {
+ max77843->i2c_chg = i2c_new_dummy_device(max77843->i2c->adapter,
I2C_ADDR_CHG);
+ if (IS_ERR(max77843->i2c_chg)) {
dev_err(&max77843->i2c->dev,
"Cannot allocate I2C device for Charger\n");
- return -ENODEV;
+ return PTR_ERR(max77843->i2c_chg);
}
i2c_set_clientdata(max77843->i2c_chg, max77843);
--
2.20.1
[PATCH 14/14] mfd: twl-core: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/mfd/twl-core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 448d9397ff04..20cf8cfe4f3b 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -1141,12 +1141,12 @@ twl_probe(struct i2c_client *client, const struct
i2c_device_id *id)
if (i == 0) {
twl->client = client;
} else {
- twl->client = i2c_new_dummy(client->adapter,
+ twl->client = i2c_new_dummy_device(client->adapter,
client->addr + i);
- if (!twl->client) {
+ if (IS_ERR(twl->client)) {
dev_err(&client->dev,
"can't attach client %d\n", i);
- status = -ENOMEM;
+ status = PTR_ERR(twl->client);
goto fail;
}
}
--
2.20.1
[PATCH] mfd: tps80031: convert to devm_i2c_new_dummy_device
Move from i2c_new_dummy() to devm_i2c_new_dummy_device(). So, we now get
an ERRPTR which we use in error handling and we can skip removal of the
created devices.
Signed-off-by: Wolfram Sang
---
Only build tested. Part of a tree-wide move to deprecate
i2c_new_dummy().
drivers/mfd/tps80031.c | 23 ---
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/mfd/tps80031.c b/drivers/mfd/tps80031.c
index 865257ade8ac..907452b86e32 100644
--- a/drivers/mfd/tps80031.c
+++ b/drivers/mfd/tps80031.c
@@ -437,12 +437,11 @@ static int tps80031_probe(struct i2c_client *client,
if (tps80031_slave_address[i] == client->addr)
tps80031->clients[i] = client;
else
- tps80031->clients[i] = i2c_new_dummy(client->adapter,
- tps80031_slave_address[i]);
- if (!tps80031->clients[i]) {
+ tps80031->clients[i] =
devm_i2c_new_dummy_device(&client->dev,
+ client->adapter,
tps80031_slave_address[i]);
+ if (IS_ERR(tps80031->clients[i])) {
dev_err(&client->dev, "can't attach client %d\n", i);
- ret = -ENOMEM;
- goto fail_client_reg;
+ return PTR_ERR(tps80031->clients[i]);
}
i2c_set_clientdata(tps80031->clients[i], tps80031);
@@ -452,7 +451,7 @@ static int tps80031_probe(struct i2c_client *client,
ret = PTR_ERR(tps80031->regmap[i]);
dev_err(&client->dev,
"regmap %d init failed, err %d\n", i, ret);
- goto fail_client_reg;
+ return ret;
}
}
@@ -461,7 +460,7 @@ static int tps80031_probe(struct i2c_client *client,
if (ret < 0) {
dev_err(&client->dev,
"Silicon version number read failed: %d\n", ret);
- goto fail_client_reg;
+ return ret;
}
ret = tps80031_read(&client->dev, TPS80031_SLAVE_ID3,
@@ -469,7 +468,7 @@ static int tps80031_probe(struct i2c_client *client,
if (ret < 0) {
dev_err(&client->dev,
"Silicon eeprom version read failed: %d\n", ret);
- goto fail_client_reg;
+ return ret;
}
dev_info(&client->dev, "ES version 0x%02x and EPROM version 0x%02x\n",
@@ -482,7 +481,7 @@ static int tps80031_probe(struct i2c_client *client,
ret = tps80031_irq_init(tps80031, client->irq, pdata->irq_base);
if (ret) {
dev_err(&client->dev, "IRQ init failed: %d\n", ret);
- goto fail_client_reg;
+ return ret;
}
tps80031_pupd_init(tps80031, pdata);
@@ -506,12 +505,6 @@ static int tps80031_probe(struct i2c_client *client,
fail_mfd_add:
regmap_del_irq_chip(client->irq, tps80031->irq_data);
-
-fail_client_reg:
- for (i = 0; i < TPS80031_NUM_SLAVES; i++) {
- if (tps80031->clients[i] && (tps80031->clients[i] != client))
- i2c_unregister_device(tps80031->clients[i]);
- }
return ret;
}
--
2.20.1
[PATCH 0/3] iio: convert subsystem to i2c_new_dummy_device()
This series is part of a tree-wide movement to replace the I2C API call 'i2c_new_dummy' which returns NULL with its new counterpart returning an ERRPTR. The series was generated with coccinelle (audited afterwards, of course) and build tested by me and by buildbot. No tests on HW have been performed. The branch is based on v5.3-rc1. A branch (with some more stuff included) can be found here: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/i2c/new_dummy Some drivers still need to be manually converted. Patches for those will be sent out individually. Wolfram Sang (3): iio: light: cm36651: convert to i2c_new_dummy_device iio: light: veml6070: convert to i2c_new_dummy_device iio: pressure: hp03: convert to i2c_new_dummy_device drivers/iio/light/cm36651.c | 12 ++-- drivers/iio/light/veml6070.c | 6 +++--- drivers/iio/pressure/hp03.c | 6 +++--- 3 files changed, 12 insertions(+), 12 deletions(-) -- 2.20.1
[PATCH 12/14] mfd: max8998: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/mfd/max8998.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
index 56409df120f8..785f8e9841b7 100644
--- a/drivers/mfd/max8998.c
+++ b/drivers/mfd/max8998.c
@@ -195,10 +195,10 @@ static int max8998_i2c_probe(struct i2c_client *i2c,
}
mutex_init(&max8998->iolock);
- max8998->rtc = i2c_new_dummy(i2c->adapter, RTC_I2C_ADDR);
- if (!max8998->rtc) {
+ max8998->rtc = i2c_new_dummy_device(i2c->adapter, RTC_I2C_ADDR);
+ if (IS_ERR(max8998->rtc)) {
dev_err(&i2c->dev, "Failed to allocate I2C device for RTC\n");
- return -ENODEV;
+ return PTR_ERR(max8998->rtc);
}
i2c_set_clientdata(max8998->rtc, max8998);
--
2.20.1
[PATCH 3/3] hwmon: w83781d: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/hwmon/w83781d.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hwmon/w83781d.c b/drivers/hwmon/w83781d.c
index d2c04b6a3f2b..015f1ea31966 100644
--- a/drivers/hwmon/w83781d.c
+++ b/drivers/hwmon/w83781d.c
@@ -894,12 +894,12 @@ w83781d_detect_subclients(struct i2c_client *new_client)
}
for (i = 0; i < num_sc; i++) {
- data->lm75[i] = i2c_new_dummy(adapter, sc_addr[i]);
- if (!data->lm75[i]) {
+ data->lm75[i] = i2c_new_dummy_device(adapter, sc_addr[i]);
+ if (IS_ERR(data->lm75[i])) {
dev_err(&new_client->dev,
"Subclient %d registration at address 0x%x
failed.\n",
i, sc_addr[i]);
- err = -ENOMEM;
+ err = PTR_ERR(data->lm75[i]);
if (i == 1)
goto ERROR_SC_3;
goto ERROR_SC_2;
--
2.20.1
[PATCH 00/14] mfd: convert subsystem to i2c_new_dummy_device()
This series is part of a tree-wide movement to replace the I2C API call 'i2c_new_dummy' which returns NULL with its new counterpart returning an ERRPTR. The series was generated with coccinelle (audited afterwards, of course) and build tested by me and by buildbot. No tests on HW have been performed. The branch is based on v5.3-rc1. A branch (with some more stuff included) can be found here: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/i2c/new_dummy Some drivers still need to be manually converted. Patches for those will be sent out individually. Wolfram Sang (14): mfd: 88pm800: convert to i2c_new_dummy_device mfd: 88pm860x-core: convert to i2c_new_dummy_device mfd: ab3100-core: convert to i2c_new_dummy_device mfd: bcm590xx: convert to i2c_new_dummy_device mfd: da9150-core: convert to i2c_new_dummy_device mfd: max14577: convert to i2c_new_dummy_device mfd: max77693: convert to i2c_new_dummy_device mfd: max77843: convert to i2c_new_dummy_device mfd: max8907: convert to i2c_new_dummy_device mfd: max8925-i2c: convert to i2c_new_dummy_device mfd: max8997: convert to i2c_new_dummy_device mfd: max8998: convert to i2c_new_dummy_device mfd: palmas: convert to i2c_new_dummy_device mfd: twl-core: convert to i2c_new_dummy_device drivers/mfd/88pm800.c | 12 ++-- drivers/mfd/88pm860x-core.c | 6 +++--- drivers/mfd/ab3100-core.c | 6 +++--- drivers/mfd/bcm590xx.c | 6 +++--- drivers/mfd/da9150-core.c | 6 +++--- drivers/mfd/max14577.c | 6 +++--- drivers/mfd/max77693.c | 12 ++-- drivers/mfd/max77843.c | 6 +++--- drivers/mfd/max8907.c | 6 +++--- drivers/mfd/max8925-i2c.c | 12 ++-- drivers/mfd/max8997.c | 18 +- drivers/mfd/max8998.c | 6 +++--- drivers/mfd/palmas.c| 6 +++--- drivers/mfd/twl-core.c | 6 +++--- 14 files changed, 57 insertions(+), 57 deletions(-) -- 2.20.1
[PATCH 2/3] iio: light: veml6070: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/iio/light/veml6070.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/light/veml6070.c b/drivers/iio/light/veml6070.c
index a3138e1b5803..0be553ad5989 100644
--- a/drivers/iio/light/veml6070.c
+++ b/drivers/iio/light/veml6070.c
@@ -158,10 +158,10 @@ static int veml6070_probe(struct i2c_client *client,
indio_dev->name = VEML6070_DRV_NAME;
indio_dev->modes = INDIO_DIRECT_MODE;
- data->client2 = i2c_new_dummy(client->adapter, VEML6070_ADDR_DATA_LSB);
- if (!data->client2) {
+ data->client2 = i2c_new_dummy_device(client->adapter,
VEML6070_ADDR_DATA_LSB);
+ if (IS_ERR(data->client2)) {
dev_err(&client->dev, "i2c device for second chip address
failed\n");
- return -ENODEV;
+ return PTR_ERR(data->client2);
}
data->config = VEML6070_IT_10 | VEML6070_COMMAND_RSRVD |
--
2.20.1
[PATCH 3/3] iio: pressure: hp03: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/iio/pressure/hp03.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/pressure/hp03.c b/drivers/iio/pressure/hp03.c
index f00102577fd5..026ba15ef68f 100644
--- a/drivers/iio/pressure/hp03.c
+++ b/drivers/iio/pressure/hp03.c
@@ -243,10 +243,10 @@ static int hp03_probe(struct i2c_client *client,
* which has it's dedicated I2C address and contains
* the calibration constants for the sensor.
*/
- priv->eeprom_client = i2c_new_dummy(client->adapter, HP03_EEPROM_ADDR);
- if (!priv->eeprom_client) {
+ priv->eeprom_client = i2c_new_dummy_device(client->adapter,
HP03_EEPROM_ADDR);
+ if (IS_ERR(priv->eeprom_client)) {
dev_err(dev, "New EEPROM I2C device failed\n");
- return -ENODEV;
+ return PTR_ERR(priv->eeprom_client);
}
priv->eeprom_regmap = regmap_init_i2c(priv->eeprom_client,
--
2.20.1
[PATCH 10/14] mfd: max8925-i2c: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/mfd/max8925-i2c.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/mfd/max8925-i2c.c b/drivers/mfd/max8925-i2c.c
index 20bb19b71109..114e905bef25 100644
--- a/drivers/mfd/max8925-i2c.c
+++ b/drivers/mfd/max8925-i2c.c
@@ -176,18 +176,18 @@ static int max8925_probe(struct i2c_client *client,
dev_set_drvdata(chip->dev, chip);
mutex_init(&chip->io_lock);
- chip->rtc = i2c_new_dummy(chip->i2c->adapter, RTC_I2C_ADDR);
- if (!chip->rtc) {
+ chip->rtc = i2c_new_dummy_device(chip->i2c->adapter, RTC_I2C_ADDR);
+ if (IS_ERR(chip->rtc)) {
dev_err(chip->dev, "Failed to allocate I2C device for RTC\n");
- return -ENODEV;
+ return PTR_ERR(chip->rtc);
}
i2c_set_clientdata(chip->rtc, chip);
- chip->adc = i2c_new_dummy(chip->i2c->adapter, ADC_I2C_ADDR);
- if (!chip->adc) {
+ chip->adc = i2c_new_dummy_device(chip->i2c->adapter, ADC_I2C_ADDR);
+ if (IS_ERR(chip->adc)) {
dev_err(chip->dev, "Failed to allocate I2C device for ADC\n");
i2c_unregister_device(chip->rtc);
- return -ENODEV;
+ return PTR_ERR(chip->adc);
}
i2c_set_clientdata(chip->adc, chip);
--
2.20.1
[PATCH 13/14] mfd: palmas: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/mfd/palmas.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
index 6818ff34837c..f5b3fa973b13 100644
--- a/drivers/mfd/palmas.c
+++ b/drivers/mfd/palmas.c
@@ -549,12 +549,12 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
palmas->i2c_clients[i] = i2c;
else {
palmas->i2c_clients[i] =
- i2c_new_dummy(i2c->adapter,
+ i2c_new_dummy_device(i2c->adapter,
i2c->addr + i);
- if (!palmas->i2c_clients[i]) {
+ if (IS_ERR(palmas->i2c_clients[i])) {
dev_err(palmas->dev,
"can't attach client %d\n", i);
- ret = -ENOMEM;
+ ret = PTR_ERR(palmas->i2c_clients[i]);
goto err_i2c;
}
palmas->i2c_clients[i]->dev.of_node = of_node_get(node);
--
2.20.1
[PATCH] net: sfc: falcon: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(). So, we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Only build tested. Part of a tree-wide move to deprecate
i2c_new_dummy().
drivers/net/ethernet/sfc/falcon/falcon_boards.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/sfc/falcon/falcon_boards.c
b/drivers/net/ethernet/sfc/falcon/falcon_boards.c
index 839189dab98e..189ce9b9dfa7 100644
--- a/drivers/net/ethernet/sfc/falcon/falcon_boards.c
+++ b/drivers/net/ethernet/sfc/falcon/falcon_boards.c
@@ -454,13 +454,13 @@ static int sfe4001_init(struct ef4_nic *efx)
#if IS_ENABLED(CONFIG_SENSORS_LM90)
board->hwmon_client =
- i2c_new_device(&board->i2c_adap, &sfe4001_hwmon_info);
+ i2c_new_client_device(&board->i2c_adap, &sfe4001_hwmon_info);
#else
board->hwmon_client =
- i2c_new_dummy(&board->i2c_adap, sfe4001_hwmon_info.addr);
+ i2c_new_dummy_device(&board->i2c_adap, sfe4001_hwmon_info.addr);
#endif
- if (!board->hwmon_client)
- return -EIO;
+ if (IS_ERR(board->hwmon_client))
+ return PTR_ERR(board->hwmon_client);
/* Raise board/PHY high limit from 85 to 90 degrees Celsius */
rc = i2c_smbus_write_byte_data(board->hwmon_client,
@@ -468,9 +468,9 @@ static int sfe4001_init(struct ef4_nic *efx)
if (rc)
goto fail_hwmon;
- board->ioexp_client = i2c_new_dummy(&board->i2c_adap, PCA9539);
- if (!board->ioexp_client) {
- rc = -EIO;
+ board->ioexp_client = i2c_new_dummy_device(&board->i2c_adap, PCA9539);
+ if (IS_ERR(board->ioexp_client)) {
+ rc = PTR_ERR(board->ioexp_client);
goto fail_hwmon;
}
--
2.20.1
[PATCH 1/2] media: ir-kbd-i2c: prevent potential NULL pointer access
i2c_new_dummy() can fail returning a NULL pointer. The code does not
bail out in this case and the returned pointer is blindly used. Convert
to devm_i2c_new_dummy_device() which returns an ERR_PTR and also bail
out when failing the validity check.
Signed-off-by: Wolfram Sang
---
drivers/media/i2c/ir-kbd-i2c.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/media/i2c/ir-kbd-i2c.c b/drivers/media/i2c/ir-kbd-i2c.c
index 876d7587a1da..f46717052efc 100644
--- a/drivers/media/i2c/ir-kbd-i2c.c
+++ b/drivers/media/i2c/ir-kbd-i2c.c
@@ -885,9 +885,12 @@ static int ir_probe(struct i2c_client *client, const
struct i2c_device_id *id)
INIT_DELAYED_WORK(&ir->work, ir_work);
if (probe_tx) {
- ir->tx_c = i2c_new_dummy(client->adapter, 0x70);
- if (!ir->tx_c) {
+ ir->tx_c = devm_i2c_new_dummy_device(&client->dev,
+client->adapter, 0x70);
+ if (IS_ERR(ir->tx_c)) {
dev_err(&client->dev, "failed to setup tx i2c address");
+ err = PTR_ERR(ir->tx_c);
+ goto err_out_free;
} else if (!zilog_init(ir)) {
ir->carrier = 38000;
ir->duty_cycle = 40;
@@ -904,9 +907,6 @@ static int ir_probe(struct i2c_client *client, const struct
i2c_device_id *id)
return 0;
err_out_free:
- if (ir->tx_c)
- i2c_unregister_device(ir->tx_c);
-
/* Only frees rc if it were allocated internally */
rc_free_device(rc);
return err;
@@ -919,9 +919,6 @@ static int ir_remove(struct i2c_client *client)
/* kill outstanding polls */
cancel_delayed_work_sync(&ir->work);
- if (ir->tx_c)
- i2c_unregister_device(ir->tx_c);
-
/* unregister device */
rc_unregister_device(ir->rc);
--
2.20.1
[PATCH 11/14] mfd: max8997: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/mfd/max8997.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
index 8c06c09e36d1..68d8f2b95287 100644
--- a/drivers/mfd/max8997.c
+++ b/drivers/mfd/max8997.c
@@ -185,25 +185,25 @@ static int max8997_i2c_probe(struct i2c_client *i2c,
mutex_init(&max8997->iolock);
- max8997->rtc = i2c_new_dummy(i2c->adapter, I2C_ADDR_RTC);
- if (!max8997->rtc) {
+ max8997->rtc = i2c_new_dummy_device(i2c->adapter, I2C_ADDR_RTC);
+ if (IS_ERR(max8997->rtc)) {
dev_err(max8997->dev, "Failed to allocate I2C device for
RTC\n");
- return -ENODEV;
+ return PTR_ERR(max8997->rtc);
}
i2c_set_clientdata(max8997->rtc, max8997);
- max8997->haptic = i2c_new_dummy(i2c->adapter, I2C_ADDR_HAPTIC);
- if (!max8997->haptic) {
+ max8997->haptic = i2c_new_dummy_device(i2c->adapter, I2C_ADDR_HAPTIC);
+ if (IS_ERR(max8997->haptic)) {
dev_err(max8997->dev, "Failed to allocate I2C device for
Haptic\n");
- ret = -ENODEV;
+ ret = PTR_ERR(max8997->haptic);
goto err_i2c_haptic;
}
i2c_set_clientdata(max8997->haptic, max8997);
- max8997->muic = i2c_new_dummy(i2c->adapter, I2C_ADDR_MUIC);
- if (!max8997->muic) {
+ max8997->muic = i2c_new_dummy_device(i2c->adapter, I2C_ADDR_MUIC);
+ if (IS_ERR(max8997->muic)) {
dev_err(max8997->dev, "Failed to allocate I2C device for
MUIC\n");
- ret = -ENODEV;
+ ret = PTR_ERR(max8997->muic);
goto err_i2c_muic;
}
i2c_set_clientdata(max8997->muic, max8997);
--
2.20.1
[PATCH 06/14] mfd: max14577: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/mfd/max14577.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mfd/max14577.c b/drivers/mfd/max14577.c
index ebb13d5de530..fd8864cafd25 100644
--- a/drivers/mfd/max14577.c
+++ b/drivers/mfd/max14577.c
@@ -297,11 +297,11 @@ static int max77836_init(struct max14577 *max14577)
int ret;
u8 intsrc_mask;
- max14577->i2c_pmic = i2c_new_dummy(max14577->i2c->adapter,
+ max14577->i2c_pmic = i2c_new_dummy_device(max14577->i2c->adapter,
I2C_ADDR_PMIC);
- if (!max14577->i2c_pmic) {
+ if (IS_ERR(max14577->i2c_pmic)) {
dev_err(max14577->dev, "Failed to register PMIC I2C device\n");
- return -ENODEV;
+ return PTR_ERR(max14577->i2c_pmic);
}
i2c_set_clientdata(max14577->i2c_pmic, max14577);
--
2.20.1
[PATCH 05/14] mfd: da9150-core: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/mfd/da9150-core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mfd/da9150-core.c b/drivers/mfd/da9150-core.c
index 13033068721a..7f0aa1e8db96 100644
--- a/drivers/mfd/da9150-core.c
+++ b/drivers/mfd/da9150-core.c
@@ -420,10 +420,10 @@ static int da9150_probe(struct i2c_client *client,
qif_addr = da9150_reg_read(da9150, DA9150_CORE2WIRE_CTRL_A);
qif_addr = (qif_addr & DA9150_CORE_BASE_ADDR_MASK) >> 1;
qif_addr |= DA9150_QIF_I2C_ADDR_LSB;
- da9150->core_qif = i2c_new_dummy(client->adapter, qif_addr);
- if (!da9150->core_qif) {
+ da9150->core_qif = i2c_new_dummy_device(client->adapter, qif_addr);
+ if (IS_ERR(da9150->core_qif)) {
dev_err(da9150->dev, "Failed to attach QIF client\n");
- return -ENODEV;
+ return PTR_ERR(da9150->core_qif);
}
i2c_set_clientdata(da9150->core_qif, da9150);
--
2.20.1
[PATCH 04/14] mfd: bcm590xx: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/mfd/bcm590xx.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mfd/bcm590xx.c b/drivers/mfd/bcm590xx.c
index 1aeb5e498d91..bfac5dc091ca 100644
--- a/drivers/mfd/bcm590xx.c
+++ b/drivers/mfd/bcm590xx.c
@@ -61,11 +61,11 @@ static int bcm590xx_i2c_probe(struct i2c_client *i2c_pri,
}
/* Secondary I2C slave address is the base address with A(2) asserted */
- bcm590xx->i2c_sec = i2c_new_dummy(i2c_pri->adapter,
+ bcm590xx->i2c_sec = i2c_new_dummy_device(i2c_pri->adapter,
i2c_pri->addr | BIT(2));
- if (!bcm590xx->i2c_sec) {
+ if (IS_ERR(bcm590xx->i2c_sec)) {
dev_err(&i2c_pri->dev, "failed to add secondary I2C device\n");
- return -ENODEV;
+ return PTR_ERR(bcm590xx->i2c_sec);
}
i2c_set_clientdata(bcm590xx->i2c_sec, bcm590xx);
--
2.20.1
[PATCH 1/2] misc: eeprom: ee1004: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/misc/eeprom/ee1004.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
index 6f00c33cfe22..b081c67416d7 100644
--- a/drivers/misc/eeprom/ee1004.c
+++ b/drivers/misc/eeprom/ee1004.c
@@ -195,13 +195,13 @@ static int ee1004_probe(struct i2c_client *client,
mutex_lock(&ee1004_bus_lock);
if (++ee1004_dev_count == 1) {
for (cnr = 0; cnr < 2; cnr++) {
- ee1004_set_page[cnr] = i2c_new_dummy(client->adapter,
+ ee1004_set_page[cnr] =
i2c_new_dummy_device(client->adapter,
EE1004_ADDR_SET_PAGE + cnr);
- if (!ee1004_set_page[cnr]) {
+ if (IS_ERR(ee1004_set_page[cnr])) {
dev_err(&client->dev,
"address 0x%02x unavailable\n",
EE1004_ADDR_SET_PAGE + cnr);
- err = -EADDRINUSE;
+ err = PTR_ERR(ee1004_set_page[cnr]);
goto err_clients;
}
}
--
2.20.1
[PATCH 0/4] rtc: convert subsystem to i2c_new_dummy_device()
This series is part of a tree-wide movement to replace the I2C API call 'i2c_new_dummy' which returns NULL with its new counterpart returning an ERRPTR. The series was generated with coccinelle (audited afterwards, of course) and build tested by me and by buildbot. No tests on HW have been performed. The branch is based on v5.3-rc1. A branch (with some more stuff included) can be found here: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/i2c/new_dummy Some drivers still need to be manually converted. Patches for those will be sent out individually. Wolfram Sang (4): rtc: isl12026: convert to i2c_new_dummy_device rtc: max77686: convert to i2c_new_dummy_device rtc: s35390a: convert to i2c_new_dummy_device rtc: s5m: convert to i2c_new_dummy_device drivers/rtc/rtc-isl12026.c | 6 +++--- drivers/rtc/rtc-max77686.c | 6 +++--- drivers/rtc/rtc-s35390a.c | 6 +++--- drivers/rtc/rtc-s5m.c | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) -- 2.20.1
[PATCH 4/4] rtc: s5m: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/rtc/rtc-s5m.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c
index c7f1bf823ea0..eb9dde4095a9 100644
--- a/drivers/rtc/rtc-s5m.c
+++ b/drivers/rtc/rtc-s5m.c
@@ -760,10 +760,10 @@ static int s5m_rtc_probe(struct platform_device *pdev)
return -ENODEV;
}
- info->i2c = i2c_new_dummy(s5m87xx->i2c->adapter, RTC_I2C_ADDR);
- if (!info->i2c) {
+ info->i2c = i2c_new_dummy_device(s5m87xx->i2c->adapter, RTC_I2C_ADDR);
+ if (IS_ERR(info->i2c)) {
dev_err(&pdev->dev, "Failed to allocate I2C for RTC\n");
- return -ENODEV;
+ return PTR_ERR(info->i2c);
}
info->regmap = devm_regmap_init_i2c(info->i2c, regmap_cfg);
--
2.20.1
[PATCH 0/2] misc: convert subsystem to i2c_new_dummy_device()
This series is part of a tree-wide movement to replace the I2C API call 'i2c_new_dummy' which returns NULL with its new counterpart returning an ERRPTR. The series was generated with coccinelle (audited afterwards, of course) and build tested by me and by buildbot. No tests on HW have been performed. The branch is based on v5.3-rc1. A branch (with some more stuff included) can be found here: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/i2c/new_dummy Some drivers still need to be manually converted. Patches for those will be sent out individually. Wolfram Sang (2): misc: eeprom: ee1004: convert to i2c_new_dummy_device misc: eeprom: max6875: convert to i2c_new_dummy_device drivers/misc/eeprom/ee1004.c | 6 +++--- drivers/misc/eeprom/max6875.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) -- 2.20.1
[PATCH 03/14] mfd: ab3100-core: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/mfd/ab3100-core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mfd/ab3100-core.c b/drivers/mfd/ab3100-core.c
index e350ab64238e..7e007df857b8 100644
--- a/drivers/mfd/ab3100-core.c
+++ b/drivers/mfd/ab3100-core.c
@@ -896,10 +896,10 @@ static int ab3100_probe(struct i2c_client *client,
&ab3100->chip_name[0]);
/* Attach a second dummy i2c_client to the test register address */
- ab3100->testreg_client = i2c_new_dummy(client->adapter,
+ ab3100->testreg_client = i2c_new_dummy_device(client->adapter,
client->addr + 1);
- if (!ab3100->testreg_client) {
- err = -ENOMEM;
+ if (IS_ERR(ab3100->testreg_client)) {
+ err = PTR_ERR(ab3100->testreg_client);
goto exit_no_testreg_client;
}
--
2.20.1
[PATCH 2/4] rtc: max77686: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/rtc/rtc-max77686.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 4aff349ae301..d04fd1024697 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -693,11 +693,11 @@ static int max77686_init_rtc_regmap(struct
max77686_rtc_info *info)
goto add_rtc_irq;
}
- info->rtc = i2c_new_dummy(parent_i2c->adapter,
+ info->rtc = i2c_new_dummy_device(parent_i2c->adapter,
info->drv_data->rtc_i2c_addr);
- if (!info->rtc) {
+ if (IS_ERR(info->rtc)) {
dev_err(info->dev, "Failed to allocate I2C device for RTC\n");
- return -ENODEV;
+ return PTR_ERR(info->rtc);
}
info->rtc_regmap = devm_regmap_init_i2c(info->rtc,
--
2.20.1
[PATCH 0/1] gpu: convert subsystem to i2c_new_dummy_device()
This series is part of a tree-wide movement to replace the I2C API call 'i2c_new_dummy' which returns NULL with its new counterpart returning an ERRPTR. The series was generated with coccinelle (audited afterwards, of course) and build tested by me and by buildbot. No tests on HW have been performed. The branch is based on v5.3-rc1. A branch (with some more stuff included) can be found here: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/i2c/new_dummy Some drivers still need to be manually converted. Patches for those will be sent out individually. Wolfram Sang (1): gpu: drm: bridge: analogix-anx78xx: convert to i2c_new_dummy_device drivers/gpu/drm/bridge/analogix-anx78xx.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) -- 2.20.1
[PATCH 2/2] misc: eeprom: max6875: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/misc/eeprom/max6875.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/eeprom/max6875.c b/drivers/misc/eeprom/max6875.c
index 4d0cb90f4aeb..9da81f6d4a1c 100644
--- a/drivers/misc/eeprom/max6875.c
+++ b/drivers/misc/eeprom/max6875.c
@@ -150,9 +150,9 @@ static int max6875_probe(struct i2c_client *client,
return -ENOMEM;
/* A fake client is created on the odd address */
- data->fake_client = i2c_new_dummy(client->adapter, client->addr + 1);
- if (!data->fake_client) {
- err = -ENOMEM;
+ data->fake_client = i2c_new_dummy_device(client->adapter, client->addr
+ 1);
+ if (IS_ERR(data->fake_client)) {
+ err = PTR_ERR(data->fake_client);
goto exit_kfree;
}
--
2.20.1
[PATCH 1/4] rtc: isl12026: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an ERRPTR which we use in error handling. Signed-off-by: Wolfram Sang --- Generated with coccinelle. Build tested by me and buildbot. Not tested on HW. drivers/rtc/rtc-isl12026.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/rtc/rtc-isl12026.c b/drivers/rtc/rtc-isl12026.c index 97f594f9667c..5b6b17fb6d62 100644 --- a/drivers/rtc/rtc-isl12026.c +++ b/drivers/rtc/rtc-isl12026.c @@ -454,9 +454,9 @@ static int isl12026_probe_new(struct i2c_client *client) isl12026_force_power_modes(client); - priv->nvm_client = i2c_new_dummy(client->adapter, ISL12026_EEPROM_ADDR); - if (!priv->nvm_client) - return -ENOMEM; + priv->nvm_client = i2c_new_dummy_device(client->adapter, ISL12026_EEPROM_ADDR); + if (IS_ERR(priv->nvm_client)) + return PTR_ERR(priv->nvm_client); priv->rtc = devm_rtc_allocate_device(&client->dev); ret = PTR_ERR_OR_ZERO(priv->rtc); -- 2.20.1
[PATCH 3/4] rtc: s35390a: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/rtc/rtc-s35390a.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index 84806ff763cf..5826209a3f30 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -450,12 +450,12 @@ static int s35390a_probe(struct i2c_client *client,
/* This chip uses multiple addresses, use dummy devices for them */
for (i = 1; i < 8; ++i) {
- s35390a->client[i] = i2c_new_dummy(client->adapter,
+ s35390a->client[i] = i2c_new_dummy_device(client->adapter,
client->addr + i);
- if (!s35390a->client[i]) {
+ if (IS_ERR(s35390a->client[i])) {
dev_err(dev, "Address %02x unavailable\n",
client->addr + i);
- err = -EBUSY;
+ err = PTR_ERR(s35390a->client[i]);
goto exit_dummy;
}
}
--
2.20.1
[PATCH 8/8] media: usb: go7007: s2250-board: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/media/usb/go7007/s2250-board.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/usb/go7007/s2250-board.c
b/drivers/media/usb/go7007/s2250-board.c
index 179d4d642dae..49e75a1a1f3f 100644
--- a/drivers/media/usb/go7007/s2250-board.c
+++ b/drivers/media/usb/go7007/s2250-board.c
@@ -505,9 +505,9 @@ static int s2250_probe(struct i2c_client *client,
struct go7007 *go = i2c_get_adapdata(adapter);
struct go7007_usb *usb = go->hpi_context;
- audio = i2c_new_dummy(adapter, TLV320_ADDRESS >> 1);
- if (audio == NULL)
- return -ENOMEM;
+ audio = i2c_new_dummy_device(adapter, TLV320_ADDRESS >> 1);
+ if (IS_ERR(audio))
+ return PTR_ERR(audio);
state = kzalloc(sizeof(struct s2250), GFP_KERNEL);
if (state == NULL) {
--
2.20.1
[PATCH 02/14] mfd: 88pm860x-core: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/mfd/88pm860x-core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mfd/88pm860x-core.c b/drivers/mfd/88pm860x-core.c
index 9e0bd135730f..c9bae71f643a 100644
--- a/drivers/mfd/88pm860x-core.c
+++ b/drivers/mfd/88pm860x-core.c
@@ -1178,12 +1178,12 @@ static int pm860x_probe(struct i2c_client *client)
*/
if (pdata->companion_addr && (pdata->companion_addr != client->addr)) {
chip->companion_addr = pdata->companion_addr;
- chip->companion = i2c_new_dummy(chip->client->adapter,
+ chip->companion = i2c_new_dummy_device(chip->client->adapter,
chip->companion_addr);
- if (!chip->companion) {
+ if (IS_ERR(chip->companion)) {
dev_err(&client->dev,
"Failed to allocate I2C companion device\n");
- return -ENODEV;
+ return PTR_ERR(chip->companion);
}
chip->regmap_companion = regmap_init_i2c(chip->companion,
&pm860x_regmap_config);
--
2.20.1
[PATCH 01/14] mfd: 88pm800: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/mfd/88pm800.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
index f2d9fb4c4e8e..4e8d0d6b9b5c 100644
--- a/drivers/mfd/88pm800.c
+++ b/drivers/mfd/88pm800.c
@@ -425,10 +425,10 @@ static int pm800_pages_init(struct pm80x_chip *chip)
return -ENODEV;
/* PM800 block power page */
- subchip->power_page = i2c_new_dummy(client->adapter,
+ subchip->power_page = i2c_new_dummy_device(client->adapter,
subchip->power_page_addr);
- if (subchip->power_page == NULL) {
- ret = -ENODEV;
+ if (IS_ERR(subchip->power_page)) {
+ ret = PTR_ERR(subchip->power_page);
goto out;
}
@@ -444,10 +444,10 @@ static int pm800_pages_init(struct pm80x_chip *chip)
i2c_set_clientdata(subchip->power_page, chip);
/* PM800 block GPADC */
- subchip->gpadc_page = i2c_new_dummy(client->adapter,
+ subchip->gpadc_page = i2c_new_dummy_device(client->adapter,
subchip->gpadc_page_addr);
- if (subchip->gpadc_page == NULL) {
- ret = -ENODEV;
+ if (IS_ERR(subchip->gpadc_page)) {
+ ret = PTR_ERR(subchip->gpadc_page);
goto out;
}
--
2.20.1
[PATCH 4/8] media: dvb-frontends: mn88473: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/media/dvb-frontends/mn88473.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/media/dvb-frontends/mn88473.c
b/drivers/media/dvb-frontends/mn88473.c
index 08118b38533b..71cedce6c7d0 100644
--- a/drivers/media/dvb-frontends/mn88473.c
+++ b/drivers/media/dvb-frontends/mn88473.c
@@ -657,9 +657,9 @@ static int mn88473_probe(struct i2c_client *client,
* Also, register bank 2 do not support sequential I/O. Only single
* register write or read is allowed to that bank.
*/
- dev->client[1] = i2c_new_dummy(client->adapter, 0x1a);
- if (dev->client[1] == NULL) {
- ret = -ENODEV;
+ dev->client[1] = i2c_new_dummy_device(client->adapter, 0x1a);
+ if (IS_ERR(dev->client[1])) {
+ ret = PTR_ERR(dev->client[1]);
dev_err(&client->dev, "I2C registration failed\n");
if (ret)
goto err_regmap_0_regmap_exit;
@@ -671,9 +671,9 @@ static int mn88473_probe(struct i2c_client *client,
}
i2c_set_clientdata(dev->client[1], dev);
- dev->client[2] = i2c_new_dummy(client->adapter, 0x1c);
- if (dev->client[2] == NULL) {
- ret = -ENODEV;
+ dev->client[2] = i2c_new_dummy_device(client->adapter, 0x1c);
+ if (IS_ERR(dev->client[2])) {
+ ret = PTR_ERR(dev->client[2]);
dev_err(&client->dev, "2nd I2C registration failed\n");
if (ret)
goto err_regmap_1_regmap_exit;
--
2.20.1
[PATCH 2/3] hwmon: smm665: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/hwmon/smm665.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hwmon/smm665.c b/drivers/hwmon/smm665.c
index d8c91c2cb8cf..6eff14fe395d 100644
--- a/drivers/hwmon/smm665.c
+++ b/drivers/hwmon/smm665.c
@@ -586,10 +586,10 @@ static int smm665_probe(struct i2c_client *client,
data->client = client;
data->type = id->driver_data;
- data->cmdreg = i2c_new_dummy(adapter, (client->addr & ~SMM665_REGMASK)
+ data->cmdreg = i2c_new_dummy_device(adapter, (client->addr &
~SMM665_REGMASK)
| SMM665_CMDREG_BASE);
- if (!data->cmdreg)
- return -ENOMEM;
+ if (IS_ERR(data->cmdreg))
+ return PTR_ERR(data->cmdreg);
switch (data->type) {
case smm465:
--
2.20.1
[PATCH 6/8] media: i2c: adv7180: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/media/i2c/adv7180.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 6f3dc8862622..e780969cc2f2 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -1329,17 +1329,17 @@ static int adv7180_probe(struct i2c_client *client,
}
if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
- state->csi_client = i2c_new_dummy(client->adapter,
+ state->csi_client = i2c_new_dummy_device(client->adapter,
ADV7180_DEFAULT_CSI_I2C_ADDR);
- if (!state->csi_client)
- return -ENOMEM;
+ if (IS_ERR(state->csi_client))
+ return PTR_ERR(state->csi_client);
}
if (state->chip_info->flags & ADV7180_FLAG_I2P) {
- state->vpp_client = i2c_new_dummy(client->adapter,
+ state->vpp_client = i2c_new_dummy_device(client->adapter,
ADV7180_DEFAULT_VPP_I2C_ADDR);
- if (!state->vpp_client) {
- ret = -ENOMEM;
+ if (IS_ERR(state->vpp_client)) {
+ ret = PTR_ERR(state->vpp_client);
goto err_unregister_csi_client;
}
}
--
2.20.1
[PATCH 1/3] hwmon: asb100: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/hwmon/asb100.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/hwmon/asb100.c b/drivers/hwmon/asb100.c
index c9fa84b25678..4c609e23a4ef 100644
--- a/drivers/hwmon/asb100.c
+++ b/drivers/hwmon/asb100.c
@@ -706,21 +706,21 @@ static int asb100_detect_subclients(struct i2c_client
*client)
goto ERROR_SC_2;
}
- data->lm75[0] = i2c_new_dummy(adapter, sc_addr[0]);
- if (!data->lm75[0]) {
+ data->lm75[0] = i2c_new_dummy_device(adapter, sc_addr[0]);
+ if (IS_ERR(data->lm75[0])) {
dev_err(&client->dev,
"subclient %d registration at address 0x%x failed.\n",
1, sc_addr[0]);
- err = -ENOMEM;
+ err = PTR_ERR(data->lm75[0]);
goto ERROR_SC_2;
}
- data->lm75[1] = i2c_new_dummy(adapter, sc_addr[1]);
- if (!data->lm75[1]) {
+ data->lm75[1] = i2c_new_dummy_device(adapter, sc_addr[1]);
+ if (IS_ERR(data->lm75[1])) {
dev_err(&client->dev,
"subclient %d registration at address 0x%x failed.\n",
2, sc_addr[1]);
- err = -ENOMEM;
+ err = PTR_ERR(data->lm75[1]);
goto ERROR_SC_3;
}
--
2.20.1
[PATCH 2/8] media: dvb-frontends: mn88443x: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/media/dvb-frontends/mn88443x.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/dvb-frontends/mn88443x.c
b/drivers/media/dvb-frontends/mn88443x.c
index 9ec1aeef03d5..e4528784f847 100644
--- a/drivers/media/dvb-frontends/mn88443x.c
+++ b/drivers/media/dvb-frontends/mn88443x.c
@@ -722,9 +722,9 @@ static int mn88443x_probe(struct i2c_client *client,
* Chip has two I2C addresses for each satellite/terrestrial system.
* ISDB-T uses address ISDB-S + 4, so we register a dummy client.
*/
- chip->client_t = i2c_new_dummy(client->adapter, client->addr + 4);
- if (!chip->client_t)
- return -ENODEV;
+ chip->client_t = i2c_new_dummy_device(client->adapter, client->addr +
4);
+ if (IS_ERR(chip->client_t))
+ return PTR_ERR(chip->client_t);
chip->regmap_t = devm_regmap_init_i2c(chip->client_t, ®map_config);
if (IS_ERR(chip->regmap_t)) {
--
2.20.1
[PATCH 0/8] media: convert subsystem to i2c_new_dummy_device()
This series is part of a tree-wide movement to replace the I2C API call 'i2c_new_dummy' which returns NULL with its new counterpart returning an ERRPTR. The series was generated with coccinelle (audited afterwards, of course) and build tested by me and by buildbot. No tests on HW have been performed. The branch is based on v5.3-rc1. A branch (with some more stuff included) can be found here: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/i2c/new_dummy Some drivers still need to be manually converted. Patches for those will be sent out individually. Wolfram Sang (8): media: dvb-frontends: cxd2820r_core: convert to i2c_new_dummy_device media: dvb-frontends: mn88443x: convert to i2c_new_dummy_device media: dvb-frontends: mn88472: convert to i2c_new_dummy_device media: dvb-frontends: mn88473: convert to i2c_new_dummy_device media: i2c: ad9389b: convert to i2c_new_dummy_device media: i2c: adv7180: convert to i2c_new_dummy_device media: i2c: adv7511-v4l2: convert to i2c_new_dummy_device media: usb: go7007: s2250-board: convert to i2c_new_dummy_device drivers/media/dvb-frontends/cxd2820r_core.c | 6 +++--- drivers/media/dvb-frontends/mn88443x.c | 6 +++--- drivers/media/dvb-frontends/mn88472.c | 12 ++-- drivers/media/dvb-frontends/mn88473.c | 12 ++-- drivers/media/i2c/ad9389b.c | 6 +++--- drivers/media/i2c/adv7180.c | 12 ++-- drivers/media/i2c/adv7511-v4l2.c| 18 +- drivers/media/usb/go7007/s2250-board.c | 6 +++--- 8 files changed, 39 insertions(+), 39 deletions(-) -- 2.20.1
[PATCH 1/1] extcon: extcon-max77843: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/extcon/extcon-max77843.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/extcon/extcon-max77843.c b/drivers/extcon/extcon-max77843.c
index a343a6ef3506..e6b50ca83008 100644
--- a/drivers/extcon/extcon-max77843.c
+++ b/drivers/extcon/extcon-max77843.c
@@ -774,12 +774,12 @@ static int max77843_init_muic_regmap(struct max77693_dev
*max77843)
{
int ret;
- max77843->i2c_muic = i2c_new_dummy(max77843->i2c->adapter,
+ max77843->i2c_muic = i2c_new_dummy_device(max77843->i2c->adapter,
I2C_ADDR_MUIC);
- if (!max77843->i2c_muic) {
+ if (IS_ERR(max77843->i2c_muic)) {
dev_err(&max77843->i2c->dev,
"Cannot allocate I2C device for MUIC\n");
- return -ENOMEM;
+ return PTR_ERR(max77843->i2c_muic);
}
i2c_set_clientdata(max77843->i2c_muic, max77843);
--
2.20.1
[PATCH 0/1] extcon: convert subsystem to i2c_new_dummy_device()
This series is part of a tree-wide movement to replace the I2C API call 'i2c_new_dummy' which returns NULL with its new counterpart returning an ERRPTR. The series was generated with coccinelle (audited afterwards, of course) and build tested by me and by buildbot. No tests on HW have been performed. The branch is based on v5.3-rc1. A branch (with some more stuff included) can be found here: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/i2c/new_dummy Some drivers still need to be manually converted. Patches for those will be sent out individually. Wolfram Sang (1): extcon: extcon-max77843: convert to i2c_new_dummy_device drivers/extcon/extcon-max77843.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) -- 2.20.1
[PATCH 5/8] media: i2c: ad9389b: convert to i2c_new_dummy_device
Move from i2c_new_dummy() to i2c_new_dummy_device(), so we now get an
ERRPTR which we use in error handling.
Signed-off-by: Wolfram Sang
---
Generated with coccinelle. Build tested by me and buildbot. Not tested on HW.
drivers/media/i2c/ad9389b.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/i2c/ad9389b.c b/drivers/media/i2c/ad9389b.c
index aa8b04cfed0f..1b92048a4a67 100644
--- a/drivers/media/i2c/ad9389b.c
+++ b/drivers/media/i2c/ad9389b.c
@@ -1148,10 +1148,10 @@ static int ad9389b_probe(struct i2c_client *client,
const struct i2c_device_id *
v4l2_dbg(1, debug, sd, "reg 0x41 0x%x, chip version (reg 0x00) 0x%x\n",
ad9389b_rd(sd, 0x41), state->chip_revision);
- state->edid_i2c_client = i2c_new_dummy(client->adapter, (0x7e>>1));
- if (state->edid_i2c_client == NULL) {
+ state->edid_i2c_client = i2c_new_dummy_device(client->adapter, (0x7e >>
1));
+ if (IS_ERR(state->edid_i2c_client)) {
v4l2_err(sd, "failed to register edid i2c client\n");
- err = -ENOMEM;
+ err = PTR_ERR(state->edid_i2c_client);
goto err_entity;
}
--
2.20.1
[PATCH RFC v3 0/14] sched,fair: flatten CPU controller runqueues
The current implementation of the CPU controller uses hierarchical runqueues, where on wakeup a task is enqueued on its group's runqueue, the group is enqueued on the runqueue of the group above it, etc. This increases a fairly large amount of overhead for workloads that do a lot of wakeups a second, especially given that the default systemd hierarchy is 2 or 3 levels deep. This patch series is an attempt at reducing that overhead, by placing all the tasks on the same runqueue, and scaling the task priority by the priority of the group, which is calculated periodically. My main TODO items for the next period of time are likely going to be testing, testing, and testing. I hope to find and flush out any corner case I can find, and make sure performance does not regress with any workloads, and hopefully improves some. Other TODO items: - More code cleanups. - Remove some more now unused code. - Reimplement CONFIG_CFS_BANDWIDTH. Plan for the CONFIG_CFS_BANDWIDTH reimplementation: - When a cgroup gets throttled, mark the cgroup and its children as throttled. - When pick_next_entity finds a task that is on a throttled cgroup, stash it on the cgroup runqueue (which is not used for runnable tasks any more). Leave the vruntime unchanged, and adjust that runqueue's vruntime to be that of the left-most task. - When a cgroup gets unthrottled, and has tasks on it, place it on a vruntime ordered heap separate from the main runqueue. - Have pick_next_task_fair grab one task off that heap every time it is called, and the min vruntime of that heap is lower than the vruntime of the CPU's cfs_rq (or the CPU has no other runnable tasks). - Place that selected task on the CPU's cfs_rq, renormalizing its vruntime with the GENTLE_FAIR_SLEEPERS logic. That should help interleave the already runnable tasks with the recently unthrottled group, and prevent thundering herd issues. - If the group gets throttled again before all of its task had a chance to run, vruntime sorting ensures all the tasks in the throttled cgroup get a chance to run over time. Changes from v2: - fixed the web server performance regression, in a way vaguely similar to what Josef Bacik suggested (blame me for the implementation) - removed some code duplication so the diffstat is redder than before - propagate sum_exec_runtime up the tree, in preparation for CFS_BANDWIDTH - small cleanups left and right Changes from v1: - use task_se_h_weight instead of task_se_h_load in calc_delta_fair and sched_slice, this seems to improve performance a little, but I still have some remaining regression to chase with our web server workload - implement a number of the changes suggested by Dietmar Eggemann (still holding out for a better name for group_cfs_rq_of_parent) This series applies on top of 5.2 include/linux/sched.h |7 kernel/sched/core.c |3 kernel/sched/debug.c | 17 - kernel/sched/fair.c | 780 -- kernel/sched/pelt.c | 69 ++-- kernel/sched/pelt.h |3 kernel/sched/sched.h | 11 7 files changed, 372 insertions(+), 518 deletions(-)
Re: [RFC PATCH] string.h: Add stracpy/stracpy_pad (was: Re: [PATCH] checkpatch: Added warnings in favor of strscpy().)
On Thu, Jul 04, 2019 at 05:15:57PM -0700, Joe Perches wrote:
> On Thu, 2019-07-04 at 13:46 -0700, Joe Perches wrote:
> > On Thu, 2019-07-04 at 11:24 +0530, Nitin Gote wrote:
> > > Added warnings in checkpatch.pl script to :
> > >
> > > 1. Deprecate strcpy() in favor of strscpy().
> > > 2. Deprecate strlcpy() in favor of strscpy().
> > > 3. Deprecate strncpy() in favor of strscpy() or strscpy_pad().
> > >
> > > Updated strncpy() section in Documentation/process/deprecated.rst
> > > to cover strscpy_pad() case.
>
> []
>
> I sent a patch series for some strscpy/strlcpy misuses.
>
> How about adding a macro helper to avoid the misuses like:
> ---
> include/linux/string.h | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 4deb11f7976b..ef01bd6f19df 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -35,6 +35,22 @@ ssize_t strscpy(char *, const char *, size_t);
> /* Wraps calls to strscpy()/memset(), no arch specific code required */
> ssize_t strscpy_pad(char *dest, const char *src, size_t count);
>
> +#define stracpy(to, from)\
> +({ \
> + size_t size = ARRAY_SIZE(to); \
> + BUILD_BUG_ON(!__same_type(typeof(*to), char)); \
> + \
> + strscpy(to, from, size);\
> +})
> +
> +#define stracpy_pad(to, from)\
> +({ \
> + size_t size = ARRAY_SIZE(to); \
> + BUILD_BUG_ON(!__same_type(typeof(*to), char)); \
> + \
> + strscpy_pad(to, from, size);\
> +})
> +
> #ifndef __HAVE_ARCH_STRCAT
> extern char * strcat(char *, const char *);
> #endif
This seems like a reasonable addition, yes. I think Coccinelle might
actually be able to find all the existing strscpy(dst, src, sizeof(dst))
cases to jump-start this conversion.
Devil's advocate: this adds yet more string handling functions... will
this cause even more confusion?
--
Kees Cook
Re: [PATCH 23/79] libperf: Make libperf.a part of the build
Em Mon, Jul 22, 2019 at 05:54:17PM +0200, Jiri Olsa escreveu: > On Mon, Jul 22, 2019 at 09:39:24AM -0300, Arnaldo Carvalho de Melo wrote: > > Em Sun, Jul 21, 2019 at 01:24:10PM +0200, Jiri Olsa escreveu: > > > Adding empty libperf.a under toos/perf/lib and > > > linking it with perf. > > > > So, I noticed you created a subdirectory under tools/perf/, while I > > first thought you would have it in tools/lib/perf/, why not move it > > there? > > yea I started like this, and then there was only one reason > I could think of ;-) I mentioned it in the cover letter: > - move under tools/lib after the interface is more stable-ish > > if you insist to move it there now, would it be ok > to make it under separate patch afterwards? it would > safe me zillions of small changes ;-) Humm, moving it after will be just a git rename, its ok. - Arnaldo > thanks, > jirka > > > > > - Arnaldo > > > > > It can also be built separately with: > > > > > > $ cd tools/perf/lib && make > > > CC core.o > > > LD libperf-in.o > > > AR libperf.a > > > LINK libperf.so > > > > > > Link: http://lkml.kernel.org/n/tip-lzrlfu3hutepbeqyntjks...@git.kernel.org > > > Signed-off-by: Jiri Olsa > > > --- > > > tools/perf/Makefile.config | 1 + > > > tools/perf/Makefile.perf | 30 > > > tools/perf/lib/Build | 1 + > > > tools/perf/lib/Makefile| 74 ++ > > > tools/perf/lib/core.c | 0 > > > 5 files changed, 92 insertions(+), 14 deletions(-) > > > create mode 100644 tools/perf/lib/Build > > > create mode 100644 tools/perf/lib/Makefile > > > create mode 100644 tools/perf/lib/core.c > > > > > > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config > > > index 89ac5a1f1550..e4988f49ea79 100644 > > > --- a/tools/perf/Makefile.config > > > +++ b/tools/perf/Makefile.config > > > @@ -277,6 +277,7 @@ ifeq ($(DEBUG),0) > > >endif > > > endif > > > > > > +INC_FLAGS += -I$(src-perf)/lib/include > > > INC_FLAGS += -I$(src-perf)/util/include > > > INC_FLAGS += -I$(src-perf)/arch/$(SRCARCH)/include > > > INC_FLAGS += -I$(srctree)/tools/include/uapi > > > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf > > > index 0fffd2bb6cd9..6e7e7d44ffac 100644 > > > --- a/tools/perf/Makefile.perf > > > +++ b/tools/perf/Makefile.perf > > > @@ -224,6 +224,7 @@ LIB_DIR = $(srctree)/tools/lib/api/ > > > TRACE_EVENT_DIR = $(srctree)/tools/lib/traceevent/ > > > BPF_DIR = $(srctree)/tools/lib/bpf/ > > > SUBCMD_DIR = $(srctree)/tools/lib/subcmd/ > > > +LIBPERF_DIR = $(srctree)/tools/perf/lib/ > > > > > > # Set FEATURE_TESTS to 'all' so all possible feature checkers are > > > executed. > > > # Without this setting the output feature dump file misses some > > > features, for > > > @@ -272,6 +273,7 @@ ifneq ($(OUTPUT),) > > >TE_PATH=$(OUTPUT) > > >BPF_PATH=$(OUTPUT) > > >SUBCMD_PATH=$(OUTPUT) > > > + LIBPERF_PATH=$(OUTPUT) > > > ifneq ($(subdir),) > > >API_PATH=$(OUTPUT)/../lib/api/ > > > else > > > @@ -282,6 +284,7 @@ else > > >API_PATH=$(LIB_DIR) > > >BPF_PATH=$(BPF_DIR) > > >SUBCMD_PATH=$(SUBCMD_DIR) > > > + LIBPERF_PATH=$(LIBPERF_DIR) > > > endif > > > > > > LIBTRACEEVENT = $(TE_PATH)libtraceevent.a > > > @@ -303,6 +306,8 @@ LIBBPF = $(BPF_PATH)libbpf.a > > > > > > LIBSUBCMD = $(SUBCMD_PATH)libsubcmd.a > > > > > > +LIBPERF = $(LIBPERF_PATH)libperf.a > > > + > > > # python extension build directories > > > PYTHON_EXTBUILD := $(OUTPUT)python_ext_build/ > > > PYTHON_EXTBUILD_LIB := $(PYTHON_EXTBUILD)lib/ > > > @@ -348,9 +353,7 @@ endif > > > > > > export PERL_PATH > > > > > > -LIBPERF_A=$(OUTPUT)libperf.a > > > - > > > -PERFLIBS = $(LIBAPI) $(LIBTRACEEVENT) $(LIBSUBCMD) > > > +PERFLIBS = $(LIBAPI) $(LIBTRACEEVENT) $(LIBSUBCMD) $(LIBPERF) > > > ifndef NO_LIBBPF > > >PERFLIBS += $(LIBBPF) > > > endif > > > @@ -583,8 +586,6 @@ JEVENTS_IN:= $(OUTPUT)pmu-events/jevents-in.o > > > > > > PMU_EVENTS_IN := $(OUTPUT)pmu-events/pmu-events-in.o > > > > > > -LIBPERF_IN := $(OUTPUT)libperf-in.o > > > - > > > export JEVENTS > > > > > > build := -f $(srctree)/tools/build/Makefile.build dir=. obj > > > @@ -601,12 +602,9 @@ $(JEVENTS): $(JEVENTS_IN) > > > $(PMU_EVENTS_IN): $(JEVENTS) FORCE > > > $(Q)$(MAKE) -f $(srctree)/tools/build/Makefile.build dir=pmu-events > > > obj=pmu-events > > > > > > -$(LIBPERF_IN): prepare FORCE > > > - $(Q)$(MAKE) $(build)=libperf > > > - > > > -$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(PMU_EVENTS_IN) $(LIBPERF_IN) > > > $(LIBTRACEEVENT_DYNAMIC_LIST) > > > +$(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(PMU_EVENTS_IN) > > > $(LIBTRACEEVENT_DYNAMIC_LIST) > > > $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) > > > $(LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS) \ > > > - $(PERF_IN) $(PMU_EVENTS_IN) $(LIBPERF_IN) $(LIBS) -o $@ > > > + $(PERF_IN) $(PMU_EVENTS_IN) $(LIBS) -o $@ > > > > > > $(GTK_IN): F
[PATCH 03/14] sched,fair: redefine runnable_load_avg as the sum of task_h_load
The runnable_load magic is used to quickly propagate information about
runnable tasks up the hierarchy of runqueues. The runnable_load_avg is
mostly used for the load balancing code, which only examines the value at
the root cfs_rq.
Redefine the root cfs_rq runnable_load_avg to be the sum of task_h_loads
of the runnable tasks. This works because the hierarchical runnable_load of
a task is already equal to the task_se_h_load today. This provides enough
information to the load balancer.
The runnable_load_avg of the cgroup cfs_rqs does not appear to be
used for anything, so don't bother calculating those.
This removes one of the things that the code currently traverses the
cgroup hierarchy for, and getting rid of it brings us one step closer
to a flat runqueue for the CPU controller.
Signed-off-by: Rik van Riel
---
include/linux/sched.h | 3 +-
kernel/sched/core.c | 2 -
kernel/sched/debug.c | 1 +
kernel/sched/fair.c | 125 +-
kernel/sched/pelt.c | 49 ++---
kernel/sched/sched.h | 6 --
6 files changed, 55 insertions(+), 131 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 11837410690f..f5bb6948e40c 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -391,7 +391,6 @@ struct util_est {
struct sched_avg {
u64 last_update_time;
u64 load_sum;
- u64 runnable_load_sum;
u32 util_sum;
u32 period_contrib;
unsigned long load_avg;
@@ -439,7 +438,6 @@ struct sched_statistics {
struct sched_entity {
/* For load-balancing: */
struct load_weight load;
- unsigned long runnable_weight;
struct rb_node run_node;
struct list_headgroup_node;
unsigned inton_rq;
@@ -455,6 +453,7 @@ struct sched_entity {
#ifdef CONFIG_FAIR_GROUP_SCHED
int depth;
+ unsigned long enqueued_h_load;
struct sched_entity *parent;
/* rq on which this entity is (to be) queued: */
struct cfs_rq *cfs_rq;
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 874c427742a9..fbd96900f715 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -744,7 +744,6 @@ static void set_load_weight(struct task_struct *p, bool
update_load)
if (task_has_idle_policy(p)) {
load->weight = scale_load(WEIGHT_IDLEPRIO);
load->inv_weight = WMULT_IDLEPRIO;
- p->se.runnable_weight = load->weight;
return;
}
@@ -757,7 +756,6 @@ static void set_load_weight(struct task_struct *p, bool
update_load)
} else {
load->weight = scale_load(sched_prio_to_weight[prio]);
load->inv_weight = sched_prio_to_wmult[prio];
- p->se.runnable_weight = load->weight;
}
}
diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index f6beaca97a09..cefc1b171c0b 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -962,6 +962,7 @@ void proc_sched_show_task(struct task_struct *p, struct
pid_namespace *ns,
P(se.avg.load_avg);
P(se.avg.runnable_load_avg);
P(se.avg.util_avg);
+ P(se.enqueued_h_load);
P(se.avg.last_update_time);
P(se.avg.util_est.ewma);
P(se.avg.util_est.enqueued);
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index eadf9a96b3e1..860708b687a7 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -723,9 +723,7 @@ void init_entity_runnable_average(struct sched_entity *se)
* nothing has been attached to the task group yet.
*/
if (entity_is_task(se))
- sa->runnable_load_avg = sa->load_avg =
scale_load_down(se->load.weight);
-
- se->runnable_weight = se->load.weight;
+ sa->load_avg = scale_load_down(se->load.weight);
/* when this task enqueue'ed, it will contribute to its cfs_rq's
load_avg */
}
@@ -2766,20 +2764,39 @@ account_entity_dequeue(struct cfs_rq *cfs_rq, struct
sched_entity *se)
static inline void
enqueue_runnable_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se)
{
- cfs_rq->runnable_weight += se->runnable_weight;
+ if (entity_is_task(se)) {
+ struct cfs_rq *root_cfs_rq = &cfs_rq->rq->cfs;
+ se->enqueued_h_load = task_se_h_load(se);
- cfs_rq->avg.runnable_load_avg += se->avg.runnable_load_avg;
- cfs_rq->avg.runnable_load_sum += se_runnable(se) *
se->avg.runnable_load_sum;
+ root_cfs_rq->avg.runnable_load_avg += se->enqueued_h_load;
+ }
}
static inline void
dequeue_runnable_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se)
{
-
[PATCH 10/14] sched,fair: add helper functions for flattened runqueue
Add helper functions to make the flattened runqueue patch a little smaller.
The task_se_h_weight function is similar to task_se_h_load, but scales the
task weight by the group weight, without taking the task's duty cycle into
account.
The task_se_in_cgroup helper is functionally identical to parent_entity,
but directly calling a function with that name obscures what the other
code is trying to use it for, and would make the code harder to understand.
Signed-off-by: Rik van Riel
---
kernel/sched/fair.c | 31 +++
1 file changed, 31 insertions(+)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index c944729423bd..6073fabb171b 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -243,6 +243,7 @@ static u64 __calc_delta(u64 delta_exec, unsigned long
weight, struct load_weight
const struct sched_class fair_sched_class;
static unsigned long task_se_h_load(struct sched_entity *se);
+static unsigned long task_se_h_weight(struct sched_entity *se);
/**
* CFS operations on generic schedulable entities:
@@ -431,6 +432,12 @@ static inline struct sched_entity *parent_entity(struct
sched_entity *se)
return se->parent;
}
+/* Is this (task) sched_entity in a non-root cgroup? */
+static inline bool task_se_in_cgroup(struct sched_entity *se)
+{
+ return parent_entity(se);
+}
+
static void
find_matching_se(struct sched_entity **se, struct sched_entity **pse)
{
@@ -513,6 +520,11 @@ static inline struct sched_entity *parent_entity(struct
sched_entity *se)
return NULL;
}
+static inline bool task_se_in_cgroup(struct sched_entity *se)
+{
+ return false;
+}
+
static inline void
find_matching_se(struct sched_entity **se, struct sched_entity **pse)
{
@@ -7835,6 +7847,20 @@ static void update_cfs_rq_h_load(struct cfs_rq *cfs_rq)
}
}
+static unsigned long task_se_h_weight(struct sched_entity *se)
+{
+ struct cfs_rq *cfs_rq;
+
+ if (!task_se_in_cgroup(se))
+ return se->load.weight;
+
+ cfs_rq = group_cfs_rq_of_parent(se);
+ update_cfs_rq_h_load(cfs_rq);
+
+ /* Reduce the load.weight by the h_load of the group the task is in. */
+ return (cfs_rq->h_load * se->load.weight) >> SCHED_FIXEDPOINT_SHIFT;
+}
+
static unsigned long task_se_h_load(struct sched_entity *se)
{
struct cfs_rq *cfs_rq = group_cfs_rq_of_parent(se);
@@ -7871,6 +7897,11 @@ static unsigned long task_se_h_load(struct sched_entity
*se)
{
return se->avg.load_avg;
}
+
+static unsigned long task_se_h_weight(struct sched_entity *se)
+{
+ return se->load.weight;
+}
#endif
/** Helpers for find_busiest_group /
--
2.20.1
[PATCH 07/14] sched,cfs: fix zero length timeslice calculation
The way the time slice length is currently calculated, not only do high priority tasks get longer time slices than low priority tasks, but due to fixed point math, low priority tasks could end up with a zero length time slice. This can lead to cache thrashing and other inefficiencies. Cap the minimum time slice length to sysctl_sched_min_granularity. Tasks that end up getting a time slice length too long for their relative priority will simply end up having their vruntime advanced much faster than other tasks, resulting in them receiving time slices less frequently. Signed-off-by: Rik van Riel --- kernel/sched/fair.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 39f7a2d810e1..9ff69b927a3c 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -732,6 +732,13 @@ static u64 sched_slice(struct cfs_rq *cfs_rq, struct sched_entity *se) } slice = __calc_delta(slice, se->load.weight, load); } + + /* +* To avoid cache thrashing, run at least sysctl_sched_min_granularity. +* The vruntime of a low priority task advances faster; those tasks +* will simply get time slices less frequently. +*/ + slice = max_t(u64, slice, sysctl_sched_min_granularity); return slice; } -- 2.20.1
[PATCH 04/14] sched,fair: move runnable_load_avg to cfs_rq
Since only the root cfs_rq runnable_load_avg field is used any more,
we can move the field from struct sched_avg, which every sched_entity
has one of, directly into the struct cfs_rq, of which we have way fewer.
No functional changes.
Suggested-by: Dietmar Eggemann
Signed-off-by: Rik van Riel
---
include/linux/sched.h | 1 -
kernel/sched/debug.c | 3 +--
kernel/sched/fair.c | 8
kernel/sched/sched.h | 1 +
4 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index f5bb6948e40c..84a6cc6f5c47 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -394,7 +394,6 @@ struct sched_avg {
u32 util_sum;
u32 period_contrib;
unsigned long load_avg;
- unsigned long runnable_load_avg;
unsigned long util_avg;
struct util_est util_est;
} cacheline_aligned;
diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index cefc1b171c0b..6e7c8ff210a8 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -539,7 +539,7 @@ void print_cfs_rq(struct seq_file *m, int cpu, struct
cfs_rq *cfs_rq)
SEQ_printf(m, " .%-30s: %lu\n", "load_avg",
cfs_rq->avg.load_avg);
SEQ_printf(m, " .%-30s: %lu\n", "runnable_load_avg",
- cfs_rq->avg.runnable_load_avg);
+ cfs_rq->runnable_load_avg);
SEQ_printf(m, " .%-30s: %lu\n", "util_avg",
cfs_rq->avg.util_avg);
SEQ_printf(m, " .%-30s: %u\n", "util_est_enqueued",
@@ -960,7 +960,6 @@ void proc_sched_show_task(struct task_struct *p, struct
pid_namespace *ns,
P(se.avg.load_sum);
P(se.avg.util_sum);
P(se.avg.load_avg);
- P(se.avg.runnable_load_avg);
P(se.avg.util_avg);
P(se.enqueued_h_load);
P(se.avg.last_update_time);
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 860708b687a7..63cb40253b26 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -2768,7 +2768,7 @@ enqueue_runnable_load_avg(struct cfs_rq *cfs_rq, struct
sched_entity *se)
struct cfs_rq *root_cfs_rq = &cfs_rq->rq->cfs;
se->enqueued_h_load = task_se_h_load(se);
- root_cfs_rq->avg.runnable_load_avg += se->enqueued_h_load;
+ root_cfs_rq->runnable_load_avg += se->enqueued_h_load;
}
}
@@ -2777,7 +2777,7 @@ dequeue_runnable_load_avg(struct cfs_rq *cfs_rq, struct
sched_entity *se)
{
if (entity_is_task(se)) {
struct cfs_rq *root_cfs_rq = &cfs_rq->rq->cfs;
- sub_positive(&root_cfs_rq->avg.runnable_load_avg,
+ sub_positive(&root_cfs_rq->runnable_load_avg,
se->enqueued_h_load);
}
}
@@ -2795,7 +2795,7 @@ update_runnable_load_avg(struct sched_entity *se)
new_h_load = task_se_h_load(se);
delta = new_h_load - se->enqueued_h_load;
- root_cfs_rq->avg.runnable_load_avg += delta;
+ root_cfs_rq->runnable_load_avg += delta;
se->enqueued_h_load = new_h_load;
}
@@ -3559,7 +3559,7 @@ static void remove_entity_load_avg(struct sched_entity
*se)
static inline unsigned long cfs_rq_runnable_load_avg(struct cfs_rq *cfs_rq)
{
- return cfs_rq->avg.runnable_load_avg;
+ return cfs_rq->runnable_load_avg;
}
static inline unsigned long cfs_rq_load_avg(struct cfs_rq *cfs_rq)
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 5be14cee61f9..32978a8de8ce 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -516,6 +516,7 @@ struct cfs_rq {
* CFS load tracking
*/
struct sched_avgavg;
+ unsigned long runnable_load_avg;
#ifndef CONFIG_64BIT
u64 load_last_update_time_copy;
#endif
--
2.20.1
