Re: BUG: soft lockup in snd_virmidi_output_trigger
On Fri, Jul 27, 2018 at 07:20:30AM +0200, Takashi Iwai wrote:
I think there is a mistake on the reported-by tag.
The reported-by tag (e4c8abb920ef...) is for WARNING in port_delete.
I guess the tag below is correct.
Reported-by: syzbot+619d9f40141d826b0...@syzkaller.appspotmail.com
Could you please make sure that the reported-by tag is correct?
By the way, I can't reproduce the crash WARNING in port_delete so
far. I feel sad for saying this..
I really want to reproduce the crash and thus, to help to fix the
crash. If there is any progress on it, I will let you know
immediately.
Best regards,
Dae R. Jeong
> On Fri, 27 Jul 2018 06:13:22 +0200,
> Dae R. Jeong wrote:
> >
> > I tested it and it worked.
> > Thanks a lot!
>
> Good to hear. Below is the final patch with a proper comment (and
> with syzbot reported-by, too) I'm going to queue to sound.git tree.
>
>
> thanks,
>
> Takashi
>
> -- 8< --
>
> From: Takashi Iwai
> Subject: [PATCH v2] ALSA: virmidi: Fix too long output trigger loop
>
> The virmidi output trigger tries to parse the all available bytes and
> process sequencer events as much as possible. In a normal situation,
> this is supposed to be relatively short, but a program may give a huge
> buffer and it'll take a long time in a single spin lock, which may
> eventually lead to a soft lockup.
>
> This patch simply adds a workaround, a cond_resched() call in the loop
> if applicable. A better solution would be to move the event processor
> into a work, but let's put a duct-tape quickly at first.
>
> Reported-and-tested-by: Dae R. Jeong
> Reported-by: syzbot+e4c8abb920efa77ba...@syzkaller.appspotmail.com
> Cc:
> Signed-off-by: Takashi Iwai
> ---
> sound/core/seq/seq_virmidi.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/sound/core/seq/seq_virmidi.c b/sound/core/seq/seq_virmidi.c
> index 289ae6bb81d9..8ebbca554e99 100644
> --- a/sound/core/seq/seq_virmidi.c
> +++ b/sound/core/seq/seq_virmidi.c
> @@ -163,6 +163,7 @@ static void snd_virmidi_output_trigger(struct
> snd_rawmidi_substream *substream,
> int count, res;
> unsigned char buf[32], *pbuf;
> unsigned long flags;
> + bool check_resched = !in_atomic();
>
> if (up) {
> vmidi->trigger = 1;
> @@ -200,6 +201,15 @@ static void snd_virmidi_output_trigger(struct
> snd_rawmidi_substream *substream,
> vmidi->event.type =
> SNDRV_SEQ_EVENT_NONE;
> }
> }
> + if (!check_resched)
> + continue;
> + /* do temporary unlock & cond_resched() for avoiding
> + * CPU soft lockup, which may happen via a write from
> + * a huge rawmidi buffer
> + */
> + spin_unlock_irqrestore(>runtime->lock,
> flags);
> + cond_resched();
> + spin_lock_irqsave(>runtime->lock, flags);
> }
> out:
> spin_unlock_irqrestore(>runtime->lock, flags);
> --
> 2.18.0
>
Re: BUG: soft lockup in snd_virmidi_output_trigger
On Fri, Jul 27, 2018 at 07:20:30AM +0200, Takashi Iwai wrote:
I think there is a mistake on the reported-by tag.
The reported-by tag (e4c8abb920ef...) is for WARNING in port_delete.
I guess the tag below is correct.
Reported-by: syzbot+619d9f40141d826b0...@syzkaller.appspotmail.com
Could you please make sure that the reported-by tag is correct?
By the way, I can't reproduce the crash WARNING in port_delete so
far. I feel sad for saying this..
I really want to reproduce the crash and thus, to help to fix the
crash. If there is any progress on it, I will let you know
immediately.
Best regards,
Dae R. Jeong
> On Fri, 27 Jul 2018 06:13:22 +0200,
> Dae R. Jeong wrote:
> >
> > I tested it and it worked.
> > Thanks a lot!
>
> Good to hear. Below is the final patch with a proper comment (and
> with syzbot reported-by, too) I'm going to queue to sound.git tree.
>
>
> thanks,
>
> Takashi
>
> -- 8< --
>
> From: Takashi Iwai
> Subject: [PATCH v2] ALSA: virmidi: Fix too long output trigger loop
>
> The virmidi output trigger tries to parse the all available bytes and
> process sequencer events as much as possible. In a normal situation,
> this is supposed to be relatively short, but a program may give a huge
> buffer and it'll take a long time in a single spin lock, which may
> eventually lead to a soft lockup.
>
> This patch simply adds a workaround, a cond_resched() call in the loop
> if applicable. A better solution would be to move the event processor
> into a work, but let's put a duct-tape quickly at first.
>
> Reported-and-tested-by: Dae R. Jeong
> Reported-by: syzbot+e4c8abb920efa77ba...@syzkaller.appspotmail.com
> Cc:
> Signed-off-by: Takashi Iwai
> ---
> sound/core/seq/seq_virmidi.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/sound/core/seq/seq_virmidi.c b/sound/core/seq/seq_virmidi.c
> index 289ae6bb81d9..8ebbca554e99 100644
> --- a/sound/core/seq/seq_virmidi.c
> +++ b/sound/core/seq/seq_virmidi.c
> @@ -163,6 +163,7 @@ static void snd_virmidi_output_trigger(struct
> snd_rawmidi_substream *substream,
> int count, res;
> unsigned char buf[32], *pbuf;
> unsigned long flags;
> + bool check_resched = !in_atomic();
>
> if (up) {
> vmidi->trigger = 1;
> @@ -200,6 +201,15 @@ static void snd_virmidi_output_trigger(struct
> snd_rawmidi_substream *substream,
> vmidi->event.type =
> SNDRV_SEQ_EVENT_NONE;
> }
> }
> + if (!check_resched)
> + continue;
> + /* do temporary unlock & cond_resched() for avoiding
> + * CPU soft lockup, which may happen via a write from
> + * a huge rawmidi buffer
> + */
> + spin_unlock_irqrestore(>runtime->lock,
> flags);
> + cond_resched();
> + spin_lock_irqsave(>runtime->lock, flags);
> }
> out:
> spin_unlock_irqrestore(>runtime->lock, flags);
> --
> 2.18.0
>
Re: [PATCH v23 2/4] mailbox: mediatek: Add Mediatek CMDQ driver
Hi, Jassi: In this patch, mediatek mailbox controller implement its own data queue rather than using mailbox framework's queue. I've an idea that could let mediatek mailbox controller use framework's queue but the framework should provide abort function. You could take a look on the sample in [1]. Let me explain why need the abort function. One client of mediatek mailbox controller is display driver. When a cursor is moving, display continuously update the register related to the cursor. Display hardware has a limitation that register should be updated in the vblank period which is a small interval. In tradition, display hardware would trigger an interrupt when vblank start, and driver could update register in this irq handler. But the interrupt handler has the risk that it could be delayed by some reason so the handler may be postponed out of this vblank interval. In order to reduce the risk, display driver use GCE hardware to write register. If a cursor move 3 times before vblank, display driver would send 3 messages sequentially to mailbox controller. If the controller use framework's queue, controller just receive the first message and the others is queued in framework. The first message could be executed exactly in vblank interval but the other messages are sent to controller when controller notify framework tx_done in interrupt handler. The interrupt may be postponed by some reason this is what we worried. So this patch has to implement its own queue to make sure that all message execute in vblank interval. My idea is that display driver send at most one message to mailbox controller. When it need to send the second message before the first message is done, it should abort the first message and then send the second message which should include first message. So how do you think about abort function? Or you have another suggestion? [1] https://github.com/ckhu-mediatek/linux.git-tags/commit/28618df0491f54956a9d5558c3af0df5be208787 Regards, CK On Wed, 2018-07-25 at 09:26 +0800, Houlong Wei wrote: > This patch is first version of Mediatek Command Queue(CMDQ) driver. The > CMDQ is used to help write registers with critical time limitation, > such as updating display configuration during the vblank. It controls > Global Command Engine (GCE) hardware to achieve this requirement. > Currently, CMDQ only supports display related hardwares, but we expect > it can be extended to other hardwares for future requirements. > > Signed-off-by: Houlong Wei > Signed-off-by: HS Liao > Signed-off-by: CK Hu > --- > drivers/mailbox/Kconfig | 10 + > drivers/mailbox/Makefile |2 + > drivers/mailbox/mtk-cmdq-mailbox.c | 569 > ++ > include/linux/mailbox/mtk-cmdq-mailbox.h | 77 > 4 files changed, 658 insertions(+) > create mode 100644 drivers/mailbox/mtk-cmdq-mailbox.c > create mode 100644 include/linux/mailbox/mtk-cmdq-mailbox.h > > diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig > index e63d29a..2bbabc9 100644 > --- a/drivers/mailbox/Kconfig > +++ b/drivers/mailbox/Kconfig > @@ -189,4 +189,14 @@ config STM32_IPCC > Mailbox implementation for STMicroelectonics STM32 family chips > with hardware for Inter-Processor Communication Controller (IPCC) > between processors. Say Y here if you want to have this support. > + > +config MTK_CMDQ_MBOX > + tristate "MediaTek CMDQ Mailbox Support" > + depends on ARCH_MEDIATEK || COMPILE_TEST > + select MTK_INFRACFG > + help > + Say yes here to add support for the MediaTek Command Queue (CMDQ) > + mailbox driver. The CMDQ is used to help read/write registers with > + critical time limitation, such as updating display configuration > + during the vblank. > endif > diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile > index 4d501be..4b00804 100644 > --- a/drivers/mailbox/Makefile > +++ b/drivers/mailbox/Makefile > @@ -40,3 +40,5 @@ obj-$(CONFIG_QCOM_APCS_IPC) += qcom-apcs-ipc-mailbox.o > obj-$(CONFIG_TEGRA_HSP_MBOX) += tegra-hsp.o > > obj-$(CONFIG_STM32_IPCC) += stm32-ipcc.o > + > +obj-$(CONFIG_MTK_CMDQ_MBOX) += mtk-cmdq-mailbox.o > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c > b/drivers/mailbox/mtk-cmdq-mailbox.c > new file mode 100644 > index 000..6f92c5e > --- /dev/null > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c > @@ -0,0 +1,569 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// > +// Copyright (c) 2018 MediaTek Inc. > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define CMDQ_OP_CODE_MASK(0xff << CMDQ_OP_CODE_SHIFT) > +#define CMDQ_IRQ_MASK0x > +#define CMDQ_NUM_CMD(t) (t->cmd_buf_size / > CMDQ_INST_SIZE) > + > +#define CMDQ_CURR_IRQ_STATUS 0x10 > +#define CMDQ_THR_SLOT_CYCLES 0x30
Re: [PATCH v23 2/4] mailbox: mediatek: Add Mediatek CMDQ driver
Hi, Jassi: In this patch, mediatek mailbox controller implement its own data queue rather than using mailbox framework's queue. I've an idea that could let mediatek mailbox controller use framework's queue but the framework should provide abort function. You could take a look on the sample in [1]. Let me explain why need the abort function. One client of mediatek mailbox controller is display driver. When a cursor is moving, display continuously update the register related to the cursor. Display hardware has a limitation that register should be updated in the vblank period which is a small interval. In tradition, display hardware would trigger an interrupt when vblank start, and driver could update register in this irq handler. But the interrupt handler has the risk that it could be delayed by some reason so the handler may be postponed out of this vblank interval. In order to reduce the risk, display driver use GCE hardware to write register. If a cursor move 3 times before vblank, display driver would send 3 messages sequentially to mailbox controller. If the controller use framework's queue, controller just receive the first message and the others is queued in framework. The first message could be executed exactly in vblank interval but the other messages are sent to controller when controller notify framework tx_done in interrupt handler. The interrupt may be postponed by some reason this is what we worried. So this patch has to implement its own queue to make sure that all message execute in vblank interval. My idea is that display driver send at most one message to mailbox controller. When it need to send the second message before the first message is done, it should abort the first message and then send the second message which should include first message. So how do you think about abort function? Or you have another suggestion? [1] https://github.com/ckhu-mediatek/linux.git-tags/commit/28618df0491f54956a9d5558c3af0df5be208787 Regards, CK On Wed, 2018-07-25 at 09:26 +0800, Houlong Wei wrote: > This patch is first version of Mediatek Command Queue(CMDQ) driver. The > CMDQ is used to help write registers with critical time limitation, > such as updating display configuration during the vblank. It controls > Global Command Engine (GCE) hardware to achieve this requirement. > Currently, CMDQ only supports display related hardwares, but we expect > it can be extended to other hardwares for future requirements. > > Signed-off-by: Houlong Wei > Signed-off-by: HS Liao > Signed-off-by: CK Hu > --- > drivers/mailbox/Kconfig | 10 + > drivers/mailbox/Makefile |2 + > drivers/mailbox/mtk-cmdq-mailbox.c | 569 > ++ > include/linux/mailbox/mtk-cmdq-mailbox.h | 77 > 4 files changed, 658 insertions(+) > create mode 100644 drivers/mailbox/mtk-cmdq-mailbox.c > create mode 100644 include/linux/mailbox/mtk-cmdq-mailbox.h > > diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig > index e63d29a..2bbabc9 100644 > --- a/drivers/mailbox/Kconfig > +++ b/drivers/mailbox/Kconfig > @@ -189,4 +189,14 @@ config STM32_IPCC > Mailbox implementation for STMicroelectonics STM32 family chips > with hardware for Inter-Processor Communication Controller (IPCC) > between processors. Say Y here if you want to have this support. > + > +config MTK_CMDQ_MBOX > + tristate "MediaTek CMDQ Mailbox Support" > + depends on ARCH_MEDIATEK || COMPILE_TEST > + select MTK_INFRACFG > + help > + Say yes here to add support for the MediaTek Command Queue (CMDQ) > + mailbox driver. The CMDQ is used to help read/write registers with > + critical time limitation, such as updating display configuration > + during the vblank. > endif > diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile > index 4d501be..4b00804 100644 > --- a/drivers/mailbox/Makefile > +++ b/drivers/mailbox/Makefile > @@ -40,3 +40,5 @@ obj-$(CONFIG_QCOM_APCS_IPC) += qcom-apcs-ipc-mailbox.o > obj-$(CONFIG_TEGRA_HSP_MBOX) += tegra-hsp.o > > obj-$(CONFIG_STM32_IPCC) += stm32-ipcc.o > + > +obj-$(CONFIG_MTK_CMDQ_MBOX) += mtk-cmdq-mailbox.o > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c > b/drivers/mailbox/mtk-cmdq-mailbox.c > new file mode 100644 > index 000..6f92c5e > --- /dev/null > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c > @@ -0,0 +1,569 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// > +// Copyright (c) 2018 MediaTek Inc. > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define CMDQ_OP_CODE_MASK(0xff << CMDQ_OP_CODE_SHIFT) > +#define CMDQ_IRQ_MASK0x > +#define CMDQ_NUM_CMD(t) (t->cmd_buf_size / > CMDQ_INST_SIZE) > + > +#define CMDQ_CURR_IRQ_STATUS 0x10 > +#define CMDQ_THR_SLOT_CYCLES 0x30
[PATCH 1/2] fs/epoll: loosen irq safety in ep_poll()
Similar to other calls, ep_poll() is not called with interrupts
disabled, and we can therefore avoid the irq save/restore dance
and just disable local irqs. In fact, the call should never be
called in irq context at all, considering that the only path is
epoll_wait(2) -> do_epoll_wait() -> ep_poll().
When running on a 2 socket 40-core (ht) IvyBridge a common pipe
based epoll_wait(2) microbenchmark, the following performance
improvements are seen:
# threads vanilla dirty
1 1805587 2106412
2 1854064 2090762
4 1805484 2017436
8 1751222 1974475
16 1725299 1962104
32 1378463 1571233
64 787368 900784
Which is a pretty constantly near 15%.
Also add a lockdep check such that we detect any mischief
before deadlocking.
Signed-off-by: Davidlohr Bueso
---
fs/eventpoll.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index b5e43e11f1e3..88473e6271ef 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -1746,11 +1746,12 @@ static int ep_poll(struct eventpoll *ep, struct
epoll_event __user *events,
int maxevents, long timeout)
{
int res = 0, eavail, timed_out = 0;
- unsigned long flags;
u64 slack = 0;
wait_queue_entry_t wait;
ktime_t expires, *to = NULL;
+ lockdep_assert_irqs_enabled();
+
if (timeout > 0) {
struct timespec64 end_time = ep_set_mstimeout(timeout);
@@ -1763,7 +1764,7 @@ static int ep_poll(struct eventpoll *ep, struct
epoll_event __user *events,
* caller specified a non blocking operation.
*/
timed_out = 1;
- spin_lock_irqsave(>wq.lock, flags);
+ spin_lock_irq(>wq.lock);
goto check_events;
}
@@ -1772,7 +1773,7 @@ static int ep_poll(struct eventpoll *ep, struct
epoll_event __user *events,
if (!ep_events_available(ep))
ep_busy_loop(ep, timed_out);
- spin_lock_irqsave(>wq.lock, flags);
+ spin_lock_irq(>wq.lock);
if (!ep_events_available(ep)) {
/*
@@ -1814,11 +1815,11 @@ static int ep_poll(struct eventpoll *ep, struct
epoll_event __user *events,
break;
}
- spin_unlock_irqrestore(>wq.lock, flags);
+ spin_unlock_irq(>wq.lock);
if (!schedule_hrtimeout_range(to, slack,
HRTIMER_MODE_ABS))
timed_out = 1;
- spin_lock_irqsave(>wq.lock, flags);
+ spin_lock_irq(>wq.lock);
}
__remove_wait_queue(>wq, );
@@ -1828,7 +1829,7 @@ static int ep_poll(struct eventpoll *ep, struct
epoll_event __user *events,
/* Is it worth to try to dig for events ? */
eavail = ep_events_available(ep);
- spin_unlock_irqrestore(>wq.lock, flags);
+ spin_unlock_irq(>wq.lock);
/*
* Try to transfer events to user space. In case we get 0 events and
--
2.16.4
[PATCH 1/2] fs/epoll: loosen irq safety in ep_poll()
Similar to other calls, ep_poll() is not called with interrupts
disabled, and we can therefore avoid the irq save/restore dance
and just disable local irqs. In fact, the call should never be
called in irq context at all, considering that the only path is
epoll_wait(2) -> do_epoll_wait() -> ep_poll().
When running on a 2 socket 40-core (ht) IvyBridge a common pipe
based epoll_wait(2) microbenchmark, the following performance
improvements are seen:
# threads vanilla dirty
1 1805587 2106412
2 1854064 2090762
4 1805484 2017436
8 1751222 1974475
16 1725299 1962104
32 1378463 1571233
64 787368 900784
Which is a pretty constantly near 15%.
Also add a lockdep check such that we detect any mischief
before deadlocking.
Signed-off-by: Davidlohr Bueso
---
fs/eventpoll.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index b5e43e11f1e3..88473e6271ef 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -1746,11 +1746,12 @@ static int ep_poll(struct eventpoll *ep, struct
epoll_event __user *events,
int maxevents, long timeout)
{
int res = 0, eavail, timed_out = 0;
- unsigned long flags;
u64 slack = 0;
wait_queue_entry_t wait;
ktime_t expires, *to = NULL;
+ lockdep_assert_irqs_enabled();
+
if (timeout > 0) {
struct timespec64 end_time = ep_set_mstimeout(timeout);
@@ -1763,7 +1764,7 @@ static int ep_poll(struct eventpoll *ep, struct
epoll_event __user *events,
* caller specified a non blocking operation.
*/
timed_out = 1;
- spin_lock_irqsave(>wq.lock, flags);
+ spin_lock_irq(>wq.lock);
goto check_events;
}
@@ -1772,7 +1773,7 @@ static int ep_poll(struct eventpoll *ep, struct
epoll_event __user *events,
if (!ep_events_available(ep))
ep_busy_loop(ep, timed_out);
- spin_lock_irqsave(>wq.lock, flags);
+ spin_lock_irq(>wq.lock);
if (!ep_events_available(ep)) {
/*
@@ -1814,11 +1815,11 @@ static int ep_poll(struct eventpoll *ep, struct
epoll_event __user *events,
break;
}
- spin_unlock_irqrestore(>wq.lock, flags);
+ spin_unlock_irq(>wq.lock);
if (!schedule_hrtimeout_range(to, slack,
HRTIMER_MODE_ABS))
timed_out = 1;
- spin_lock_irqsave(>wq.lock, flags);
+ spin_lock_irq(>wq.lock);
}
__remove_wait_queue(>wq, );
@@ -1828,7 +1829,7 @@ static int ep_poll(struct eventpoll *ep, struct
epoll_event __user *events,
/* Is it worth to try to dig for events ? */
eavail = ep_events_available(ep);
- spin_unlock_irqrestore(>wq.lock, flags);
+ spin_unlock_irq(>wq.lock);
/*
* Try to transfer events to user space. In case we get 0 events and
--
2.16.4
[PATCH 2/2] fs/eventpoll: simplify ep_is_linked callers
Instead of having each caller pass the rdllink explicitly,
just have ep_is_linked() pass it while the callers just need
the epi pointer. This helper is all about the rdllink, and
this change, furthermore, improves the function's self
documentation.
Signed-off-by: Davidlohr Bueso
---
fs/eventpoll.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 88473e6271ef..42bbe6824b4b 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -336,9 +336,9 @@ static inline int ep_cmp_ffd(struct epoll_filefd *p1,
}
/* Tells us if the item is currently linked */
-static inline int ep_is_linked(struct list_head *p)
+static inline int ep_is_linked(struct epitem *epi)
{
- return !list_empty(p);
+ return !list_empty(>rdllink);
}
static inline struct eppoll_entry *ep_pwq_from_wait(wait_queue_entry_t *p)
@@ -721,7 +721,7 @@ static __poll_t ep_scan_ready_list(struct eventpoll *ep,
* queued into ->ovflist but the "txlist" might already
* contain them, and the list_splice() below takes care of them.
*/
- if (!ep_is_linked(>rdllink)) {
+ if (!ep_is_linked(epi)) {
list_add_tail(>rdllink, >rdllist);
ep_pm_stay_awake(epi);
}
@@ -790,7 +790,7 @@ static int ep_remove(struct eventpoll *ep, struct epitem
*epi)
rb_erase_cached(>rbn, >rbr);
spin_lock_irq(>wq.lock);
- if (ep_is_linked(>rdllink))
+ if (ep_is_linked(epi))
list_del_init(>rdllink);
spin_unlock_irq(>wq.lock);
@@ -1171,7 +1171,7 @@ static int ep_poll_callback(wait_queue_entry_t *wait,
unsigned mode, int sync, v
}
/* If this file is already in the ready list we exit soon */
- if (!ep_is_linked(>rdllink)) {
+ if (!ep_is_linked(epi)) {
list_add_tail(>rdllink, >rdllist);
ep_pm_stay_awake_rcu(epi);
}
@@ -1495,7 +1495,7 @@ static int ep_insert(struct eventpoll *ep, const struct
epoll_event *event,
ep_set_busy_poll_napi_id(epi);
/* If the file is already "ready" we drop it inside the ready list */
- if (revents && !ep_is_linked(>rdllink)) {
+ if (revents && !ep_is_linked(epi)) {
list_add_tail(>rdllink, >rdllist);
ep_pm_stay_awake(epi);
@@ -1533,7 +1533,7 @@ static int ep_insert(struct eventpoll *ep, const struct
epoll_event *event,
* And ep_insert() is called with "mtx" held.
*/
spin_lock_irq(>wq.lock);
- if (ep_is_linked(>rdllink))
+ if (ep_is_linked(epi))
list_del_init(>rdllink);
spin_unlock_irq(>wq.lock);
@@ -1601,7 +1601,7 @@ static int ep_modify(struct eventpoll *ep, struct epitem
*epi,
*/
if (ep_item_poll(epi, , 1)) {
spin_lock_irq(>wq.lock);
- if (!ep_is_linked(>rdllink)) {
+ if (!ep_is_linked(epi)) {
list_add_tail(>rdllink, >rdllist);
ep_pm_stay_awake(epi);
--
2.16.4
[PATCH 2/2] fs/eventpoll: simplify ep_is_linked callers
Instead of having each caller pass the rdllink explicitly,
just have ep_is_linked() pass it while the callers just need
the epi pointer. This helper is all about the rdllink, and
this change, furthermore, improves the function's self
documentation.
Signed-off-by: Davidlohr Bueso
---
fs/eventpoll.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 88473e6271ef..42bbe6824b4b 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -336,9 +336,9 @@ static inline int ep_cmp_ffd(struct epoll_filefd *p1,
}
/* Tells us if the item is currently linked */
-static inline int ep_is_linked(struct list_head *p)
+static inline int ep_is_linked(struct epitem *epi)
{
- return !list_empty(p);
+ return !list_empty(>rdllink);
}
static inline struct eppoll_entry *ep_pwq_from_wait(wait_queue_entry_t *p)
@@ -721,7 +721,7 @@ static __poll_t ep_scan_ready_list(struct eventpoll *ep,
* queued into ->ovflist but the "txlist" might already
* contain them, and the list_splice() below takes care of them.
*/
- if (!ep_is_linked(>rdllink)) {
+ if (!ep_is_linked(epi)) {
list_add_tail(>rdllink, >rdllist);
ep_pm_stay_awake(epi);
}
@@ -790,7 +790,7 @@ static int ep_remove(struct eventpoll *ep, struct epitem
*epi)
rb_erase_cached(>rbn, >rbr);
spin_lock_irq(>wq.lock);
- if (ep_is_linked(>rdllink))
+ if (ep_is_linked(epi))
list_del_init(>rdllink);
spin_unlock_irq(>wq.lock);
@@ -1171,7 +1171,7 @@ static int ep_poll_callback(wait_queue_entry_t *wait,
unsigned mode, int sync, v
}
/* If this file is already in the ready list we exit soon */
- if (!ep_is_linked(>rdllink)) {
+ if (!ep_is_linked(epi)) {
list_add_tail(>rdllink, >rdllist);
ep_pm_stay_awake_rcu(epi);
}
@@ -1495,7 +1495,7 @@ static int ep_insert(struct eventpoll *ep, const struct
epoll_event *event,
ep_set_busy_poll_napi_id(epi);
/* If the file is already "ready" we drop it inside the ready list */
- if (revents && !ep_is_linked(>rdllink)) {
+ if (revents && !ep_is_linked(epi)) {
list_add_tail(>rdllink, >rdllist);
ep_pm_stay_awake(epi);
@@ -1533,7 +1533,7 @@ static int ep_insert(struct eventpoll *ep, const struct
epoll_event *event,
* And ep_insert() is called with "mtx" held.
*/
spin_lock_irq(>wq.lock);
- if (ep_is_linked(>rdllink))
+ if (ep_is_linked(epi))
list_del_init(>rdllink);
spin_unlock_irq(>wq.lock);
@@ -1601,7 +1601,7 @@ static int ep_modify(struct eventpoll *ep, struct epitem
*epi,
*/
if (ep_item_poll(epi, , 1)) {
spin_lock_irq(>wq.lock);
- if (!ep_is_linked(>rdllink)) {
+ if (!ep_is_linked(epi)) {
list_add_tail(>rdllink, >rdllist);
ep_pm_stay_awake(epi);
--
2.16.4
[PATCH -next 0/2] epoll: loosen irq safety in ep_poll()
Hi, Along the same lines than the previous work. Details are in patch 1. Patch 2 is an add on while eyeballing the code. Similar to the previous patches, this has survived ltp testcases and various workloads. Thanks, Davidlohr Davidlohr Bueso (2): fs/epoll: loosen irq safety in ep_poll() fs/eventpoll: simplify ep_is_linked callers fs/eventpoll.c | 29 +++-- 1 file changed, 15 insertions(+), 14 deletions(-) -- 2.16.4
[PATCH -next 0/2] epoll: loosen irq safety in ep_poll()
Hi, Along the same lines than the previous work. Details are in patch 1. Patch 2 is an add on while eyeballing the code. Similar to the previous patches, this has survived ltp testcases and various workloads. Thanks, Davidlohr Davidlohr Bueso (2): fs/epoll: loosen irq safety in ep_poll() fs/eventpoll: simplify ep_is_linked callers fs/eventpoll.c | 29 +++-- 1 file changed, 15 insertions(+), 14 deletions(-) -- 2.16.4
[PATCH] timers: fix offset calculation when the expires align with LVL_GRAN
when the expires of timer is align with LVL_GRAN(n), it will be trigged
in 'expires + LVL_GRAN(n)'.
Some drivers like power runtime use the timer to start a power down
of device, it could saves power if the timer is triggerd in time,
especially when LEVEL=0 with low expires.
Signed-off-by: Xu YiPing
---
kernel/time/timer.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index cc2d23e..76655e2 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -487,7 +487,8 @@ static inline void timer_set_idx(struct timer_list *timer,
unsigned int idx)
*/
static inline unsigned calc_index(unsigned expires, unsigned lvl)
{
- expires = (expires + LVL_GRAN(lvl)) >> LVL_SHIFT(lvl);
+ expires = (expires + LVL_GRAN(lvl) - 1) >> LVL_SHIFT(lvl);
+
return LVL_OFFS(lvl) + (expires & LVL_MASK);
}
--
2.7.4
[PATCH] timers: fix offset calculation when the expires align with LVL_GRAN
when the expires of timer is align with LVL_GRAN(n), it will be trigged
in 'expires + LVL_GRAN(n)'.
Some drivers like power runtime use the timer to start a power down
of device, it could saves power if the timer is triggerd in time,
especially when LEVEL=0 with low expires.
Signed-off-by: Xu YiPing
---
kernel/time/timer.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index cc2d23e..76655e2 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -487,7 +487,8 @@ static inline void timer_set_idx(struct timer_list *timer,
unsigned int idx)
*/
static inline unsigned calc_index(unsigned expires, unsigned lvl)
{
- expires = (expires + LVL_GRAN(lvl)) >> LVL_SHIFT(lvl);
+ expires = (expires + LVL_GRAN(lvl) - 1) >> LVL_SHIFT(lvl);
+
return LVL_OFFS(lvl) + (expires & LVL_MASK);
}
--
2.7.4
Re: [PATCH] mtd: spi-nor: cadence-quadspi: make return type fit wait_for_completion_timeout
On Thu, Jul 26, 2018 at 10:09:28PM +0200, Boris Brezillon wrote:
> On Wed, 25 Jul 2018 06:31:37 +
> Nicholas Mc Guire wrote:
>
> > On Tue, Jul 24, 2018 at 10:46:26PM +0200, Boris Brezillon wrote:
> > > On Sat, 21 Jul 2018 18:08:13 +0200
> > > Nicholas Mc Guire wrote:
> > >
> > > > wait_for_completion_timeout returns an unsigned long not int. declare a
> > > > suitably type timeout and fix up assignment and check.
> > > >
> > > > Signed-off-by: Nicholas Mc Guire
> > > > Reported-by: Vignesh R
> > > > Fixes: 140623410536 ("mtd: spi-nor: Add driver for Cadence Quad SPI
> > > > Flash Controller")
> > >
> > > If you don't mind, I'd like to squash all wait_for_completion_timeout()
> > > fixes into a single commit.
> > >
> > makes sense - no objections to that at all.
>
>
> I also dropped the timeout vars and checked the return of
> wait_for_completion_timeout() directly + removed the Fixes tag since
> it's no longer possible to attach the fix to a single commit and also,
> I'm sure the bug really existed (no overflow possible since the timeout
> values are small enough to fit in a signed int).
>
> Here is the resulting patch. Let me know if you're okay with this version.
>
looks good to me - quit a bit simpler than what I did - thanks !
thx!
hofrat
> Thanks,
>
> Boris
>
> --->8---
> From 69edac4ef4d1c8dac519b6d12afd4097ce72a54e Mon Sep 17 00:00:00 2001
> From: Nicholas Mc Guire
> Date: Sat, 21 Jul 2018 16:21:51 +0200
> Subject: [PATCH] mtd: spi-nor: cadence-quadspi: fix timeout handling
>
> wait_for_completion_timeout returns an unsigned long not an int, so
> let's check its return value directly instead of storing it in ret,
> and avoid checking for negative values since this cannot happen.
>
> Signed-off-by: Nicholas Mc Guire
> Signed-off-by: Boris Brezillon
> ---
> drivers/mtd/spi-nor/cadence-quadspi.c | 20
> 1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c
> b/drivers/mtd/spi-nor/cadence-quadspi.c
> index c3f7aaa5d18f..7a19dae717fa 100644
> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -525,15 +525,14 @@ static int cqspi_indirect_read_execute(struct spi_nor
> *nor, u8 *rxbuf,
> reg_base + CQSPI_REG_INDIRECTRD);
>
> while (remaining > 0) {
> - ret = wait_for_completion_timeout(>transfer_complete,
> - msecs_to_jiffies
> - (CQSPI_READ_TIMEOUT_MS));
> + if (!wait_for_completion_timeout(>transfer_complete,
> + msecs_to_jiffies(CQSPI_READ_TIMEOUT_MS)))
> + ret = -ETIMEDOUT;
>
> bytes_to_read = cqspi_get_rd_sram_level(cqspi);
>
> - if (!ret && bytes_to_read == 0) {
> + if (ret && bytes_to_read == 0) {
> dev_err(nor->dev, "Indirect read timeout, no bytes\n");
> - ret = -ETIMEDOUT;
> goto failrd;
> }
>
> @@ -649,10 +648,8 @@ static int cqspi_indirect_write_execute(struct spi_nor
> *nor, loff_t to_addr,
> iowrite32_rep(cqspi->ahb_base, txbuf,
> DIV_ROUND_UP(write_bytes, 4));
>
> - ret = wait_for_completion_timeout(>transfer_complete,
> - msecs_to_jiffies
> - (CQSPI_TIMEOUT_MS));
> - if (!ret) {
> + if (!wait_for_completion_timeout(>transfer_complete,
> + msecs_to_jiffies(CQSPI_TIMEOUT_MS))) {
> dev_err(nor->dev, "Indirect write timeout\n");
> ret = -ETIMEDOUT;
> goto failwr;
> @@ -986,9 +983,8 @@ static int cqspi_direct_read_execute(struct spi_nor *nor,
> u_char *buf,
> }
>
> dma_async_issue_pending(cqspi->rx_chan);
> - ret = wait_for_completion_timeout(>rx_dma_complete,
> - msecs_to_jiffies(len));
> - if (ret <= 0) {
> + if (!wait_for_completion_timeout(>rx_dma_complete,
> + msecs_to_jiffies(len))) {
> dmaengine_terminate_sync(cqspi->rx_chan);
> dev_err(nor->dev, "DMA wait_for_completion_timeout\n");
> ret = -ETIMEDOUT;
Re: [PATCH v12 04/16] powerpc, kexec_file: factor out memblock-based arch_kexec_walk_mem()
On Wed, Jul 25, 2018 at 08:31:29PM +0800, Dave Young wrote:
> On 07/24/18 at 03:57pm, AKASHI Takahiro wrote:
> > Memblock list is another source for usable system memory layout.
> > So move powerpc's arch_kexec_walk_mem() to common code so that other
> > memblock-based architectures, particularly arm64, can also utilise it.
> > A moved function is now renamed to kexec_walk_memblock() and integrated
> > into kexec_locate_mem_hole(), which will now be usable for all
> > architectures with no need for overriding arch_kexec_walk_mem().
> >
> > kexec_walk_memblock() will not work for kdump in this form, this will be
> > fixed in the next patch.
> >
> > Signed-off-by: AKASHI Takahiro
> > Cc: "Eric W. Biederman"
> > Cc: Dave Young
> > Cc: Vivek Goyal
> > Cc: Baoquan He
> > Acked-by: James Morse
> > ---
> > arch/powerpc/kernel/machine_kexec_file_64.c | 54 ---
> > include/linux/kexec.h | 2 -
> > kernel/kexec_file.c | 58 -
> > 3 files changed, 56 insertions(+), 58 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/machine_kexec_file_64.c
> > b/arch/powerpc/kernel/machine_kexec_file_64.c
> > index 0bd23dc789a4..5357b09902c5 100644
> > --- a/arch/powerpc/kernel/machine_kexec_file_64.c
> > +++ b/arch/powerpc/kernel/machine_kexec_file_64.c
> > @@ -24,7 +24,6 @@
> >
> > #include
> > #include
> > -#include
> > #include
> > #include
> > #include
> > @@ -46,59 +45,6 @@ int arch_kexec_kernel_image_probe(struct kimage *image,
> > void *buf,
> > return kexec_image_probe_default(image, buf, buf_len);
> > }
> >
> > -/**
> > - * arch_kexec_walk_mem - call func(data) for each unreserved memory block
> > - * @kbuf: Context info for the search. Also passed to @func.
> > - * @func: Function to call for each memory block.
> > - *
> > - * This function is used by kexec_add_buffer and kexec_locate_mem_hole
> > - * to find unreserved memory to load kexec segments into.
> > - *
> > - * Return: The memory walk will stop when func returns a non-zero value
> > - * and that value will be returned. If all free regions are visited without
> > - * func returning non-zero, then zero will be returned.
> > - */
> > -int arch_kexec_walk_mem(struct kexec_buf *kbuf,
> > - int (*func)(struct resource *, void *))
> > -{
> > - int ret = 0;
> > - u64 i;
> > - phys_addr_t mstart, mend;
> > - struct resource res = { };
> > -
> > - if (kbuf->top_down) {
> > - for_each_free_mem_range_reverse(i, NUMA_NO_NODE, 0,
> > - , , NULL) {
> > - /*
> > -* In memblock, end points to the first byte after the
> > -* range while in kexec, end points to the last byte
> > -* in the range.
> > -*/
> > - res.start = mstart;
> > - res.end = mend - 1;
> > - ret = func(, kbuf);
> > - if (ret)
> > - break;
> > - }
> > - } else {
> > - for_each_free_mem_range(i, NUMA_NO_NODE, 0, , ,
> > - NULL) {
> > - /*
> > -* In memblock, end points to the first byte after the
> > -* range while in kexec, end points to the last byte
> > -* in the range.
> > -*/
> > - res.start = mstart;
> > - res.end = mend - 1;
> > - ret = func(, kbuf);
> > - if (ret)
> > - break;
> > - }
> > - }
> > -
> > - return ret;
> > -}
> > -
> > /**
> > * setup_purgatory - initialize the purgatory's global variables
> > * @image: kexec image.
> > diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> > index 49ab758f4d91..c196bfd11bee 100644
> > --- a/include/linux/kexec.h
> > +++ b/include/linux/kexec.h
> > @@ -184,8 +184,6 @@ int __weak arch_kexec_apply_relocations(struct
> > purgatory_info *pi,
> > const Elf_Shdr *relsec,
> > const Elf_Shdr *symtab);
> >
> > -int __weak arch_kexec_walk_mem(struct kexec_buf *kbuf,
> > - int (*func)(struct resource *, void *));
> > extern int kexec_add_buffer(struct kexec_buf *kbuf);
> > int kexec_locate_mem_hole(struct kexec_buf *kbuf);
> >
> > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> > index bf39df5e5bb9..2f0691b0f8ad 100644
> > --- a/kernel/kexec_file.c
> > +++ b/kernel/kexec_file.c
> > @@ -16,6 +16,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -501,6 +502,55 @@ static int locate_mem_hole_callback(struct resource
> > *res, void *arg)
> > return locate_mem_hole_bottom_up(start, end, kbuf);
> > }
> >
> > +#if
Re: [PATCH] mtd: spi-nor: cadence-quadspi: make return type fit wait_for_completion_timeout
On Thu, Jul 26, 2018 at 10:09:28PM +0200, Boris Brezillon wrote:
> On Wed, 25 Jul 2018 06:31:37 +
> Nicholas Mc Guire wrote:
>
> > On Tue, Jul 24, 2018 at 10:46:26PM +0200, Boris Brezillon wrote:
> > > On Sat, 21 Jul 2018 18:08:13 +0200
> > > Nicholas Mc Guire wrote:
> > >
> > > > wait_for_completion_timeout returns an unsigned long not int. declare a
> > > > suitably type timeout and fix up assignment and check.
> > > >
> > > > Signed-off-by: Nicholas Mc Guire
> > > > Reported-by: Vignesh R
> > > > Fixes: 140623410536 ("mtd: spi-nor: Add driver for Cadence Quad SPI
> > > > Flash Controller")
> > >
> > > If you don't mind, I'd like to squash all wait_for_completion_timeout()
> > > fixes into a single commit.
> > >
> > makes sense - no objections to that at all.
>
>
> I also dropped the timeout vars and checked the return of
> wait_for_completion_timeout() directly + removed the Fixes tag since
> it's no longer possible to attach the fix to a single commit and also,
> I'm sure the bug really existed (no overflow possible since the timeout
> values are small enough to fit in a signed int).
>
> Here is the resulting patch. Let me know if you're okay with this version.
>
looks good to me - quit a bit simpler than what I did - thanks !
thx!
hofrat
> Thanks,
>
> Boris
>
> --->8---
> From 69edac4ef4d1c8dac519b6d12afd4097ce72a54e Mon Sep 17 00:00:00 2001
> From: Nicholas Mc Guire
> Date: Sat, 21 Jul 2018 16:21:51 +0200
> Subject: [PATCH] mtd: spi-nor: cadence-quadspi: fix timeout handling
>
> wait_for_completion_timeout returns an unsigned long not an int, so
> let's check its return value directly instead of storing it in ret,
> and avoid checking for negative values since this cannot happen.
>
> Signed-off-by: Nicholas Mc Guire
> Signed-off-by: Boris Brezillon
> ---
> drivers/mtd/spi-nor/cadence-quadspi.c | 20
> 1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c
> b/drivers/mtd/spi-nor/cadence-quadspi.c
> index c3f7aaa5d18f..7a19dae717fa 100644
> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -525,15 +525,14 @@ static int cqspi_indirect_read_execute(struct spi_nor
> *nor, u8 *rxbuf,
> reg_base + CQSPI_REG_INDIRECTRD);
>
> while (remaining > 0) {
> - ret = wait_for_completion_timeout(>transfer_complete,
> - msecs_to_jiffies
> - (CQSPI_READ_TIMEOUT_MS));
> + if (!wait_for_completion_timeout(>transfer_complete,
> + msecs_to_jiffies(CQSPI_READ_TIMEOUT_MS)))
> + ret = -ETIMEDOUT;
>
> bytes_to_read = cqspi_get_rd_sram_level(cqspi);
>
> - if (!ret && bytes_to_read == 0) {
> + if (ret && bytes_to_read == 0) {
> dev_err(nor->dev, "Indirect read timeout, no bytes\n");
> - ret = -ETIMEDOUT;
> goto failrd;
> }
>
> @@ -649,10 +648,8 @@ static int cqspi_indirect_write_execute(struct spi_nor
> *nor, loff_t to_addr,
> iowrite32_rep(cqspi->ahb_base, txbuf,
> DIV_ROUND_UP(write_bytes, 4));
>
> - ret = wait_for_completion_timeout(>transfer_complete,
> - msecs_to_jiffies
> - (CQSPI_TIMEOUT_MS));
> - if (!ret) {
> + if (!wait_for_completion_timeout(>transfer_complete,
> + msecs_to_jiffies(CQSPI_TIMEOUT_MS))) {
> dev_err(nor->dev, "Indirect write timeout\n");
> ret = -ETIMEDOUT;
> goto failwr;
> @@ -986,9 +983,8 @@ static int cqspi_direct_read_execute(struct spi_nor *nor,
> u_char *buf,
> }
>
> dma_async_issue_pending(cqspi->rx_chan);
> - ret = wait_for_completion_timeout(>rx_dma_complete,
> - msecs_to_jiffies(len));
> - if (ret <= 0) {
> + if (!wait_for_completion_timeout(>rx_dma_complete,
> + msecs_to_jiffies(len))) {
> dmaengine_terminate_sync(cqspi->rx_chan);
> dev_err(nor->dev, "DMA wait_for_completion_timeout\n");
> ret = -ETIMEDOUT;
Re: [PATCH v12 04/16] powerpc, kexec_file: factor out memblock-based arch_kexec_walk_mem()
On Wed, Jul 25, 2018 at 08:31:29PM +0800, Dave Young wrote:
> On 07/24/18 at 03:57pm, AKASHI Takahiro wrote:
> > Memblock list is another source for usable system memory layout.
> > So move powerpc's arch_kexec_walk_mem() to common code so that other
> > memblock-based architectures, particularly arm64, can also utilise it.
> > A moved function is now renamed to kexec_walk_memblock() and integrated
> > into kexec_locate_mem_hole(), which will now be usable for all
> > architectures with no need for overriding arch_kexec_walk_mem().
> >
> > kexec_walk_memblock() will not work for kdump in this form, this will be
> > fixed in the next patch.
> >
> > Signed-off-by: AKASHI Takahiro
> > Cc: "Eric W. Biederman"
> > Cc: Dave Young
> > Cc: Vivek Goyal
> > Cc: Baoquan He
> > Acked-by: James Morse
> > ---
> > arch/powerpc/kernel/machine_kexec_file_64.c | 54 ---
> > include/linux/kexec.h | 2 -
> > kernel/kexec_file.c | 58 -
> > 3 files changed, 56 insertions(+), 58 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/machine_kexec_file_64.c
> > b/arch/powerpc/kernel/machine_kexec_file_64.c
> > index 0bd23dc789a4..5357b09902c5 100644
> > --- a/arch/powerpc/kernel/machine_kexec_file_64.c
> > +++ b/arch/powerpc/kernel/machine_kexec_file_64.c
> > @@ -24,7 +24,6 @@
> >
> > #include
> > #include
> > -#include
> > #include
> > #include
> > #include
> > @@ -46,59 +45,6 @@ int arch_kexec_kernel_image_probe(struct kimage *image,
> > void *buf,
> > return kexec_image_probe_default(image, buf, buf_len);
> > }
> >
> > -/**
> > - * arch_kexec_walk_mem - call func(data) for each unreserved memory block
> > - * @kbuf: Context info for the search. Also passed to @func.
> > - * @func: Function to call for each memory block.
> > - *
> > - * This function is used by kexec_add_buffer and kexec_locate_mem_hole
> > - * to find unreserved memory to load kexec segments into.
> > - *
> > - * Return: The memory walk will stop when func returns a non-zero value
> > - * and that value will be returned. If all free regions are visited without
> > - * func returning non-zero, then zero will be returned.
> > - */
> > -int arch_kexec_walk_mem(struct kexec_buf *kbuf,
> > - int (*func)(struct resource *, void *))
> > -{
> > - int ret = 0;
> > - u64 i;
> > - phys_addr_t mstart, mend;
> > - struct resource res = { };
> > -
> > - if (kbuf->top_down) {
> > - for_each_free_mem_range_reverse(i, NUMA_NO_NODE, 0,
> > - , , NULL) {
> > - /*
> > -* In memblock, end points to the first byte after the
> > -* range while in kexec, end points to the last byte
> > -* in the range.
> > -*/
> > - res.start = mstart;
> > - res.end = mend - 1;
> > - ret = func(, kbuf);
> > - if (ret)
> > - break;
> > - }
> > - } else {
> > - for_each_free_mem_range(i, NUMA_NO_NODE, 0, , ,
> > - NULL) {
> > - /*
> > -* In memblock, end points to the first byte after the
> > -* range while in kexec, end points to the last byte
> > -* in the range.
> > -*/
> > - res.start = mstart;
> > - res.end = mend - 1;
> > - ret = func(, kbuf);
> > - if (ret)
> > - break;
> > - }
> > - }
> > -
> > - return ret;
> > -}
> > -
> > /**
> > * setup_purgatory - initialize the purgatory's global variables
> > * @image: kexec image.
> > diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> > index 49ab758f4d91..c196bfd11bee 100644
> > --- a/include/linux/kexec.h
> > +++ b/include/linux/kexec.h
> > @@ -184,8 +184,6 @@ int __weak arch_kexec_apply_relocations(struct
> > purgatory_info *pi,
> > const Elf_Shdr *relsec,
> > const Elf_Shdr *symtab);
> >
> > -int __weak arch_kexec_walk_mem(struct kexec_buf *kbuf,
> > - int (*func)(struct resource *, void *));
> > extern int kexec_add_buffer(struct kexec_buf *kbuf);
> > int kexec_locate_mem_hole(struct kexec_buf *kbuf);
> >
> > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> > index bf39df5e5bb9..2f0691b0f8ad 100644
> > --- a/kernel/kexec_file.c
> > +++ b/kernel/kexec_file.c
> > @@ -16,6 +16,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -501,6 +502,55 @@ static int locate_mem_hole_callback(struct resource
> > *res, void *arg)
> > return locate_mem_hole_bottom_up(start, end, kbuf);
> > }
> >
> > +#if
Re: BUG: soft lockup in snd_virmidi_output_trigger
On Fri, 27 Jul 2018 06:13:22 +0200,
Dae R. Jeong wrote:
>
> I tested it and it worked.
> Thanks a lot!
Good to hear. Below is the final patch with a proper comment (and
with syzbot reported-by, too) I'm going to queue to sound.git tree.
thanks,
Takashi
-- 8< --
From: Takashi Iwai
Subject: [PATCH v2] ALSA: virmidi: Fix too long output trigger loop
The virmidi output trigger tries to parse the all available bytes and
process sequencer events as much as possible. In a normal situation,
this is supposed to be relatively short, but a program may give a huge
buffer and it'll take a long time in a single spin lock, which may
eventually lead to a soft lockup.
This patch simply adds a workaround, a cond_resched() call in the loop
if applicable. A better solution would be to move the event processor
into a work, but let's put a duct-tape quickly at first.
Reported-and-tested-by: Dae R. Jeong
Reported-by: syzbot+e4c8abb920efa77ba...@syzkaller.appspotmail.com
Cc:
Signed-off-by: Takashi Iwai
---
sound/core/seq/seq_virmidi.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/sound/core/seq/seq_virmidi.c b/sound/core/seq/seq_virmidi.c
index 289ae6bb81d9..8ebbca554e99 100644
--- a/sound/core/seq/seq_virmidi.c
+++ b/sound/core/seq/seq_virmidi.c
@@ -163,6 +163,7 @@ static void snd_virmidi_output_trigger(struct
snd_rawmidi_substream *substream,
int count, res;
unsigned char buf[32], *pbuf;
unsigned long flags;
+ bool check_resched = !in_atomic();
if (up) {
vmidi->trigger = 1;
@@ -200,6 +201,15 @@ static void snd_virmidi_output_trigger(struct
snd_rawmidi_substream *substream,
vmidi->event.type =
SNDRV_SEQ_EVENT_NONE;
}
}
+ if (!check_resched)
+ continue;
+ /* do temporary unlock & cond_resched() for avoiding
+* CPU soft lockup, which may happen via a write from
+* a huge rawmidi buffer
+*/
+ spin_unlock_irqrestore(>runtime->lock,
flags);
+ cond_resched();
+ spin_lock_irqsave(>runtime->lock, flags);
}
out:
spin_unlock_irqrestore(>runtime->lock, flags);
--
2.18.0
Re: BUG: soft lockup in snd_virmidi_output_trigger
On Fri, 27 Jul 2018 06:13:22 +0200,
Dae R. Jeong wrote:
>
> I tested it and it worked.
> Thanks a lot!
Good to hear. Below is the final patch with a proper comment (and
with syzbot reported-by, too) I'm going to queue to sound.git tree.
thanks,
Takashi
-- 8< --
From: Takashi Iwai
Subject: [PATCH v2] ALSA: virmidi: Fix too long output trigger loop
The virmidi output trigger tries to parse the all available bytes and
process sequencer events as much as possible. In a normal situation,
this is supposed to be relatively short, but a program may give a huge
buffer and it'll take a long time in a single spin lock, which may
eventually lead to a soft lockup.
This patch simply adds a workaround, a cond_resched() call in the loop
if applicable. A better solution would be to move the event processor
into a work, but let's put a duct-tape quickly at first.
Reported-and-tested-by: Dae R. Jeong
Reported-by: syzbot+e4c8abb920efa77ba...@syzkaller.appspotmail.com
Cc:
Signed-off-by: Takashi Iwai
---
sound/core/seq/seq_virmidi.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/sound/core/seq/seq_virmidi.c b/sound/core/seq/seq_virmidi.c
index 289ae6bb81d9..8ebbca554e99 100644
--- a/sound/core/seq/seq_virmidi.c
+++ b/sound/core/seq/seq_virmidi.c
@@ -163,6 +163,7 @@ static void snd_virmidi_output_trigger(struct
snd_rawmidi_substream *substream,
int count, res;
unsigned char buf[32], *pbuf;
unsigned long flags;
+ bool check_resched = !in_atomic();
if (up) {
vmidi->trigger = 1;
@@ -200,6 +201,15 @@ static void snd_virmidi_output_trigger(struct
snd_rawmidi_substream *substream,
vmidi->event.type =
SNDRV_SEQ_EVENT_NONE;
}
}
+ if (!check_resched)
+ continue;
+ /* do temporary unlock & cond_resched() for avoiding
+* CPU soft lockup, which may happen via a write from
+* a huge rawmidi buffer
+*/
+ spin_unlock_irqrestore(>runtime->lock,
flags);
+ cond_resched();
+ spin_lock_irqsave(>runtime->lock, flags);
}
out:
spin_unlock_irqrestore(>runtime->lock, flags);
--
2.18.0
Re: [PATCH v3 1/2] leds: core: Introduce generic pattern interface
Hi Pavel,
On 24 July 2018 at 19:41, Pavel Machek wrote:
> Hi!
>
>> >> > >Please keep in mind that this is ABI documentation for the pattern file
>> >> > >to be exposed by LED core, and not by the pattern trigger, that, as we
>> >> > >agreed, will be implemented later. In this case, I'd go for
>> >> >
>> >> > Gosh, I got completely distracted by the recent discussion about
>> >> > pattern synchronization.
>> >> >
>> >> > So, to recap, we need to decide if we are taking Baolin's solution
>> >> > or we're opting for implementing pattern trigger.
>> >> >
>> >> > If we choose the latter, then we will also need some software
>> >> > pattern engine in the trigger, to be applied as a software pattern
>> >> > fallback for the devices without hardware pattern support.
>> >> > It will certainly delay the contribution process, provided that Baolin
>> >> > would find time for this work at all.
>> >>
>> >> I'd recommend the latter. Yes, software pattern as a fallback would be
>> >> nice, but I have that code already... let me get it back to running
>> >> state, and figure out where to add interface for "hardware
>> >> acceleration". I'd like to have same interface to userland, whether
>> >> pattern can be done by hardware or by software.
>> >
>> > For the record, I'd like something like this. (Software pattern should
>> > work. Hardware pattern... needs more work).
>>
>> Thanks for showing your thoughts. But I failed to compile your code,
>> would you like to send out formal patches (Or only including software
>> pattern, I will help to add hardware pattern part and do some testing
>> with our driver)? Thanks.
>
> This should be a bit better. I attempted to compile it with your
> driver, but whether it works is an open question.
Sorry for late reply. I've compiled and tested this version on my
platform, the hardware pattern can work with one small fix as below.
if (led_cdev->pattern_set && !led_cdev->pattern_set(led_cdev,
data->steps, data->nsteps)) {
return;
}
But I saw there are lots coding style issues and something can be
improved in this patch, so will you send out one clean patch (I will
help to test the hardware pattern support)? Or I can help to improve
this code and try to upstream it again?
> Signed-off-by: Pavel Machek
>
>
> diff --git a/drivers/leds/leds-sc27xx-bltc.c b/drivers/leds/leds-sc27xx-bltc.c
> index 9d9b7aa..898f92d 100644
> --- a/drivers/leds/leds-sc27xx-bltc.c
> +++ b/drivers/leds/leds-sc27xx-bltc.c
> @@ -6,6 +6,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> /* PMIC global control register definition */
> @@ -32,8 +33,13 @@
> #define SC27XX_DUTY_MASK GENMASK(15, 0)
> #define SC27XX_MOD_MASKGENMASK(7, 0)
>
> +#define SC27XX_CURVE_SHIFT 8
> +#define SC27XX_CURVE_L_MASKGENMASK(7, 0)
> +#define SC27XX_CURVE_H_MASKGENMASK(15, 8)
> +
> #define SC27XX_LEDS_OFFSET 0x10
> #define SC27XX_LEDS_MAX3
> +#define SC27XX_LEDS_PATTERN_CNT4
>
> struct sc27xx_led {
> char name[LED_MAX_NAME_SIZE];
> @@ -122,6 +128,157 @@ static int sc27xx_led_set(struct led_classdev *ldev,
> enum led_brightness value)
> return err;
> }
>
> +static int sc27xx_led_pattern_clear(struct led_classdev *ldev)
> +{
> + struct sc27xx_led *leds = to_sc27xx_led(ldev);
> + struct regmap *regmap = leds->priv->regmap;
> + u32 base = sc27xx_led_get_offset(leds);
> + u32 ctrl_base = leds->priv->base + SC27XX_LEDS_CTRL;
> + u8 ctrl_shift = SC27XX_CTRL_SHIFT * leds->line;
> + int err;
> +
> + mutex_lock(>priv->lock);
> +
> + /* Reset the rise, high, fall and low time to zero. */
> + regmap_write(regmap, base + SC27XX_LEDS_CURVE0, 0);
> + regmap_write(regmap, base + SC27XX_LEDS_CURVE1, 0);
> +
> + err = regmap_update_bits(regmap, ctrl_base,
> + (SC27XX_LED_RUN | SC27XX_LED_TYPE) << ctrl_shift, 0);
> +
> + mutex_unlock(>priv->lock);
> +
> + return err;
> +}
> +
> +static int sc27xx_led_pattern_set(struct led_classdev *ldev,
> + struct led_pattern *pattern,
> + int len)
> +{
> + struct sc27xx_led *leds = to_sc27xx_led(ldev);
> + u32 base = sc27xx_led_get_offset(leds);
> + u32 ctrl_base = leds->priv->base + SC27XX_LEDS_CTRL;
> + u8 ctrl_shift = SC27XX_CTRL_SHIFT * leds->line;
> + struct regmap *regmap = leds->priv->regmap;
> + int err;
> +
> + /*
> +* Must contain 4 patterns to configure the rise time, high time, fall
> +* time and low time to enable the breathing mode.
> +*/
> + if (len != SC27XX_LEDS_PATTERN_CNT)
> + return -EINVAL;
> +
> + mutex_lock(>priv->lock);
> +
> + err = regmap_update_bits(regmap, base + SC27XX_LEDS_CURVE0,
> +SC27XX_CURVE_L_MASK, pattern[0].delta_t);
> + if (err)
> + goto out;
> +
> +
Re: [PATCH v3 1/2] leds: core: Introduce generic pattern interface
Hi Pavel,
On 24 July 2018 at 19:41, Pavel Machek wrote:
> Hi!
>
>> >> > >Please keep in mind that this is ABI documentation for the pattern file
>> >> > >to be exposed by LED core, and not by the pattern trigger, that, as we
>> >> > >agreed, will be implemented later. In this case, I'd go for
>> >> >
>> >> > Gosh, I got completely distracted by the recent discussion about
>> >> > pattern synchronization.
>> >> >
>> >> > So, to recap, we need to decide if we are taking Baolin's solution
>> >> > or we're opting for implementing pattern trigger.
>> >> >
>> >> > If we choose the latter, then we will also need some software
>> >> > pattern engine in the trigger, to be applied as a software pattern
>> >> > fallback for the devices without hardware pattern support.
>> >> > It will certainly delay the contribution process, provided that Baolin
>> >> > would find time for this work at all.
>> >>
>> >> I'd recommend the latter. Yes, software pattern as a fallback would be
>> >> nice, but I have that code already... let me get it back to running
>> >> state, and figure out where to add interface for "hardware
>> >> acceleration". I'd like to have same interface to userland, whether
>> >> pattern can be done by hardware or by software.
>> >
>> > For the record, I'd like something like this. (Software pattern should
>> > work. Hardware pattern... needs more work).
>>
>> Thanks for showing your thoughts. But I failed to compile your code,
>> would you like to send out formal patches (Or only including software
>> pattern, I will help to add hardware pattern part and do some testing
>> with our driver)? Thanks.
>
> This should be a bit better. I attempted to compile it with your
> driver, but whether it works is an open question.
Sorry for late reply. I've compiled and tested this version on my
platform, the hardware pattern can work with one small fix as below.
if (led_cdev->pattern_set && !led_cdev->pattern_set(led_cdev,
data->steps, data->nsteps)) {
return;
}
But I saw there are lots coding style issues and something can be
improved in this patch, so will you send out one clean patch (I will
help to test the hardware pattern support)? Or I can help to improve
this code and try to upstream it again?
> Signed-off-by: Pavel Machek
>
>
> diff --git a/drivers/leds/leds-sc27xx-bltc.c b/drivers/leds/leds-sc27xx-bltc.c
> index 9d9b7aa..898f92d 100644
> --- a/drivers/leds/leds-sc27xx-bltc.c
> +++ b/drivers/leds/leds-sc27xx-bltc.c
> @@ -6,6 +6,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> /* PMIC global control register definition */
> @@ -32,8 +33,13 @@
> #define SC27XX_DUTY_MASK GENMASK(15, 0)
> #define SC27XX_MOD_MASKGENMASK(7, 0)
>
> +#define SC27XX_CURVE_SHIFT 8
> +#define SC27XX_CURVE_L_MASKGENMASK(7, 0)
> +#define SC27XX_CURVE_H_MASKGENMASK(15, 8)
> +
> #define SC27XX_LEDS_OFFSET 0x10
> #define SC27XX_LEDS_MAX3
> +#define SC27XX_LEDS_PATTERN_CNT4
>
> struct sc27xx_led {
> char name[LED_MAX_NAME_SIZE];
> @@ -122,6 +128,157 @@ static int sc27xx_led_set(struct led_classdev *ldev,
> enum led_brightness value)
> return err;
> }
>
> +static int sc27xx_led_pattern_clear(struct led_classdev *ldev)
> +{
> + struct sc27xx_led *leds = to_sc27xx_led(ldev);
> + struct regmap *regmap = leds->priv->regmap;
> + u32 base = sc27xx_led_get_offset(leds);
> + u32 ctrl_base = leds->priv->base + SC27XX_LEDS_CTRL;
> + u8 ctrl_shift = SC27XX_CTRL_SHIFT * leds->line;
> + int err;
> +
> + mutex_lock(>priv->lock);
> +
> + /* Reset the rise, high, fall and low time to zero. */
> + regmap_write(regmap, base + SC27XX_LEDS_CURVE0, 0);
> + regmap_write(regmap, base + SC27XX_LEDS_CURVE1, 0);
> +
> + err = regmap_update_bits(regmap, ctrl_base,
> + (SC27XX_LED_RUN | SC27XX_LED_TYPE) << ctrl_shift, 0);
> +
> + mutex_unlock(>priv->lock);
> +
> + return err;
> +}
> +
> +static int sc27xx_led_pattern_set(struct led_classdev *ldev,
> + struct led_pattern *pattern,
> + int len)
> +{
> + struct sc27xx_led *leds = to_sc27xx_led(ldev);
> + u32 base = sc27xx_led_get_offset(leds);
> + u32 ctrl_base = leds->priv->base + SC27XX_LEDS_CTRL;
> + u8 ctrl_shift = SC27XX_CTRL_SHIFT * leds->line;
> + struct regmap *regmap = leds->priv->regmap;
> + int err;
> +
> + /*
> +* Must contain 4 patterns to configure the rise time, high time, fall
> +* time and low time to enable the breathing mode.
> +*/
> + if (len != SC27XX_LEDS_PATTERN_CNT)
> + return -EINVAL;
> +
> + mutex_lock(>priv->lock);
> +
> + err = regmap_update_bits(regmap, base + SC27XX_LEDS_CURVE0,
> +SC27XX_CURVE_L_MASK, pattern[0].delta_t);
> + if (err)
> + goto out;
> +
> +
Re: [PATCH 01/14] thermal: ti-soc-thermal: fix TALERT IRQ handling for DRA752
On Wednesday 25 July 2018 07:57 PM, Bartlomiej Zolnierkiewicz wrote: > On Wednesday, July 11, 2018 07:49:41 AM J, KEERTHY wrote: >> >> On 5/14/2018 5:12 PM, Bartlomiej Zolnierkiewicz wrote: >>> .report_temperature is not set in dra752_data which >>> results in temperature updates not being propagated by >>> ti_bandgap_talert_irq_handler() (it doesn't make much >>> sense to handle TALERT IRQ without reporting temperature >>> updates to the thermal core). Fix it. >> >> ATM no one is using TALERT as the thermal software polls on the >> temperature. No real benefit from TALERT. >> >> TALERT is set at different temperature and software polling thresholds >> come from Device tree and i believe its best for software to go by >> polling and then act on trip points. > > Could you please explain what do you mean by "no one is using > TALERT"? > > The code in ti_bandgap_probe() sets TALERT thresholds and requests > IRQ if the TI_BANDGAP_FEATURE_TALERT feature flag is set (and this > flag is set in omap4460_data, omap4470_data, omap5430_data and > dra752_data). The software thresholds and the polling takes care of reducing the temperature. What i actually meant was we never relied on talert and the polling takes care of keeping a check on the temperature. Regards, Keerthy > > Best regards, > -- > Bartlomiej Zolnierkiewicz > Samsung R Institute Poland > Samsung Electronics >
Re: [PATCH 01/14] thermal: ti-soc-thermal: fix TALERT IRQ handling for DRA752
On Wednesday 25 July 2018 07:57 PM, Bartlomiej Zolnierkiewicz wrote: > On Wednesday, July 11, 2018 07:49:41 AM J, KEERTHY wrote: >> >> On 5/14/2018 5:12 PM, Bartlomiej Zolnierkiewicz wrote: >>> .report_temperature is not set in dra752_data which >>> results in temperature updates not being propagated by >>> ti_bandgap_talert_irq_handler() (it doesn't make much >>> sense to handle TALERT IRQ without reporting temperature >>> updates to the thermal core). Fix it. >> >> ATM no one is using TALERT as the thermal software polls on the >> temperature. No real benefit from TALERT. >> >> TALERT is set at different temperature and software polling thresholds >> come from Device tree and i believe its best for software to go by >> polling and then act on trip points. > > Could you please explain what do you mean by "no one is using > TALERT"? > > The code in ti_bandgap_probe() sets TALERT thresholds and requests > IRQ if the TI_BANDGAP_FEATURE_TALERT feature flag is set (and this > flag is set in omap4460_data, omap4470_data, omap5430_data and > dra752_data). The software thresholds and the polling takes care of reducing the temperature. What i actually meant was we never relied on talert and the polling takes care of keeping a check on the temperature. Regards, Keerthy > > Best regards, > -- > Bartlomiej Zolnierkiewicz > Samsung R Institute Poland > Samsung Electronics >
Re: m68k allmodconfig build errors
On Thu, 26 Jul 2018, Randy Dunlap wrote: > > > > The untested patch below may work. It seems that it may be relevant to > > both arc and m68k: > > > I got back to looking at the build errors. I agree with Andreas that > all of the fields in question are null-terminated, so Finn's patch looks > good to me. > > That patch is insufficient: ERROR: "strcmp" [fs/hfsplus/hfsplus.ko] undefined! And I haven't even attempted "make ARCH=m68k ... allyesconfig" let alone "make ARCH=arc ... allyesconfig". And we have to anticipate an ongoing game of whack-a-mole here, as new library calls get added to linux and new optimizations get added to gcc. The real problem here seems to be a compiler bug: converting strncmp to strcmp is bogus. Some implicit assumptions in that code transformation: 1. that strcmp is equivalient to strncmp in this case 2. that strcmp is faster than strncmp in this case 3. that strcmp is actually available to the linker Why doesn't gcc convert strncmp to __builtin_strcmp? That would require fewer assumptions, and it would actually link, and we wouldn't have to keep patching things... --
Re: m68k allmodconfig build errors
On Thu, 26 Jul 2018, Randy Dunlap wrote: > > > > The untested patch below may work. It seems that it may be relevant to > > both arc and m68k: > > > I got back to looking at the build errors. I agree with Andreas that > all of the fields in question are null-terminated, so Finn's patch looks > good to me. > > That patch is insufficient: ERROR: "strcmp" [fs/hfsplus/hfsplus.ko] undefined! And I haven't even attempted "make ARCH=m68k ... allyesconfig" let alone "make ARCH=arc ... allyesconfig". And we have to anticipate an ongoing game of whack-a-mole here, as new library calls get added to linux and new optimizations get added to gcc. The real problem here seems to be a compiler bug: converting strncmp to strcmp is bogus. Some implicit assumptions in that code transformation: 1. that strcmp is equivalient to strncmp in this case 2. that strcmp is faster than strncmp in this case 3. that strcmp is actually available to the linker Why doesn't gcc convert strncmp to __builtin_strcmp? That would require fewer assumptions, and it would actually link, and we wouldn't have to keep patching things... --
Re: [PATCH v6 5/6] Uprobes/sdt: Prevent multiple reference counter for same uprobe
Hi Oleg,
On 07/25/2018 04:38 PM, Oleg Nesterov wrote:
> No, I can't understand this patch...
>
> On 07/16, Ravi Bangoria wrote:
>>
>> --- a/kernel/events/uprobes.c
>> +++ b/kernel/events/uprobes.c
>> @@ -63,6 +63,8 @@ static struct percpu_rw_semaphore dup_mmap_sem;
>>
>> /* Have a copy of original instruction */
>> #define UPROBE_COPY_INSN0
>> +/* Reference counter offset is reloaded with non-zero value. */
>> +#define REF_CTR_OFF_RELOADED1
>>
>> struct uprobe {
>> struct rb_node rb_node;/* node in the rb tree */
>> @@ -476,9 +478,23 @@ int uprobe_write_opcode(struct arch_uprobe *auprobe,
>> struct mm_struct *mm,
>> return ret;
>>
>> ret = verify_opcode(old_page, vaddr, );
>> -if (ret <= 0)
>> +if (ret < 0)
>> goto put_old;
>
> I agree, "ret <= 0" wasn't nice even before this change, but "ret < 0" looks
> worse because this is simply not possible.
Ok. Any better idea?
I think if we don't track all mms patched by uprobe, we have to rely
on current instruction.
>
>> +/*
>> + * If instruction is already patched but reference counter offset
>> + * has been reloaded to non-zero value, increment the reference
>> + * counter and return.
>> + */
>> +if (ret == 0) {
>> +if (is_register &&
>> +test_bit(REF_CTR_OFF_RELOADED, >flags)) {
>> +WARN_ON(!uprobe->ref_ctr_offset);
>> +ret = update_ref_ctr(uprobe, mm, true);
>> +}
>> +goto put_old;
>> +}
>
> So we need to force update_ref_ctr(true) in case when uprobe_register_refctr()
> detects the already registered uprobe with ref_ctr_offset == 0, and then it
> calls
> register_for_each_vma().
>
> Why this can't race with uprobe_mmap() ?
>
> uprobe_mmap() can do install_breakpoint() right after REF_CTR_OFF_RELOADED
> was set,
> then register_for_each_vma() will find this vma and do install_breakpoint()
> too.
> If ref_ctr_vma was already mmaped, the counter will be incremented twice, no?
Hmm right. Probably, I can fix this race by using some lock, but I don't
know if it's good to do that inside uprobe_mmap().
>
>> @@ -971,6 +1011,7 @@ register_for_each_vma(struct uprobe *uprobe, struct
>> uprobe_consumer *new)
>> bool is_register = !!new;
>> struct map_info *info;
>> int err = 0;
>> +bool installed = false;
>>
>> percpu_down_write(_mmap_sem);
>> info = build_map_info(uprobe->inode->i_mapping,
>> @@ -1000,8 +1041,10 @@ register_for_each_vma(struct uprobe *uprobe, struct
>> uprobe_consumer *new)
>> if (is_register) {
>> /* consult only the "caller", new consumer. */
>> if (consumer_filter(new,
>> -UPROBE_FILTER_REGISTER, mm))
>> +UPROBE_FILTER_REGISTER, mm)) {
>> err = install_breakpoint(uprobe, mm, vma,
>> info->vaddr);
>> +installed = true;
>> +}
>> } else if (test_bit(MMF_HAS_UPROBES, >flags)) {
>> if (!filter_chain(uprobe,
>> UPROBE_FILTER_UNREGISTER, mm))
>> @@ -1016,6 +1059,8 @@ register_for_each_vma(struct uprobe *uprobe, struct
>> uprobe_consumer *new)
>> }
>> out:
>> percpu_up_write(_mmap_sem);
>> +if (installed)
>> +clear_bit(REF_CTR_OFF_RELOADED, >flags);
>
> I simply can't understand this "bool installed"
That boolean is needed because consumer_filter() returns false when this
function gets called first time from uprobe_register(). And consumer_filter
returns true when it gets called by uprobe_apply(). If I make it
unconditional, there is no effect of setting REF_CTR_OFF_RELOADED bit. So
this boolean is needed.
Though, there is one more issue I found. Let's say there are two processes
running and user probes on both of them using uprobe_register() using, let's
say systemtap. Now, some other guy does uprobe_register_refctr() using
'perf -p PID' on same uprobe but he is interested in only one process. Here,
we will increment the reference counter only in the "PID" process and we will
clear REF_CTR_OFF_RELOADED flag. Later, some other guy does 'perf -a' which
will call uprobe_register_refctr() for both the target but we will fail to
increment the counter in "non-PID" process because we had already clear
REF_CTR_OFF_RELOADED.
I have a solution for this. Idea is, if reference counter is reloaded, save
of all mms for which consumer_filter() denied to updated when being called
from register_for_each_vma(). Use this list of mms as checklist next time
onwards. I don't know if it's good to do that or not.
>
> shouldn't we clear REF_CTR_OFF_RELOADED unconditionally after
> register_for_each_vma()?
>
No, because uprobe_register() is simply NOP and breakpoint is actually
installed by uprobe_apply().
>
>
> Also.
Re: [PATCH v6 5/6] Uprobes/sdt: Prevent multiple reference counter for same uprobe
Hi Oleg,
On 07/25/2018 04:38 PM, Oleg Nesterov wrote:
> No, I can't understand this patch...
>
> On 07/16, Ravi Bangoria wrote:
>>
>> --- a/kernel/events/uprobes.c
>> +++ b/kernel/events/uprobes.c
>> @@ -63,6 +63,8 @@ static struct percpu_rw_semaphore dup_mmap_sem;
>>
>> /* Have a copy of original instruction */
>> #define UPROBE_COPY_INSN0
>> +/* Reference counter offset is reloaded with non-zero value. */
>> +#define REF_CTR_OFF_RELOADED1
>>
>> struct uprobe {
>> struct rb_node rb_node;/* node in the rb tree */
>> @@ -476,9 +478,23 @@ int uprobe_write_opcode(struct arch_uprobe *auprobe,
>> struct mm_struct *mm,
>> return ret;
>>
>> ret = verify_opcode(old_page, vaddr, );
>> -if (ret <= 0)
>> +if (ret < 0)
>> goto put_old;
>
> I agree, "ret <= 0" wasn't nice even before this change, but "ret < 0" looks
> worse because this is simply not possible.
Ok. Any better idea?
I think if we don't track all mms patched by uprobe, we have to rely
on current instruction.
>
>> +/*
>> + * If instruction is already patched but reference counter offset
>> + * has been reloaded to non-zero value, increment the reference
>> + * counter and return.
>> + */
>> +if (ret == 0) {
>> +if (is_register &&
>> +test_bit(REF_CTR_OFF_RELOADED, >flags)) {
>> +WARN_ON(!uprobe->ref_ctr_offset);
>> +ret = update_ref_ctr(uprobe, mm, true);
>> +}
>> +goto put_old;
>> +}
>
> So we need to force update_ref_ctr(true) in case when uprobe_register_refctr()
> detects the already registered uprobe with ref_ctr_offset == 0, and then it
> calls
> register_for_each_vma().
>
> Why this can't race with uprobe_mmap() ?
>
> uprobe_mmap() can do install_breakpoint() right after REF_CTR_OFF_RELOADED
> was set,
> then register_for_each_vma() will find this vma and do install_breakpoint()
> too.
> If ref_ctr_vma was already mmaped, the counter will be incremented twice, no?
Hmm right. Probably, I can fix this race by using some lock, but I don't
know if it's good to do that inside uprobe_mmap().
>
>> @@ -971,6 +1011,7 @@ register_for_each_vma(struct uprobe *uprobe, struct
>> uprobe_consumer *new)
>> bool is_register = !!new;
>> struct map_info *info;
>> int err = 0;
>> +bool installed = false;
>>
>> percpu_down_write(_mmap_sem);
>> info = build_map_info(uprobe->inode->i_mapping,
>> @@ -1000,8 +1041,10 @@ register_for_each_vma(struct uprobe *uprobe, struct
>> uprobe_consumer *new)
>> if (is_register) {
>> /* consult only the "caller", new consumer. */
>> if (consumer_filter(new,
>> -UPROBE_FILTER_REGISTER, mm))
>> +UPROBE_FILTER_REGISTER, mm)) {
>> err = install_breakpoint(uprobe, mm, vma,
>> info->vaddr);
>> +installed = true;
>> +}
>> } else if (test_bit(MMF_HAS_UPROBES, >flags)) {
>> if (!filter_chain(uprobe,
>> UPROBE_FILTER_UNREGISTER, mm))
>> @@ -1016,6 +1059,8 @@ register_for_each_vma(struct uprobe *uprobe, struct
>> uprobe_consumer *new)
>> }
>> out:
>> percpu_up_write(_mmap_sem);
>> +if (installed)
>> +clear_bit(REF_CTR_OFF_RELOADED, >flags);
>
> I simply can't understand this "bool installed"
That boolean is needed because consumer_filter() returns false when this
function gets called first time from uprobe_register(). And consumer_filter
returns true when it gets called by uprobe_apply(). If I make it
unconditional, there is no effect of setting REF_CTR_OFF_RELOADED bit. So
this boolean is needed.
Though, there is one more issue I found. Let's say there are two processes
running and user probes on both of them using uprobe_register() using, let's
say systemtap. Now, some other guy does uprobe_register_refctr() using
'perf -p PID' on same uprobe but he is interested in only one process. Here,
we will increment the reference counter only in the "PID" process and we will
clear REF_CTR_OFF_RELOADED flag. Later, some other guy does 'perf -a' which
will call uprobe_register_refctr() for both the target but we will fail to
increment the counter in "non-PID" process because we had already clear
REF_CTR_OFF_RELOADED.
I have a solution for this. Idea is, if reference counter is reloaded, save
of all mms for which consumer_filter() denied to updated when being called
from register_for_each_vma(). Use this list of mms as checklist next time
onwards. I don't know if it's good to do that or not.
>
> shouldn't we clear REF_CTR_OFF_RELOADED unconditionally after
> register_for_each_vma()?
>
No, because uprobe_register() is simply NOP and breakpoint is actually
installed by uprobe_apply().
>
>
> Also.
Re: BUG: soft lockup in snd_virmidi_output_trigger
On Thu, Jul 26, 2018 at 02:50:25PM +0200, Takashi Iwai wrote: > On Thu, 26 Jul 2018 07:53:26 +0200, > Dae R. Jeong wrote: > > > > Reporting the crash: BUG: soft lockup in snd_virmidi_output_trigger > > > > This crash has been found in v4.18-rc3 using RaceFuzzer (a modified > > version of Syzkaller), which we describe more at the end of this > > report. > > > > Note that this bug is previously reported by Syzkaller a few month ago. > > (https://syzkaller.appspot.com/bug?id=642c927972318775e0ea1b4779ccd8624ce9e6f5) > > Nonetheless, the crash still can be detected. We guess that the crash has > > not fixed yet. > > We report the crash log and the attached reproducer with a hope that they > > are > > helpful to diagnose the crash and to fix a bug. > > > > C Reproducer: > > https://kiwi.cs.purdue.edu/static/race-fuzzer/repro-dab082.c > > > > Kernel config: > > https://kiwi.cs.purdue.edu/static/race-fuzzer/config-dab082 > > > > (Please disregard all code related to "should_hypercall" in the C repro, > > as this is only for our debugging purposes using our own hypervisor.) > > > > > > Crash log: > > == > > watchdog: BUG: soft lockup - CPU#1 stuck for 22s! [syz-executor0:24261] > > Modules linked in: > > irq event stamp: 6692 > > hardirqs last enabled at (6691): [] > > __raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:160 [inline] > > hardirqs last enabled at (6691): [] > > _raw_spin_unlock_irqrestore+0x62/0x90 kernel/locking/spinlock.c:184 > > hardirqs last disabled at (6692): [] > > interrupt_entry+0xb5/0xf0 arch/x86/entry/entry_64.S:625 > > softirqs last enabled at (1468): [] > > __do_softirq+0x5ff/0x7f4 kernel/softirq.c:314 > > softirqs last disabled at (1455): [] invoke_softirq > > kernel/softirq.c:368 [inline] > > softirqs last disabled at (1455): [] irq_exit+0x126/0x130 > > kernel/softirq.c:408 > > CPU: 1 PID: 24261 Comm: syz-executor0 Not tainted 4.18.0-rc3 #1 > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS > > rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014 > > RIP: 0010:arch_local_irq_restore arch/x86/include/asm/paravirt.h:783 > > [inline] > > RIP: 0010:__raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:160 > > [inline] > > RIP: 0010:_raw_spin_unlock_irqrestore+0x7d/0x90 > > kernel/locking/spinlock.c:184 > > Code: 0d c8 ce 16 7b 5b 41 5c 5d c3 e8 3e 0f 52 fc 48 c7 c7 68 b0 71 86 e8 > > 62 4c 8d fc 48 83 3d 42 91 86 01 00 74 0e 48 89 df 57 9d <0f> 1f 44 00 00 > > eb cd 0f 0b 0f 0b 0f 1f 84 00 00 00 00 00 55 48 89 > > RSP: 0018:8801b8e4f7e8 EFLAGS: 0282 ORIG_RAX: ff13 > > RAX: RBX: 0282 RCX: 84eb1f1e > > RDX: dc00 RSI: dc00 RDI: 0282 > > RBP: 8801b8e4f7f8 R08: ed002573a06a R09: > > R10: R11: R12: 88012b9d0348 > > R13: 8801e6417580 R14: 8801b8e4f868 R15: 8801e64175a8 > > FS: 7f3447a49700() GS:8801f6d0() knlGS: > > CS: 0010 DS: ES: CR0: 80050033 > > CR2: 7f3447a48db8 CR3: 0001ee58d000 CR4: 06e0 > > Call Trace: > > spin_unlock_irqrestore include/linux/spinlock.h:365 [inline] > > snd_virmidi_output_trigger+0x37d/0x420 sound/core/seq/seq_virmidi.c:205 > > snd_rawmidi_output_trigger sound/core/rawmidi.c:150 [inline] > > snd_rawmidi_kernel_write1+0x383/0x410 sound/core/rawmidi.c:1288 > > snd_rawmidi_write+0x249/0xa50 sound/core/rawmidi.c:1338 > > This seems to be a long loop in snd_virmidi_output_trigger processed > in a spinlock. The below patch puts a cond_resched() for avoiding the > possible stalls, and it seems working. > > Let me know if this works for you. > > > thanks, > > Takashi > > -- 8< -- > From: Takashi Iwai > Subject: [PATCH] ALSA: virmidi: Fix too long output trigger loop > > The virmidi output trigger tries to parse the all available bytes and > process sequencer events as much as possible. In a normal situation, > this is supposed to be relatively short, but a program may give a huge > buffer and it'll take a long time in a single spin lock, which may > eventually lead to a soft lockup. > > This patch simply adds a workaround, a cond_resched() call in the loop > if applicable. A better solution would be to move the event processor > into a work, but let's put a duct-tape quickly at first. > > Reported-by: Dae R. Jeong > Cc: > Signed-off-by: Takashi Iwai > --- > sound/core/seq/seq_virmidi.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/sound/core/seq/seq_virmidi.c b/sound/core/seq/seq_virmidi.c > index 289ae6bb81d9..65549d41533e 100644 > --- a/sound/core/seq/seq_virmidi.c > +++ b/sound/core/seq/seq_virmidi.c > @@ -163,6 +163,7 @@ static void snd_virmidi_output_trigger(struct > snd_rawmidi_substream *substream, > int count, res; > unsigned char buf[32], *pbuf; >
Re: BUG: soft lockup in snd_virmidi_output_trigger
On Thu, Jul 26, 2018 at 02:50:25PM +0200, Takashi Iwai wrote: > On Thu, 26 Jul 2018 07:53:26 +0200, > Dae R. Jeong wrote: > > > > Reporting the crash: BUG: soft lockup in snd_virmidi_output_trigger > > > > This crash has been found in v4.18-rc3 using RaceFuzzer (a modified > > version of Syzkaller), which we describe more at the end of this > > report. > > > > Note that this bug is previously reported by Syzkaller a few month ago. > > (https://syzkaller.appspot.com/bug?id=642c927972318775e0ea1b4779ccd8624ce9e6f5) > > Nonetheless, the crash still can be detected. We guess that the crash has > > not fixed yet. > > We report the crash log and the attached reproducer with a hope that they > > are > > helpful to diagnose the crash and to fix a bug. > > > > C Reproducer: > > https://kiwi.cs.purdue.edu/static/race-fuzzer/repro-dab082.c > > > > Kernel config: > > https://kiwi.cs.purdue.edu/static/race-fuzzer/config-dab082 > > > > (Please disregard all code related to "should_hypercall" in the C repro, > > as this is only for our debugging purposes using our own hypervisor.) > > > > > > Crash log: > > == > > watchdog: BUG: soft lockup - CPU#1 stuck for 22s! [syz-executor0:24261] > > Modules linked in: > > irq event stamp: 6692 > > hardirqs last enabled at (6691): [] > > __raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:160 [inline] > > hardirqs last enabled at (6691): [] > > _raw_spin_unlock_irqrestore+0x62/0x90 kernel/locking/spinlock.c:184 > > hardirqs last disabled at (6692): [] > > interrupt_entry+0xb5/0xf0 arch/x86/entry/entry_64.S:625 > > softirqs last enabled at (1468): [] > > __do_softirq+0x5ff/0x7f4 kernel/softirq.c:314 > > softirqs last disabled at (1455): [] invoke_softirq > > kernel/softirq.c:368 [inline] > > softirqs last disabled at (1455): [] irq_exit+0x126/0x130 > > kernel/softirq.c:408 > > CPU: 1 PID: 24261 Comm: syz-executor0 Not tainted 4.18.0-rc3 #1 > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS > > rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014 > > RIP: 0010:arch_local_irq_restore arch/x86/include/asm/paravirt.h:783 > > [inline] > > RIP: 0010:__raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:160 > > [inline] > > RIP: 0010:_raw_spin_unlock_irqrestore+0x7d/0x90 > > kernel/locking/spinlock.c:184 > > Code: 0d c8 ce 16 7b 5b 41 5c 5d c3 e8 3e 0f 52 fc 48 c7 c7 68 b0 71 86 e8 > > 62 4c 8d fc 48 83 3d 42 91 86 01 00 74 0e 48 89 df 57 9d <0f> 1f 44 00 00 > > eb cd 0f 0b 0f 0b 0f 1f 84 00 00 00 00 00 55 48 89 > > RSP: 0018:8801b8e4f7e8 EFLAGS: 0282 ORIG_RAX: ff13 > > RAX: RBX: 0282 RCX: 84eb1f1e > > RDX: dc00 RSI: dc00 RDI: 0282 > > RBP: 8801b8e4f7f8 R08: ed002573a06a R09: > > R10: R11: R12: 88012b9d0348 > > R13: 8801e6417580 R14: 8801b8e4f868 R15: 8801e64175a8 > > FS: 7f3447a49700() GS:8801f6d0() knlGS: > > CS: 0010 DS: ES: CR0: 80050033 > > CR2: 7f3447a48db8 CR3: 0001ee58d000 CR4: 06e0 > > Call Trace: > > spin_unlock_irqrestore include/linux/spinlock.h:365 [inline] > > snd_virmidi_output_trigger+0x37d/0x420 sound/core/seq/seq_virmidi.c:205 > > snd_rawmidi_output_trigger sound/core/rawmidi.c:150 [inline] > > snd_rawmidi_kernel_write1+0x383/0x410 sound/core/rawmidi.c:1288 > > snd_rawmidi_write+0x249/0xa50 sound/core/rawmidi.c:1338 > > This seems to be a long loop in snd_virmidi_output_trigger processed > in a spinlock. The below patch puts a cond_resched() for avoiding the > possible stalls, and it seems working. > > Let me know if this works for you. > > > thanks, > > Takashi > > -- 8< -- > From: Takashi Iwai > Subject: [PATCH] ALSA: virmidi: Fix too long output trigger loop > > The virmidi output trigger tries to parse the all available bytes and > process sequencer events as much as possible. In a normal situation, > this is supposed to be relatively short, but a program may give a huge > buffer and it'll take a long time in a single spin lock, which may > eventually lead to a soft lockup. > > This patch simply adds a workaround, a cond_resched() call in the loop > if applicable. A better solution would be to move the event processor > into a work, but let's put a duct-tape quickly at first. > > Reported-by: Dae R. Jeong > Cc: > Signed-off-by: Takashi Iwai > --- > sound/core/seq/seq_virmidi.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/sound/core/seq/seq_virmidi.c b/sound/core/seq/seq_virmidi.c > index 289ae6bb81d9..65549d41533e 100644 > --- a/sound/core/seq/seq_virmidi.c > +++ b/sound/core/seq/seq_virmidi.c > @@ -163,6 +163,7 @@ static void snd_virmidi_output_trigger(struct > snd_rawmidi_substream *substream, > int count, res; > unsigned char buf[32], *pbuf; >
[PATCH] serial: 8250_dw: Add ACPI support for uart on Broadcom SoC
Add ACPI identifier HID for UART DW 8250 on Broadcom SoCs
to match the HID passed through ACPI tables to enable
UART controller.
Signed-off-by: Srinath Mannam
Reviewed-by: Vladimir Olovyannikov
Tested-by: Vladimir Olovyannikov
Reviewed-by: Ray Jui
---
drivers/tty/serial/8250/8250_dw.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/tty/serial/8250/8250_dw.c
b/drivers/tty/serial/8250/8250_dw.c
index aff04f1..7ea94ec 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -708,6 +708,7 @@ static const struct acpi_device_id dw8250_acpi_match[] = {
{ "AMD0020", 0 },
{ "AMDI0020", 0 },
{ "HISI0031", 0 },
+ { "BRCM2032", 0 },
{ },
};
MODULE_DEVICE_TABLE(acpi, dw8250_acpi_match);
--
2.7.4
[PATCH] serial: 8250_dw: Add ACPI support for uart on Broadcom SoC
Add ACPI identifier HID for UART DW 8250 on Broadcom SoCs
to match the HID passed through ACPI tables to enable
UART controller.
Signed-off-by: Srinath Mannam
Reviewed-by: Vladimir Olovyannikov
Tested-by: Vladimir Olovyannikov
Reviewed-by: Ray Jui
---
drivers/tty/serial/8250/8250_dw.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/tty/serial/8250/8250_dw.c
b/drivers/tty/serial/8250/8250_dw.c
index aff04f1..7ea94ec 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -708,6 +708,7 @@ static const struct acpi_device_id dw8250_acpi_match[] = {
{ "AMD0020", 0 },
{ "AMDI0020", 0 },
{ "HISI0031", 0 },
+ { "BRCM2032", 0 },
{ },
};
MODULE_DEVICE_TABLE(acpi, dw8250_acpi_match);
--
2.7.4
Request for Partnership!
Sir/Madam, I have access to very vital information that can be used to move huge amount of money. I have done my homework very well and i have the machineries in place to get it done since I am still in active service. If it was possible for me to do it alone I would not have bothered contacting you. Ultimately I need you to play an important role in the completion of this business transaction. Reply if you are willing to do the business. Regards, Richard Willis
Request for Partnership!
Sir/Madam, I have access to very vital information that can be used to move huge amount of money. I have done my homework very well and i have the machineries in place to get it done since I am still in active service. If it was possible for me to do it alone I would not have bothered contacting you. Ultimately I need you to play an important role in the completion of this business transaction. Reply if you are willing to do the business. Regards, Richard Willis
Re: [PATCH RT v3] arm64: fpsimd: use preemp_disable in addition to local_bh_disable()
On Thu, 2018-07-26 at 17:06 +0200, Sebastian Andrzej Siewior wrote: > > @@ -1115,6 +1139,7 @@ void kernel_neon_begin(void) > > BUG_ON(!may_use_simd()); > > + preempt_disable(); > local_bh_disable(); > > __this_cpu_write(kernel_neon_busy, true); > @@ -1131,6 +1156,7 @@ void kernel_neon_begin(void) > preempt_disable(); Nit: this preempt_disable() could be removed... > local_bh_enable(); > + preempt_enable(); > } > EXPORT_SYMBOL(kernel_neon_begin); ...instead of adding this one. -Mike
Re: [PATCH RT v3] arm64: fpsimd: use preemp_disable in addition to local_bh_disable()
On Thu, 2018-07-26 at 17:06 +0200, Sebastian Andrzej Siewior wrote: > > @@ -1115,6 +1139,7 @@ void kernel_neon_begin(void) > > BUG_ON(!may_use_simd()); > > + preempt_disable(); > local_bh_disable(); > > __this_cpu_write(kernel_neon_busy, true); > @@ -1131,6 +1156,7 @@ void kernel_neon_begin(void) > preempt_disable(); Nit: this preempt_disable() could be removed... > local_bh_enable(); > + preempt_enable(); > } > EXPORT_SYMBOL(kernel_neon_begin); ...instead of adding this one. -Mike
Re: [LKP] [rcutorture] 3b745c8969: WARNING:at_mm/slab_common.c:#kmalloc_slab
On Thu, Jul 26, 2018 at 04:34:36PM -0700, Joel Fernandes wrote:
> On Thu, Jul 26, 2018 at 04:53:25AM -0700, Paul E. McKenney wrote:
> > On Thu, Jul 26, 2018 at 04:50:15PM +0800, kernel test robot wrote:
> > >
> > > FYI, we noticed the following commit (built with gcc-5):
> > >
> > > commit: 3b745c8969c752601cb68c82a06735363563ab42 ("rcutorture: Make boost
> > > test more robust")
> > > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
> > >
> > > in testcase: boot
> > >
> > > on test machine: qemu-system-x86_64 -enable-kvm -smp 2 -m 512M
> > >
> > > caused below changes (please refer to attached dmesg/kmsg for entire
> > > log/backtrace):
> >
> > Is this fixed by 4babd855fd61 ("rcutorture: Add support to detect
> > if boost kthread prio is too low")? That could address the
> > rcu_torture_stats_print() failures, depending on exactly what they were.
> > (Yes, I should have reversed these two commits, but they are in -tip
> > now, so that ship has sailed.)
> >
> > Joel, any other thoughts?
>
> I ran the next tree myself and was not able to reproduce the issue with the
> same configuration. Although I don't have rcupdate.rcu_cpu_stall_timeout=100
> passed in like they do (which I can also try if you think its of significance
> here).
>
> It seems from their logs that most Locking API self tests are failing. the
> Lock API test suite is run before rcutorture where rcutorture hasn't even
> started.
>
> Also as per their rcutorture output, it appears the 'rtf' value is also
> non-zero (rcu_torture_free test). Which makes sense if the earlier memory
> allocation warnings are somehow related.
It could potentially be an OOM issue. Something to keep an eye out for,
then.
Thanx, Paul
Re: [LKP] [rcutorture] 3b745c8969: WARNING:at_mm/slab_common.c:#kmalloc_slab
On Thu, Jul 26, 2018 at 04:34:36PM -0700, Joel Fernandes wrote:
> On Thu, Jul 26, 2018 at 04:53:25AM -0700, Paul E. McKenney wrote:
> > On Thu, Jul 26, 2018 at 04:50:15PM +0800, kernel test robot wrote:
> > >
> > > FYI, we noticed the following commit (built with gcc-5):
> > >
> > > commit: 3b745c8969c752601cb68c82a06735363563ab42 ("rcutorture: Make boost
> > > test more robust")
> > > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
> > >
> > > in testcase: boot
> > >
> > > on test machine: qemu-system-x86_64 -enable-kvm -smp 2 -m 512M
> > >
> > > caused below changes (please refer to attached dmesg/kmsg for entire
> > > log/backtrace):
> >
> > Is this fixed by 4babd855fd61 ("rcutorture: Add support to detect
> > if boost kthread prio is too low")? That could address the
> > rcu_torture_stats_print() failures, depending on exactly what they were.
> > (Yes, I should have reversed these two commits, but they are in -tip
> > now, so that ship has sailed.)
> >
> > Joel, any other thoughts?
>
> I ran the next tree myself and was not able to reproduce the issue with the
> same configuration. Although I don't have rcupdate.rcu_cpu_stall_timeout=100
> passed in like they do (which I can also try if you think its of significance
> here).
>
> It seems from their logs that most Locking API self tests are failing. the
> Lock API test suite is run before rcutorture where rcutorture hasn't even
> started.
>
> Also as per their rcutorture output, it appears the 'rtf' value is also
> non-zero (rcu_torture_free test). Which makes sense if the earlier memory
> allocation warnings are somehow related.
It could potentially be an OOM issue. Something to keep an eye out for,
then.
Thanx, Paul
[PATCH 00/10] staging: gasket: logging cleanups
From: Todd Poynor Kill off gasket logging functions, convert to standard. Fixup a few formatting/style problems in the process. Todd Poynor (10): staging: gasket: save struct device for a gasket device staging: gasket: core: convert to standard logging staging: gasket: interrupt: convert to standard logging staging: gasket: ioctl: convert to standard logging staging: gasket: page table: convert to standard logging staging: gasket: sysfs: convert to standard logging staging: gasket: apex: convert to standard logging staging: gasket: remove gasket logging header staging: gasket: TODO: remove entry for convert to standard logging staging: gasket: don't print device addresses as kernel pointers drivers/staging/gasket/TODO| 1 - drivers/staging/gasket/apex_driver.c | 61 ++--- drivers/staging/gasket/gasket_core.c | 300 ++--- drivers/staging/gasket/gasket_core.h | 3 + drivers/staging/gasket/gasket_interrupt.c | 67 +++-- drivers/staging/gasket/gasket_ioctl.c | 23 +- drivers/staging/gasket/gasket_logging.h| 64 - drivers/staging/gasket/gasket_page_table.c | 131 - drivers/staging/gasket/gasket_sysfs.c | 73 +++-- 9 files changed, 296 insertions(+), 427 deletions(-) delete mode 100644 drivers/staging/gasket/gasket_logging.h -- 2.18.0.345.g5c9ce644c3-goog
[PATCH 00/10] staging: gasket: logging cleanups
From: Todd Poynor Kill off gasket logging functions, convert to standard. Fixup a few formatting/style problems in the process. Todd Poynor (10): staging: gasket: save struct device for a gasket device staging: gasket: core: convert to standard logging staging: gasket: interrupt: convert to standard logging staging: gasket: ioctl: convert to standard logging staging: gasket: page table: convert to standard logging staging: gasket: sysfs: convert to standard logging staging: gasket: apex: convert to standard logging staging: gasket: remove gasket logging header staging: gasket: TODO: remove entry for convert to standard logging staging: gasket: don't print device addresses as kernel pointers drivers/staging/gasket/TODO| 1 - drivers/staging/gasket/apex_driver.c | 61 ++--- drivers/staging/gasket/gasket_core.c | 300 ++--- drivers/staging/gasket/gasket_core.h | 3 + drivers/staging/gasket/gasket_interrupt.c | 67 +++-- drivers/staging/gasket/gasket_ioctl.c | 23 +- drivers/staging/gasket/gasket_logging.h| 64 - drivers/staging/gasket/gasket_page_table.c | 131 - drivers/staging/gasket/gasket_sysfs.c | 73 +++-- 9 files changed, 296 insertions(+), 427 deletions(-) delete mode 100644 drivers/staging/gasket/gasket_logging.h -- 2.18.0.345.g5c9ce644c3-goog
[PATCH 05/10] staging: gasket: page table: convert to standard logging
From: Todd Poynor
Replace gasket logging calls with standard logging calls.
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/gasket_page_table.c | 131 +
1 file changed, 54 insertions(+), 77 deletions(-)
diff --git a/drivers/staging/gasket/gasket_page_table.c
b/drivers/staging/gasket/gasket_page_table.c
index 55ab59303247a..8ea8ea1c5174c 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -33,6 +33,7 @@
*/
#include "gasket_page_table.h"
+#include
#include
#include
#include
@@ -43,7 +44,6 @@
#include "gasket_constants.h"
#include "gasket_core.h"
-#include "gasket_logging.h"
/* Constants & utility macros */
/* The number of pages that can be mapped into each second-level page table. */
@@ -79,11 +79,6 @@
*/
#define GASKET_EXTENDED_LVL1_SHIFT 12
-/* Page-table specific error logging. */
-#define gasket_pg_tbl_error(pg_tbl, format, arg...)
\
- gasket_dev_log(err, (pg_tbl)->device, (struct pci_dev *)NULL, format, \
- ##arg)
-
/* Type declarations */
/* Valid states for a struct gasket_page_table_entry. */
enum pte_status {
@@ -306,24 +301,23 @@ int gasket_page_table_init(
* hardware register that contains the page table size.
*/
if (total_entries == ULONG_MAX) {
- gasket_nodev_debug(
- "Error reading page table size. "
- "Initializing page table with size 0.");
+ dev_dbg(device, "Error reading page table size. "
+ "Initializing page table with size 0\n");
total_entries = 0;
}
- gasket_nodev_debug(
- "Attempting to initialize page table of size 0x%lx.",
+ dev_dbg(device,
+ "Attempting to initialize page table of size 0x%lx\n",
total_entries);
- gasket_nodev_debug(
- "Table has base reg 0x%x, extended offset reg 0x%x.",
+ dev_dbg(device,
+ "Table has base reg 0x%x, extended offset reg 0x%x\n",
page_table_config->base_reg,
page_table_config->extended_reg);
*ppg_tbl = kzalloc(sizeof(**ppg_tbl), GFP_KERNEL);
if (!*ppg_tbl) {
- gasket_nodev_debug("No memory for page table.");
+ dev_dbg(device, "No memory for page table\n");
return -ENOMEM;
}
@@ -332,8 +326,8 @@ int gasket_page_table_init(
if (bytes != 0) {
pg_tbl->entries = vzalloc(bytes);
if (!pg_tbl->entries) {
- gasket_nodev_debug(
- "No memory for address translation metadata.");
+ dev_dbg(device,
+ "No memory for address translation metadata\n");
kfree(pg_tbl);
*ppg_tbl = NULL;
return -ENOMEM;
@@ -361,7 +355,7 @@ int gasket_page_table_init(
pg_tbl->pci_dev = pci_dev;
pg_tbl->dma_ops = has_dma_ops;
- gasket_nodev_debug("Page table initialized successfully.");
+ dev_dbg(device, "Page table initialized successfully\n");
return 0;
}
@@ -398,7 +392,7 @@ int gasket_page_table_partition(
for (i = start; i < pg_tbl->config.total_entries; i++) {
if (pg_tbl->entries[i].status != PTE_FREE) {
- gasket_pg_tbl_error(pg_tbl, "entry %d is not free", i);
+ dev_err(pg_tbl->device, "entry %d is not free\n", i);
mutex_unlock(_tbl->mutex);
return -EBUSY;
}
@@ -443,11 +437,9 @@ int gasket_page_table_map(
mutex_unlock(_tbl->mutex);
- gasket_nodev_debug(
- "%s done: ha %llx daddr %llx num %d, "
- "ret %d\n",
- __func__,
- (unsigned long long)host_addr,
+ dev_dbg(pg_tbl->device,
+ "%s done: ha %llx daddr %llx num %d, ret %d\n",
+ __func__, (unsigned long long)host_addr,
(unsigned long long)dev_addr, num_pages, ret);
return ret;
}
@@ -562,9 +554,8 @@ bool gasket_page_table_are_addrs_bad(
ulong bytes)
{
if (host_addr & (PAGE_SIZE - 1)) {
- gasket_pg_tbl_error(
- pg_tbl,
- "host mapping address 0x%lx must be page aligned",
+ dev_err(pg_tbl->device,
+ "host mapping address 0x%lx must be page aligned\n",
host_addr);
return true;
}
@@ -580,16 +571,14 @@ bool gasket_page_table_is_dev_addr_bad(
uint num_pages = bytes / PAGE_SIZE;
if (bytes & (PAGE_SIZE - 1)) {
- gasket_pg_tbl_error(
- pg_tbl,
- "mapping size 0x%lX must be page aligned", bytes);
+
[PATCH 10/10] staging: gasket: don't print device addresses as kernel pointers
From: Todd Poynor
Print device addresses as unsigned long, not as kernel pointers.
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/gasket_page_table.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/gasket/gasket_page_table.c
b/drivers/staging/gasket/gasket_page_table.c
index 8ea8ea1c5174c..32f1c1e10c7e2 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -1333,8 +1333,8 @@ static bool gasket_is_extended_dev_addr_bad(
/* check if the device address is out of bound */
addr = dev_addr & ~((pg_tbl)->extended_flag);
if (addr >> (GASKET_EXTENDED_LVL0_WIDTH + GASKET_EXTENDED_LVL0_SHIFT)) {
- dev_err(pg_tbl->device, "device address out of bound, 0x%p\n",
- (void *)dev_addr);
+ dev_err(pg_tbl->device, "device address out of bounds: 0x%lx\n",
+ dev_addr);
return true;
}
@@ -1351,8 +1351,8 @@ static bool gasket_is_extended_dev_addr_bad(
if (gasket_components_to_dev_address(
pg_tbl, 0, page_global_idx, page_offset) != dev_addr) {
- dev_err(pg_tbl->device, "address is invalid, 0x%p\n",
- (void *)dev_addr);
+ dev_err(pg_tbl->device, "address is invalid: 0x%lx\n",
+ dev_addr);
return true;
}
--
2.18.0.345.g5c9ce644c3-goog
[PATCH 03/10] staging: gasket: interrupt: convert to standard logging
From: Todd Poynor
Convert gasket logging calls to standard functions.
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/gasket_interrupt.c | 67 +++
1 file changed, 32 insertions(+), 35 deletions(-)
diff --git a/drivers/staging/gasket/gasket_interrupt.c
b/drivers/staging/gasket/gasket_interrupt.c
index 2b8c26d065336..3be8e24c126d0 100644
--- a/drivers/staging/gasket/gasket_interrupt.c
+++ b/drivers/staging/gasket/gasket_interrupt.c
@@ -5,9 +5,10 @@
#include "gasket_constants.h"
#include "gasket_core.h"
-#include "gasket_logging.h"
#include "gasket_sysfs.h"
+#include
#include
+#include
#include
#ifdef GASKET_KERNEL_TRACE_SUPPORT
#define CREATE_TRACE_POINTS
@@ -165,8 +166,8 @@ int gasket_interrupt_init(
case PCI_MSI:
case PLATFORM_WIRE:
default:
- gasket_nodev_error(
- "Cannot handle unsupported interrupt type %d.",
+ dev_err(gasket_dev->dev,
+ "Cannot handle unsupported interrupt type %d\n",
interrupt_data->type);
ret = -EINVAL;
};
@@ -175,8 +176,8 @@ int gasket_interrupt_init(
/* Failing to setup interrupts will cause the device to report
* GASKET_STATUS_LAMED. But it is not fatal.
*/
- gasket_log_warn(
- gasket_dev, "Couldn't initialize interrupts: %d", ret);
+ dev_warn(gasket_dev->dev,
+"Couldn't initialize interrupts: %d\n", ret);
return 0;
}
@@ -216,7 +217,7 @@ static int gasket_interrupt_msix_init(
interrupt_data);
if (ret) {
- gasket_nodev_error(
+ dev_err(_data->pci_dev->dev,
"Cannot get IRQ for interrupt %d, vector %d; "
"%d\n",
i, interrupt_data->msix_entries[i].vector, ret);
@@ -287,9 +288,8 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
int ret;
if (!gasket_dev->interrupt_data) {
- gasket_log_debug(
- gasket_dev,
- "Attempted to reinit uninitialized interrupt data.");
+ dev_dbg(gasket_dev->dev,
+ "Attempted to reinit uninitialized interrupt data\n");
return -EINVAL;
}
@@ -305,8 +305,8 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
case PCI_MSI:
case PLATFORM_WIRE:
default:
- gasket_nodev_debug(
- "Cannot handle unsupported interrupt type %d.",
+ dev_dbg(gasket_dev->dev,
+ "Cannot handle unsupported interrupt type %d\n",
gasket_dev->interrupt_data->type);
ret = -EINVAL;
};
@@ -315,7 +315,7 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
/* Failing to setup MSIx will cause the device
* to report GASKET_STATUS_LAMED, but is not fatal.
*/
- gasket_log_warn(gasket_dev, "Couldn't init msix: %d", ret);
+ dev_warn(gasket_dev->dev, "Couldn't init msix: %d\n", ret);
return 0;
}
@@ -327,7 +327,7 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
/* See gasket_interrupt.h for description. */
int gasket_interrupt_reset_counts(struct gasket_dev *gasket_dev)
{
- gasket_log_debug(gasket_dev, "Clearing interrupt counts.");
+ dev_dbg(gasket_dev->dev, "Clearing interrupt counts\n");
memset(gasket_dev->interrupt_data->interrupt_counts, 0,
gasket_dev->interrupt_data->num_interrupts *
sizeof(*gasket_dev->interrupt_data->interrupt_counts));
@@ -351,12 +351,11 @@ static void gasket_interrupt_setup(struct gasket_dev
*gasket_dev)
gasket_dev->interrupt_data;
if (!interrupt_data) {
- gasket_log_debug(
- gasket_dev, "Interrupt data is not initialized.");
+ dev_dbg(gasket_dev->dev, "Interrupt data is not initialized\n");
return;
}
- gasket_log_debug(gasket_dev, "Running interrupt setup.");
+ dev_dbg(gasket_dev->dev, "Running interrupt setup\n");
if (interrupt_data->type == PLATFORM_WIRE ||
interrupt_data->type == PCI_MSI) {
@@ -365,8 +364,8 @@ static void gasket_interrupt_setup(struct gasket_dev
*gasket_dev)
}
if (interrupt_data->type != PCI_MSIX) {
- gasket_nodev_debug(
- "Cannot handle unsupported interrupt type %d.",
+ dev_dbg(gasket_dev->dev,
+ "Cannot handle unsupported interrupt type %d\n",
interrupt_data->type);
return;
}
@@ -379,10 +378,9 @@
[PATCH 04/10] staging: gasket: ioctl: convert to standard logging
From: Todd Poynor
Replace gasket logging calls with standard logging calls.
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/gasket_ioctl.c | 23 +--
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/gasket/gasket_ioctl.c
b/drivers/staging/gasket/gasket_ioctl.c
index 63e139ab7ff89..78a132a60cc4f 100644
--- a/drivers/staging/gasket/gasket_ioctl.c
+++ b/drivers/staging/gasket/gasket_ioctl.c
@@ -5,9 +5,9 @@
#include "gasket_constants.h"
#include "gasket_core.h"
#include "gasket_interrupt.h"
-#include "gasket_logging.h"
#include "gasket_page_table.h"
#include
+#include
#include
#include
@@ -73,7 +73,7 @@ long gasket_handle_ioctl(struct file *filp, uint cmd, void
__user *argp)
}
} else if (!gasket_ioctl_check_permissions(filp, cmd)) {
trace_gasket_ioctl_exit(-EPERM);
- gasket_log_debug(gasket_dev, "ioctl cmd=%x noperm.", cmd);
+ dev_dbg(gasket_dev->dev, "ioctl cmd=%x noperm\n", cmd);
return -EPERM;
}
@@ -132,10 +132,9 @@ long gasket_handle_ioctl(struct file *filp, uint cmd, void
__user *argp)
* the arg.
*/
trace_gasket_ioctl_integer_data(arg);
- gasket_log_debug(
- gasket_dev,
+ dev_dbg(gasket_dev->dev,
"Unknown ioctl cmd=0x%x not caught by "
- "gasket_is_supported_ioctl",
+ "gasket_is_supported_ioctl\n",
cmd);
retval = -EINVAL;
break;
@@ -186,12 +185,9 @@ static bool gasket_ioctl_check_permissions(struct file
*filp, uint cmd)
struct gasket_dev *gasket_dev = (struct gasket_dev *)filp->private_data;
alive = (gasket_dev->status == GASKET_STATUS_ALIVE);
- if (!alive) {
- gasket_nodev_error(
- "%s alive %d status %d.",
- __func__,
- alive, gasket_dev->status);
- }
+ if (!alive)
+ dev_dbg(gasket_dev->dev, "%s alive %d status %d\n",
+ __func__, alive, gasket_dev->status);
read = !!(filp->f_mode & FMODE_READ);
write = !!(filp->f_mode & FMODE_WRITE);
@@ -329,9 +325,8 @@ static int gasket_partition_page_table(
gasket_dev->page_table[ibuf.page_table_index]);
if (ibuf.size > max_page_table_size) {
- gasket_log_debug(
- gasket_dev,
- "Partition request 0x%llx too large, max is 0x%x.",
+ dev_dbg(gasket_dev->dev,
+ "Partition request 0x%llx too large, max is 0x%x\n",
ibuf.size, max_page_table_size);
return -EINVAL;
}
--
2.18.0.345.g5c9ce644c3-goog
[PATCH 07/10] staging: gasket: apex: convert to standard logging
From: Todd Poynor
Drop gasket logging calls in favor of standard logging.
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/apex_driver.c | 61
1 file changed, 26 insertions(+), 35 deletions(-)
diff --git a/drivers/staging/gasket/apex_driver.c
b/drivers/staging/gasket/apex_driver.c
index 6396b18b246a5..73fc2683a834d 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -7,11 +7,13 @@
#include
#include
+#include
#include
#include
#include
#include
#include
+#include
#include
#include
@@ -19,7 +21,6 @@
#include "gasket_core.h"
#include "gasket_interrupt.h"
-#include "gasket_logging.h"
#include "gasket_page_table.h"
#include "gasket_sysfs.h"
@@ -362,11 +363,9 @@ static int apex_add_dev_cb(struct gasket_dev *gasket_dev)
if (retries == APEX_RESET_RETRY) {
if (!page_table_ready)
- gasket_log_error(
- gasket_dev, "Page table init timed out.");
+ dev_err(gasket_dev->dev, "Page table init timed out\n");
if (!msix_table_ready)
- gasket_log_error(
- gasket_dev, "MSI-X table init timed out.");
+ dev_err(gasket_dev->dev, "MSI-X table init timed
out\n");
return -ETIMEDOUT;
}
@@ -420,12 +419,9 @@ static int apex_device_cleanup(struct gasket_dev
*gasket_dev)
gasket_dev, APEX_BAR_INDEX,
APEX_BAR2_REG_SCALAR_CORE_ERROR_STATUS);
- gasket_log_debug(
- gasket_dev,
- "%s 0x%p hib_error 0x%llx scalar_error "
- "0x%llx.",
- __func__,
- gasket_dev, hib_error, scalar_error);
+ dev_dbg(gasket_dev->dev,
+ "%s 0x%p hib_error 0x%llx scalar_error 0x%llx\n",
+ __func__, gasket_dev, hib_error, scalar_error);
if (allow_power_save)
ret = apex_enter_reset(gasket_dev, APEX_CHIP_REINIT_RESET);
@@ -452,7 +448,7 @@ static int apex_reset(struct gasket_dev *gasket_dev, uint
type)
/* We are not in reset - toggle the reset bit so as to force
* re-init of custom block
*/
- gasket_log_debug(gasket_dev, "%s: toggle reset.", __func__);
+ dev_dbg(gasket_dev->dev, "%s: toggle reset\n", __func__);
ret = apex_enter_reset(gasket_dev, type);
if (ret)
@@ -489,9 +485,9 @@ static int apex_enter_reset(struct gasket_dev *gasket_dev,
uint type)
if (gasket_wait_with_reschedule(gasket_dev, APEX_BAR_INDEX,
APEX_BAR2_REG_USER_HIB_DMA_PAUSED, 1, 1,
APEX_RESET_DELAY, APEX_RESET_RETRY)) {
- gasket_log_error(gasket_dev,
-"DMAs did not quiesce within timeout (%d ms)",
-APEX_RESET_RETRY * APEX_RESET_DELAY);
+ dev_err(gasket_dev->dev,
+ "DMAs did not quiesce within timeout (%d ms)\n",
+ APEX_RESET_RETRY * APEX_RESET_DELAY);
return -ETIMEDOUT;
}
@@ -511,9 +507,8 @@ static int apex_enter_reset(struct gasket_dev *gasket_dev,
uint type)
if (gasket_wait_with_reschedule(gasket_dev, APEX_BAR_INDEX,
APEX_BAR2_REG_SCU_3, 1 << 6, 1 << 6,
APEX_RESET_DELAY, APEX_RESET_RETRY)) {
- gasket_log_error(
- gasket_dev,
- "RAM did not shut down within timeout (%d ms)",
+ dev_err(gasket_dev->dev,
+ "RAM did not shut down within timeout (%d ms)\n",
APEX_RESET_RETRY * APEX_RESET_DELAY);
return -ETIMEDOUT;
}
@@ -560,9 +555,8 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev,
uint type)
if (gasket_wait_with_reschedule(gasket_dev, APEX_BAR_INDEX,
APEX_BAR2_REG_SCU_3, 1 << 6, 0,
APEX_RESET_DELAY, APEX_RESET_RETRY)) {
- gasket_log_error(
- gasket_dev,
- "RAM did not enable within timeout (%d ms)",
+ dev_err(gasket_dev->dev,
+ "RAM did not enable within timeout (%d ms)\n",
APEX_RESET_RETRY * APEX_RESET_DELAY);
return -ETIMEDOUT;
}
@@ -572,9 +566,8 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev,
uint type)
APEX_BAR2_REG_SCU_3,
SCU3_CUR_RST_GCB_BIT_MASK, 0,
APEX_RESET_DELAY, APEX_RESET_RETRY)) {
- gasket_log_error(
-
[PATCH 05/10] staging: gasket: page table: convert to standard logging
From: Todd Poynor
Replace gasket logging calls with standard logging calls.
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/gasket_page_table.c | 131 +
1 file changed, 54 insertions(+), 77 deletions(-)
diff --git a/drivers/staging/gasket/gasket_page_table.c
b/drivers/staging/gasket/gasket_page_table.c
index 55ab59303247a..8ea8ea1c5174c 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -33,6 +33,7 @@
*/
#include "gasket_page_table.h"
+#include
#include
#include
#include
@@ -43,7 +44,6 @@
#include "gasket_constants.h"
#include "gasket_core.h"
-#include "gasket_logging.h"
/* Constants & utility macros */
/* The number of pages that can be mapped into each second-level page table. */
@@ -79,11 +79,6 @@
*/
#define GASKET_EXTENDED_LVL1_SHIFT 12
-/* Page-table specific error logging. */
-#define gasket_pg_tbl_error(pg_tbl, format, arg...)
\
- gasket_dev_log(err, (pg_tbl)->device, (struct pci_dev *)NULL, format, \
- ##arg)
-
/* Type declarations */
/* Valid states for a struct gasket_page_table_entry. */
enum pte_status {
@@ -306,24 +301,23 @@ int gasket_page_table_init(
* hardware register that contains the page table size.
*/
if (total_entries == ULONG_MAX) {
- gasket_nodev_debug(
- "Error reading page table size. "
- "Initializing page table with size 0.");
+ dev_dbg(device, "Error reading page table size. "
+ "Initializing page table with size 0\n");
total_entries = 0;
}
- gasket_nodev_debug(
- "Attempting to initialize page table of size 0x%lx.",
+ dev_dbg(device,
+ "Attempting to initialize page table of size 0x%lx\n",
total_entries);
- gasket_nodev_debug(
- "Table has base reg 0x%x, extended offset reg 0x%x.",
+ dev_dbg(device,
+ "Table has base reg 0x%x, extended offset reg 0x%x\n",
page_table_config->base_reg,
page_table_config->extended_reg);
*ppg_tbl = kzalloc(sizeof(**ppg_tbl), GFP_KERNEL);
if (!*ppg_tbl) {
- gasket_nodev_debug("No memory for page table.");
+ dev_dbg(device, "No memory for page table\n");
return -ENOMEM;
}
@@ -332,8 +326,8 @@ int gasket_page_table_init(
if (bytes != 0) {
pg_tbl->entries = vzalloc(bytes);
if (!pg_tbl->entries) {
- gasket_nodev_debug(
- "No memory for address translation metadata.");
+ dev_dbg(device,
+ "No memory for address translation metadata\n");
kfree(pg_tbl);
*ppg_tbl = NULL;
return -ENOMEM;
@@ -361,7 +355,7 @@ int gasket_page_table_init(
pg_tbl->pci_dev = pci_dev;
pg_tbl->dma_ops = has_dma_ops;
- gasket_nodev_debug("Page table initialized successfully.");
+ dev_dbg(device, "Page table initialized successfully\n");
return 0;
}
@@ -398,7 +392,7 @@ int gasket_page_table_partition(
for (i = start; i < pg_tbl->config.total_entries; i++) {
if (pg_tbl->entries[i].status != PTE_FREE) {
- gasket_pg_tbl_error(pg_tbl, "entry %d is not free", i);
+ dev_err(pg_tbl->device, "entry %d is not free\n", i);
mutex_unlock(_tbl->mutex);
return -EBUSY;
}
@@ -443,11 +437,9 @@ int gasket_page_table_map(
mutex_unlock(_tbl->mutex);
- gasket_nodev_debug(
- "%s done: ha %llx daddr %llx num %d, "
- "ret %d\n",
- __func__,
- (unsigned long long)host_addr,
+ dev_dbg(pg_tbl->device,
+ "%s done: ha %llx daddr %llx num %d, ret %d\n",
+ __func__, (unsigned long long)host_addr,
(unsigned long long)dev_addr, num_pages, ret);
return ret;
}
@@ -562,9 +554,8 @@ bool gasket_page_table_are_addrs_bad(
ulong bytes)
{
if (host_addr & (PAGE_SIZE - 1)) {
- gasket_pg_tbl_error(
- pg_tbl,
- "host mapping address 0x%lx must be page aligned",
+ dev_err(pg_tbl->device,
+ "host mapping address 0x%lx must be page aligned\n",
host_addr);
return true;
}
@@ -580,16 +571,14 @@ bool gasket_page_table_is_dev_addr_bad(
uint num_pages = bytes / PAGE_SIZE;
if (bytes & (PAGE_SIZE - 1)) {
- gasket_pg_tbl_error(
- pg_tbl,
- "mapping size 0x%lX must be page aligned", bytes);
+
[PATCH 10/10] staging: gasket: don't print device addresses as kernel pointers
From: Todd Poynor
Print device addresses as unsigned long, not as kernel pointers.
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/gasket_page_table.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/gasket/gasket_page_table.c
b/drivers/staging/gasket/gasket_page_table.c
index 8ea8ea1c5174c..32f1c1e10c7e2 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -1333,8 +1333,8 @@ static bool gasket_is_extended_dev_addr_bad(
/* check if the device address is out of bound */
addr = dev_addr & ~((pg_tbl)->extended_flag);
if (addr >> (GASKET_EXTENDED_LVL0_WIDTH + GASKET_EXTENDED_LVL0_SHIFT)) {
- dev_err(pg_tbl->device, "device address out of bound, 0x%p\n",
- (void *)dev_addr);
+ dev_err(pg_tbl->device, "device address out of bounds: 0x%lx\n",
+ dev_addr);
return true;
}
@@ -1351,8 +1351,8 @@ static bool gasket_is_extended_dev_addr_bad(
if (gasket_components_to_dev_address(
pg_tbl, 0, page_global_idx, page_offset) != dev_addr) {
- dev_err(pg_tbl->device, "address is invalid, 0x%p\n",
- (void *)dev_addr);
+ dev_err(pg_tbl->device, "address is invalid: 0x%lx\n",
+ dev_addr);
return true;
}
--
2.18.0.345.g5c9ce644c3-goog
[PATCH 03/10] staging: gasket: interrupt: convert to standard logging
From: Todd Poynor
Convert gasket logging calls to standard functions.
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/gasket_interrupt.c | 67 +++
1 file changed, 32 insertions(+), 35 deletions(-)
diff --git a/drivers/staging/gasket/gasket_interrupt.c
b/drivers/staging/gasket/gasket_interrupt.c
index 2b8c26d065336..3be8e24c126d0 100644
--- a/drivers/staging/gasket/gasket_interrupt.c
+++ b/drivers/staging/gasket/gasket_interrupt.c
@@ -5,9 +5,10 @@
#include "gasket_constants.h"
#include "gasket_core.h"
-#include "gasket_logging.h"
#include "gasket_sysfs.h"
+#include
#include
+#include
#include
#ifdef GASKET_KERNEL_TRACE_SUPPORT
#define CREATE_TRACE_POINTS
@@ -165,8 +166,8 @@ int gasket_interrupt_init(
case PCI_MSI:
case PLATFORM_WIRE:
default:
- gasket_nodev_error(
- "Cannot handle unsupported interrupt type %d.",
+ dev_err(gasket_dev->dev,
+ "Cannot handle unsupported interrupt type %d\n",
interrupt_data->type);
ret = -EINVAL;
};
@@ -175,8 +176,8 @@ int gasket_interrupt_init(
/* Failing to setup interrupts will cause the device to report
* GASKET_STATUS_LAMED. But it is not fatal.
*/
- gasket_log_warn(
- gasket_dev, "Couldn't initialize interrupts: %d", ret);
+ dev_warn(gasket_dev->dev,
+"Couldn't initialize interrupts: %d\n", ret);
return 0;
}
@@ -216,7 +217,7 @@ static int gasket_interrupt_msix_init(
interrupt_data);
if (ret) {
- gasket_nodev_error(
+ dev_err(_data->pci_dev->dev,
"Cannot get IRQ for interrupt %d, vector %d; "
"%d\n",
i, interrupt_data->msix_entries[i].vector, ret);
@@ -287,9 +288,8 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
int ret;
if (!gasket_dev->interrupt_data) {
- gasket_log_debug(
- gasket_dev,
- "Attempted to reinit uninitialized interrupt data.");
+ dev_dbg(gasket_dev->dev,
+ "Attempted to reinit uninitialized interrupt data\n");
return -EINVAL;
}
@@ -305,8 +305,8 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
case PCI_MSI:
case PLATFORM_WIRE:
default:
- gasket_nodev_debug(
- "Cannot handle unsupported interrupt type %d.",
+ dev_dbg(gasket_dev->dev,
+ "Cannot handle unsupported interrupt type %d\n",
gasket_dev->interrupt_data->type);
ret = -EINVAL;
};
@@ -315,7 +315,7 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
/* Failing to setup MSIx will cause the device
* to report GASKET_STATUS_LAMED, but is not fatal.
*/
- gasket_log_warn(gasket_dev, "Couldn't init msix: %d", ret);
+ dev_warn(gasket_dev->dev, "Couldn't init msix: %d\n", ret);
return 0;
}
@@ -327,7 +327,7 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
/* See gasket_interrupt.h for description. */
int gasket_interrupt_reset_counts(struct gasket_dev *gasket_dev)
{
- gasket_log_debug(gasket_dev, "Clearing interrupt counts.");
+ dev_dbg(gasket_dev->dev, "Clearing interrupt counts\n");
memset(gasket_dev->interrupt_data->interrupt_counts, 0,
gasket_dev->interrupt_data->num_interrupts *
sizeof(*gasket_dev->interrupt_data->interrupt_counts));
@@ -351,12 +351,11 @@ static void gasket_interrupt_setup(struct gasket_dev
*gasket_dev)
gasket_dev->interrupt_data;
if (!interrupt_data) {
- gasket_log_debug(
- gasket_dev, "Interrupt data is not initialized.");
+ dev_dbg(gasket_dev->dev, "Interrupt data is not initialized\n");
return;
}
- gasket_log_debug(gasket_dev, "Running interrupt setup.");
+ dev_dbg(gasket_dev->dev, "Running interrupt setup\n");
if (interrupt_data->type == PLATFORM_WIRE ||
interrupt_data->type == PCI_MSI) {
@@ -365,8 +364,8 @@ static void gasket_interrupt_setup(struct gasket_dev
*gasket_dev)
}
if (interrupt_data->type != PCI_MSIX) {
- gasket_nodev_debug(
- "Cannot handle unsupported interrupt type %d.",
+ dev_dbg(gasket_dev->dev,
+ "Cannot handle unsupported interrupt type %d\n",
interrupt_data->type);
return;
}
@@ -379,10 +378,9 @@
[PATCH 04/10] staging: gasket: ioctl: convert to standard logging
From: Todd Poynor
Replace gasket logging calls with standard logging calls.
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/gasket_ioctl.c | 23 +--
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/gasket/gasket_ioctl.c
b/drivers/staging/gasket/gasket_ioctl.c
index 63e139ab7ff89..78a132a60cc4f 100644
--- a/drivers/staging/gasket/gasket_ioctl.c
+++ b/drivers/staging/gasket/gasket_ioctl.c
@@ -5,9 +5,9 @@
#include "gasket_constants.h"
#include "gasket_core.h"
#include "gasket_interrupt.h"
-#include "gasket_logging.h"
#include "gasket_page_table.h"
#include
+#include
#include
#include
@@ -73,7 +73,7 @@ long gasket_handle_ioctl(struct file *filp, uint cmd, void
__user *argp)
}
} else if (!gasket_ioctl_check_permissions(filp, cmd)) {
trace_gasket_ioctl_exit(-EPERM);
- gasket_log_debug(gasket_dev, "ioctl cmd=%x noperm.", cmd);
+ dev_dbg(gasket_dev->dev, "ioctl cmd=%x noperm\n", cmd);
return -EPERM;
}
@@ -132,10 +132,9 @@ long gasket_handle_ioctl(struct file *filp, uint cmd, void
__user *argp)
* the arg.
*/
trace_gasket_ioctl_integer_data(arg);
- gasket_log_debug(
- gasket_dev,
+ dev_dbg(gasket_dev->dev,
"Unknown ioctl cmd=0x%x not caught by "
- "gasket_is_supported_ioctl",
+ "gasket_is_supported_ioctl\n",
cmd);
retval = -EINVAL;
break;
@@ -186,12 +185,9 @@ static bool gasket_ioctl_check_permissions(struct file
*filp, uint cmd)
struct gasket_dev *gasket_dev = (struct gasket_dev *)filp->private_data;
alive = (gasket_dev->status == GASKET_STATUS_ALIVE);
- if (!alive) {
- gasket_nodev_error(
- "%s alive %d status %d.",
- __func__,
- alive, gasket_dev->status);
- }
+ if (!alive)
+ dev_dbg(gasket_dev->dev, "%s alive %d status %d\n",
+ __func__, alive, gasket_dev->status);
read = !!(filp->f_mode & FMODE_READ);
write = !!(filp->f_mode & FMODE_WRITE);
@@ -329,9 +325,8 @@ static int gasket_partition_page_table(
gasket_dev->page_table[ibuf.page_table_index]);
if (ibuf.size > max_page_table_size) {
- gasket_log_debug(
- gasket_dev,
- "Partition request 0x%llx too large, max is 0x%x.",
+ dev_dbg(gasket_dev->dev,
+ "Partition request 0x%llx too large, max is 0x%x\n",
ibuf.size, max_page_table_size);
return -EINVAL;
}
--
2.18.0.345.g5c9ce644c3-goog
[PATCH 07/10] staging: gasket: apex: convert to standard logging
From: Todd Poynor
Drop gasket logging calls in favor of standard logging.
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/apex_driver.c | 61
1 file changed, 26 insertions(+), 35 deletions(-)
diff --git a/drivers/staging/gasket/apex_driver.c
b/drivers/staging/gasket/apex_driver.c
index 6396b18b246a5..73fc2683a834d 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -7,11 +7,13 @@
#include
#include
+#include
#include
#include
#include
#include
#include
+#include
#include
#include
@@ -19,7 +21,6 @@
#include "gasket_core.h"
#include "gasket_interrupt.h"
-#include "gasket_logging.h"
#include "gasket_page_table.h"
#include "gasket_sysfs.h"
@@ -362,11 +363,9 @@ static int apex_add_dev_cb(struct gasket_dev *gasket_dev)
if (retries == APEX_RESET_RETRY) {
if (!page_table_ready)
- gasket_log_error(
- gasket_dev, "Page table init timed out.");
+ dev_err(gasket_dev->dev, "Page table init timed out\n");
if (!msix_table_ready)
- gasket_log_error(
- gasket_dev, "MSI-X table init timed out.");
+ dev_err(gasket_dev->dev, "MSI-X table init timed
out\n");
return -ETIMEDOUT;
}
@@ -420,12 +419,9 @@ static int apex_device_cleanup(struct gasket_dev
*gasket_dev)
gasket_dev, APEX_BAR_INDEX,
APEX_BAR2_REG_SCALAR_CORE_ERROR_STATUS);
- gasket_log_debug(
- gasket_dev,
- "%s 0x%p hib_error 0x%llx scalar_error "
- "0x%llx.",
- __func__,
- gasket_dev, hib_error, scalar_error);
+ dev_dbg(gasket_dev->dev,
+ "%s 0x%p hib_error 0x%llx scalar_error 0x%llx\n",
+ __func__, gasket_dev, hib_error, scalar_error);
if (allow_power_save)
ret = apex_enter_reset(gasket_dev, APEX_CHIP_REINIT_RESET);
@@ -452,7 +448,7 @@ static int apex_reset(struct gasket_dev *gasket_dev, uint
type)
/* We are not in reset - toggle the reset bit so as to force
* re-init of custom block
*/
- gasket_log_debug(gasket_dev, "%s: toggle reset.", __func__);
+ dev_dbg(gasket_dev->dev, "%s: toggle reset\n", __func__);
ret = apex_enter_reset(gasket_dev, type);
if (ret)
@@ -489,9 +485,9 @@ static int apex_enter_reset(struct gasket_dev *gasket_dev,
uint type)
if (gasket_wait_with_reschedule(gasket_dev, APEX_BAR_INDEX,
APEX_BAR2_REG_USER_HIB_DMA_PAUSED, 1, 1,
APEX_RESET_DELAY, APEX_RESET_RETRY)) {
- gasket_log_error(gasket_dev,
-"DMAs did not quiesce within timeout (%d ms)",
-APEX_RESET_RETRY * APEX_RESET_DELAY);
+ dev_err(gasket_dev->dev,
+ "DMAs did not quiesce within timeout (%d ms)\n",
+ APEX_RESET_RETRY * APEX_RESET_DELAY);
return -ETIMEDOUT;
}
@@ -511,9 +507,8 @@ static int apex_enter_reset(struct gasket_dev *gasket_dev,
uint type)
if (gasket_wait_with_reschedule(gasket_dev, APEX_BAR_INDEX,
APEX_BAR2_REG_SCU_3, 1 << 6, 1 << 6,
APEX_RESET_DELAY, APEX_RESET_RETRY)) {
- gasket_log_error(
- gasket_dev,
- "RAM did not shut down within timeout (%d ms)",
+ dev_err(gasket_dev->dev,
+ "RAM did not shut down within timeout (%d ms)\n",
APEX_RESET_RETRY * APEX_RESET_DELAY);
return -ETIMEDOUT;
}
@@ -560,9 +555,8 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev,
uint type)
if (gasket_wait_with_reschedule(gasket_dev, APEX_BAR_INDEX,
APEX_BAR2_REG_SCU_3, 1 << 6, 0,
APEX_RESET_DELAY, APEX_RESET_RETRY)) {
- gasket_log_error(
- gasket_dev,
- "RAM did not enable within timeout (%d ms)",
+ dev_err(gasket_dev->dev,
+ "RAM did not enable within timeout (%d ms)\n",
APEX_RESET_RETRY * APEX_RESET_DELAY);
return -ETIMEDOUT;
}
@@ -572,9 +566,8 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev,
uint type)
APEX_BAR2_REG_SCU_3,
SCU3_CUR_RST_GCB_BIT_MASK, 0,
APEX_RESET_DELAY, APEX_RESET_RETRY)) {
- gasket_log_error(
-
[PATCH 08/10] staging: gasket: remove gasket logging header
From: Todd Poynor
Gasket logging functions no longer used.
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/gasket_logging.h | 64 -
1 file changed, 64 deletions(-)
delete mode 100644 drivers/staging/gasket/gasket_logging.h
diff --git a/drivers/staging/gasket/gasket_logging.h
b/drivers/staging/gasket/gasket_logging.h
deleted file mode 100644
index 54bbe516b0716..0
--- a/drivers/staging/gasket/gasket_logging.h
+++ /dev/null
@@ -1,64 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Common logging utilities for the Gasket driver framework.
- *
- * Copyright (C) 2018 Google, Inc.
- */
-
-#include
-#include
-
-#ifndef _GASKET_LOGGING_H_
-#define _GASKET_LOGGING_H_
-
-/* Base macro; other logging can/should be built on top of this. */
-#define gasket_dev_log(level, device, pci_dev, format, arg...)
\
- if (false) { \
- if (pci_dev) { \
- dev_##level(&(pci_dev)->dev, "%s: " format "\n", \
- __func__, ##arg); \
- } else { \
- gasket_nodev_log(level, format, ##arg);\
- } \
- }
-
-/* "No-device" logging macros. */
-#define gasket_nodev_log(level, format, arg...)
\
- if (false) pr_##level("gasket: %s: " format "\n", __func__, ##arg)
-
-#define gasket_nodev_debug(format, arg...)
\
- if (false) gasket_nodev_log(debug, format, ##arg)
-
-#define gasket_nodev_info(format, arg...)
\
- if (false) gasket_nodev_log(info, format, ##arg)
-
-#define gasket_nodev_warn(format, arg...)
\
- if (false) gasket_nodev_log(warn, format, ##arg)
-
-#define gasket_nodev_error(format, arg...)
\
- if (false) gasket_nodev_log(err, format, ##arg)
-
-/* gasket_dev logging macros */
-#define gasket_log_info(gasket_dev, format, arg...)
\
- if (false) gasket_dev_log(info, (gasket_dev)->dev_info.device, \
- (gasket_dev)->pci_dev, format, ##arg)
-
-#define gasket_log_warn(gasket_dev, format, arg...)
\
- if (false) gasket_dev_log(warn, (gasket_dev)->dev_info.device, \
- (gasket_dev)->pci_dev, format, ##arg)
-
-#define gasket_log_error(gasket_dev, format, arg...)
\
- if (false) gasket_dev_log(err, (gasket_dev)->dev_info.device, \
- (gasket_dev)->pci_dev, format, ##arg)
-
-#define gasket_log_debug(gasket_dev, format, arg...)
\
- if (false){\
- if ((gasket_dev)->pci_dev) { \
- dev_dbg(&((gasket_dev)->pci_dev)->dev, "%s: " format \
- "\n", __func__, ##arg);\
- } else { \
- gasket_nodev_log(debug, format, ##arg);\
- } \
- }
-
-#endif
--
2.18.0.345.g5c9ce644c3-goog
[PATCH 06/10] staging: gasket: sysfs: convert to standard logging
From: Todd Poynor
Drop gasket logging calls in favor of standard logging.
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/gasket_sysfs.c | 73 +--
1 file changed, 35 insertions(+), 38 deletions(-)
diff --git a/drivers/staging/gasket/gasket_sysfs.c
b/drivers/staging/gasket/gasket_sysfs.c
index 1c5f7502e0d5e..418b81797e638 100644
--- a/drivers/staging/gasket/gasket_sysfs.c
+++ b/drivers/staging/gasket/gasket_sysfs.c
@@ -3,7 +3,9 @@
#include "gasket_sysfs.h"
#include "gasket_core.h"
-#include "gasket_logging.h"
+
+#include
+#include
/*
* Pair of kernel device and user-specified pointer. Used in lookups in sysfs
@@ -66,7 +68,7 @@ static struct gasket_sysfs_mapping *get_mapping(struct device
*device)
int i;
if (!device) {
- gasket_nodev_error("Received NULL device!");
+ pr_debug("%s: Received NULL device\n", __func__);
return NULL;
}
@@ -80,7 +82,8 @@ static struct gasket_sysfs_mapping *get_mapping(struct device
*device)
mutex_unlock(_mappings[i].mutex);
}
- gasket_nodev_info("Mapping to device %s not found.", device->kobj.name);
+ dev_dbg(device, "%s: Mapping to device %s not found\n",
+ __func__, device->kobj.name);
return NULL;
}
@@ -103,16 +106,15 @@ static void put_mapping(struct gasket_sysfs_mapping
*mapping)
struct device *device;
if (!mapping) {
- gasket_nodev_info("Mapping should not be NULL.");
+ pr_debug("%s: Mapping should not be NULL\n", __func__);
return;
}
mutex_lock(>mutex);
if (refcount_read(>refcount.refcount) == 0)
- gasket_nodev_error("Refcount is already 0!");
+ dev_err(mapping->device, "Refcount is already 0\n");
if (kref_put(>refcount, release_entry)) {
- gasket_nodev_info("Removing Gasket sysfs mapping, device %s",
- mapping->device->kobj.name);
+ dev_dbg(mapping->device, "Removing Gasket sysfs mapping\n");
/*
* We can't remove the sysfs nodes in the kref callback, since
* device_remove_file() blocks until the node is free.
@@ -182,16 +184,13 @@ int gasket_sysfs_create_mapping(
static DEFINE_MUTEX(function_mutex);
mutex_lock(_mutex);
-
- gasket_nodev_info(
- "Creating sysfs entries for device pointer 0x%p.", device);
+ dev_dbg(device, "Creating sysfs entries for device\n");
/* Check that the device we're adding hasn't already been added. */
mapping = get_mapping(device);
if (mapping) {
- gasket_nodev_error(
- "Attempting to re-initialize sysfs mapping for device "
- "0x%p.", device);
+ dev_err(device,
+ "Attempting to re-initialize sysfs mapping for
device\n");
put_mapping(mapping);
mutex_unlock(_mutex);
return -EBUSY;
@@ -207,13 +206,13 @@ int gasket_sysfs_create_mapping(
}
if (map_idx == GASKET_SYSFS_NUM_MAPPINGS) {
- gasket_nodev_error("All mappings have been exhausted!");
+ dev_err(device, "All mappings have been exhausted\n");
mutex_unlock(_mutex);
return -ENOMEM;
}
- gasket_nodev_info(
- "Creating sysfs mapping for device %s.", device->kobj.name);
+ dev_dbg(device, "Creating sysfs mapping for device %s\n",
+ device->kobj.name);
mapping = _mappings[map_idx];
kref_init(>refcount);
@@ -224,7 +223,7 @@ int gasket_sysfs_create_mapping(
GFP_KERNEL);
mapping->attribute_count = 0;
if (!mapping->attributes) {
- gasket_nodev_error("Unable to allocate sysfs attribute array.");
+ dev_dbg(device, "Unable to allocate sysfs attribute array\n");
mapping->device = NULL;
mapping->gasket_dev = NULL;
mutex_unlock(>mutex);
@@ -247,10 +246,9 @@ int gasket_sysfs_create_entries(
struct gasket_sysfs_mapping *mapping = get_mapping(device);
if (!mapping) {
- gasket_nodev_error(
- "Creating entries for device 0x%p without first "
- "initializing mapping.",
- device);
+ dev_dbg(device,
+ "Creating entries for device without first "
+ "initializing mapping\n");
return -EINVAL;
}
@@ -258,9 +256,9 @@ int gasket_sysfs_create_entries(
for (i = 0; strcmp(attrs[i].attr.attr.name, GASKET_ARRAY_END_MARKER);
i++) {
if (mapping->attribute_count == GASKET_SYSFS_MAX_NODES) {
-
[PATCH 08/10] staging: gasket: remove gasket logging header
From: Todd Poynor
Gasket logging functions no longer used.
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/gasket_logging.h | 64 -
1 file changed, 64 deletions(-)
delete mode 100644 drivers/staging/gasket/gasket_logging.h
diff --git a/drivers/staging/gasket/gasket_logging.h
b/drivers/staging/gasket/gasket_logging.h
deleted file mode 100644
index 54bbe516b0716..0
--- a/drivers/staging/gasket/gasket_logging.h
+++ /dev/null
@@ -1,64 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Common logging utilities for the Gasket driver framework.
- *
- * Copyright (C) 2018 Google, Inc.
- */
-
-#include
-#include
-
-#ifndef _GASKET_LOGGING_H_
-#define _GASKET_LOGGING_H_
-
-/* Base macro; other logging can/should be built on top of this. */
-#define gasket_dev_log(level, device, pci_dev, format, arg...)
\
- if (false) { \
- if (pci_dev) { \
- dev_##level(&(pci_dev)->dev, "%s: " format "\n", \
- __func__, ##arg); \
- } else { \
- gasket_nodev_log(level, format, ##arg);\
- } \
- }
-
-/* "No-device" logging macros. */
-#define gasket_nodev_log(level, format, arg...)
\
- if (false) pr_##level("gasket: %s: " format "\n", __func__, ##arg)
-
-#define gasket_nodev_debug(format, arg...)
\
- if (false) gasket_nodev_log(debug, format, ##arg)
-
-#define gasket_nodev_info(format, arg...)
\
- if (false) gasket_nodev_log(info, format, ##arg)
-
-#define gasket_nodev_warn(format, arg...)
\
- if (false) gasket_nodev_log(warn, format, ##arg)
-
-#define gasket_nodev_error(format, arg...)
\
- if (false) gasket_nodev_log(err, format, ##arg)
-
-/* gasket_dev logging macros */
-#define gasket_log_info(gasket_dev, format, arg...)
\
- if (false) gasket_dev_log(info, (gasket_dev)->dev_info.device, \
- (gasket_dev)->pci_dev, format, ##arg)
-
-#define gasket_log_warn(gasket_dev, format, arg...)
\
- if (false) gasket_dev_log(warn, (gasket_dev)->dev_info.device, \
- (gasket_dev)->pci_dev, format, ##arg)
-
-#define gasket_log_error(gasket_dev, format, arg...)
\
- if (false) gasket_dev_log(err, (gasket_dev)->dev_info.device, \
- (gasket_dev)->pci_dev, format, ##arg)
-
-#define gasket_log_debug(gasket_dev, format, arg...)
\
- if (false){\
- if ((gasket_dev)->pci_dev) { \
- dev_dbg(&((gasket_dev)->pci_dev)->dev, "%s: " format \
- "\n", __func__, ##arg);\
- } else { \
- gasket_nodev_log(debug, format, ##arg);\
- } \
- }
-
-#endif
--
2.18.0.345.g5c9ce644c3-goog
[PATCH 06/10] staging: gasket: sysfs: convert to standard logging
From: Todd Poynor
Drop gasket logging calls in favor of standard logging.
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/gasket_sysfs.c | 73 +--
1 file changed, 35 insertions(+), 38 deletions(-)
diff --git a/drivers/staging/gasket/gasket_sysfs.c
b/drivers/staging/gasket/gasket_sysfs.c
index 1c5f7502e0d5e..418b81797e638 100644
--- a/drivers/staging/gasket/gasket_sysfs.c
+++ b/drivers/staging/gasket/gasket_sysfs.c
@@ -3,7 +3,9 @@
#include "gasket_sysfs.h"
#include "gasket_core.h"
-#include "gasket_logging.h"
+
+#include
+#include
/*
* Pair of kernel device and user-specified pointer. Used in lookups in sysfs
@@ -66,7 +68,7 @@ static struct gasket_sysfs_mapping *get_mapping(struct device
*device)
int i;
if (!device) {
- gasket_nodev_error("Received NULL device!");
+ pr_debug("%s: Received NULL device\n", __func__);
return NULL;
}
@@ -80,7 +82,8 @@ static struct gasket_sysfs_mapping *get_mapping(struct device
*device)
mutex_unlock(_mappings[i].mutex);
}
- gasket_nodev_info("Mapping to device %s not found.", device->kobj.name);
+ dev_dbg(device, "%s: Mapping to device %s not found\n",
+ __func__, device->kobj.name);
return NULL;
}
@@ -103,16 +106,15 @@ static void put_mapping(struct gasket_sysfs_mapping
*mapping)
struct device *device;
if (!mapping) {
- gasket_nodev_info("Mapping should not be NULL.");
+ pr_debug("%s: Mapping should not be NULL\n", __func__);
return;
}
mutex_lock(>mutex);
if (refcount_read(>refcount.refcount) == 0)
- gasket_nodev_error("Refcount is already 0!");
+ dev_err(mapping->device, "Refcount is already 0\n");
if (kref_put(>refcount, release_entry)) {
- gasket_nodev_info("Removing Gasket sysfs mapping, device %s",
- mapping->device->kobj.name);
+ dev_dbg(mapping->device, "Removing Gasket sysfs mapping\n");
/*
* We can't remove the sysfs nodes in the kref callback, since
* device_remove_file() blocks until the node is free.
@@ -182,16 +184,13 @@ int gasket_sysfs_create_mapping(
static DEFINE_MUTEX(function_mutex);
mutex_lock(_mutex);
-
- gasket_nodev_info(
- "Creating sysfs entries for device pointer 0x%p.", device);
+ dev_dbg(device, "Creating sysfs entries for device\n");
/* Check that the device we're adding hasn't already been added. */
mapping = get_mapping(device);
if (mapping) {
- gasket_nodev_error(
- "Attempting to re-initialize sysfs mapping for device "
- "0x%p.", device);
+ dev_err(device,
+ "Attempting to re-initialize sysfs mapping for
device\n");
put_mapping(mapping);
mutex_unlock(_mutex);
return -EBUSY;
@@ -207,13 +206,13 @@ int gasket_sysfs_create_mapping(
}
if (map_idx == GASKET_SYSFS_NUM_MAPPINGS) {
- gasket_nodev_error("All mappings have been exhausted!");
+ dev_err(device, "All mappings have been exhausted\n");
mutex_unlock(_mutex);
return -ENOMEM;
}
- gasket_nodev_info(
- "Creating sysfs mapping for device %s.", device->kobj.name);
+ dev_dbg(device, "Creating sysfs mapping for device %s\n",
+ device->kobj.name);
mapping = _mappings[map_idx];
kref_init(>refcount);
@@ -224,7 +223,7 @@ int gasket_sysfs_create_mapping(
GFP_KERNEL);
mapping->attribute_count = 0;
if (!mapping->attributes) {
- gasket_nodev_error("Unable to allocate sysfs attribute array.");
+ dev_dbg(device, "Unable to allocate sysfs attribute array\n");
mapping->device = NULL;
mapping->gasket_dev = NULL;
mutex_unlock(>mutex);
@@ -247,10 +246,9 @@ int gasket_sysfs_create_entries(
struct gasket_sysfs_mapping *mapping = get_mapping(device);
if (!mapping) {
- gasket_nodev_error(
- "Creating entries for device 0x%p without first "
- "initializing mapping.",
- device);
+ dev_dbg(device,
+ "Creating entries for device without first "
+ "initializing mapping\n");
return -EINVAL;
}
@@ -258,9 +256,9 @@ int gasket_sysfs_create_entries(
for (i = 0; strcmp(attrs[i].attr.attr.name, GASKET_ARRAY_END_MARKER);
i++) {
if (mapping->attribute_count == GASKET_SYSFS_MAX_NODES) {
-
[PATCH 09/10] staging: gasket: TODO: remove entry for convert to standard logging
From: Todd Poynor Gasket/apex drivers now use standard logging, remove TODO entry for this. Signed-off-by: Todd Poynor --- drivers/staging/gasket/TODO | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/gasket/TODO b/drivers/staging/gasket/TODO index d3c44ca4fda25..fb71997fb5612 100644 --- a/drivers/staging/gasket/TODO +++ b/drivers/staging/gasket/TODO @@ -4,7 +4,6 @@ staging directory. - Document sysfs files with Documentation/ABI/ entries. - Use misc interface instead of major number for driver version description. - Add descriptions of module_param's -- Remove gasket-specific logging functions. - apex_get_status() should actually check status. - Static functions don't need kernel doc formatting, can be simplified. - Fix multi-line alignment formatting to look like: -- 2.18.0.345.g5c9ce644c3-goog
[PATCH 09/10] staging: gasket: TODO: remove entry for convert to standard logging
From: Todd Poynor Gasket/apex drivers now use standard logging, remove TODO entry for this. Signed-off-by: Todd Poynor --- drivers/staging/gasket/TODO | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/gasket/TODO b/drivers/staging/gasket/TODO index d3c44ca4fda25..fb71997fb5612 100644 --- a/drivers/staging/gasket/TODO +++ b/drivers/staging/gasket/TODO @@ -4,7 +4,6 @@ staging directory. - Document sysfs files with Documentation/ABI/ entries. - Use misc interface instead of major number for driver version description. - Add descriptions of module_param's -- Remove gasket-specific logging functions. - apex_get_status() should actually check status. - Static functions don't need kernel doc formatting, can be simplified. - Fix multi-line alignment formatting to look like: -- 2.18.0.345.g5c9ce644c3-goog
[PATCH 02/10] staging: gasket: core: convert to standard logging
From: Todd Poynor
Use standard logging functions, drop use of gasket log functions.
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/gasket_core.c | 295 ---
1 file changed, 134 insertions(+), 161 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c
b/drivers/staging/gasket/gasket_core.c
index e8f3b021c20d1..f44805c38159b 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -10,15 +10,16 @@
#include "gasket_interrupt.h"
#include "gasket_ioctl.h"
-#include "gasket_logging.h"
#include "gasket_page_table.h"
#include "gasket_sysfs.h"
#include
#include
+#include
#include
#include
#include
+#include
#ifdef GASKET_KERNEL_TRACE_SUPPORT
#define CREATE_TRACE_POINTS
@@ -205,8 +206,8 @@ static inline int check_and_invoke_callback(
{
int ret = 0;
- gasket_log_debug(gasket_dev, "check_and_invoke_callback %p",
-cb_function);
+ dev_dbg(gasket_dev->dev, "check_and_invoke_callback %p\n",
+ cb_function);
if (cb_function) {
mutex_lock(_dev->mutex);
ret = cb_function(gasket_dev);
@@ -228,8 +229,8 @@ static inline int gasket_check_and_invoke_callback_nolock(
int ret = 0;
if (cb_function) {
- gasket_log_debug(
- gasket_dev, "Invoking device-specific callback.");
+ dev_dbg(gasket_dev->dev,
+ "Invoking device-specific callback.\n");
ret = cb_function(gasket_dev);
}
return ret;
@@ -250,7 +251,7 @@ static int __init gasket_init(void)
{
int i;
- gasket_nodev_info("Performing one-time init of the Gasket framework.");
+ pr_info("Performing one-time init of the Gasket framework.\n");
/* Check for duplicates and find a free slot. */
mutex_lock(_mutex);
for (i = 0; i < GASKET_FRAMEWORK_DESC_MAX; i++) {
@@ -267,7 +268,7 @@ static int __init gasket_init(void)
static void __exit gasket_exit(void)
{
/* No deinit/dealloc needed at present. */
- gasket_nodev_info("Removing Gasket framework module.");
+ pr_info("Removing Gasket framework module.\n");
}
/* See gasket_core.h for description. */
@@ -277,15 +278,14 @@ int gasket_register_device(const struct
gasket_driver_desc *driver_desc)
int desc_idx = -1;
struct gasket_internal_desc *internal;
- gasket_nodev_info("Initializing Gasket framework device");
+ pr_info("Initializing Gasket framework device\n");
/* Check for duplicates and find a free slot. */
mutex_lock(_mutex);
for (i = 0; i < GASKET_FRAMEWORK_DESC_MAX; i++) {
if (g_descs[i].driver_desc == driver_desc) {
- gasket_nodev_error(
- "%s driver already loaded/registered",
- driver_desc->name);
+ pr_err("%s driver already loaded/registered\n",
+ driver_desc->name);
mutex_unlock(_mutex);
return -EBUSY;
}
@@ -301,17 +301,17 @@ int gasket_register_device(const struct
gasket_driver_desc *driver_desc)
}
mutex_unlock(_mutex);
- gasket_nodev_info("Loaded %s driver, framework version %s",
- driver_desc->name, GASKET_FRAMEWORK_VERSION);
+ pr_info("Loaded %s driver, framework version %s\n",
+ driver_desc->name, GASKET_FRAMEWORK_VERSION);
if (desc_idx == -1) {
- gasket_nodev_error("Too many Gasket drivers loaded: %d\n",
- GASKET_FRAMEWORK_DESC_MAX);
+ pr_err("Too many Gasket drivers loaded: %d\n",
+ GASKET_FRAMEWORK_DESC_MAX);
return -EBUSY;
}
/* Internal structure setup. */
- gasket_nodev_info("Performing initial internal structure setup.");
+ pr_debug("Performing initial internal structure setup.\n");
internal = _descs[desc_idx];
mutex_init(>mutex);
memset(internal->devs, 0, sizeof(struct gasket_dev *) * GASKET_DEV_MAX);
@@ -324,8 +324,8 @@ int gasket_register_device(const struct gasket_driver_desc
*driver_desc)
class_create(driver_desc->module, driver_desc->name);
if (IS_ERR(internal->class)) {
- gasket_nodev_error("Cannot register %s class [ret=%ld]",
- driver_desc->name, PTR_ERR(internal->class));
+ pr_err("Cannot register %s class [ret=%ld]\n",
+ driver_desc->name, PTR_ERR(internal->class));
ret = PTR_ERR(internal->class);
goto unregister_gasket_driver;
}
@@ -334,25 +334,24 @@ int gasket_register_device(const struct
gasket_driver_desc *driver_desc)
* Not using pci_register_driver() (without
[PATCH 01/10] staging: gasket: save struct device for a gasket device
From: Todd Poynor
Save the struct device pointer to a gasket device in gasket's metadata,
to facilitate use of standard logging calls and in anticipation of
non-PCI gasket devices in the future.
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/gasket_core.c | 5 +++--
drivers/staging/gasket/gasket_core.h | 3 +++
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c
b/drivers/staging/gasket/gasket_core.c
index 732218773c3c6..e8f3b021c20d1 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -450,6 +450,7 @@ static int gasket_alloc_dev(
gasket_dev->internal_desc = internal_desc;
gasket_dev->dev_idx = dev_idx;
snprintf(gasket_dev->kobj_name, GASKET_NAME_MAX, "%s", kobj_name);
+ gasket_dev->dev = parent;
/* gasket_bar_data is uninitialized. */
gasket_dev->num_page_tables = driver_desc->num_page_tables;
/* max_page_table_size and *page table are uninit'ed */
@@ -925,7 +926,7 @@ static int gasket_enable_dev(
_dev->bar_data[
driver_desc->page_table_bar_index],
_desc->page_table_configs[tbl_idx],
- _dev->pci_dev->dev, gasket_dev->pci_dev, true);
+ gasket_dev->dev, gasket_dev->pci_dev, true);
if (ret) {
gasket_log_error(
gasket_dev,
@@ -2028,7 +2029,7 @@ const struct gasket_driver_desc
*gasket_get_driver_desc(struct gasket_dev *dev)
*/
struct device *gasket_get_device(struct gasket_dev *dev)
{
- return >pci_dev->dev;
+ return dev->dev;
}
/**
diff --git a/drivers/staging/gasket/gasket_core.h
b/drivers/staging/gasket/gasket_core.h
index bf4ed3769efb2..8bd431ad3b58b 100644
--- a/drivers/staging/gasket/gasket_core.h
+++ b/drivers/staging/gasket/gasket_core.h
@@ -263,6 +263,9 @@ struct gasket_dev {
/* Pointer to the internal driver description for this device. */
struct gasket_internal_desc *internal_desc;
+ /* Device info */
+ struct device *dev;
+
/* PCI subsystem metadata. */
struct pci_dev *pci_dev;
--
2.18.0.345.g5c9ce644c3-goog
[PATCH 02/10] staging: gasket: core: convert to standard logging
From: Todd Poynor
Use standard logging functions, drop use of gasket log functions.
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/gasket_core.c | 295 ---
1 file changed, 134 insertions(+), 161 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c
b/drivers/staging/gasket/gasket_core.c
index e8f3b021c20d1..f44805c38159b 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -10,15 +10,16 @@
#include "gasket_interrupt.h"
#include "gasket_ioctl.h"
-#include "gasket_logging.h"
#include "gasket_page_table.h"
#include "gasket_sysfs.h"
#include
#include
+#include
#include
#include
#include
+#include
#ifdef GASKET_KERNEL_TRACE_SUPPORT
#define CREATE_TRACE_POINTS
@@ -205,8 +206,8 @@ static inline int check_and_invoke_callback(
{
int ret = 0;
- gasket_log_debug(gasket_dev, "check_and_invoke_callback %p",
-cb_function);
+ dev_dbg(gasket_dev->dev, "check_and_invoke_callback %p\n",
+ cb_function);
if (cb_function) {
mutex_lock(_dev->mutex);
ret = cb_function(gasket_dev);
@@ -228,8 +229,8 @@ static inline int gasket_check_and_invoke_callback_nolock(
int ret = 0;
if (cb_function) {
- gasket_log_debug(
- gasket_dev, "Invoking device-specific callback.");
+ dev_dbg(gasket_dev->dev,
+ "Invoking device-specific callback.\n");
ret = cb_function(gasket_dev);
}
return ret;
@@ -250,7 +251,7 @@ static int __init gasket_init(void)
{
int i;
- gasket_nodev_info("Performing one-time init of the Gasket framework.");
+ pr_info("Performing one-time init of the Gasket framework.\n");
/* Check for duplicates and find a free slot. */
mutex_lock(_mutex);
for (i = 0; i < GASKET_FRAMEWORK_DESC_MAX; i++) {
@@ -267,7 +268,7 @@ static int __init gasket_init(void)
static void __exit gasket_exit(void)
{
/* No deinit/dealloc needed at present. */
- gasket_nodev_info("Removing Gasket framework module.");
+ pr_info("Removing Gasket framework module.\n");
}
/* See gasket_core.h for description. */
@@ -277,15 +278,14 @@ int gasket_register_device(const struct
gasket_driver_desc *driver_desc)
int desc_idx = -1;
struct gasket_internal_desc *internal;
- gasket_nodev_info("Initializing Gasket framework device");
+ pr_info("Initializing Gasket framework device\n");
/* Check for duplicates and find a free slot. */
mutex_lock(_mutex);
for (i = 0; i < GASKET_FRAMEWORK_DESC_MAX; i++) {
if (g_descs[i].driver_desc == driver_desc) {
- gasket_nodev_error(
- "%s driver already loaded/registered",
- driver_desc->name);
+ pr_err("%s driver already loaded/registered\n",
+ driver_desc->name);
mutex_unlock(_mutex);
return -EBUSY;
}
@@ -301,17 +301,17 @@ int gasket_register_device(const struct
gasket_driver_desc *driver_desc)
}
mutex_unlock(_mutex);
- gasket_nodev_info("Loaded %s driver, framework version %s",
- driver_desc->name, GASKET_FRAMEWORK_VERSION);
+ pr_info("Loaded %s driver, framework version %s\n",
+ driver_desc->name, GASKET_FRAMEWORK_VERSION);
if (desc_idx == -1) {
- gasket_nodev_error("Too many Gasket drivers loaded: %d\n",
- GASKET_FRAMEWORK_DESC_MAX);
+ pr_err("Too many Gasket drivers loaded: %d\n",
+ GASKET_FRAMEWORK_DESC_MAX);
return -EBUSY;
}
/* Internal structure setup. */
- gasket_nodev_info("Performing initial internal structure setup.");
+ pr_debug("Performing initial internal structure setup.\n");
internal = _descs[desc_idx];
mutex_init(>mutex);
memset(internal->devs, 0, sizeof(struct gasket_dev *) * GASKET_DEV_MAX);
@@ -324,8 +324,8 @@ int gasket_register_device(const struct gasket_driver_desc
*driver_desc)
class_create(driver_desc->module, driver_desc->name);
if (IS_ERR(internal->class)) {
- gasket_nodev_error("Cannot register %s class [ret=%ld]",
- driver_desc->name, PTR_ERR(internal->class));
+ pr_err("Cannot register %s class [ret=%ld]\n",
+ driver_desc->name, PTR_ERR(internal->class));
ret = PTR_ERR(internal->class);
goto unregister_gasket_driver;
}
@@ -334,25 +334,24 @@ int gasket_register_device(const struct
gasket_driver_desc *driver_desc)
* Not using pci_register_driver() (without
[PATCH 01/10] staging: gasket: save struct device for a gasket device
From: Todd Poynor
Save the struct device pointer to a gasket device in gasket's metadata,
to facilitate use of standard logging calls and in anticipation of
non-PCI gasket devices in the future.
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/gasket_core.c | 5 +++--
drivers/staging/gasket/gasket_core.h | 3 +++
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c
b/drivers/staging/gasket/gasket_core.c
index 732218773c3c6..e8f3b021c20d1 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -450,6 +450,7 @@ static int gasket_alloc_dev(
gasket_dev->internal_desc = internal_desc;
gasket_dev->dev_idx = dev_idx;
snprintf(gasket_dev->kobj_name, GASKET_NAME_MAX, "%s", kobj_name);
+ gasket_dev->dev = parent;
/* gasket_bar_data is uninitialized. */
gasket_dev->num_page_tables = driver_desc->num_page_tables;
/* max_page_table_size and *page table are uninit'ed */
@@ -925,7 +926,7 @@ static int gasket_enable_dev(
_dev->bar_data[
driver_desc->page_table_bar_index],
_desc->page_table_configs[tbl_idx],
- _dev->pci_dev->dev, gasket_dev->pci_dev, true);
+ gasket_dev->dev, gasket_dev->pci_dev, true);
if (ret) {
gasket_log_error(
gasket_dev,
@@ -2028,7 +2029,7 @@ const struct gasket_driver_desc
*gasket_get_driver_desc(struct gasket_dev *dev)
*/
struct device *gasket_get_device(struct gasket_dev *dev)
{
- return >pci_dev->dev;
+ return dev->dev;
}
/**
diff --git a/drivers/staging/gasket/gasket_core.h
b/drivers/staging/gasket/gasket_core.h
index bf4ed3769efb2..8bd431ad3b58b 100644
--- a/drivers/staging/gasket/gasket_core.h
+++ b/drivers/staging/gasket/gasket_core.h
@@ -263,6 +263,9 @@ struct gasket_dev {
/* Pointer to the internal driver description for this device. */
struct gasket_internal_desc *internal_desc;
+ /* Device info */
+ struct device *dev;
+
/* PCI subsystem metadata. */
struct pci_dev *pci_dev;
--
2.18.0.345.g5c9ce644c3-goog
[PATCH] ASoC: uniphier: change functions to static
This patch changes some functions that are not used by other objects
to static.
Signed-off-by: Katsuhiro Suzuki
---
sound/soc/uniphier/aio-core.c | 6 +++---
sound/soc/uniphier/aio.h | 3 ---
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/sound/soc/uniphier/aio-core.c b/sound/soc/uniphier/aio-core.c
index 8b09bbb0f8d0..9bcba06ba52e 100644
--- a/sound/soc/uniphier/aio-core.c
+++ b/sound/soc/uniphier/aio-core.c
@@ -327,7 +327,7 @@ static int aio_port_set_ch(struct uniphier_aio_sub *sub)
*
* Return: Zero if successful, otherwise a negative value on error.
*/
-int aio_port_set_rate(struct uniphier_aio_sub *sub, int rate)
+static int aio_port_set_rate(struct uniphier_aio_sub *sub, int rate)
{
struct regmap *r = sub->aio->chip->regmap;
struct device *dev = >aio->chip->pdev->dev;
@@ -446,7 +446,7 @@ int aio_port_set_rate(struct uniphier_aio_sub *sub, int
rate)
*
* Return: Zero if successful, otherwise a negative value on error.
*/
-int aio_port_set_fmt(struct uniphier_aio_sub *sub)
+static int aio_port_set_fmt(struct uniphier_aio_sub *sub)
{
struct regmap *r = sub->aio->chip->regmap;
struct device *dev = >aio->chip->pdev->dev;
@@ -511,7 +511,7 @@ int aio_port_set_fmt(struct uniphier_aio_sub *sub)
*
* Return: Zero if successful, otherwise a negative value on error.
*/
-int aio_port_set_clk(struct uniphier_aio_sub *sub)
+static int aio_port_set_clk(struct uniphier_aio_sub *sub)
{
struct uniphier_aio_chip *chip = sub->aio->chip;
struct device *dev = >aio->chip->pdev->dev;
diff --git a/sound/soc/uniphier/aio.h b/sound/soc/uniphier/aio.h
index 23a5c3c68658..ca6ccbae0ee8 100644
--- a/sound/soc/uniphier/aio.h
+++ b/sound/soc/uniphier/aio.h
@@ -325,9 +325,6 @@ int aio_chip_set_pll(struct uniphier_aio_chip *chip, int
pll_id,
void aio_chip_init(struct uniphier_aio_chip *chip);
int aio_init(struct uniphier_aio_sub *sub);
void aio_port_reset(struct uniphier_aio_sub *sub);
-int aio_port_set_rate(struct uniphier_aio_sub *sub, int rate);
-int aio_port_set_fmt(struct uniphier_aio_sub *sub);
-int aio_port_set_clk(struct uniphier_aio_sub *sub);
int aio_port_set_param(struct uniphier_aio_sub *sub, int pass_through,
const struct snd_pcm_hw_params *params);
void aio_port_set_enable(struct uniphier_aio_sub *sub, int enable);
--
2.18.0
[PATCH] ASoC: uniphier: add support for multichannel output
This patch adds multichannel PCM output support for LD11/LD20.
Currently driver tested and supported only 2ch, 6ch, and 8ch.
Signed-off-by: Katsuhiro Suzuki
---
sound/soc/uniphier/aio-core.c | 78 ---
sound/soc/uniphier/aio-ld11.c | 2 +-
sound/soc/uniphier/aio-reg.h | 1 +
sound/soc/uniphier/aio.h | 3 ++
4 files changed, 78 insertions(+), 6 deletions(-)
diff --git a/sound/soc/uniphier/aio-core.c b/sound/soc/uniphier/aio-core.c
index 638cb3fc5f7b..8b09bbb0f8d0 100644
--- a/sound/soc/uniphier/aio-core.c
+++ b/sound/soc/uniphier/aio-core.c
@@ -264,6 +264,57 @@ void aio_port_reset(struct uniphier_aio_sub *sub)
}
}
+/**
+ * aio_port_set_ch - set channels of LPCM
+ * @sub: the AIO substream pointer, PCM substream only
+ * @ch : count of channels
+ *
+ * Set suitable slot selecting to input/output port block of AIO.
+ *
+ * This function may return error if non-PCM substream.
+ *
+ * Return: Zero if successful, otherwise a negative value on error.
+ */
+static int aio_port_set_ch(struct uniphier_aio_sub *sub)
+{
+ struct regmap *r = sub->aio->chip->regmap;
+ u32 slotsel_2ch[] = {
+ 0, 0, 0, 0, 0,
+ };
+ u32 slotsel_multi[] = {
+ OPORTMXTYSLOTCTR_SLOTSEL_SLOT0,
+ OPORTMXTYSLOTCTR_SLOTSEL_SLOT1,
+ OPORTMXTYSLOTCTR_SLOTSEL_SLOT2,
+ OPORTMXTYSLOTCTR_SLOTSEL_SLOT3,
+ OPORTMXTYSLOTCTR_SLOTSEL_SLOT4,
+ };
+ u32 mode, *slotsel;
+ int i;
+
+ switch (params_channels(>params)) {
+ case 8:
+ case 6:
+ mode = OPORTMXTYSLOTCTR_MODE;
+ slotsel = slotsel_multi;
+ break;
+ case 2:
+ mode = 0;
+ slotsel = slotsel_2ch;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ for (i = 0; i < AUD_MAX_SLOTSEL; i++) {
+ regmap_update_bits(r, OPORTMXTYSLOTCTR(sub->swm->oport.map, i),
+ OPORTMXTYSLOTCTR_MODE, mode);
+ regmap_update_bits(r, OPORTMXTYSLOTCTR(sub->swm->oport.map, i),
+ OPORTMXTYSLOTCTR_SLOTSEL_MASK, slotsel[i]);
+ }
+
+ return 0;
+}
+
/**
* aio_port_set_rate - set sampling rate of LPCM
* @sub: the AIO substream pointer, PCM substream only
@@ -575,6 +626,10 @@ int aio_port_set_param(struct uniphier_aio_sub *sub, int
pass_through,
rate = params_rate(params);
}
+ ret = aio_port_set_ch(sub);
+ if (ret)
+ return ret;
+
ret = aio_port_set_rate(sub, rate);
if (ret)
return ret;
@@ -731,15 +786,28 @@ void aio_port_set_volume(struct uniphier_aio_sub *sub,
int vol)
int aio_if_set_param(struct uniphier_aio_sub *sub, int pass_through)
{
struct regmap *r = sub->aio->chip->regmap;
- u32 v;
+ u32 memfmt, v;
if (sub->swm->dir == PORT_DIR_OUTPUT) {
- if (pass_through)
+ if (pass_through) {
v = PBOUTMXCTR0_ENDIAN_0123 |
PBOUTMXCTR0_MEMFMT_STREAM;
- else
- v = PBOUTMXCTR0_ENDIAN_3210 |
- PBOUTMXCTR0_MEMFMT_2CH;
+ } else {
+ switch (params_channels(>params)) {
+ case 2:
+ memfmt = PBOUTMXCTR0_MEMFMT_2CH;
+ break;
+ case 6:
+ memfmt = PBOUTMXCTR0_MEMFMT_6CH;
+ break;
+ case 8:
+ memfmt = PBOUTMXCTR0_MEMFMT_8CH;
+ break;
+ default:
+ return -EINVAL;
+ }
+ v = PBOUTMXCTR0_ENDIAN_3210 | memfmt;
+ }
regmap_write(r, PBOUTMXCTR0(sub->swm->oif.map), v);
regmap_write(r, PBOUTMXCTR1(sub->swm->oif.map), 0);
diff --git a/sound/soc/uniphier/aio-ld11.c b/sound/soc/uniphier/aio-ld11.c
index ab04d3331be9..de962df245ba 100644
--- a/sound/soc/uniphier/aio-ld11.c
+++ b/sound/soc/uniphier/aio-ld11.c
@@ -286,7 +286,7 @@ static struct snd_soc_dai_driver uniphier_aio_dai_ld11[] = {
.formats = SNDRV_PCM_FMTBIT_S32_LE,
.rates = SNDRV_PCM_RATE_48000,
.channels_min = 2,
- .channels_max = 2,
+ .channels_max = 8,
},
.ops = _aio_i2s_ops,
},
diff --git a/sound/soc/uniphier/aio-reg.h b/sound/soc/uniphier/aio-reg.h
index 45fdc6ae358a..734395dbcffb 100644
--- a/sound/soc/uniphier/aio-reg.h
+++ b/sound/soc/uniphier/aio-reg.h
@@ -374,6 +374,7 @@
#define
[PATCH] ASoC: uniphier: change functions to static
This patch changes some functions that are not used by other objects
to static.
Signed-off-by: Katsuhiro Suzuki
---
sound/soc/uniphier/aio-core.c | 6 +++---
sound/soc/uniphier/aio.h | 3 ---
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/sound/soc/uniphier/aio-core.c b/sound/soc/uniphier/aio-core.c
index 8b09bbb0f8d0..9bcba06ba52e 100644
--- a/sound/soc/uniphier/aio-core.c
+++ b/sound/soc/uniphier/aio-core.c
@@ -327,7 +327,7 @@ static int aio_port_set_ch(struct uniphier_aio_sub *sub)
*
* Return: Zero if successful, otherwise a negative value on error.
*/
-int aio_port_set_rate(struct uniphier_aio_sub *sub, int rate)
+static int aio_port_set_rate(struct uniphier_aio_sub *sub, int rate)
{
struct regmap *r = sub->aio->chip->regmap;
struct device *dev = >aio->chip->pdev->dev;
@@ -446,7 +446,7 @@ int aio_port_set_rate(struct uniphier_aio_sub *sub, int
rate)
*
* Return: Zero if successful, otherwise a negative value on error.
*/
-int aio_port_set_fmt(struct uniphier_aio_sub *sub)
+static int aio_port_set_fmt(struct uniphier_aio_sub *sub)
{
struct regmap *r = sub->aio->chip->regmap;
struct device *dev = >aio->chip->pdev->dev;
@@ -511,7 +511,7 @@ int aio_port_set_fmt(struct uniphier_aio_sub *sub)
*
* Return: Zero if successful, otherwise a negative value on error.
*/
-int aio_port_set_clk(struct uniphier_aio_sub *sub)
+static int aio_port_set_clk(struct uniphier_aio_sub *sub)
{
struct uniphier_aio_chip *chip = sub->aio->chip;
struct device *dev = >aio->chip->pdev->dev;
diff --git a/sound/soc/uniphier/aio.h b/sound/soc/uniphier/aio.h
index 23a5c3c68658..ca6ccbae0ee8 100644
--- a/sound/soc/uniphier/aio.h
+++ b/sound/soc/uniphier/aio.h
@@ -325,9 +325,6 @@ int aio_chip_set_pll(struct uniphier_aio_chip *chip, int
pll_id,
void aio_chip_init(struct uniphier_aio_chip *chip);
int aio_init(struct uniphier_aio_sub *sub);
void aio_port_reset(struct uniphier_aio_sub *sub);
-int aio_port_set_rate(struct uniphier_aio_sub *sub, int rate);
-int aio_port_set_fmt(struct uniphier_aio_sub *sub);
-int aio_port_set_clk(struct uniphier_aio_sub *sub);
int aio_port_set_param(struct uniphier_aio_sub *sub, int pass_through,
const struct snd_pcm_hw_params *params);
void aio_port_set_enable(struct uniphier_aio_sub *sub, int enable);
--
2.18.0
[PATCH] ASoC: uniphier: add support for multichannel output
This patch adds multichannel PCM output support for LD11/LD20.
Currently driver tested and supported only 2ch, 6ch, and 8ch.
Signed-off-by: Katsuhiro Suzuki
---
sound/soc/uniphier/aio-core.c | 78 ---
sound/soc/uniphier/aio-ld11.c | 2 +-
sound/soc/uniphier/aio-reg.h | 1 +
sound/soc/uniphier/aio.h | 3 ++
4 files changed, 78 insertions(+), 6 deletions(-)
diff --git a/sound/soc/uniphier/aio-core.c b/sound/soc/uniphier/aio-core.c
index 638cb3fc5f7b..8b09bbb0f8d0 100644
--- a/sound/soc/uniphier/aio-core.c
+++ b/sound/soc/uniphier/aio-core.c
@@ -264,6 +264,57 @@ void aio_port_reset(struct uniphier_aio_sub *sub)
}
}
+/**
+ * aio_port_set_ch - set channels of LPCM
+ * @sub: the AIO substream pointer, PCM substream only
+ * @ch : count of channels
+ *
+ * Set suitable slot selecting to input/output port block of AIO.
+ *
+ * This function may return error if non-PCM substream.
+ *
+ * Return: Zero if successful, otherwise a negative value on error.
+ */
+static int aio_port_set_ch(struct uniphier_aio_sub *sub)
+{
+ struct regmap *r = sub->aio->chip->regmap;
+ u32 slotsel_2ch[] = {
+ 0, 0, 0, 0, 0,
+ };
+ u32 slotsel_multi[] = {
+ OPORTMXTYSLOTCTR_SLOTSEL_SLOT0,
+ OPORTMXTYSLOTCTR_SLOTSEL_SLOT1,
+ OPORTMXTYSLOTCTR_SLOTSEL_SLOT2,
+ OPORTMXTYSLOTCTR_SLOTSEL_SLOT3,
+ OPORTMXTYSLOTCTR_SLOTSEL_SLOT4,
+ };
+ u32 mode, *slotsel;
+ int i;
+
+ switch (params_channels(>params)) {
+ case 8:
+ case 6:
+ mode = OPORTMXTYSLOTCTR_MODE;
+ slotsel = slotsel_multi;
+ break;
+ case 2:
+ mode = 0;
+ slotsel = slotsel_2ch;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ for (i = 0; i < AUD_MAX_SLOTSEL; i++) {
+ regmap_update_bits(r, OPORTMXTYSLOTCTR(sub->swm->oport.map, i),
+ OPORTMXTYSLOTCTR_MODE, mode);
+ regmap_update_bits(r, OPORTMXTYSLOTCTR(sub->swm->oport.map, i),
+ OPORTMXTYSLOTCTR_SLOTSEL_MASK, slotsel[i]);
+ }
+
+ return 0;
+}
+
/**
* aio_port_set_rate - set sampling rate of LPCM
* @sub: the AIO substream pointer, PCM substream only
@@ -575,6 +626,10 @@ int aio_port_set_param(struct uniphier_aio_sub *sub, int
pass_through,
rate = params_rate(params);
}
+ ret = aio_port_set_ch(sub);
+ if (ret)
+ return ret;
+
ret = aio_port_set_rate(sub, rate);
if (ret)
return ret;
@@ -731,15 +786,28 @@ void aio_port_set_volume(struct uniphier_aio_sub *sub,
int vol)
int aio_if_set_param(struct uniphier_aio_sub *sub, int pass_through)
{
struct regmap *r = sub->aio->chip->regmap;
- u32 v;
+ u32 memfmt, v;
if (sub->swm->dir == PORT_DIR_OUTPUT) {
- if (pass_through)
+ if (pass_through) {
v = PBOUTMXCTR0_ENDIAN_0123 |
PBOUTMXCTR0_MEMFMT_STREAM;
- else
- v = PBOUTMXCTR0_ENDIAN_3210 |
- PBOUTMXCTR0_MEMFMT_2CH;
+ } else {
+ switch (params_channels(>params)) {
+ case 2:
+ memfmt = PBOUTMXCTR0_MEMFMT_2CH;
+ break;
+ case 6:
+ memfmt = PBOUTMXCTR0_MEMFMT_6CH;
+ break;
+ case 8:
+ memfmt = PBOUTMXCTR0_MEMFMT_8CH;
+ break;
+ default:
+ return -EINVAL;
+ }
+ v = PBOUTMXCTR0_ENDIAN_3210 | memfmt;
+ }
regmap_write(r, PBOUTMXCTR0(sub->swm->oif.map), v);
regmap_write(r, PBOUTMXCTR1(sub->swm->oif.map), 0);
diff --git a/sound/soc/uniphier/aio-ld11.c b/sound/soc/uniphier/aio-ld11.c
index ab04d3331be9..de962df245ba 100644
--- a/sound/soc/uniphier/aio-ld11.c
+++ b/sound/soc/uniphier/aio-ld11.c
@@ -286,7 +286,7 @@ static struct snd_soc_dai_driver uniphier_aio_dai_ld11[] = {
.formats = SNDRV_PCM_FMTBIT_S32_LE,
.rates = SNDRV_PCM_RATE_48000,
.channels_min = 2,
- .channels_max = 2,
+ .channels_max = 8,
},
.ops = _aio_i2s_ops,
},
diff --git a/sound/soc/uniphier/aio-reg.h b/sound/soc/uniphier/aio-reg.h
index 45fdc6ae358a..734395dbcffb 100644
--- a/sound/soc/uniphier/aio-reg.h
+++ b/sound/soc/uniphier/aio-reg.h
@@ -374,6 +374,7 @@
#define
[PATCH] input: touchscreen: wdt87xx_i2c: Replace mdelay() with msleep() in wdt87xx_resume()
wdt87xx_resume() is never called in atomic context. It calls mdelay() to busily wait, which is not necessary. mdelay() can be replaced with msleep(). This is found by a static analysis tool named DCNS written by myself. Signed-off-by: Jia-Ju Bai --- drivers/input/touchscreen/wdt87xx_i2c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/input/touchscreen/wdt87xx_i2c.c b/drivers/input/touchscreen/wdt87xx_i2c.c index 20f7f3902757..166edeb6 100644 --- a/drivers/input/touchscreen/wdt87xx_i2c.c +++ b/drivers/input/touchscreen/wdt87xx_i2c.c @@ -1142,7 +1142,7 @@ static int __maybe_unused wdt87xx_resume(struct device *dev) * The chip may have been reset while system is resuming, * give it some time to settle. */ - mdelay(100); + msleep(100); error = wdt87xx_send_command(client, VND_CMD_START, 0); if (error) -- 2.17.0
[PATCH] input: touchscreen: wdt87xx_i2c: Replace mdelay() with msleep() in wdt87xx_resume()
wdt87xx_resume() is never called in atomic context. It calls mdelay() to busily wait, which is not necessary. mdelay() can be replaced with msleep(). This is found by a static analysis tool named DCNS written by myself. Signed-off-by: Jia-Ju Bai --- drivers/input/touchscreen/wdt87xx_i2c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/input/touchscreen/wdt87xx_i2c.c b/drivers/input/touchscreen/wdt87xx_i2c.c index 20f7f3902757..166edeb6 100644 --- a/drivers/input/touchscreen/wdt87xx_i2c.c +++ b/drivers/input/touchscreen/wdt87xx_i2c.c @@ -1142,7 +1142,7 @@ static int __maybe_unused wdt87xx_resume(struct device *dev) * The chip may have been reset while system is resuming, * give it some time to settle. */ - mdelay(100); + msleep(100); error = wdt87xx_send_command(client, VND_CMD_START, 0); if (error) -- 2.17.0
[PATCH] input: tablet: aiptek: Replace GFP_ATOMIC with GFP_KERNEL in aiptek_probe()
aiptek_probe() is never called in atomic context.
It calls usb_alloc_coherent() with GFP_ATOMIC, which is not necessary.
GFP_ATOMIC can be replaced with GFP_KERNEL.
This is found by a static analysis tool named DCNS written by myself.
Signed-off-by: Jia-Ju Bai
---
drivers/input/tablet/aiptek.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/tablet/aiptek.c b/drivers/input/tablet/aiptek.c
index 545fa6e89035..c82cd5079d0e 100644
--- a/drivers/input/tablet/aiptek.c
+++ b/drivers/input/tablet/aiptek.c
@@ -1712,7 +1712,7 @@ aiptek_probe(struct usb_interface *intf, const struct
usb_device_id *id)
}
aiptek->data = usb_alloc_coherent(usbdev, AIPTEK_PACKET_LENGTH,
- GFP_ATOMIC, >data_dma);
+ GFP_KERNEL, >data_dma);
if (!aiptek->data) {
dev_warn(>dev, "cannot allocate usb buffer\n");
goto fail1;
--
2.17.0
[PATCH] input: tablet: aiptek: Replace GFP_ATOMIC with GFP_KERNEL in aiptek_probe()
aiptek_probe() is never called in atomic context.
It calls usb_alloc_coherent() with GFP_ATOMIC, which is not necessary.
GFP_ATOMIC can be replaced with GFP_KERNEL.
This is found by a static analysis tool named DCNS written by myself.
Signed-off-by: Jia-Ju Bai
---
drivers/input/tablet/aiptek.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/tablet/aiptek.c b/drivers/input/tablet/aiptek.c
index 545fa6e89035..c82cd5079d0e 100644
--- a/drivers/input/tablet/aiptek.c
+++ b/drivers/input/tablet/aiptek.c
@@ -1712,7 +1712,7 @@ aiptek_probe(struct usb_interface *intf, const struct
usb_device_id *id)
}
aiptek->data = usb_alloc_coherent(usbdev, AIPTEK_PACKET_LENGTH,
- GFP_ATOMIC, >data_dma);
+ GFP_KERNEL, >data_dma);
if (!aiptek->data) {
dev_warn(>dev, "cannot allocate usb buffer\n");
goto fail1;
--
2.17.0
[PATCH] input: mouse: appletouch: Replace GFP_ATOMIC with GFP_KERNEL
atp_open(), atp_recover() and atp_resume() are never
called in atomic context.
They call usb_submit_urb() with GFP_ATOMIC, which is not necessary.
GFP_ATOMIC can be replaced with GFP_KERNEL.
This is found by a static analysis tool named DCNS written by myself.
Signed-off-by: Jia-Ju Bai
---
drivers/input/mouse/appletouch.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 032d27983b6c..f593ec96c95f 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -810,7 +810,7 @@ static int atp_open(struct input_dev *input)
{
struct atp *dev = input_get_drvdata(input);
- if (usb_submit_urb(dev->urb, GFP_ATOMIC))
+ if (usb_submit_urb(dev->urb, GFP_KERNEL))
return -EIO;
dev->open = true;
@@ -976,7 +976,7 @@ static int atp_recover(struct atp *dev)
if (error)
return error;
- if (dev->open && usb_submit_urb(dev->urb, GFP_ATOMIC))
+ if (dev->open && usb_submit_urb(dev->urb, GFP_KERNEL))
return -EIO;
return 0;
@@ -994,7 +994,7 @@ static int atp_resume(struct usb_interface *iface)
{
struct atp *dev = usb_get_intfdata(iface);
- if (dev->open && usb_submit_urb(dev->urb, GFP_ATOMIC))
+ if (dev->open && usb_submit_urb(dev->urb, GFP_KERNEL))
return -EIO;
return 0;
--
2.17.0
[PATCH] input: mouse: appletouch: Replace GFP_ATOMIC with GFP_KERNEL
atp_open(), atp_recover() and atp_resume() are never
called in atomic context.
They call usb_submit_urb() with GFP_ATOMIC, which is not necessary.
GFP_ATOMIC can be replaced with GFP_KERNEL.
This is found by a static analysis tool named DCNS written by myself.
Signed-off-by: Jia-Ju Bai
---
drivers/input/mouse/appletouch.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 032d27983b6c..f593ec96c95f 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -810,7 +810,7 @@ static int atp_open(struct input_dev *input)
{
struct atp *dev = input_get_drvdata(input);
- if (usb_submit_urb(dev->urb, GFP_ATOMIC))
+ if (usb_submit_urb(dev->urb, GFP_KERNEL))
return -EIO;
dev->open = true;
@@ -976,7 +976,7 @@ static int atp_recover(struct atp *dev)
if (error)
return error;
- if (dev->open && usb_submit_urb(dev->urb, GFP_ATOMIC))
+ if (dev->open && usb_submit_urb(dev->urb, GFP_KERNEL))
return -EIO;
return 0;
@@ -994,7 +994,7 @@ static int atp_resume(struct usb_interface *iface)
{
struct atp *dev = usb_get_intfdata(iface);
- if (dev->open && usb_submit_urb(dev->urb, GFP_ATOMIC))
+ if (dev->open && usb_submit_urb(dev->urb, GFP_KERNEL))
return -EIO;
return 0;
--
2.17.0
[PATCH] input: misc: yealink: Replace GFP_ATOMIC with GFP_KERNEL in usb_probe()
usb_probe() is never called in atomic context. It calls usb_alloc_coherent() with GFP_ATOMIC, which is not necessary. GFP_ATOMIC can be replaced with GFP_KERNEL. This is found by a static analysis tool named DCNS written by myself. Signed-off-by: Jia-Ju Bai --- drivers/input/misc/yealink.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/input/misc/yealink.c b/drivers/input/misc/yealink.c index f0c9bf87b4e3..1365cd94ed9b 100644 --- a/drivers/input/misc/yealink.c +++ b/drivers/input/misc/yealink.c @@ -894,12 +894,12 @@ static int usb_probe(struct usb_interface *intf, const struct usb_device_id *id) /* allocate usb buffers */ yld->irq_data = usb_alloc_coherent(udev, USB_PKT_LEN, - GFP_ATOMIC, >irq_dma); + GFP_KERNEL, >irq_dma); if (yld->irq_data == NULL) return usb_cleanup(yld, -ENOMEM); yld->ctl_data = usb_alloc_coherent(udev, USB_PKT_LEN, - GFP_ATOMIC, >ctl_dma); + GFP_KERNEL, >ctl_dma); if (!yld->ctl_data) return usb_cleanup(yld, -ENOMEM); -- 2.17.0
[PATCH] input: misc: yealink: Replace GFP_ATOMIC with GFP_KERNEL in usb_probe()
usb_probe() is never called in atomic context. It calls usb_alloc_coherent() with GFP_ATOMIC, which is not necessary. GFP_ATOMIC can be replaced with GFP_KERNEL. This is found by a static analysis tool named DCNS written by myself. Signed-off-by: Jia-Ju Bai --- drivers/input/misc/yealink.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/input/misc/yealink.c b/drivers/input/misc/yealink.c index f0c9bf87b4e3..1365cd94ed9b 100644 --- a/drivers/input/misc/yealink.c +++ b/drivers/input/misc/yealink.c @@ -894,12 +894,12 @@ static int usb_probe(struct usb_interface *intf, const struct usb_device_id *id) /* allocate usb buffers */ yld->irq_data = usb_alloc_coherent(udev, USB_PKT_LEN, - GFP_ATOMIC, >irq_dma); + GFP_KERNEL, >irq_dma); if (yld->irq_data == NULL) return usb_cleanup(yld, -ENOMEM); yld->ctl_data = usb_alloc_coherent(udev, USB_PKT_LEN, - GFP_ATOMIC, >ctl_dma); + GFP_KERNEL, >ctl_dma); if (!yld->ctl_data) return usb_cleanup(yld, -ENOMEM); -- 2.17.0
[PATCH] input: misc: powermate: Replace GFP_ATOMIC with GFP_KERNEL in powermate_alloc_buffers()
powermate_alloc_buffers() is never called in atomic context.
It calls usb_alloc_coherent() with GFP_ATOMIC, which is not necessary.
GFP_ATOMIC can be replaced with GFP_KERNEL.
This is found by a static analysis tool named DCNS written by myself.
Signed-off-by: Jia-Ju Bai
---
drivers/input/misc/powermate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/misc/powermate.c b/drivers/input/misc/powermate.c
index 5c8c79623c87..e8de3aaf9f63 100644
--- a/drivers/input/misc/powermate.c
+++ b/drivers/input/misc/powermate.c
@@ -277,7 +277,7 @@ static int powermate_input_event(struct input_dev *dev,
unsigned int type, unsig
static int powermate_alloc_buffers(struct usb_device *udev, struct
powermate_device *pm)
{
pm->data = usb_alloc_coherent(udev, POWERMATE_PAYLOAD_SIZE_MAX,
- GFP_ATOMIC, >data_dma);
+ GFP_KERNEL, >data_dma);
if (!pm->data)
return -1;
--
2.17.0
[PATCH] input: misc: powermate: Replace GFP_ATOMIC with GFP_KERNEL in powermate_alloc_buffers()
powermate_alloc_buffers() is never called in atomic context.
It calls usb_alloc_coherent() with GFP_ATOMIC, which is not necessary.
GFP_ATOMIC can be replaced with GFP_KERNEL.
This is found by a static analysis tool named DCNS written by myself.
Signed-off-by: Jia-Ju Bai
---
drivers/input/misc/powermate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/misc/powermate.c b/drivers/input/misc/powermate.c
index 5c8c79623c87..e8de3aaf9f63 100644
--- a/drivers/input/misc/powermate.c
+++ b/drivers/input/misc/powermate.c
@@ -277,7 +277,7 @@ static int powermate_input_event(struct input_dev *dev,
unsigned int type, unsig
static int powermate_alloc_buffers(struct usb_device *udev, struct
powermate_device *pm)
{
pm->data = usb_alloc_coherent(udev, POWERMATE_PAYLOAD_SIZE_MAX,
- GFP_ATOMIC, >data_dma);
+ GFP_KERNEL, >data_dma);
if (!pm->data)
return -1;
--
2.17.0
[PATCH] input: misc: keyspan_remote: Replace GFP_ATOMIC with GFP_KERNEL in keyspan_probe()
keyspan_probe() is never called in atomic context.
It calls usb_alloc_coherent() with GFP_ATOMIC, which is not necessary.
GFP_ATOMIC can be replace with GFP_KERNEL.
This is found by a static analysis tool named DCNS written by myself.
Signed-off-by: Jia-Ju Bai
---
drivers/input/misc/keyspan_remote.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/misc/keyspan_remote.c
b/drivers/input/misc/keyspan_remote.c
index 67482b248b2d..a8937ceac66a 100644
--- a/drivers/input/misc/keyspan_remote.c
+++ b/drivers/input/misc/keyspan_remote.c
@@ -466,7 +466,7 @@ static int keyspan_probe(struct usb_interface *interface,
const struct usb_devic
remote->in_endpoint = endpoint;
remote->toggle = -1;/* Set to -1 so we will always not match the
toggle from the first remote message. */
- remote->in_buffer = usb_alloc_coherent(udev, RECV_SIZE, GFP_ATOMIC,
>in_dma);
+ remote->in_buffer = usb_alloc_coherent(udev, RECV_SIZE, GFP_KERNEL,
>in_dma);
if (!remote->in_buffer) {
error = -ENOMEM;
goto fail1;
--
2.17.0
[PATCH] input: misc: keyspan_remote: Replace GFP_ATOMIC with GFP_KERNEL in keyspan_probe()
keyspan_probe() is never called in atomic context.
It calls usb_alloc_coherent() with GFP_ATOMIC, which is not necessary.
GFP_ATOMIC can be replace with GFP_KERNEL.
This is found by a static analysis tool named DCNS written by myself.
Signed-off-by: Jia-Ju Bai
---
drivers/input/misc/keyspan_remote.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/misc/keyspan_remote.c
b/drivers/input/misc/keyspan_remote.c
index 67482b248b2d..a8937ceac66a 100644
--- a/drivers/input/misc/keyspan_remote.c
+++ b/drivers/input/misc/keyspan_remote.c
@@ -466,7 +466,7 @@ static int keyspan_probe(struct usb_interface *interface,
const struct usb_devic
remote->in_endpoint = endpoint;
remote->toggle = -1;/* Set to -1 so we will always not match the
toggle from the first remote message. */
- remote->in_buffer = usb_alloc_coherent(udev, RECV_SIZE, GFP_ATOMIC,
>in_dma);
+ remote->in_buffer = usb_alloc_coherent(udev, RECV_SIZE, GFP_KERNEL,
>in_dma);
if (!remote->in_buffer) {
error = -ENOMEM;
goto fail1;
--
2.17.0
Re: [PATCH] linux/bitmap.h: fix BITMAP_LAST_WORD_MASK
On 07/26/2018 08:10 PM, Yury Norov wrote:
On Thu, Jul 26, 2018 at 06:15:59PM +0800, Wei Wang wrote:
External Email
On 07/26/2018 05:37 PM, Yury Norov wrote:
On Thu, Jul 26, 2018 at 04:07:51PM +0800, Wei Wang wrote:
The existing BITMAP_LAST_WORD_MASK macro returns 0x if nbits is
0. This patch changes the macro to return 0 when there is no bit needs to
be masked.
I think this is intentional behavour. Previous version did return ~0UL
explicitly in this case. See patch 89c1e79eb3023 (linux/bitmap.h: improve
BITMAP_{LAST,FIRST}_WORD_MASK) from Rasmus.
Yes, I saw that. But it seems confusing for the corner case that nbits=0
(no bits to mask), the macro returns with all the bits set.
Introducing conditional branch would affect performance. All existing
code checks nbits for 0 before handling last word where needed
explicitly. So I think we'd better change nothing here.
I think that didn't save the conditional branch essentially, because
it's just moved from inside this macro to the caller as you mentioned.
If callers missed the check for some reason and passed 0 to the macro,
they will get something unexpected.
Current callers like __bitmap_weight, __bitmap_equal, and others, they have
if (bits % BITS_PER_LONG)
w += hweight_long(bitmap[k] & BITMAP_LAST_WORD_MASK(bits));
we could remove the "if" check by "w += hweight_long(bitmap[k] &
BITMAP_LAST_WORD_MASK(bits % BITS_PER_LONG));" the branch is the same.
But your patch doesn't remove external conditional, and it fact
introduces overhead, right? Also, in some cases it's not so trivial to
remove it. Consider __bitmap_intersects() for example.
Anyway, this patch changes the very basic API. In that case you should
check every user of the macro to be safe against the change, including
possible performance downsides.
If you find this corner case behavior of macro confusing, I think that
the better option would be introducing detailed comment to the
BITMAP_LAST_WORD_MASK(), or writing wrapper around it that handles
nbits == 0 as you expect.
OK. Thanks Yury and Andy for the discussion. It seems the more preferred
way is just to add comments as a note. Agree with that.
Best,
Wei
Re: [PATCH] linux/bitmap.h: fix BITMAP_LAST_WORD_MASK
On 07/26/2018 08:10 PM, Yury Norov wrote:
On Thu, Jul 26, 2018 at 06:15:59PM +0800, Wei Wang wrote:
External Email
On 07/26/2018 05:37 PM, Yury Norov wrote:
On Thu, Jul 26, 2018 at 04:07:51PM +0800, Wei Wang wrote:
The existing BITMAP_LAST_WORD_MASK macro returns 0x if nbits is
0. This patch changes the macro to return 0 when there is no bit needs to
be masked.
I think this is intentional behavour. Previous version did return ~0UL
explicitly in this case. See patch 89c1e79eb3023 (linux/bitmap.h: improve
BITMAP_{LAST,FIRST}_WORD_MASK) from Rasmus.
Yes, I saw that. But it seems confusing for the corner case that nbits=0
(no bits to mask), the macro returns with all the bits set.
Introducing conditional branch would affect performance. All existing
code checks nbits for 0 before handling last word where needed
explicitly. So I think we'd better change nothing here.
I think that didn't save the conditional branch essentially, because
it's just moved from inside this macro to the caller as you mentioned.
If callers missed the check for some reason and passed 0 to the macro,
they will get something unexpected.
Current callers like __bitmap_weight, __bitmap_equal, and others, they have
if (bits % BITS_PER_LONG)
w += hweight_long(bitmap[k] & BITMAP_LAST_WORD_MASK(bits));
we could remove the "if" check by "w += hweight_long(bitmap[k] &
BITMAP_LAST_WORD_MASK(bits % BITS_PER_LONG));" the branch is the same.
But your patch doesn't remove external conditional, and it fact
introduces overhead, right? Also, in some cases it's not so trivial to
remove it. Consider __bitmap_intersects() for example.
Anyway, this patch changes the very basic API. In that case you should
check every user of the macro to be safe against the change, including
possible performance downsides.
If you find this corner case behavior of macro confusing, I think that
the better option would be introducing detailed comment to the
BITMAP_LAST_WORD_MASK(), or writing wrapper around it that handles
nbits == 0 as you expect.
OK. Thanks Yury and Andy for the discussion. It seems the more preferred
way is just to add comments as a note. Agree with that.
Best,
Wei
RE: [PATCH v8 1/6] ARM: imx6q: provide documentation for new fsl,pmic-stby-poweroff property
> -Original Message- > From: Oleksij Rempel [mailto:o.rem...@pengutronix.de] > Sent: 2018年7月26日 19:38 > To: Robin Gong ; Shawn Guo ; > Mark Brown ; Rafael J. Wysocki > > Cc: ker...@pengutronix.de; devicet...@vger.kernel.org; > linux-arm-ker...@lists.infradead.org; linux-...@vger.kernel.org; > linux-kernel@vger.kernel.org; Andrew Morton ; > Liam Girdwood ; Leonard Crestez > ; Rob Herring ; Mark > Rutland ; Michael Turquette > ; Stephen Boyd ; Fabio > Estevam ; Russell King ; > dl-linux-imx ; A.s. Dong > Subject: Re: [PATCH v8 1/6] ARM: imx6q: provide documentation for new > fsl,pmic-stby-poweroff property > > Hi, > > On 26.07.2018 11:51, Robin Gong wrote: > > > > > >> -Original Message- > >> From: Oleksij Rempel [mailto:o.rem...@pengutronix.de] > >> Sent: 2018年7月26日 17:22 > >> To: Shawn Guo ; Mark Brown > ; > >> Rafael J. Wysocki > >> Cc: Oleksij Rempel ; ker...@pengutronix.de; > >> devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; > >> linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; Andrew > >> Morton ; Liam Girdwood > >> ; Leonard Crestez ; > Rob > >> Herring ; Mark Rutland ; > >> Michael Turquette ; Stephen Boyd > >> ; Fabio Estevam ; > >> Russell King ; dl-linux-imx > >> ; Robin Gong ; A.s. Dong > >> > >> Subject: [PATCH v8 1/6] ARM: imx6q: provide documentation for new > >> fsl,pmic-stby-poweroff property > >> > >> Signed-off-by: Oleksij Rempel > >> Acked-by: Rob Herring > >> --- > >> Documentation/devicetree/bindings/clock/imx6q-clock.txt | 8 > >> 1 file changed, 8 insertions(+) > >> > >> diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt > >> b/Documentation/devicetree/bindings/clock/imx6q-clock.txt > >> index a45ca67a9d5f..e1308346e00d 100644 > >> --- a/Documentation/devicetree/bindings/clock/imx6q-clock.txt > >> +++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt > >> @@ -6,6 +6,14 @@ Required properties: > >> - interrupts: Should contain CCM interrupt > >> - #clock-cells: Should be <1> > >> > >> +Optional properties: > >> +- fsl,pmic-stby-poweroff: Configure CCM to assert PMIC_STBY_REQ > >> +signal > >> + on power off. > >> + Use this property if the SoC should be powered off by external > >> +power > >> + management IC (PMIC) triggered via PMIC_STBY_REQ signal. > > PMIC_ON_REQ didn't connect to any pin of PMIC in your case? > > No. First, it was only one customer specific issue. After some research I > found > even publicly available boards (for example RioTboard) which has same/similar > design. After seeing this in imx6 documentation as valid power off way, I have > no doubts - there should be even more devices doin this in the wild. Not sure why the customer didn't follow reference design, since PMIC_ON_REQ can provide power off/on feature by pressing ONOFF key which connected ONOFF pin of i.mx6(ONOFF will toggle PMIC_ON_REQ to power off/on PMIC). The official power off/on way (PMIC_ON_REQ) can also power off all power rails of PFUZE except snvs as your patch did on PMIC_STBY_REQ. PMIC_STBY_REQ is used to notify pmic switch power mode (PFM/APS) or decrease voltage to save power in kernel suspend, not power offI am not sure if we need this patchset to 'workaround' the board issue. > > > Don't understand > > why not follow normal board design guide to power off pmic by > PMIC_ON_REQ. > > How to power on board again then? > > Power cycle. Without this patch, power of is not real power off. So, power > cycle, > is expected behavior for user interaction. On usual PC, reset button will not > enable PC as well. Your board can't support wakeup by RTC alarm if not use PMIC_ON_REQ to power off. Again, why your board not follow the design guide?
RE: [PATCH v8 1/6] ARM: imx6q: provide documentation for new fsl,pmic-stby-poweroff property
> -Original Message- > From: Oleksij Rempel [mailto:o.rem...@pengutronix.de] > Sent: 2018年7月26日 19:38 > To: Robin Gong ; Shawn Guo ; > Mark Brown ; Rafael J. Wysocki > > Cc: ker...@pengutronix.de; devicet...@vger.kernel.org; > linux-arm-ker...@lists.infradead.org; linux-...@vger.kernel.org; > linux-kernel@vger.kernel.org; Andrew Morton ; > Liam Girdwood ; Leonard Crestez > ; Rob Herring ; Mark > Rutland ; Michael Turquette > ; Stephen Boyd ; Fabio > Estevam ; Russell King ; > dl-linux-imx ; A.s. Dong > Subject: Re: [PATCH v8 1/6] ARM: imx6q: provide documentation for new > fsl,pmic-stby-poweroff property > > Hi, > > On 26.07.2018 11:51, Robin Gong wrote: > > > > > >> -Original Message- > >> From: Oleksij Rempel [mailto:o.rem...@pengutronix.de] > >> Sent: 2018年7月26日 17:22 > >> To: Shawn Guo ; Mark Brown > ; > >> Rafael J. Wysocki > >> Cc: Oleksij Rempel ; ker...@pengutronix.de; > >> devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; > >> linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; Andrew > >> Morton ; Liam Girdwood > >> ; Leonard Crestez ; > Rob > >> Herring ; Mark Rutland ; > >> Michael Turquette ; Stephen Boyd > >> ; Fabio Estevam ; > >> Russell King ; dl-linux-imx > >> ; Robin Gong ; A.s. Dong > >> > >> Subject: [PATCH v8 1/6] ARM: imx6q: provide documentation for new > >> fsl,pmic-stby-poweroff property > >> > >> Signed-off-by: Oleksij Rempel > >> Acked-by: Rob Herring > >> --- > >> Documentation/devicetree/bindings/clock/imx6q-clock.txt | 8 > >> 1 file changed, 8 insertions(+) > >> > >> diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt > >> b/Documentation/devicetree/bindings/clock/imx6q-clock.txt > >> index a45ca67a9d5f..e1308346e00d 100644 > >> --- a/Documentation/devicetree/bindings/clock/imx6q-clock.txt > >> +++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt > >> @@ -6,6 +6,14 @@ Required properties: > >> - interrupts: Should contain CCM interrupt > >> - #clock-cells: Should be <1> > >> > >> +Optional properties: > >> +- fsl,pmic-stby-poweroff: Configure CCM to assert PMIC_STBY_REQ > >> +signal > >> + on power off. > >> + Use this property if the SoC should be powered off by external > >> +power > >> + management IC (PMIC) triggered via PMIC_STBY_REQ signal. > > PMIC_ON_REQ didn't connect to any pin of PMIC in your case? > > No. First, it was only one customer specific issue. After some research I > found > even publicly available boards (for example RioTboard) which has same/similar > design. After seeing this in imx6 documentation as valid power off way, I have > no doubts - there should be even more devices doin this in the wild. Not sure why the customer didn't follow reference design, since PMIC_ON_REQ can provide power off/on feature by pressing ONOFF key which connected ONOFF pin of i.mx6(ONOFF will toggle PMIC_ON_REQ to power off/on PMIC). The official power off/on way (PMIC_ON_REQ) can also power off all power rails of PFUZE except snvs as your patch did on PMIC_STBY_REQ. PMIC_STBY_REQ is used to notify pmic switch power mode (PFM/APS) or decrease voltage to save power in kernel suspend, not power offI am not sure if we need this patchset to 'workaround' the board issue. > > > Don't understand > > why not follow normal board design guide to power off pmic by > PMIC_ON_REQ. > > How to power on board again then? > > Power cycle. Without this patch, power of is not real power off. So, power > cycle, > is expected behavior for user interaction. On usual PC, reset button will not > enable PC as well. Your board can't support wakeup by RTC alarm if not use PMIC_ON_REQ to power off. Again, why your board not follow the design guide?
linux-next: build failure after merge of the v4l-dvb tree
Hi Mauro,
After merging the v4l-dvb tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:
fs/compat_ioctl.c: In function 'do_video_set_spu_palette':
fs/compat_ioctl.c:218:45: error: invalid application of 'sizeof' to incomplete
type 'struct video_spu_palette'
up_native = compat_alloc_user_space(sizeof(struct video_spu_palette));
^~
In file included from include/linux/uaccess.h:14:0,
from include/linux/compat.h:19,
from fs/compat_ioctl.c:17:
fs/compat_ioctl.c:219:46: error: dereferencing pointer to incomplete type
'struct video_spu_palette'
err = put_user(compat_ptr(palp), _native->palette);
^
At top level:
fs/compat_ioctl.c:206:12: warning: 'do_video_set_spu_palette' defined but not
used [-Wunused-function]
static int do_video_set_spu_palette(struct file *file,
^~~~
Caused by commit
f863ce990030 ("media: dvb: get rid of VIDEO_SET_SPU_PALETTE")
grep is your friend ...
I have reverted that commit for today.
--
Cheers,
Stephen Rothwell
pgpz9D4vMW0yA.pgp
Description: OpenPGP digital signature
linux-next: build failure after merge of the v4l-dvb tree
Hi Mauro,
After merging the v4l-dvb tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:
fs/compat_ioctl.c: In function 'do_video_set_spu_palette':
fs/compat_ioctl.c:218:45: error: invalid application of 'sizeof' to incomplete
type 'struct video_spu_palette'
up_native = compat_alloc_user_space(sizeof(struct video_spu_palette));
^~
In file included from include/linux/uaccess.h:14:0,
from include/linux/compat.h:19,
from fs/compat_ioctl.c:17:
fs/compat_ioctl.c:219:46: error: dereferencing pointer to incomplete type
'struct video_spu_palette'
err = put_user(compat_ptr(palp), _native->palette);
^
At top level:
fs/compat_ioctl.c:206:12: warning: 'do_video_set_spu_palette' defined but not
used [-Wunused-function]
static int do_video_set_spu_palette(struct file *file,
^~~~
Caused by commit
f863ce990030 ("media: dvb: get rid of VIDEO_SET_SPU_PALETTE")
grep is your friend ...
I have reverted that commit for today.
--
Cheers,
Stephen Rothwell
pgpz9D4vMW0yA.pgp
Description: OpenPGP digital signature
Re: include architecture Kconfig files from top-level Kconfig v3
2018-07-27 2:21 GMT+09:00 Christoph Hellwig : > On Wed, Jul 25, 2018 at 01:30:24PM +0900, Masahiro Yamada wrote: >> Could you check the difference of CONFIG_PREEMPT_COUNT, please? >> >> >> For alpha, hexagon, um, >> CONFIG_PREEMPT_COUNT was previously never enabled >> because kernel/Kconfig.preempt was not included. >> >> Now, CONFIG_PREEMPT_COUNT can be enabled >> if it is select'ed by someone. >> >> I guess this change will be OK, but >> I'd like you and Randy to double-check it just in case. > > It might be fine, but with this little fix folded in we should be > safer: > > diff --git a/kernel/Kconfig.preempt b/kernel/Kconfig.preempt > index cd1655122ec0..1321a4a7a677 100644 > --- a/kernel/Kconfig.preempt > +++ b/kernel/Kconfig.preempt > @@ -57,4 +57,5 @@ config PREEMPT > endchoice > > config PREEMPT_COUNT > - bool > \ No newline at end of file > + depends on !ARCH_NO_PREEMPT > + bool > -- > To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html This will just add a new unmet dependency warning. CONFIG_PREEMPT_COUNT will be still selected. $ make ARCH=alpha allyesconfig HOSTCC scripts/basic/fixdep HOSTCC scripts/kconfig/conf.o YACCscripts/kconfig/zconf.tab.c LEX scripts/kconfig/zconf.lex.c HOSTCC scripts/kconfig/zconf.tab.o HOSTLD scripts/kconfig/conf scripts/kconfig/conf --allyesconfig Kconfig WARNING: unmet direct dependencies detected for PREEMPT_COUNT Depends on [n]: !ARCH_NO_PREEMPT [=y] Selected by [y]: - DEBUG_ATOMIC_SLEEP [=y] && DEBUG_KERNEL [=y] # # configuration written to .config # -- Best Regards Masahiro Yamada
Re: include architecture Kconfig files from top-level Kconfig v3
2018-07-27 2:21 GMT+09:00 Christoph Hellwig : > On Wed, Jul 25, 2018 at 01:30:24PM +0900, Masahiro Yamada wrote: >> Could you check the difference of CONFIG_PREEMPT_COUNT, please? >> >> >> For alpha, hexagon, um, >> CONFIG_PREEMPT_COUNT was previously never enabled >> because kernel/Kconfig.preempt was not included. >> >> Now, CONFIG_PREEMPT_COUNT can be enabled >> if it is select'ed by someone. >> >> I guess this change will be OK, but >> I'd like you and Randy to double-check it just in case. > > It might be fine, but with this little fix folded in we should be > safer: > > diff --git a/kernel/Kconfig.preempt b/kernel/Kconfig.preempt > index cd1655122ec0..1321a4a7a677 100644 > --- a/kernel/Kconfig.preempt > +++ b/kernel/Kconfig.preempt > @@ -57,4 +57,5 @@ config PREEMPT > endchoice > > config PREEMPT_COUNT > - bool > \ No newline at end of file > + depends on !ARCH_NO_PREEMPT > + bool > -- > To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html This will just add a new unmet dependency warning. CONFIG_PREEMPT_COUNT will be still selected. $ make ARCH=alpha allyesconfig HOSTCC scripts/basic/fixdep HOSTCC scripts/kconfig/conf.o YACCscripts/kconfig/zconf.tab.c LEX scripts/kconfig/zconf.lex.c HOSTCC scripts/kconfig/zconf.tab.o HOSTLD scripts/kconfig/conf scripts/kconfig/conf --allyesconfig Kconfig WARNING: unmet direct dependencies detected for PREEMPT_COUNT Depends on [n]: !ARCH_NO_PREEMPT [=y] Selected by [y]: - DEBUG_ATOMIC_SLEEP [=y] && DEBUG_KERNEL [=y] # # configuration written to .config # -- Best Regards Masahiro Yamada
Re: [PATCH] checkpatch: Warn when a patch doesn't have a description
On Thu, 2018-07-26 at 15:08 -0700, Andrew Morton wrote:
> On Wed, 25 Jul 2018 19:22:47 -0700 Joe Perches wrote:
>
> > Potential patches should have a commit description.
> > Emit a warning when there isn't one.
[]
> I did this:
>
> ---
> a/scripts/checkpatch.pl~checkpatch-warn-when-a-patch-doesnt-have-a-description-fix
> +++ a/scripts/checkpatch.pl
> @@ -2517,7 +2517,7 @@ sub process {
> if ($line !~ /^\s*$/) {
> $commit_log_lines++;#could be a $signature
> }
> - } else if ($has_commit_log && $commit_log_lines < 2) {
> + } elsif ($has_commit_log && $commit_log_lines < 2) {
> WARN("COMMIT_MESSAGE",
>"Missing commit description - Add an appropriate
> one\n");
> $commit_log_lines = 2; #warn only once
>
> But I worry that you didn't send out the version which you tested, so
> please check.
You're right, I inserted the wrong one.
Anyway, what you did is correct and that was the only change.
Thanks.
$ diff -urN cp_commit_log_lines.diff cp_commit_message.diff
--- cp_commit_log_lines.diff2018-07-13 17:06:29.115337477 -0700
+++ cp_commit_message.diff 2018-07-15 05:46:29.878805336 -0700
@@ -2,7 +2,7 @@
1 file changed, 13 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
-index b5c875d7132b..8b5f3dae31c9 100755
+index b5c875d7132b..4ccf84079ea4 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2240,6 +2240,7 @@ sub process {
@@ -23,7 +23,7 @@
+ if ($line !~ /^\s*$/) {
+ $commit_log_lines++;#could be a $signature
+ }
-+ } else if ($has_commit_log && $commit_log_lines < 2) {
++ } elsif ($has_commit_log && $commit_log_lines < 2) {
+ WARN("COMMIT_MESSAGE",
+ "Missing commit description - Add an appropriate
one\n");
+ $commit_log_lines = 2; #warn only once
Re: [PATCH] checkpatch: Warn when a patch doesn't have a description
On Thu, 2018-07-26 at 15:08 -0700, Andrew Morton wrote:
> On Wed, 25 Jul 2018 19:22:47 -0700 Joe Perches wrote:
>
> > Potential patches should have a commit description.
> > Emit a warning when there isn't one.
[]
> I did this:
>
> ---
> a/scripts/checkpatch.pl~checkpatch-warn-when-a-patch-doesnt-have-a-description-fix
> +++ a/scripts/checkpatch.pl
> @@ -2517,7 +2517,7 @@ sub process {
> if ($line !~ /^\s*$/) {
> $commit_log_lines++;#could be a $signature
> }
> - } else if ($has_commit_log && $commit_log_lines < 2) {
> + } elsif ($has_commit_log && $commit_log_lines < 2) {
> WARN("COMMIT_MESSAGE",
>"Missing commit description - Add an appropriate
> one\n");
> $commit_log_lines = 2; #warn only once
>
> But I worry that you didn't send out the version which you tested, so
> please check.
You're right, I inserted the wrong one.
Anyway, what you did is correct and that was the only change.
Thanks.
$ diff -urN cp_commit_log_lines.diff cp_commit_message.diff
--- cp_commit_log_lines.diff2018-07-13 17:06:29.115337477 -0700
+++ cp_commit_message.diff 2018-07-15 05:46:29.878805336 -0700
@@ -2,7 +2,7 @@
1 file changed, 13 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
-index b5c875d7132b..8b5f3dae31c9 100755
+index b5c875d7132b..4ccf84079ea4 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2240,6 +2240,7 @@ sub process {
@@ -23,7 +23,7 @@
+ if ($line !~ /^\s*$/) {
+ $commit_log_lines++;#could be a $signature
+ }
-+ } else if ($has_commit_log && $commit_log_lines < 2) {
++ } elsif ($has_commit_log && $commit_log_lines < 2) {
+ WARN("COMMIT_MESSAGE",
+ "Missing commit description - Add an appropriate
one\n");
+ $commit_log_lines = 2; #warn only once
Re: [PATCH v2 1/1] remoteproc: correct rproc_free_vring() to avoid invalid kernel paging
On 07/26/2018 06:51 PM, Suman Anna wrote:
> Hi Loic,
>
> On 07/26/2018 02:48 AM, Loic PALLARDY wrote:
>> Hi Suman,
>>>
>>> Hi Loic,
>>>
>>> On 07/06/2018 02:46 AM, Loic Pallardy wrote:
If rproc_start() failed, rproc_resource_cleanup() is called to clean
debugfs entries, then associated iommu mappings, carveouts and vdev.
Issue occurs when rproc_free_vring() is trying to reset vring resource
table entry.
At this time, table_ptr is pointing on loaded resource table and carveouts
already released, so access to loaded resource table is generating a kernel
paging error:
>>>
>>> Are you using a device specific CMA pool or carveout, and if so, where
>>> the pool is? If not, where is the default CMA pool? I am trying to
>>> reproduce the issue on my platform with the start failure as you
>>> suggested, but haven't seen it so far. That said, I have seen the exact
>>> same crash when using HighMEM CMA pools on my downstream kernel
>>> when
>>> stopping the processor, and the root cause is essentially the same as
>>> what you summarized here. The issue was present with LowMem pools as
>>> well, but got masked because of the kernel linear mapping.
>>
>> I have a carveout declared in firmware resource table for co-processor code
>> and data, and st driver has a specific
>> reserved memory region to fit fix address space requested by co-processor.
>> So CPU access to code and loaded resource table area is granted thanks to
>> allocation done by rproc_handle_carveout().
>
> Where are the vrings getting allocated from?
>
> In anycase, I prefer that we should actually reset the table_ptr in
> rproc_start() in failure cases (undo the operation essentially) as we
> don't call rproc_stop() in those cases. This will result in symmetric
> code. We already have the reset handled in rproc_stop() added recently
> in commit 0a8b81cb2e41 ("remoteproc: Reset table_ptr on stop"). Let me
> know what you think, I can send a quick patch.
FYI, patch for the same posted here,
https://patchwork.kernel.org/patch/10546555/
regards
Suman
>>
>>>
[ 12.696535] Unable to handle kernel paging request at virtual address
>>> f0f357cc
[ 12.696540] pgd = (ptrval)
[ 12.696542] [f0f357cc] *pgd=6d2d0811, *pte=, *ppte=
[ 12.696558] Internal error: Oops: 807 [#1] SMP ARM
[ 12.696563] Modules linked in: rpmsg_core v4l2_mem2mem
>>> videobuf2_dma_contig sti_drm v4l2_common vida
[ 12.696598] CPU: 1 PID: 48 Comm: kworker/1:1 Tainted: GW
>>> 4.18.0-rc2-00018-g3170fdd-8
[ 12.696602] Hardware name: STi SoC with Flattened Device Tree
[ 12.696625] Workqueue: events request_firmware_work_func
[ 12.696659] PC is at rproc_free_vring+0x84/0xbc [remoteproc]
[ 12.696667] LR is at rproc_free_vring+0x70/0xbc [remoteproc]
This patch proposes to simply remove reset of resource table vring entries,
as firmware and resource table are reloaded at each rproc boot.
rproc_trigger_recovery() not impacted as resources not touched during
>>> recovery
procedure.
>>>
>>> And error recovery doesn't work for me after the rproc_start, stop got
>>> introduced.
>> Recovery no available on B2260, but I'll test it on another platform this
>> week
>>
>> Regards,
>> Loic
>>>
>>> regards
>>> Suman
>>>
Signed-off-by: Loic Pallardy
---
Changes from V1: typo fixes in commit message
drivers/remoteproc/remoteproc_core.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c
>>> b/drivers/remoteproc/remoteproc_core.c
index a9609d9..9a8b47c 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -289,16 +289,10 @@ void rproc_free_vring(struct rproc_vring *rvring)
{
int size = PAGE_ALIGN(vring_size(rvring->len, rvring->align));
struct rproc *rproc = rvring->rvdev->rproc;
- int idx = rvring->rvdev->vring - rvring;
- struct fw_rsc_vdev *rsc;
dma_free_coherent(rproc->dev.parent, size, rvring->va, rvring-
dma);
idr_remove(>notifyids, rvring->notifyid);
- /* reset resource entry info */
- rsc = (void *)rproc->table_ptr + rvring->rvdev->rsc_offset;
- rsc->vring[idx].da = 0;
- rsc->vring[idx].notifyid = -1;
}
static int rproc_vdev_do_probe(struct rproc_subdev *subdev)
>>
>
Re: [PATCH v2 1/1] remoteproc: correct rproc_free_vring() to avoid invalid kernel paging
On 07/26/2018 06:51 PM, Suman Anna wrote:
> Hi Loic,
>
> On 07/26/2018 02:48 AM, Loic PALLARDY wrote:
>> Hi Suman,
>>>
>>> Hi Loic,
>>>
>>> On 07/06/2018 02:46 AM, Loic Pallardy wrote:
If rproc_start() failed, rproc_resource_cleanup() is called to clean
debugfs entries, then associated iommu mappings, carveouts and vdev.
Issue occurs when rproc_free_vring() is trying to reset vring resource
table entry.
At this time, table_ptr is pointing on loaded resource table and carveouts
already released, so access to loaded resource table is generating a kernel
paging error:
>>>
>>> Are you using a device specific CMA pool or carveout, and if so, where
>>> the pool is? If not, where is the default CMA pool? I am trying to
>>> reproduce the issue on my platform with the start failure as you
>>> suggested, but haven't seen it so far. That said, I have seen the exact
>>> same crash when using HighMEM CMA pools on my downstream kernel
>>> when
>>> stopping the processor, and the root cause is essentially the same as
>>> what you summarized here. The issue was present with LowMem pools as
>>> well, but got masked because of the kernel linear mapping.
>>
>> I have a carveout declared in firmware resource table for co-processor code
>> and data, and st driver has a specific
>> reserved memory region to fit fix address space requested by co-processor.
>> So CPU access to code and loaded resource table area is granted thanks to
>> allocation done by rproc_handle_carveout().
>
> Where are the vrings getting allocated from?
>
> In anycase, I prefer that we should actually reset the table_ptr in
> rproc_start() in failure cases (undo the operation essentially) as we
> don't call rproc_stop() in those cases. This will result in symmetric
> code. We already have the reset handled in rproc_stop() added recently
> in commit 0a8b81cb2e41 ("remoteproc: Reset table_ptr on stop"). Let me
> know what you think, I can send a quick patch.
FYI, patch for the same posted here,
https://patchwork.kernel.org/patch/10546555/
regards
Suman
>>
>>>
[ 12.696535] Unable to handle kernel paging request at virtual address
>>> f0f357cc
[ 12.696540] pgd = (ptrval)
[ 12.696542] [f0f357cc] *pgd=6d2d0811, *pte=, *ppte=
[ 12.696558] Internal error: Oops: 807 [#1] SMP ARM
[ 12.696563] Modules linked in: rpmsg_core v4l2_mem2mem
>>> videobuf2_dma_contig sti_drm v4l2_common vida
[ 12.696598] CPU: 1 PID: 48 Comm: kworker/1:1 Tainted: GW
>>> 4.18.0-rc2-00018-g3170fdd-8
[ 12.696602] Hardware name: STi SoC with Flattened Device Tree
[ 12.696625] Workqueue: events request_firmware_work_func
[ 12.696659] PC is at rproc_free_vring+0x84/0xbc [remoteproc]
[ 12.696667] LR is at rproc_free_vring+0x70/0xbc [remoteproc]
This patch proposes to simply remove reset of resource table vring entries,
as firmware and resource table are reloaded at each rproc boot.
rproc_trigger_recovery() not impacted as resources not touched during
>>> recovery
procedure.
>>>
>>> And error recovery doesn't work for me after the rproc_start, stop got
>>> introduced.
>> Recovery no available on B2260, but I'll test it on another platform this
>> week
>>
>> Regards,
>> Loic
>>>
>>> regards
>>> Suman
>>>
Signed-off-by: Loic Pallardy
---
Changes from V1: typo fixes in commit message
drivers/remoteproc/remoteproc_core.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c
>>> b/drivers/remoteproc/remoteproc_core.c
index a9609d9..9a8b47c 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -289,16 +289,10 @@ void rproc_free_vring(struct rproc_vring *rvring)
{
int size = PAGE_ALIGN(vring_size(rvring->len, rvring->align));
struct rproc *rproc = rvring->rvdev->rproc;
- int idx = rvring->rvdev->vring - rvring;
- struct fw_rsc_vdev *rsc;
dma_free_coherent(rproc->dev.parent, size, rvring->va, rvring-
dma);
idr_remove(>notifyids, rvring->notifyid);
- /* reset resource entry info */
- rsc = (void *)rproc->table_ptr + rvring->rvdev->rsc_offset;
- rsc->vring[idx].da = 0;
- rsc->vring[idx].notifyid = -1;
}
static int rproc_vdev_do_probe(struct rproc_subdev *subdev)
>>
>
[PATCH] remoteproc: Reset table_ptr in rproc_start() failure paths
Unwind the modified table_ptr and restore it to the local copy
upon any subsequent failures in the rproc_start() function. This
keeps the function to remain balanced on failures without the need
to balance any modified variables elsewhere.
While at this, do some minor cleanup of the extra lines between
the failure labels as well.
Signed-off-by: Suman Anna
---
drivers/remoteproc/remoteproc_core.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c
b/drivers/remoteproc/remoteproc_core.c
index eadff6ce2f7f..afef2d491c5b 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -953,7 +953,7 @@ static int rproc_start(struct rproc *rproc, const struct
firmware *fw)
if (ret) {
dev_err(dev, "failed to prepare subdevices for %s: %d\n",
rproc->name, ret);
- return ret;
+ goto reset_table_ptr;
}
/* power up the remote processor */
@@ -979,10 +979,11 @@ static int rproc_start(struct rproc *rproc, const struct
firmware *fw)
stop_rproc:
rproc->ops->stop(rproc);
-
unprepare_subdevices:
rproc_unprepare_subdevices(rproc);
-
+reset_table_ptr:
+ if (loaded_table)
+ rproc->table_ptr = rproc->cached_table;
return ret;
}
--
2.18.0
[PATCH] remoteproc: Reset table_ptr in rproc_start() failure paths
Unwind the modified table_ptr and restore it to the local copy
upon any subsequent failures in the rproc_start() function. This
keeps the function to remain balanced on failures without the need
to balance any modified variables elsewhere.
While at this, do some minor cleanup of the extra lines between
the failure labels as well.
Signed-off-by: Suman Anna
---
drivers/remoteproc/remoteproc_core.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c
b/drivers/remoteproc/remoteproc_core.c
index eadff6ce2f7f..afef2d491c5b 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -953,7 +953,7 @@ static int rproc_start(struct rproc *rproc, const struct
firmware *fw)
if (ret) {
dev_err(dev, "failed to prepare subdevices for %s: %d\n",
rproc->name, ret);
- return ret;
+ goto reset_table_ptr;
}
/* power up the remote processor */
@@ -979,10 +979,11 @@ static int rproc_start(struct rproc *rproc, const struct
firmware *fw)
stop_rproc:
rproc->ops->stop(rproc);
-
unprepare_subdevices:
rproc_unprepare_subdevices(rproc);
-
+reset_table_ptr:
+ if (loaded_table)
+ rproc->table_ptr = rproc->cached_table;
return ret;
}
--
2.18.0
Re: [NOMERGE] [RFC PATCH 00/12] erofs: introduce erofs file system
On Thu, Jul 26, 2018 at 02:55:11PM -1000, Joey Pabalinas wrote: > I thought the exact same thing; opened the thread and was a bit confused > at first. In my opinion the name feels slightly misleading. Oooh, reading more of the thread I see there is some fun wordplay going on which I didn't catch at first. I rescind my comment :) -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [NOMERGE] [RFC PATCH 00/12] erofs: introduce erofs file system
On Thu, Jul 26, 2018 at 02:55:11PM -1000, Joey Pabalinas wrote: > I thought the exact same thing; opened the thread and was a bit confused > at first. In my opinion the name feels slightly misleading. Oooh, reading more of the thread I see there is some fun wordplay going on which I didn't catch at first. I rescind my comment :) -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH] get_maintainer: Allow usage outside of kernel tree
On Thu, 2018-07-26 at 15:39 -0700, Andrew Morton wrote:
> On Tue, 26 Jun 2018 11:07:03 +0100 Antonio Nino Diaz
> wrote:
>
> > Add option '--no-tree' to get_maintainer.pl script to allow using this
> > script in projects that aren't the Linux kernel if they use the same
> > format for their MAINTAINERS file. This command is also available in
> > checkpatch.pl, for example.
> >
>
> Seems OK to me?
Me too. (modulo the spaces)
> Please note that your email client is converting tabs to spaces.
>
> > --- a/scripts/get_maintainer.pl
> > +++ b/scripts/get_maintainer.pl
> > @@ -48,6 +48,7 @@ my $output_roles = 0;
> > my $output_rolestats = 1;
> > my $output_section_maxlen = 50;
> > my $scm = 0;
> > +my $tree = 1;
> > my $web = 0;
> > my $subsystem = 0;
> > my $status = 0;
> > @@ -255,6 +256,7 @@ if (!GetOptions(
> > 'subsystem!' => \$subsystem,
> > 'status!' => \$status,
> > 'scm!' => \$scm,
> > + 'tree!' => \$tree,
> > 'web!' => \$web,
> > 'letters=s' => \$letters,
> > 'pattern-depth=i' => \$pattern_depth,
> > @@ -319,7 +321,7 @@ if ($email &&
> > die "$P: Please select at least 1 email option\n";
> > }
> >
> > -if (!top_of_kernel_tree($lk_path)) {
> > +if ($tree && !top_of_kernel_tree($lk_path)) {
> > die "$P: The current directory does not appear to be "
> > . "a linux kernel source tree.\n";
> > }
> > @@ -1031,13 +1033,14 @@ Other options:
> >--sections => print all of the subsystem sections with pattern matches
> >--letters => print all matching 'letter' types from all matching sections
> >--mailmap => use .mailmap file (default: $email_use_mailmap)
> > + --no-tree => run without a kernel tree
> >--self-test => show potential issues with MAINTAINERS file content
> >--version => show version
> >--help => show this help information
> >
> > Default options:
> > - [--email --nogit --git-fallback --m --r --n --l --multiline
> > --pattern-depth=0
> > - --remove-duplicates --rolestats]
> > + [--email --tree --nogit --git-fallback --m --r --n --l --multiline
> > + --pattern-depth=0 --remove-duplicates --rolestats]
> >
> > Notes:
> >Using "-f directory" may give unexpected results:
>
>
Re: [PATCH] get_maintainer: Allow usage outside of kernel tree
On Thu, 2018-07-26 at 15:39 -0700, Andrew Morton wrote:
> On Tue, 26 Jun 2018 11:07:03 +0100 Antonio Nino Diaz
> wrote:
>
> > Add option '--no-tree' to get_maintainer.pl script to allow using this
> > script in projects that aren't the Linux kernel if they use the same
> > format for their MAINTAINERS file. This command is also available in
> > checkpatch.pl, for example.
> >
>
> Seems OK to me?
Me too. (modulo the spaces)
> Please note that your email client is converting tabs to spaces.
>
> > --- a/scripts/get_maintainer.pl
> > +++ b/scripts/get_maintainer.pl
> > @@ -48,6 +48,7 @@ my $output_roles = 0;
> > my $output_rolestats = 1;
> > my $output_section_maxlen = 50;
> > my $scm = 0;
> > +my $tree = 1;
> > my $web = 0;
> > my $subsystem = 0;
> > my $status = 0;
> > @@ -255,6 +256,7 @@ if (!GetOptions(
> > 'subsystem!' => \$subsystem,
> > 'status!' => \$status,
> > 'scm!' => \$scm,
> > + 'tree!' => \$tree,
> > 'web!' => \$web,
> > 'letters=s' => \$letters,
> > 'pattern-depth=i' => \$pattern_depth,
> > @@ -319,7 +321,7 @@ if ($email &&
> > die "$P: Please select at least 1 email option\n";
> > }
> >
> > -if (!top_of_kernel_tree($lk_path)) {
> > +if ($tree && !top_of_kernel_tree($lk_path)) {
> > die "$P: The current directory does not appear to be "
> > . "a linux kernel source tree.\n";
> > }
> > @@ -1031,13 +1033,14 @@ Other options:
> >--sections => print all of the subsystem sections with pattern matches
> >--letters => print all matching 'letter' types from all matching sections
> >--mailmap => use .mailmap file (default: $email_use_mailmap)
> > + --no-tree => run without a kernel tree
> >--self-test => show potential issues with MAINTAINERS file content
> >--version => show version
> >--help => show this help information
> >
> > Default options:
> > - [--email --nogit --git-fallback --m --r --n --l --multiline
> > --pattern-depth=0
> > - --remove-duplicates --rolestats]
> > + [--email --tree --nogit --git-fallback --m --r --n --l --multiline
> > + --pattern-depth=0 --remove-duplicates --rolestats]
> >
> > Notes:
> >Using "-f directory" may give unexpected results:
>
>
Re: [NOMERGE] [RFC PATCH 00/12] erofs: introduce erofs file system
On Fri, Jun 01, 2018 at 11:28:07AM +0200, Richard Weinberger wrote: > > We also think of other candidate full names, such as > > Enhanced / Extented Read-only File System, all the names short for "erofs" > > are okay. > > TBH, I read "erofs" as "error fs". ;-) I thought the exact same thing; opened the thread and was a bit confused at first. In my opinion the name feels slightly misleading. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [NOMERGE] [RFC PATCH 00/12] erofs: introduce erofs file system
On Fri, Jun 01, 2018 at 11:28:07AM +0200, Richard Weinberger wrote: > > We also think of other candidate full names, such as > > Enhanced / Extented Read-only File System, all the names short for "erofs" > > are okay. > > TBH, I read "erofs" as "error fs". ;-) I thought the exact same thing; opened the thread and was a bit confused at first. In my opinion the name feels slightly misleading. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH v2 08/12] sched/core: uclamp: extend cpu's cgroup controller
On Tue, Jul 24, 2018 at 06:29:02AM -0700, Tejun Heo wrote: > Hello, Patrick. > > On Mon, Jul 23, 2018 at 06:22:15PM +0100, Patrick Bellasi wrote: > > However, the "best effort" bandwidth control we have for CFS and RT > > can be further improved if, instead of just looking at time spent on > > CPUs, we provide some more hints to the scheduler to know at which > > min/max "MIPS" we want to consume the (best effort) time we have been > > allocated on a CPU. > > > > Such a simple extension is still quite useful to satisfy many use-case > > we have, mainly on mobile systems, like the ones I've described in the > >"Newcomer's Short Abstract (Updated)" > > section of the cover letter: > > > > https://lore.kernel.org/lkml/20180716082906.6061-1-patrick.bell...@arm.com/T/#u > > So, that's all completely fine but then let's please not give it a > name which doesn't quite match what it does. We can just call it > e.g. cpufreq range control. But then what name can one give it if it does more than one thing, like task-placement and CPU frequency control? It doesn't make sense to name it cpufreq IMHO. Its a clamp on the utilization of the task which can be used for many purposes. thanks, - Joel
Re: [PATCH v2 08/12] sched/core: uclamp: extend cpu's cgroup controller
On Tue, Jul 24, 2018 at 06:29:02AM -0700, Tejun Heo wrote: > Hello, Patrick. > > On Mon, Jul 23, 2018 at 06:22:15PM +0100, Patrick Bellasi wrote: > > However, the "best effort" bandwidth control we have for CFS and RT > > can be further improved if, instead of just looking at time spent on > > CPUs, we provide some more hints to the scheduler to know at which > > min/max "MIPS" we want to consume the (best effort) time we have been > > allocated on a CPU. > > > > Such a simple extension is still quite useful to satisfy many use-case > > we have, mainly on mobile systems, like the ones I've described in the > >"Newcomer's Short Abstract (Updated)" > > section of the cover letter: > > > > https://lore.kernel.org/lkml/20180716082906.6061-1-patrick.bell...@arm.com/T/#u > > So, that's all completely fine but then let's please not give it a > name which doesn't quite match what it does. We can just call it > e.g. cpufreq range control. But then what name can one give it if it does more than one thing, like task-placement and CPU frequency control? It doesn't make sense to name it cpufreq IMHO. Its a clamp on the utilization of the task which can be used for many purposes. thanks, - Joel
Re: [PATCH 00/25] staging: erofs: introduce erofs file system
On Thu, 26 Jul 2018, Gao Xiang wrote: > EROFS file system is a read-only file system with compression > support designed for certain devices (especially embeded > devices) with very limited physical memory and lots of memory Out of curiousity, and as Richard already asked[0] - what about existing file system, why can't they be used or extended instead of introducing yet another file system into the kernel? JFFS2? UBIFS? CramFs? SquashFS? ROMFS? F2FS? YAFFS? Christian. [0] https://marc.info/?l=linux-kernel=152783930418348=2 -- BOFH excuse #247: Due to Federal Budget problems we have been forced to cut back on the number of users able to access the system at one time. (namely none allowed)
Re: [PATCH 00/25] staging: erofs: introduce erofs file system
On Thu, 26 Jul 2018, Gao Xiang wrote: > EROFS file system is a read-only file system with compression > support designed for certain devices (especially embeded > devices) with very limited physical memory and lots of memory Out of curiousity, and as Richard already asked[0] - what about existing file system, why can't they be used or extended instead of introducing yet another file system into the kernel? JFFS2? UBIFS? CramFs? SquashFS? ROMFS? F2FS? YAFFS? Christian. [0] https://marc.info/?l=linux-kernel=152783930418348=2 -- BOFH excuse #247: Due to Federal Budget problems we have been forced to cut back on the number of users able to access the system at one time. (namely none allowed)
[PATCH 2/2] android: binder: Rate-limit debug and userspace triggered err msgs
Use rate-limited debug messages where userspace can trigger
excessive log spams.
Acked-by: Arve Hjønnevåg
Signed-off-by: Sherry Yang
---
drivers/android/binder.c | 5 +++--
drivers/android/binder_alloc.c | 41 +-
2 files changed, 29 insertions(+), 17 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 95283f3bb51c..78cc1190282c 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -71,6 +71,7 @@
#include
#include
#include
+#include
#include
#include "binder_alloc.h"
@@ -161,13 +162,13 @@ module_param_call(stop_on_user_error,
binder_set_stop_on_user_error,
#define binder_debug(mask, x...) \
do { \
if (binder_debug_mask & mask) \
- pr_info(x); \
+ pr_info_ratelimited(x); \
} while (0)
#define binder_user_error(x...) \
do { \
if (binder_debug_mask & BINDER_DEBUG_USER_ERROR) \
- pr_info(x); \
+ pr_info_ratelimited(x); \
if (binder_stop_on_user_error) \
binder_stop_on_user_error = 2; \
} while (0)
diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 2628806c64a2..e16116e9ad1f 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include "binder_alloc.h"
#include "binder_trace.h"
@@ -36,11 +37,12 @@ struct list_lru binder_alloc_lru;
static DEFINE_MUTEX(binder_alloc_mmap_lock);
enum {
+ BINDER_DEBUG_USER_ERROR = 1U << 0,
BINDER_DEBUG_OPEN_CLOSE = 1U << 1,
BINDER_DEBUG_BUFFER_ALLOC = 1U << 2,
BINDER_DEBUG_BUFFER_ALLOC_ASYNC = 1U << 3,
};
-static uint32_t binder_alloc_debug_mask;
+static uint32_t binder_alloc_debug_mask = BINDER_DEBUG_USER_ERROR;
module_param_named(debug_mask, binder_alloc_debug_mask,
uint, 0644);
@@ -48,7 +50,7 @@ module_param_named(debug_mask, binder_alloc_debug_mask,
#define binder_alloc_debug(mask, x...) \
do { \
if (binder_alloc_debug_mask & mask) \
- pr_info(x); \
+ pr_info_ratelimited(x); \
} while (0)
static struct binder_buffer *binder_buffer_next(struct binder_buffer *buffer)
@@ -152,8 +154,10 @@ static struct binder_buffer
*binder_alloc_prepare_to_free_locked(
* free the buffer twice
*/
if (buffer->free_in_progress) {
- pr_err("%d:%d FREE_BUFFER u%016llx user freed
buffer twice\n",
- alloc->pid, current->pid, (u64)user_ptr);
+ binder_alloc_debug(BINDER_DEBUG_USER_ERROR,
+ "%d:%d FREE_BUFFER u%016llx
user freed buffer twice\n",
+ alloc->pid, current->pid,
+ (u64)user_ptr);
return NULL;
}
buffer->free_in_progress = 1;
@@ -224,8 +228,9 @@ static int binder_update_page_range(struct binder_alloc
*alloc, int allocate,
}
if (!vma && need_mm) {
- pr_err("%d: binder_alloc_buf failed to map pages in userspace,
no vma\n",
- alloc->pid);
+ binder_alloc_debug(BINDER_DEBUG_USER_ERROR,
+ "%d: binder_alloc_buf failed to map pages in
userspace, no vma\n",
+ alloc->pid);
goto err_no_vma;
}
@@ -344,8 +349,9 @@ static struct binder_buffer *binder_alloc_new_buf_locked(
int ret;
if (alloc->vma == NULL) {
- pr_err("%d: binder_alloc_buf, no vma\n",
- alloc->pid);
+ binder_alloc_debug(BINDER_DEBUG_USER_ERROR,
+ "%d: binder_alloc_buf, no vma\n",
+ alloc->pid);
return ERR_PTR(-ESRCH);
}
@@ -417,11 +423,14 @@ static struct binder_buffer *binder_alloc_new_buf_locked(
if (buffer_size > largest_free_size)
largest_free_size = buffer_size;
}
- pr_err("%d: binder_alloc_buf size %zd failed, no address
space\n",
- alloc->pid, size);
- pr_err("allocated: %zd (num: %zd largest: %zd), free: %zd (num:
%zd largest: %zd)\n",
- total_alloc_size, allocated_buffers, largest_alloc_size,
- total_free_size, free_buffers, largest_free_size);
+ binder_alloc_debug(BINDER_DEBUG_USER_ERROR,
+ "%d: binder_alloc_buf size %zd failed,
[PATCH 0/2] android: binder: debug message improvements
This patch set includes extra_buffers_size in trace so that the total transaction size can be inferred. It also ratelimits debug messages to avoid excessive log spams reported by sysbot. android: binder: Show extra_buffers_size in trace android: binder: Rate-limit debug and userspace triggered err msgs drivers/android/binder.c | 5 +++-- drivers/android/binder_alloc.c | 41 ++ drivers/android/binder_trace.h | 7 +-- 3 files changed, 34 insertions(+), 19 deletions(-)
