2023-01-29 06:05:16

by Hongchen Zhang

[permalink] [raw]
Subject: [PATCH v4] pipe: use __pipe_{lock,unlock} instead of spinlock

Use spinlock in pipe_{read,write} cost too much time,IMO
pipe->{head,tail} can be protected by __pipe_{lock,unlock}.
On the other hand, we can use __pipe_{lock,unlock} to protect
the pipe->{head,tail} in pipe_resize_ring and
post_one_notification.

Reminded by Matthew, I tested this patch using UnixBench's pipe
test case on a x86_64 machine,and get the following data:
1) before this patch
System Benchmarks Partial Index BASELINE RESULT INDEX
Pipe Throughput 12440.0 493023.3 396.3
========
System Benchmarks Index Score (Partial Only) 396.3

2) after this patch
System Benchmarks Partial Index BASELINE RESULT INDEX
Pipe Throughput 12440.0 507551.4 408.0
========
System Benchmarks Index Score (Partial Only) 408.0

so we get ~3% speedup.

Reminded by Andrew, I tested this patch with the test code in
Linus's 0ddad21d3e99 and get following result:
1) before this patch
13,136.54 msec task-clock # 3.870 CPUs utilized
1,186,779 context-switches # 90.342 K/sec
668,867 cpu-migrations # 50.917 K/sec
895 page-faults # 68.131 /sec
29,875,711,543 cycles # 2.274 GHz
12,372,397,462 instructions # 0.41 insn per cycle
2,480,235,723 branches # 188.804 M/sec
47,191,943 branch-misses # 1.90% of all branches

3.394806886 seconds time elapsed

0.037869000 seconds user
0.189346000 seconds sys

2) after this patch

12,395.63 msec task-clock # 4.138 CPUs utilized
1,193,381 context-switches # 96.274 K/sec
585,543 cpu-migrations # 47.238 K/sec
1,063 page-faults # 85.756 /sec
27,691,587,226 cycles # 2.234 GHz
11,738,307,999 instructions # 0.42 insn per cycle
2,351,299,522 branches # 189.688 M/sec
45,404,526 branch-misses # 1.93% of all branches

2.995280878 seconds time elapsed

0.010615000 seconds user
0.206999000 seconds sys
After adding this patch, the time used on this test program becomes less.

Signed-off-by: Hongchen Zhang <[email protected]>

v4:
- fixes a typo in changelog when reviewed by Sedat.
v3:
- fixes the error reported by kernel test robot <[email protected]>
Link: https://lore.kernel.org/oe-lkp/[email protected]
- add perf stat data for the test code in Linus's 0ddad21d3e99 in
commit message.
v2:
- add UnixBench test data in commit message
- fixes the test error reported by kernel test robot <[email protected]>
by adding the missing fs.h header file.
---
fs/pipe.c | 22 +---------------------
include/linux/pipe_fs_i.h | 12 ++++++++++++
kernel/watch_queue.c | 8 ++++----
3 files changed, 17 insertions(+), 25 deletions(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index 42c7ff41c2db..4355ee5f754e 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -98,16 +98,6 @@ void pipe_unlock(struct pipe_inode_info *pipe)
}
EXPORT_SYMBOL(pipe_unlock);

-static inline void __pipe_lock(struct pipe_inode_info *pipe)
-{
- mutex_lock_nested(&pipe->mutex, I_MUTEX_PARENT);
-}
-
-static inline void __pipe_unlock(struct pipe_inode_info *pipe)
-{
- mutex_unlock(&pipe->mutex);
-}
-
void pipe_double_lock(struct pipe_inode_info *pipe1,
struct pipe_inode_info *pipe2)
{
@@ -253,8 +243,7 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
*/
was_full = pipe_full(pipe->head, pipe->tail, pipe->max_usage);
for (;;) {
- /* Read ->head with a barrier vs post_one_notification() */
- unsigned int head = smp_load_acquire(&pipe->head);
+ unsigned int head = pipe->head;
unsigned int tail = pipe->tail;
unsigned int mask = pipe->ring_size - 1;

@@ -322,14 +311,12 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)

if (!buf->len) {
pipe_buf_release(pipe, buf);
- spin_lock_irq(&pipe->rd_wait.lock);
#ifdef CONFIG_WATCH_QUEUE
if (buf->flags & PIPE_BUF_FLAG_LOSS)
pipe->note_loss = true;
#endif
tail++;
pipe->tail = tail;
- spin_unlock_irq(&pipe->rd_wait.lock);
}
total_len -= chars;
if (!total_len)
@@ -506,16 +493,13 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
* it, either the reader will consume it or it'll still
* be there for the next write.
*/
- spin_lock_irq(&pipe->rd_wait.lock);

head = pipe->head;
if (pipe_full(head, pipe->tail, pipe->max_usage)) {
- spin_unlock_irq(&pipe->rd_wait.lock);
continue;
}

pipe->head = head + 1;
- spin_unlock_irq(&pipe->rd_wait.lock);

/* Insert it into the buffer array */
buf = &pipe->bufs[head & mask];
@@ -1260,14 +1244,12 @@ int pipe_resize_ring(struct pipe_inode_info *pipe, unsigned int nr_slots)
if (unlikely(!bufs))
return -ENOMEM;

- spin_lock_irq(&pipe->rd_wait.lock);
mask = pipe->ring_size - 1;
head = pipe->head;
tail = pipe->tail;

n = pipe_occupancy(head, tail);
if (nr_slots < n) {
- spin_unlock_irq(&pipe->rd_wait.lock);
kfree(bufs);
return -EBUSY;
}
@@ -1303,8 +1285,6 @@ int pipe_resize_ring(struct pipe_inode_info *pipe, unsigned int nr_slots)
pipe->tail = tail;
pipe->head = head;

- spin_unlock_irq(&pipe->rd_wait.lock);
-
/* This might have made more room for writers */
wake_up_interruptible(&pipe->wr_wait);
return 0;
diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h
index 6cb65df3e3ba..f5084daf6eaf 100644
--- a/include/linux/pipe_fs_i.h
+++ b/include/linux/pipe_fs_i.h
@@ -2,6 +2,8 @@
#ifndef _LINUX_PIPE_FS_I_H
#define _LINUX_PIPE_FS_I_H

+#include <linux/fs.h>
+
#define PIPE_DEF_BUFFERS 16

#define PIPE_BUF_FLAG_LRU 0x01 /* page is on the LRU */
@@ -223,6 +225,16 @@ static inline void pipe_discard_from(struct pipe_inode_info *pipe,
#define PIPE_SIZE PAGE_SIZE

/* Pipe lock and unlock operations */
+static inline void __pipe_lock(struct pipe_inode_info *pipe)
+{
+ mutex_lock_nested(&pipe->mutex, I_MUTEX_PARENT);
+}
+
+static inline void __pipe_unlock(struct pipe_inode_info *pipe)
+{
+ mutex_unlock(&pipe->mutex);
+}
+
void pipe_lock(struct pipe_inode_info *);
void pipe_unlock(struct pipe_inode_info *);
void pipe_double_lock(struct pipe_inode_info *, struct pipe_inode_info *);
diff --git a/kernel/watch_queue.c b/kernel/watch_queue.c
index a6f9bdd956c3..92e46cfe9419 100644
--- a/kernel/watch_queue.c
+++ b/kernel/watch_queue.c
@@ -108,7 +108,7 @@ static bool post_one_notification(struct watch_queue *wqueue,
if (!pipe)
return false;

- spin_lock_irq(&pipe->rd_wait.lock);
+ __pipe_lock(pipe);

mask = pipe->ring_size - 1;
head = pipe->head;
@@ -135,17 +135,17 @@ static bool post_one_notification(struct watch_queue *wqueue,
buf->offset = offset;
buf->len = len;
buf->flags = PIPE_BUF_FLAG_WHOLE;
- smp_store_release(&pipe->head, head + 1); /* vs pipe_read() */
+ pipe->head = head + 1;

if (!test_and_clear_bit(note, wqueue->notes_bitmap)) {
- spin_unlock_irq(&pipe->rd_wait.lock);
+ __pipe_unlock(pipe);
BUG();
}
wake_up_interruptible_sync_poll_locked(&pipe->rd_wait, EPOLLIN | EPOLLRDNORM);
done = true;

out:
- spin_unlock_irq(&pipe->rd_wait.lock);
+ __pipe_unlock(pipe);
if (done)
kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
return done;
--
2.34.1



2023-01-29 07:33:53

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH v4] pipe: use __pipe_{lock,unlock} instead of spinlock

On Sat, Jan 28, 2023 at 10:05 PM Hongchen Zhang
<[email protected]> wrote:
>
> Use spinlock in pipe_{read,write} cost too much time,IMO
> pipe->{head,tail} can be protected by __pipe_{lock,unlock}.
> On the other hand, we can use __pipe_{lock,unlock} to protect
> the pipe->{head,tail} in pipe_resize_ring and
> post_one_notification.

No, we really can't.

post_one_notification() is called under the RCU lock held, *and* with
a spinlock held.

It simply cannot do a sleeping lock like __pipe_lock().

So that patch is simply fundamentally buggy, I'm afraid.

Linus

2023-02-03 01:42:40

by Hongchen Zhang

[permalink] [raw]
Subject: Re: [PATCH v4] pipe: use __pipe_{lock,unlock} instead of spinlock

Hi Linus,

Sorry to bother you. Can you help review this patch?
I tested this patch with the test code in you commit 0ddad21d3e99,
and the result looks better after applied this patch.

Best Regards
Hongchen Zhang

On 2023/1/29 am 2:04, Hongchen Zhang wrote:
> Use spinlock in pipe_{read,write} cost too much time,IMO
> pipe->{head,tail} can be protected by __pipe_{lock,unlock}.
> On the other hand, we can use __pipe_{lock,unlock} to protect
> the pipe->{head,tail} in pipe_resize_ring and
> post_one_notification.
>
> Reminded by Matthew, I tested this patch using UnixBench's pipe
> test case on a x86_64 machine,and get the following data:
> 1) before this patch
> System Benchmarks Partial Index BASELINE RESULT INDEX
> Pipe Throughput 12440.0 493023.3 396.3
> ========
> System Benchmarks Index Score (Partial Only) 396.3
>
> 2) after this patch
> System Benchmarks Partial Index BASELINE RESULT INDEX
> Pipe Throughput 12440.0 507551.4 408.0
> ========
> System Benchmarks Index Score (Partial Only) 408.0
>
> so we get ~3% speedup.
>
> Reminded by Andrew, I tested this patch with the test code in
> Linus's 0ddad21d3e99 and get following result:
> 1) before this patch
> 13,136.54 msec task-clock # 3.870 CPUs utilized
> 1,186,779 context-switches # 90.342 K/sec
> 668,867 cpu-migrations # 50.917 K/sec
> 895 page-faults # 68.131 /sec
> 29,875,711,543 cycles # 2.274 GHz
> 12,372,397,462 instructions # 0.41 insn per cycle
> 2,480,235,723 branches # 188.804 M/sec
> 47,191,943 branch-misses # 1.90% of all branches
>
> 3.394806886 seconds time elapsed
>
> 0.037869000 seconds user
> 0.189346000 seconds sys
>
> 2) after this patch
>
> 12,395.63 msec task-clock # 4.138 CPUs utilized
> 1,193,381 context-switches # 96.274 K/sec
> 585,543 cpu-migrations # 47.238 K/sec
> 1,063 page-faults # 85.756 /sec
> 27,691,587,226 cycles # 2.234 GHz
> 11,738,307,999 instructions # 0.42 insn per cycle
> 2,351,299,522 branches # 189.688 M/sec
> 45,404,526 branch-misses # 1.93% of all branches
>
> 2.995280878 seconds time elapsed
>
> 0.010615000 seconds user
> 0.206999000 seconds sys
> After adding this patch, the time used on this test program becomes less.
>
> Signed-off-by: Hongchen Zhang <[email protected]>
>
> v4:
> - fixes a typo in changelog when reviewed by Sedat.
> v3:
> - fixes the error reported by kernel test robot <[email protected]>
> Link: https://lore.kernel.org/oe-lkp/[email protected]
> - add perf stat data for the test code in Linus's 0ddad21d3e99 in
> commit message.
> v2:
> - add UnixBench test data in commit message
> - fixes the test error reported by kernel test robot <[email protected]>
> by adding the missing fs.h header file.
> ---
> fs/pipe.c | 22 +---------------------
> include/linux/pipe_fs_i.h | 12 ++++++++++++
> kernel/watch_queue.c | 8 ++++----
> 3 files changed, 17 insertions(+), 25 deletions(-)
>
> diff --git a/fs/pipe.c b/fs/pipe.c
> index 42c7ff41c2db..4355ee5f754e 100644
> --- a/fs/pipe.c
> +++ b/fs/pipe.c
> @@ -98,16 +98,6 @@ void pipe_unlock(struct pipe_inode_info *pipe)
> }
> EXPORT_SYMBOL(pipe_unlock);
>
> -static inline void __pipe_lock(struct pipe_inode_info *pipe)
> -{
> - mutex_lock_nested(&pipe->mutex, I_MUTEX_PARENT);
> -}
> -
> -static inline void __pipe_unlock(struct pipe_inode_info *pipe)
> -{
> - mutex_unlock(&pipe->mutex);
> -}
> -
> void pipe_double_lock(struct pipe_inode_info *pipe1,
> struct pipe_inode_info *pipe2)
> {
> @@ -253,8 +243,7 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
> */
> was_full = pipe_full(pipe->head, pipe->tail, pipe->max_usage);
> for (;;) {
> - /* Read ->head with a barrier vs post_one_notification() */
> - unsigned int head = smp_load_acquire(&pipe->head);
> + unsigned int head = pipe->head;
> unsigned int tail = pipe->tail;
> unsigned int mask = pipe->ring_size - 1;
>
> @@ -322,14 +311,12 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
>
> if (!buf->len) {
> pipe_buf_release(pipe, buf);
> - spin_lock_irq(&pipe->rd_wait.lock);
> #ifdef CONFIG_WATCH_QUEUE
> if (buf->flags & PIPE_BUF_FLAG_LOSS)
> pipe->note_loss = true;
> #endif
> tail++;
> pipe->tail = tail;
> - spin_unlock_irq(&pipe->rd_wait.lock);
> }
> total_len -= chars;
> if (!total_len)
> @@ -506,16 +493,13 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
> * it, either the reader will consume it or it'll still
> * be there for the next write.
> */
> - spin_lock_irq(&pipe->rd_wait.lock);
>
> head = pipe->head;
> if (pipe_full(head, pipe->tail, pipe->max_usage)) {
> - spin_unlock_irq(&pipe->rd_wait.lock);
> continue;
> }
>
> pipe->head = head + 1;
> - spin_unlock_irq(&pipe->rd_wait.lock);
>
> /* Insert it into the buffer array */
> buf = &pipe->bufs[head & mask];
> @@ -1260,14 +1244,12 @@ int pipe_resize_ring(struct pipe_inode_info *pipe, unsigned int nr_slots)
> if (unlikely(!bufs))
> return -ENOMEM;
>
> - spin_lock_irq(&pipe->rd_wait.lock);
> mask = pipe->ring_size - 1;
> head = pipe->head;
> tail = pipe->tail;
>
> n = pipe_occupancy(head, tail);
> if (nr_slots < n) {
> - spin_unlock_irq(&pipe->rd_wait.lock);
> kfree(bufs);
> return -EBUSY;
> }
> @@ -1303,8 +1285,6 @@ int pipe_resize_ring(struct pipe_inode_info *pipe, unsigned int nr_slots)
> pipe->tail = tail;
> pipe->head = head;
>
> - spin_unlock_irq(&pipe->rd_wait.lock);
> -
> /* This might have made more room for writers */
> wake_up_interruptible(&pipe->wr_wait);
> return 0;
> diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h
> index 6cb65df3e3ba..f5084daf6eaf 100644
> --- a/include/linux/pipe_fs_i.h
> +++ b/include/linux/pipe_fs_i.h
> @@ -2,6 +2,8 @@
> #ifndef _LINUX_PIPE_FS_I_H
> #define _LINUX_PIPE_FS_I_H
>
> +#include <linux/fs.h>
> +
> #define PIPE_DEF_BUFFERS 16
>
> #define PIPE_BUF_FLAG_LRU 0x01 /* page is on the LRU */
> @@ -223,6 +225,16 @@ static inline void pipe_discard_from(struct pipe_inode_info *pipe,
> #define PIPE_SIZE PAGE_SIZE
>
> /* Pipe lock and unlock operations */
> +static inline void __pipe_lock(struct pipe_inode_info *pipe)
> +{
> + mutex_lock_nested(&pipe->mutex, I_MUTEX_PARENT);
> +}
> +
> +static inline void __pipe_unlock(struct pipe_inode_info *pipe)
> +{
> + mutex_unlock(&pipe->mutex);
> +}
> +
> void pipe_lock(struct pipe_inode_info *);
> void pipe_unlock(struct pipe_inode_info *);
> void pipe_double_lock(struct pipe_inode_info *, struct pipe_inode_info *);
> diff --git a/kernel/watch_queue.c b/kernel/watch_queue.c
> index a6f9bdd956c3..92e46cfe9419 100644
> --- a/kernel/watch_queue.c
> +++ b/kernel/watch_queue.c
> @@ -108,7 +108,7 @@ static bool post_one_notification(struct watch_queue *wqueue,
> if (!pipe)
> return false;
>
> - spin_lock_irq(&pipe->rd_wait.lock);
> + __pipe_lock(pipe);
>
> mask = pipe->ring_size - 1;
> head = pipe->head;
> @@ -135,17 +135,17 @@ static bool post_one_notification(struct watch_queue *wqueue,
> buf->offset = offset;
> buf->len = len;
> buf->flags = PIPE_BUF_FLAG_WHOLE;
> - smp_store_release(&pipe->head, head + 1); /* vs pipe_read() */
> + pipe->head = head + 1;
>
> if (!test_and_clear_bit(note, wqueue->notes_bitmap)) {
> - spin_unlock_irq(&pipe->rd_wait.lock);
> + __pipe_unlock(pipe);
> BUG();
> }
> wake_up_interruptible_sync_poll_locked(&pipe->rd_wait, EPOLLIN | EPOLLRDNORM);
> done = true;
>
> out:
> - spin_unlock_irq(&pipe->rd_wait.lock);
> + __pipe_unlock(pipe);
> if (done)
> kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
> return done;
>


2023-02-03 02:25:08

by Hongchen Zhang

[permalink] [raw]
Subject: Re: Re: [PATCH v4] pipe: use __pipe_{lock,unlock} instead of spinlock

Hi Linus,

On 2023/1/29 下午3:33, Linus Torvalds wrote:
> On Sat, Jan 28, 2023 at 10:05 PM Hongchen Zhang
> <[email protected]> wrote:
>>
>> Use spinlock in pipe_{read,write} cost too much time,IMO
>> pipe->{head,tail} can be protected by __pipe_{lock,unlock}.
>> On the other hand, we can use __pipe_{lock,unlock} to protect
>> the pipe->{head,tail} in pipe_resize_ring and
>> post_one_notification.
>
> No, we really can't.
>
> post_one_notification() is called under the RCU lock held, *and* with
> a spinlock held.
>
> It simply cannot do a sleeping lock like __pipe_lock().
>
> So that patch is simply fundamentally buggy, I'm afraid.
>
> Linus
>
Thanks for your review,Let me find out if there is any way to solve the
problem you said.

Best Regards
Hongchen Zhang


2023-02-06 15:58:39

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCH v4] pipe: use __pipe_{lock,unlock} instead of spinlock

On Sat, Jan 28, 2023 at 11:33:08PM -0800, Linus Torvalds wrote:
> On Sat, Jan 28, 2023 at 10:05 PM Hongchen Zhang
> <[email protected]> wrote:
> >
> > Use spinlock in pipe_{read,write} cost too much time,IMO
> > pipe->{head,tail} can be protected by __pipe_{lock,unlock}.
> > On the other hand, we can use __pipe_{lock,unlock} to protect
> > the pipe->{head,tail} in pipe_resize_ring and
> > post_one_notification.
>
> No, we really can't.
>
> post_one_notification() is called under the RCU lock held, *and* with
> a spinlock held.
>
> It simply cannot do a sleeping lock like __pipe_lock().
>
> So that patch is simply fundamentally buggy, I'm afraid.

This patch lingered for a while until *way* later *Al Viro* and then
Linus chimed in on this. Ie, the issue for rejecting the patch wasn't so
obvious it seems.

As for Linus' point about us needing to avoid sleep under RCU +
spinlock, curious if we can capture *existing* bad users of that with
Coccinelle SmPL.

Luis

2023-02-06 16:14:07

by Julia Lawall

[permalink] [raw]
Subject: Re: [PATCH v4] pipe: use __pipe_{lock,unlock} instead of spinlock



On Mon, 6 Feb 2023, Luis Chamberlain wrote:

> On Sat, Jan 28, 2023 at 11:33:08PM -0800, Linus Torvalds wrote:
> > On Sat, Jan 28, 2023 at 10:05 PM Hongchen Zhang
> > <[email protected]> wrote:
> > >
> > > Use spinlock in pipe_{read,write} cost too much time,IMO
> > > pipe->{head,tail} can be protected by __pipe_{lock,unlock}.
> > > On the other hand, we can use __pipe_{lock,unlock} to protect
> > > the pipe->{head,tail} in pipe_resize_ring and
> > > post_one_notification.
> >
> > No, we really can't.
> >
> > post_one_notification() is called under the RCU lock held, *and* with
> > a spinlock held.
> >
> > It simply cannot do a sleeping lock like __pipe_lock().
> >
> > So that patch is simply fundamentally buggy, I'm afraid.
>
> This patch lingered for a while until *way* later *Al Viro* and then
> Linus chimed in on this. Ie, the issue for rejecting the patch wasn't so
> obvious it seems.
>
> As for Linus' point about us needing to avoid sleep under RCU +
> spinlock, curious if we can capture *existing* bad users of that with
> Coccinelle SmPL.

An analysis with Coccinelle may be highly prone to false positives if the
issue is very interprocedural. Maybe smatch would be better suited for
this?

julia

2023-02-06 16:16:16

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH v4] pipe: use __pipe_{lock,unlock} instead of spinlock

On Mon, Feb 6, 2023 at 7:58 AM Luis Chamberlain <[email protected]> wrote:
>
> As for Linus' point about us needing to avoid sleep under RCU +
> spinlock, curious if we can capture *existing* bad users of that with
> Coccinelle SmPL.

Well, we have it dynamically with the "might_sleep()" debugging. But
that obviously only triggers if that is enabled and when those
particular paths are run.

It would be lovely to have a static checker for "sleep under spinlock
or in RCU" (or any of the other situations: preemption disabled either
explicitly or due to get_cpu() and similar).

But I suspect it would be quite hard to do.

Linus

2023-02-06 16:45:38

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v4] pipe: use __pipe_{lock,unlock} instead of spinlock

On Mon, Feb 06, 2023 at 05:13:02PM +0100, Julia Lawall wrote:
> An analysis with Coccinelle may be highly prone to false positives if the
> issue is very interprocedural. Maybe smatch would be better suited for
> this?

I have a very good Smatch check for sleeping with preempt disabled
which would have detected this bug.

Where my check falls down is when the call tree is quite long.
Especially if the call tree is very long and there is some kind of
a MAY_SLEEP flag which is set and then checked several functions calls
down the call tree.

The unfortunate thing is that there are lot of sleeping in atomic bugs
and they are quite complicated to analyze because the call trees are
long so I'm not up to date on reviewing the warnings... You need the
cross function database to review these warnings. The warning looks
like:

drivers/accel/habanalabs/common/context.c:112 hl_ctx_fini() warn: sleeping in atomic context

That code looks like:
111 if (hdev->in_debug)
112 hl_device_set_debug_mode(hdev, ctx, false);
113

hl_device_set_debug_mode() take a mutex. Then you do
`smdb.py preempt hl_ctx_fini` and it prints out the call tree which
disables preemption.

cs_ioctl_unreserve_signals() <- disables preempt
-> hl_ctx_put()
-> hl_ctx_do_release()
-> hl_ctx_fini()

And so on. The unreviewed list is going to have more bugs and the other
list is probably mostly false positives outside of the staging/
directory.

regards,
dan carpenter


Attachments:
(No filename) (1.48 kB)
errs-complete (29.84 kB)
errs-unreviewed (6.21 kB)
Download all attachments

2023-02-06 17:55:32

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCH v4] pipe: use __pipe_{lock,unlock} instead of spinlock

On Mon, Feb 06, 2023 at 07:45:25PM +0300, Dan Carpenter wrote:
> On Mon, Feb 06, 2023 at 05:13:02PM +0100, Julia Lawall wrote:
> > An analysis with Coccinelle may be highly prone to false positives if the
> > issue is very interprocedural. Maybe smatch would be better suited for
> > this?
>
> I have a very good Smatch check for sleeping with preempt disabled
> which would have detected this bug.

Oh nice!

> Where my check falls down is when the call tree is quite long.
> Especially if the call tree is very long and there is some kind of
> a MAY_SLEEP flag which is set and then checked several functions calls
> down the call tree.
>
> The unfortunate thing is that there are lot of sleeping in atomic bugs
> and they are quite complicated to analyze because the call trees are
> long so I'm not up to date on reviewing the warnings... You need the
> cross function database to review these warnings. The warning looks
> like:
>
> drivers/accel/habanalabs/common/context.c:112 hl_ctx_fini() warn: sleeping in atomic context
>
> That code looks like:
> 111 if (hdev->in_debug)
> 112 hl_device_set_debug_mode(hdev, ctx, false);
> 113
>
> hl_device_set_debug_mode() take a mutex. Then you do
> `smdb.py preempt hl_ctx_fini` and it prints out the call tree which
> disables preemption.
>
> cs_ioctl_unreserve_signals() <- disables preempt

ok because of the spin_lock()

> -> hl_ctx_put()
> -> hl_ctx_do_release()
> -> hl_ctx_fini()
>
> And so on. The unreviewed list is going to have more bugs and the other
> list is probably mostly false positives outside of the staging/
> directory.
>
> regards,
> dan carpenter
>

> arch/x86/kernel/apic/x2apic_uv_x.c:1230 uv_set_vga_state() warn: sleeping in atomic context
> arch/x86/kernel/fpu/xstate.c:1476 fpstate_free() warn: sleeping in atomic context
> arch/x86/kernel/kvm.c:157 kvm_async_pf_task_wait_schedule() warn: sleeping in atomic context
> arch/x86/kernel/step.c:36 convert_ip_to_linear() warn: sleeping in atomic context
> arch/x86/lib/insn-eval.c:634 get_desc() warn: sleeping in atomic context
> arch/x86/mm/ioremap.c:471 iounmap() warn: sleeping in atomic context
> arch/x86/mm/mmio-mod.c:318 mmiotrace_iounmap() warn: sleeping in atomic context
> arch/x86/platform/uv/bios_uv.c:171 uv_bios_set_legacy_vga_target() warn: sleeping in atomic context
> arch/x86/platform/uv/bios_uv.c:45 uv_bios_call() warn: sleeping in atomic context
> arch/x86/xen/p2m.c:189 alloc_p2m_page() warn: sleeping in atomic context
> block/blk-crypto-profile.c:382 __blk_crypto_evict_key() warn: sleeping in atomic context
> block/blk-crypto-profile.c:390 __blk_crypto_evict_key() warn: sleeping in atomic context
> block/blk-crypto-profile.c:54 blk_crypto_hw_enter() warn: sleeping in atomic context
> block/blk-mq.c:166 blk_freeze_queue_start() warn: sleeping in atomic context
> block/blk-mq.c:206 blk_freeze_queue() warn: sleeping in atomic context
> block/blk-mq.c:216 blk_mq_freeze_queue() warn: sleeping in atomic context
> block/blk-mq.c:2174 __blk_mq_run_hw_queue() warn: sleeping in atomic context
> block/blk-mq.c:4083 blk_mq_destroy_queue() warn: sleeping in atomic context

Let's see as an example.

blk_mq_exit_hctx() can spin_lock() and so could disable preemption but I
can't see why this is sleeping in atomic context.

Luis

> block/blk-wbt.c:673 wbt_enable_default() warn: sleeping in atomic context
> block/blk-wbt.c:843 wbt_init() warn: sleeping in atomic context
> drivers/accel/habanalabs/common/context.c:112 hl_ctx_fini() warn: sleeping in atomic context
> drivers/accel/habanalabs/common/context.c:141 hl_ctx_do_release() warn: sleeping in atomic context
> drivers/accel/habanalabs/common/debugfs.c:272 vm_show() warn: sleeping in atomic context
> drivers/accel/habanalabs/common/device.c:1071 hl_device_set_debug_mode() warn: sleeping in atomic context
> drivers/accel/habanalabs/common/memory.c:1300 unmap_device_va() warn: sleeping in atomic context
> drivers/accel/habanalabs/common/memory.c:2842 hl_vm_ctx_fini() warn: sleeping in atomic context
> drivers/acpi/osl.c:927 acpi_debugger_write_log() warn: sleeping in atomic context
> drivers/base/core.c:189 device_links_write_lock() warn: sleeping in atomic context
> drivers/base/core.c:681 device_link_add() warn: sleeping in atomic context
> drivers/base/firmware_loader/main.c:587 firmware_free_data() warn: sleeping in atomic context
> drivers/base/firmware_loader/main.c:815 _request_firmware() warn: sleeping in atomic context
> drivers/base/power/runtime.c:1137 __pm_runtime_suspend() warn: sleeping in atomic context
> drivers/base/power/sysfs.c:779 wakeup_sysfs_add() warn: sleeping in atomic context
> drivers/base/power/sysfs.c:789 wakeup_sysfs_remove() warn: sleeping in atomic context
> drivers/base/power/wakeup.c:225 wakeup_source_register() warn: sleeping in atomic context
> drivers/base/power/wakeup.c:348 device_wakeup_enable() warn: sleeping in atomic context
> drivers/base/power/wakeup.c:91 wakeup_source_create() warn: sleeping in atomic context
> drivers/base/regmap/regmap.c:1790 _regmap_raw_write_impl() warn: sleeping in atomic context
> drivers/base/regmap/regmap.c:1855 _regmap_raw_write_impl() warn: sleeping in atomic context
> drivers/block/aoe/aoedev.c:229 aoedev_downdev() warn: sleeping in atomic context
> drivers/clk/clk.c:133 clk_prepare_lock() warn: sleeping in atomic context
> drivers/clk/clk.c:404 clk_core_get() warn: sleeping in atomic context
> drivers/clk/clk.c:5077 of_clk_get_hw_from_clkspec() warn: sleeping in atomic context
> drivers/clk/clkdev.c:77 clk_find_hw() warn: sleeping in atomic context
> drivers/crypto/hisilicon/sec/sec_algs.c:389 sec_send_request() warn: sleeping in atomic context
> drivers/crypto/hisilicon/sec/sec_algs.c:494 sec_skcipher_alg_callback() warn: sleeping in atomic context
> drivers/crypto/hisilicon/sec/sec_algs.c:506 sec_skcipher_alg_callback() warn: sleeping in atomic context
> drivers/crypto/hisilicon/sec/sec_algs.c:824 sec_alg_skcipher_crypto() warn: sleeping in atomic context
> drivers/crypto/hisilicon/sec/sec_drv.c:864 sec_queue_send() warn: sleeping in atomic context
> drivers/dma/dmaengine.c:905 dma_release_channel() warn: sleeping in atomic context
> drivers/gpio/gpio-aggregator.c:299 gpio_fwd_get_multiple() warn: sleeping in atomic context
> drivers/gpio/gpio-aggregator.c:355 gpio_fwd_set_multiple() warn: sleeping in atomic context
> drivers/gpio/gpio-it87.c:157 it87_gpio_request() warn: sleeping in atomic context
> drivers/gpio/gpio-it87.c:202 it87_gpio_direction_in() warn: sleeping in atomic context
> drivers/gpio/gpio-it87.c:245 it87_gpio_direction_out() warn: sleeping in atomic context
> drivers/gpio/gpio-it87.c:82 superio_enter() warn: sleeping in atomic context
> drivers/gpio/gpiolib.c:3263 gpiod_set_consumer_name() warn: sleeping in atomic context
> drivers/gpio/gpiolib.c:3516 gpiod_get_value_cansleep() warn: sleeping in atomic context
> drivers/gpio/gpiolib.c:3573 gpiod_get_array_value_cansleep() warn: sleeping in atomic context
> drivers/gpio/gpiolib.c:3612 gpiod_set_value_cansleep() warn: sleeping in atomic context
> drivers/gpio/gpiolib.c:3677 gpiod_set_array_value_cansleep() warn: sleeping in atomic context
> drivers/gpio/gpiolib-devres.c:118 devm_gpiod_get_index() warn: sleeping in atomic context
> drivers/gpu/drm/i915/gt/intel_gt_mcr.c:379 intel_gt_mcr_lock() warn: sleeping in atomic context
> drivers/gpu/drm/i915/gt/intel_ring.c:216 wait_for_space() warn: sleeping in atomic context
> drivers/gpu/drm/i915/gt/selftest_lrc.c:1721 garbage_reset() warn: sleeping in atomic context
> drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c:660 ct_send() warn: sleeping in atomic context
> drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c:784 intel_guc_ct_send() warn: sleeping in atomic context
> drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:2007 new_guc_id() warn: sleeping in atomic context
> drivers/gpu/drm/i915/i915_request.c:1982 i915_request_wait_timeout() warn: sleeping in atomic context
> drivers/gpu/drm/i915/i915_request.c:2117 i915_request_wait() warn: sleeping in atomic context
> drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c:372 update_cursor() warn: sleeping in atomic context
> drivers/gpu/drm/msm/msm_gem.c:495 msm_gem_get_and_pin_iova_range() warn: sleeping in atomic context
> drivers/gpu/drm/msm/msm_gem.c:506 msm_gem_get_and_pin_iova() warn: sleeping in atomic context
> drivers/gpu/drm/msm/msm_gem.h:185 msm_gem_lock() warn: sleeping in atomic context
> drivers/gpu/drm/nouveau/nvif/object.c:149 nvif_object_mthd() warn: sleeping in atomic context
> drivers/gpu/drm/nouveau/nvkm/core/subdev.c:200 nvkm_subdev_ref() warn: sleeping in atomic context
> drivers/gpu/drm/vmwgfx/vmwgfx_msg.c:516 vmw_host_printf() warn: sleeping in atomic context
> drivers/hid/hid-core.c:1935 __hid_request() warn: sleeping in atomic context
> drivers/hid/hid-core.c:2379 hid_hw_request() warn: sleeping in atomic context
> drivers/infiniband/core/restrack.c:351 rdma_restrack_del() warn: sleeping in atomic context
> drivers/infiniband/core/verbs.c:971 rdma_destroy_ah_user() warn: sleeping in atomic context
> drivers/leds/leds-ns2.c:96 ns2_led_set_mode() warn: sleeping in atomic context
> drivers/mailbox/mailbox.c:280 mbox_send_message() warn: sleeping in atomic context
> drivers/media/pci/cx88/cx88-alsa.c:743 snd_cx88_switch_put() warn: sleeping in atomic context
> drivers/media/v4l2-core/v4l2-ctrls-core.c:1396 find_ref_lock() warn: sleeping in atomic context
> drivers/mfd/ezx-pcap.c:102 ezx_pcap_read() warn: sleeping in atomic context
> drivers/mfd/ezx-pcap.c:117 ezx_pcap_set_bits() warn: sleeping in atomic context
> drivers/mfd/ezx-pcap.c:220 pcap_set_ts_bits() warn: sleeping in atomic context
> drivers/mfd/ezx-pcap.c:232 pcap_disable_adc() warn: sleeping in atomic context
> drivers/mfd/ezx-pcap.c:247 pcap_adc_trigger() warn: sleeping in atomic context
> drivers/mfd/ezx-pcap.c:252 pcap_adc_trigger() warn: sleeping in atomic context
> drivers/mfd/ezx-pcap.c:280 pcap_adc_irq() warn: sleeping in atomic context
> drivers/mfd/ezx-pcap.c:69 ezx_pcap_putget() warn: sleeping in atomic context
> drivers/mfd/ezx-pcap.c:86 ezx_pcap_write() warn: sleeping in atomic context
> drivers/misc/sgi-gru/grukservices.c:262 gru_get_cpu_resources() warn: sleeping in atomic context
> drivers/mmc/core/slot-gpio.c:69 mmc_gpio_get_ro() warn: sleeping in atomic context
> drivers/mmc/host/sdhci.c:1304 sdhci_external_dma_release() warn: sleeping in atomic context
> drivers/net/ethernet/amd/xgbe/xgbe-drv.c:976 xgbe_napi_disable() warn: sleeping in atomic context
> drivers/net/ethernet/amd/xgbe/xgbe-drv.c:982 xgbe_napi_disable() warn: sleeping in atomic context
> drivers/net/ethernet/cadence/macb_main.c:5159 macb_suspend() warn: sleeping in atomic context
> drivers/net/ethernet/cadence/macb_main.c:5171 macb_suspend() warn: sleeping in atomic context
> drivers/net/ethernet/cadence/macb_main.c:5250 macb_resume() warn: sleeping in atomic context
> drivers/net/ethernet/cavium/liquidio/octeon_device.c:704 octeon_allocate_device_mem() warn: sleeping in atomic context
> drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c:200 t4vf_wr_mbox_core() warn: sleeping in atomic context
> drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c:256 t4vf_wr_mbox_core() warn: sleeping in atomic context
> drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c:218 alloc_ctrl_skb() warn: sleeping in atomic context
> drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.c:330 ppm_destroy() warn: sleeping in atomic context
> drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.c:338 cxgbi_ppm_release() warn: sleeping in atomic context
> drivers/net/ethernet/davicom/dm9000.c:329 dm9000_phy_write() warn: sleeping in atomic context
> drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c:917 ixgbe_ipsec_vf_add_sa() warn: sleeping in atomic context
> drivers/net/ethernet/mellanox/mlx5/core/cmd.c:1861 cmd_exec() warn: sleeping in atomic context
> drivers/net/ethernet/netronome/nfp/flower/lag_conf.c:167 nfp_fl_lag_get_group_info() warn: sleeping in atomic context
> drivers/net/ethernet/netronome/nfp/flower/lag_conf.c:211 nfp_flower_lag_get_info_from_netdev() warn: sleeping in atomic context
> drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c:518 nfp_tun_write_neigh() warn: sleeping in atomic context
> drivers/net/ethernet/nvidia/forcedeth.c:1091 nv_disable_irq() warn: sleeping in atomic context
> drivers/net/ethernet/nvidia/forcedeth.c:1093 nv_disable_irq() warn: sleeping in atomic context
> drivers/net/ethernet/nvidia/forcedeth.c:1095 nv_disable_irq() warn: sleeping in atomic context
> drivers/net/ethernet/nvidia/forcedeth.c:4889 nv_set_loopback() warn: sleeping in atomic context
> drivers/net/ethernet/nvidia/forcedeth.c:4915 nv_set_loopback() warn: sleeping in atomic context
> drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c:303 netxen_pcie_sem_lock() warn: sleeping in atomic context
> drivers/net/ethernet/qlogic/qed/qed_mcp.c:200 qed_load_mcp_offsets() warn: sleeping in atomic context
> drivers/net/ethernet/rocker/rocker_main.c:1080 rocker_cmd_exec() warn: sleeping in atomic context
> drivers/net/ethernet/rocker/rocker_main.c:84 rocker_wait_event_timeout() warn: sleeping in atomic context
> drivers/net/phy/mdio_bus.c:1053 mdiobus_read() warn: sleeping in atomic context
> drivers/net/phy/mdio_bus.c:1076 mdiobus_c45_read() warn: sleeping in atomic context
> drivers/net/phy/mdio_bus.c:1152 mdiobus_write() warn: sleeping in atomic context
> drivers/net/phy/mdio_bus.c:1177 mdiobus_c45_write() warn: sleeping in atomic context
> drivers/net/phy/phy.c:1027 phy_ethtool_ksettings_set() warn: sleeping in atomic context
> drivers/net/phy/phy.c:330 phy_mii_ioctl() warn: sleeping in atomic context
> drivers/net/phy/phy.c:334 phy_mii_ioctl() warn: sleeping in atomic context
> drivers/net/phy/phy.c:386 phy_mii_ioctl() warn: sleeping in atomic context
> drivers/net/phy/phy.c:388 phy_mii_ioctl() warn: sleeping in atomic context
> drivers/net/wireless/ath/ath11k/peer.c:396 ath11k_peer_create() warn: sleeping in atomic context
> drivers/net/wireless/broadcom/b43legacy/main.c:2832 b43legacy_op_bss_info_changed() warn: sleeping in atomic context
> drivers/net/wireless/broadcom/b43legacy/main.c:590 b43legacy_synchronize_irq() warn: sleeping in atomic context
> drivers/net/wireless/broadcom/b43legacy/radio.c:213 b43legacy_synth_pu_workaround() warn: sleeping in atomic context
> drivers/net/wireless/broadcom/b43legacy/radio.c:604 b43legacy_calc_nrssi_slope() warn: sleeping in atomic context
> drivers/net/wireless/broadcom/b43legacy/radio.c:780 b43legacy_calc_nrssi_slope() warn: sleeping in atomic context
> drivers/net/wireless/intel/iwlegacy/common.c:1844 il_send_add_sta() warn: sleeping in atomic context
> drivers/net/wireless/intersil/orinoco/fw.c:114 orinoco_dl_firmware() warn: sleeping in atomic context
> drivers/net/wireless/intersil/orinoco/fw.c:224 symbol_dl_image() warn: sleeping in atomic context
> drivers/net/wireless/intersil/orinoco/fw.c:339 orinoco_download() warn: sleeping in atomic context
> drivers/net/wwan/t7xx/t7xx_state_monitor.c:416 t7xx_fsm_append_cmd() warn: sleeping in atomic context
> drivers/nvme/host/core.c:4931 nvme_remove_admin_tag_set() warn: sleeping in atomic context
> drivers/nvme/host/core.c:4993 nvme_remove_io_tag_set() warn: sleeping in atomic context
> drivers/nvme/host/core.c:571 nvme_change_ctrl_state() warn: sleeping in atomic context
> drivers/nvme/host/fc.c:2408 nvme_fc_ctrl_free() warn: sleeping in atomic context
> drivers/nvme/host/multipath.c:146 nvme_kick_requeue_lists() warn: sleeping in atomic context
> drivers/nvme/target/fc.c:495 nvmet_fc_xmt_disconnect_assoc() warn: sleeping in atomic context
> drivers/pci/iov.c:888 sriov_restore_state() warn: sleeping in atomic context
> drivers/pci/msi/msi.c:864 __pci_restore_msix_state() warn: sleeping in atomic context
> drivers/pci/pci.c:5376 __pci_reset_function_locked() warn: sleeping in atomic context
> drivers/pci/pci.c:855 pci_wait_for_pending() warn: sleeping in atomic context
> drivers/pci/pcie/aspm.c:1094 pcie_aspm_powersave_config_link() warn: sleeping in atomic context
> drivers/pci/probe.c:2360 pci_bus_wait_crs() warn: sleeping in atomic context
> drivers/pcmcia/pcmcia_resource.c:168 pcmcia_access_config() warn: sleeping in atomic context
> drivers/pcmcia/pcmcia_resource.c:208 pcmcia_write_config_byte() warn: sleeping in atomic context
> drivers/phy/phy-core.c:404 phy_set_mode_ext() warn: sleeping in atomic context
> drivers/platform/olpc/olpc-xo175-ec.c:363 olpc_xo175_ec_complete() warn: sleeping in atomic context
> drivers/platform/olpc/olpc-xo175-ec.c:567 olpc_xo175_ec_cmd() warn: sleeping in atomic context
> drivers/scsi/arcmsr/arcmsr_hba.c:409 arcmsr_hbaA_wait_msgint_ready() warn: sleeping in atomic context
> drivers/scsi/arcmsr/arcmsr_hba.c:429 arcmsr_hbaB_wait_msgint_ready() warn: sleeping in atomic context
> drivers/scsi/arcmsr/arcmsr_hba.c:447 arcmsr_hbaC_wait_msgint_ready() warn: sleeping in atomic context
> drivers/scsi/arcmsr/arcmsr_hba.c:465 arcmsr_hbaD_wait_msgint_ready() warn: sleeping in atomic context
> drivers/scsi/arcmsr/arcmsr_hba.c:483 arcmsr_hbaE_wait_msgint_ready() warn: sleeping in atomic context
> drivers/scsi/BusLogic.c:1939 blogic_initadapter() warn: sleeping in atomic context
> drivers/scsi/BusLogic.c:254 blogic_create_addlccbs() warn: sleeping in atomic context
> drivers/scsi/elx/libefc/efc_els.c:69 efc_els_io_alloc_size() warn: sleeping in atomic context
> drivers/scsi/fcoe/fcoe_ctlr.c:441 fcoe_ctlr_link_up() warn: sleeping in atomic context
> drivers/scsi/fnic/fnic_fcs.c:500 fnic_fcoe_start_fcf_disc() warn: sleeping in atomic context
> drivers/scsi/fnic/vnic_dev.c:597 fnic_dev_stats_dump() warn: sleeping in atomic context
> drivers/scsi/libfc/fc_exch.c:606 fc_seq_set_resp() warn: sleeping in atomic context
> drivers/scsi/lpfc/lpfc_hbadisc.c:5310 lpfc_unreg_rpi() warn: sleeping in atomic context
> drivers/scsi/lpfc/lpfc_hbadisc.c:5865 lpfc_issue_clear_la() warn: sleeping in atomic context
> drivers/scsi/lpfc/lpfc_hbadisc.c:5887 lpfc_issue_reg_vpi() warn: sleeping in atomic context
> drivers/scsi/lpfc/lpfc_hbadisc.c:6563 lpfc_nlp_init() warn: sleeping in atomic context
> drivers/scsi/lpfc/lpfc_init.c:8987 lpfc_sli4_create_rpi_hdr() warn: sleeping in atomic context
> drivers/scsi/lpfc/lpfc_sli.c:19541 lpfc_sli4_post_rpi_hdr() warn: sleeping in atomic context
> drivers/scsi/mvumi.c:130 mvumi_alloc_mem_resource() warn: sleeping in atomic context
> drivers/spi/spi.c:3863 __spi_validate() warn: sleeping in atomic context
> drivers/spi/spi.c:4245 spi_sync() warn: sleeping in atomic context
> drivers/ssb/driver_pcicore.c:702 ssb_pcicore_dev_irqvecs_enable() warn: sleeping in atomic context
> drivers/ssb/pcmcia.c:106 ssb_pcmcia_switch_coreidx() warn: sleeping in atomic context
> drivers/ssb/pcmcia.c:159 ssb_pcmcia_switch_core() warn: sleeping in atomic context
> drivers/ssb/pcmcia.c:210 select_core_and_segment() warn: sleeping in atomic context
> drivers/ssb/pcmcia.c:75 ssb_pcmcia_cfg_write() warn: sleeping in atomic context
> drivers/staging/r8188eu/core/rtw_pwrctrl.c:50 ips_leave() warn: sleeping in atomic context
> drivers/staging/r8188eu/hal/usb_ops_linux.c:24 usb_read() warn: sleeping in atomic context
> drivers/staging/rtl8723bs/core/rtw_pwrctrl.c:560 LeaveAllPowerSaveMode() warn: sleeping in atomic context
> drivers/staging/rtl8723bs/core/rtw_pwrctrl.c:563 LeaveAllPowerSaveMode() warn: sleeping in atomic context
> drivers/staging/rtl8723bs/core/rtw_pwrctrl.c:582 LPS_Leave_check() warn: sleeping in atomic context
> drivers/staging/rtl8723bs/core/rtw_pwrctrl.c:80 ips_leave() warn: sleeping in atomic context
> drivers/staging/rtl8723bs/core/rtw_xmit.c:2520 rtw_sctx_wait() warn: sleeping in atomic context
> drivers/staging/rts5208/xd.c:1320 xd_build_l2p_tbl() warn: sleeping in atomic context
> drivers/staging/rts5208/xd.c:800 xd_init_l2p_tbl() warn: sleeping in atomic context
> drivers/staging/rts5208/xd.c:825 free_zone() warn: sleeping in atomic context
> drivers/staging/rts5208/xd.c:852 xd_set_unused_block() warn: sleeping in atomic context
> drivers/target/iscsi/iscsi_target.c:3435 iscsit_build_sendtargets_response() warn: sleeping in atomic context
> drivers/target/iscsi/iscsi_target_erl0.c:878 iscsit_cause_connection_reinstatement() warn: sleeping in atomic context
> drivers/target/iscsi/iscsi_target_util.c:144 iscsit_wait_for_tag() warn: sleeping in atomic context
> drivers/target/target_core_tpg.c:228 target_tpg_has_node_acl() warn: sleeping in atomic context
> drivers/tty/serial/serial_core.c:1385 uart_set_rs485_termination() warn: sleeping in atomic context
> drivers/tty/serial/serial_mctrl_gpio.c:345 mctrl_gpio_disable_ms() warn: sleeping in atomic context
> drivers/usb/cdns3/core.c:64 cdns_role_stop() warn: sleeping in atomic context
> drivers/usb/gadget/udc/bdc/bdc_core.c:38 poll_oip() warn: sleeping in atomic context
> drivers/usb/gadget/udc/core.c:740 usb_gadget_disconnect() warn: sleeping in atomic context
> drivers/usb/gadget/udc/max3420_udc.c:1308 max3420_remove() warn: sleeping in atomic context
> drivers/usb/host/max3421-hcd.c:1934 max3421_remove() warn: sleeping in atomic context
> drivers/vhost/vhost.c:1291 vhost_iotlb_miss() warn: sleeping in atomic context
> drivers/vhost/vhost.c:2091 translate_desc() warn: sleeping in atomic context
> drivers/vhost/vhost.c:2585 vhost_new_msg() warn: sleeping in atomic context
> drivers/video/fbdev/omap2/omapfb/dss/dispc-compat.c:538 dispc_mgr_enable_digit_out() warn: sleeping in atomic context
> drivers/xen/events/events_base.c:1213 bind_evtchn_to_irq_chip() warn: sleeping in atomic context
> drivers/xen/events/events_base.c:1427 unbind_from_irq() warn: sleeping in atomic context
> drivers/xen/xenbus/xenbus_client.c:290 xenbus_va_dev_error() warn: sleeping in atomic context
> drivers/xen/xenbus/xenbus_client.c:321 xenbus_dev_error() warn: sleeping in atomic context
> drivers/xen/xenbus/xenbus_client.c:342 xenbus_dev_fatal() warn: sleeping in atomic context
> drivers/xen/xenbus/xenbus_client.c:473 xenbus_alloc_evtchn() warn: sleeping in atomic context
> drivers/xen/xenbus/xenbus_client.c:494 xenbus_free_evtchn() warn: sleeping in atomic context
> drivers/xen/xen-pciback/pci_stub.c:110 pcistub_device_release() warn: sleeping in atomic context
> drivers/xen/xen-scsiback.c:1016 __scsiback_del_translation_entry() warn: sleeping in atomic context
> drivers/xen/xen-scsiback.c:276 scsiback_free_translation_entry() warn: sleeping in atomic context
> fs/afs/inode.c:132 afs_inode_init_from_status() warn: sleeping in atomic context
> fs/btrfs/subpage.c:137 btrfs_attach_subpage() warn: sleeping in atomic context
> fs/btrfs/subpage.c:169 btrfs_alloc_subpage() warn: sleeping in atomic context
> fs/configfs/dir.c:1129 configfs_depend_item() warn: sleeping in atomic context
> fs/crypto/inline_crypt.c:34 fscrypt_get_devices() warn: sleeping in atomic context
> fs/debugfs/inode.c:765 debugfs_remove() warn: sleeping in atomic context
> fs/ecryptfs/inode.c:1054 ecryptfs_getxattr_lower() warn: sleeping in atomic context
> fs/fscache/volume.c:391 fscache_free_volume() warn: sleeping in atomic context
> fs/fscache/volume.c:420 fscache_put_volume() warn: sleeping in atomic context
> fs/fs_context.c:257 alloc_fs_context() warn: sleeping in atomic context
> fs/fs_context.c:304 fs_context_for_mount() warn: sleeping in atomic context
> fs/gfs2/glock.c:1336 gfs2_glock_wait() warn: sleeping in atomic context
> fs/gfs2/glock.c:1587 gfs2_glock_nq() warn: sleeping in atomic context
> fs/gfs2/super.c:542 gfs2_make_fs_ro() warn: sleeping in atomic context
> fs/gfs2/util.c:166 signal_our_withdraw() warn: sleeping in atomic context
> fs/gfs2/util.c:356 gfs2_withdraw() warn: sleeping in atomic context
> fs/inode.c:1279 iget_locked() warn: sleeping in atomic context
> fs/inode.c:1319 iget_locked() warn: sleeping in atomic context
> fs/inode.c:1490 ilookup() warn: sleeping in atomic context
> fs/inode.c:2400 inode_nohighmem() warn: sleeping in atomic context
> fs/inode.c:262 alloc_inode() warn: sleeping in atomic context
> fs/jffs2/fs.c:275 jffs2_iget() warn: sleeping in atomic context
> fs/jffs2/fs.c:660 jffs2_gc_fetch_inode() warn: sleeping in atomic context
> fs/jffs2/malloc.c:188 jffs2_alloc_refblock() warn: sleeping in atomic context
> fs/jffs2/malloc.c:221 jffs2_prealloc_raw_node_refs() warn: sleeping in atomic context
> fs/jfs/jfs_logmgr.c:2064 lbmWrite() warn: sleeping in atomic context
> fs/jfs/jfs_logmgr.c:2116 lbmStartIO() warn: sleeping in atomic context
> fs/kernfs/dir.c:888 kernfs_find_and_get_ns() warn: sleeping in atomic context
> fs/namespace.c:1055 vfs_kern_mount() warn: sleeping in atomic context
> fs/notify/mark.c:496 fsnotify_destroy_mark() warn: sleeping in atomic context
> fs/notify/mark.c:854 fsnotify_destroy_marks() warn: sleeping in atomic context
> fs/proc/inode.c:267 proc_entry_rundown() warn: sleeping in atomic context
> fs/sysv/itree.c:104 get_branch() warn: sleeping in atomic context
> fs/ubifs/lpt.c:1462 ubifs_pnode_lookup() warn: sleeping in atomic context
> fs/xfs/xfs_buf.c:283 xfs_buf_free_pages() warn: sleeping in atomic context
> ./include/asm-generic/pgalloc.h:129 pmd_alloc_one() warn: sleeping in atomic context
> ./include/linux/buffer_head.h:341 sb_bread() warn: sleeping in atomic context
> ./include/linux/dma-resv.h:345 dma_resv_lock() warn: sleeping in atomic context
> ./include/linux/fs.h:758 inode_lock() warn: sleeping in atomic context
> ./include/linux/fsnotify_backend.h:266 fsnotify_group_lock() warn: sleeping in atomic context
> include/linux/highmem-internal.h:166 kmap() warn: sleeping in atomic context
> ./include/linux/interrupt.h:215 devm_request_irq() warn: sleeping in atomic context
> ./include/linux/interrupt.h:739 tasklet_disable() warn: sleeping in atomic context
> ./include/linux/kernfs.h:597 kernfs_find_and_get() warn: sleeping in atomic context
> ./include/linux/mmap_lock.h:117 mmap_read_lock() warn: sleeping in atomic context
> ./include/linux/mm.h:2705 pmd_ptlock_init() warn: sleeping in atomic context
> ./include/linux/mm.h:2741 pgtable_pmd_page_ctor() warn: sleeping in atomic context
> ./include/linux/percpu-rwsem.h:49 percpu_down_read() warn: sleeping in atomic context
> ./include/linux/ptr_ring.h:603 ptr_ring_resize() warn: sleeping in atomic context
> ./include/linux/ptr_ring.h:642 ptr_ring_resize_multiple() warn: sleeping in atomic context
> ./include/linux/wait_bit.h:181 wait_on_bit_lock() warn: sleeping in atomic context
> ./include/linux/wait_bit.h:73 wait_on_bit() warn: sleeping in atomic context
> ./include/linux/writeback.h:201 wait_on_inode() warn: sleeping in atomic context
> ./include/media/v4l2-ctrls.h:1103 v4l2_ctrl_s_ctrl() warn: sleeping in atomic context
> ./include/media/v4l2-ctrls.h:564 v4l2_ctrl_lock() warn: sleeping in atomic context
> ./include/net/sock.h:1725 lock_sock() warn: sleeping in atomic context
> kernel/cpu.c:310 cpus_read_lock() warn: sleeping in atomic context
> kernel/entry/common.c:159 exit_to_user_mode_loop() warn: sleeping in atomic context
> kernel/events/core.c:5148 _free_event() warn: sleeping in atomic context
> kernel/irq/manage.c:137 synchronize_irq() warn: sleeping in atomic context
> kernel/irq/manage.c:732 disable_irq() warn: sleeping in atomic context
> kernel/irq/msi.c:334 msi_lock_descs() warn: sleeping in atomic context
> kernel/irq_work.c:279 irq_work_sync() warn: sleeping in atomic context
> kernel/jump_label.c:217 static_key_enable() warn: sleeping in atomic context
> kernel/kcov.c:428 kcov_put() warn: sleeping in atomic context
> kernel/kthread.c:709 kthread_stop() warn: sleeping in atomic context
> kernel/locking/mutex.c:870 ww_mutex_lock() warn: sleeping in atomic context
> kernel/locking/rtmutex.c:1586 rt_mutex_handle_deadlock() warn: sleeping in atomic context
> kernel/locking/rwsem.c:1519 down_read() warn: sleeping in atomic context
> kernel/locking/semaphore.c:58 down() warn: sleeping in atomic context
> kernel/locking/semaphore.c:82 down_interruptible() warn: sleeping in atomic context
> kernel/notifier.c:381 blocking_notifier_call_chain() warn: sleeping in atomic context
> kernel/printk/printk.c:2607 console_lock() warn: sleeping in atomic context
> kernel/printk/printk.c:3071 console_unblank() warn: sleeping in atomic context
> kernel/rcu/tree.c:3307 kvfree_call_rcu() warn: sleeping in atomic context
> kernel/rcu/tree_nocb.h:1276 rcu_nocb_rdp_offload() warn: sleeping in atomic context
> kernel/relay.c:354 relay_create_buf_file() warn: sleeping in atomic context
> kernel/relay.c:620 relay_late_setup_files() warn: sleeping in atomic context
> kernel/relay.c:798 relay_flush() warn: sleeping in atomic context
> kernel/resource.c:1221 __request_region() warn: sleeping in atomic context
> kernel/sched/completion.c:117 wait_for_common() warn: sleeping in atomic context
> kernel/sched/completion.c:138 wait_for_completion() warn: sleeping in atomic context
> kernel/softirq.c:901 tasklet_unlock_wait() warn: sleeping in atomic context
> kernel/trace/fgraph.c:63 ftrace_graph_stop() warn: sleeping in atomic context
> kernel/trace/trace_selftest.c:769 trace_graph_entry_watchdog() warn: sleeping in atomic context
> kernel/workqueue.c:2913 __flush_workqueue() warn: sleeping in atomic context
> lib/devres.c:356 pcim_iomap_table() warn: sleeping in atomic context
> lib/kobject_uevent.c:524 kobject_uevent_env() warn: sleeping in atomic context
> lib/kunit/test.c:735 kunit_kfree() warn: sleeping in atomic context
> lib/locking-selftest.c:2236 ww_test_spin_block() warn: sleeping in atomic context
> lib/locking-selftest.c:2277 ww_test_spin_context() warn: sleeping in atomic context
> lib/locking-selftest.c:2642 MUTEX_in_HARDIRQ() warn: sleeping in atomic context
> lib/maple_tree.c:6112 mas_erase() warn: sleeping in atomic context
> lib/maple_tree.c:6221 mtree_store_range() warn: sleeping in atomic context
> lib/maple_tree.c:6330 mtree_alloc_range() warn: sleeping in atomic context
> lib/maple_tree.c:6363 mtree_alloc_rrange() warn: sleeping in atomic context
> lib/scatterlist.c:900 sg_miter_next() warn: sleeping in atomic context
> lib/test_maple_tree.c:1261 check_prev_entry() warn: sleeping in atomic context
> lib/test_maple_tree.c:1296 check_root_expand() warn: sleeping in atomic context
> mm/gup.c:2254 get_user_pages_unlocked() warn: sleeping in atomic context
> mm/gup.c:2941 internal_get_user_pages_fast() warn: sleeping in atomic context
> mm/huge_memory.c:161 get_huge_zero_page() warn: sleeping in atomic context
> mm/kmemleak.c:804 delete_object_part() warn: sleeping in atomic context
> mm/memblock.c:442 memblock_double_array() warn: sleeping in atomic context
> mm/memory.c:5823 ptlock_alloc() warn: sleeping in atomic context
> mm/mempolicy.c:2615 mpol_misplaced() warn: sleeping in atomic context
> mm/vmalloc.c:2244 vm_unmap_ram() warn: sleeping in atomic context
> mm/vmalloc.c:3279 vmalloc() warn: sleeping in atomic context
> mm/vmalloc.c:3319 vzalloc() warn: sleeping in atomic context
> net/bluetooth/l2cap_core.c:3134 l2cap_build_cmd() warn: sleeping in atomic context
> net/bluetooth/sco.c:1295 sco_conn_ready() warn: sleeping in atomic context
> net/core/dev.c:6397 napi_disable() warn: sleeping in atomic context
> net/core/sock.c:3253 sock_no_sendpage_locked() warn: sleeping in atomic context
> net/core/sock.c:3474 lock_sock_nested() warn: sleeping in atomic context
> net/core/stream.c:145 sk_stream_wait_memory() warn: sleeping in atomic context
> net/core/stream.c:75 sk_stream_wait_connect() warn: sleeping in atomic context
> net/ipv4/tcp.c:4468 tcp_alloc_md5sig_pool() warn: sleeping in atomic context
> net/ipv6/addrconf.c:2192 addrconf_leave_solict() warn: sleeping in atomic context
> net/ipv6/mcast.c:925 __ipv6_dev_mc_inc() warn: sleeping in atomic context
> net/ipv6/mcast.c:970 __ipv6_dev_mc_dec() warn: sleeping in atomic context
> net/sctp/primitive.c:198 sctp_primitive_ASCONF() warn: sleeping in atomic context
> net/sctp/socket.c:482 sctp_send_asconf() warn: sleeping in atomic context
> net/smc/smc_core.c:1306 smcr_buf_free() warn: sleeping in atomic context
> net/smc/smc_wr.c:215 smc_wr_tx_get_free_slot() warn: sleeping in atomic context
> net/socket.c:3607 kernel_sendpage_locked() warn: sleeping in atomic context
> net/sunrpc/xprt.c:1120 xprt_wait_on_pinned_rqst() warn: sleeping in atomic context
> net/sunrpc/xprt.c:1448 xprt_request_dequeue_xprt() warn: sleeping in atomic context
> net/sunrpc/xprt.c:2113 xprt_destroy() warn: sleeping in atomic context
> net/sunrpc/xprt.c:2134 xprt_destroy_kref() warn: sleeping in atomic context
> net/sunrpc/xprt.c:2158 xprt_put() warn: sleeping in atomic context
> security/apparmor/lsm.c:1663 aa_get_buffer() warn: sleeping in atomic context
> sound/core/jack.c:673 snd_jack_report() warn: sleeping in atomic context
> sound/core/pcm_native.c:1226 snd_pcm_action_group() warn: sleeping in atomic context
> sound/core/pcm_native.c:168 _snd_pcm_stream_lock_irqsave() warn: sleeping in atomic context
> sound/core/pcm_native.c:95 snd_pcm_group_lock() warn: sleeping in atomic context
> sound/isa/msnd/msnd.c:183 snd_msnd_disable_irq() warn: sleeping in atomic context

> arch/x86/mm/mmio-mod.c:318 mmiotrace_iounmap() warn: sleeping in atomic context
> block/blk-wbt.c:673 wbt_enable_default() warn: sleeping in atomic context
> block/blk-wbt.c:843 wbt_init() warn: sleeping in atomic context
> drivers/accel/habanalabs/common/context.c:112 hl_ctx_fini() warn: sleeping in atomic context
> drivers/accel/habanalabs/common/context.c:141 hl_ctx_do_release() warn: sleeping in atomic context
> drivers/accel/habanalabs/common/device.c:1071 hl_device_set_debug_mode() warn: sleeping in atomic context
> drivers/accel/habanalabs/common/memory.c:1300 unmap_device_va() warn: sleeping in atomic context
> drivers/accel/habanalabs/common/memory.c:2842 hl_vm_ctx_fini() warn: sleeping in atomic context
> drivers/base/power/sysfs.c:789 wakeup_sysfs_remove() warn: sleeping in atomic context
> drivers/gpu/drm/i915/gt/intel_gt_mcr.c:379 intel_gt_mcr_lock() warn: sleeping in atomic context
> drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c:784 intel_guc_ct_send() warn: sleeping in atomic context
> drivers/mfd/ezx-pcap.c:220 pcap_set_ts_bits() warn: sleeping in atomic context
> drivers/mfd/ezx-pcap.c:232 pcap_disable_adc() warn: sleeping in atomic context
> drivers/mfd/ezx-pcap.c:247 pcap_adc_trigger() warn: sleeping in atomic context
> drivers/mfd/ezx-pcap.c:252 pcap_adc_trigger() warn: sleeping in atomic context
> drivers/mfd/ezx-pcap.c:280 pcap_adc_irq() warn: sleeping in atomic context
> drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.c:330 ppm_destroy() warn: sleeping in atomic context
> drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.c:338 cxgbi_ppm_release() warn: sleeping in atomic context
> drivers/net/ethernet/mellanox/mlx5/core/cmd.c:1861 cmd_exec() warn: sleeping in atomic context
> drivers/net/phy/phy.c:1027 phy_ethtool_ksettings_set() warn: sleeping in atomic context
> drivers/net/wireless/ath/ath11k/peer.c:396 ath11k_peer_create() warn: sleeping in atomic context
> drivers/net/wwan/t7xx/t7xx_state_monitor.c:416 t7xx_fsm_append_cmd() warn: sleeping in atomic context
> drivers/pci/msi/msi.c:864 __pci_restore_msix_state() warn: sleeping in atomic context
> drivers/pcmcia/pcmcia_resource.c:168 pcmcia_access_config() warn: sleeping in atomic context
> drivers/pcmcia/pcmcia_resource.c:208 pcmcia_write_config_byte() warn: sleeping in atomic context
> drivers/phy/phy-core.c:404 phy_set_mode_ext() warn: sleeping in atomic context
> drivers/platform/olpc/olpc-xo175-ec.c:567 olpc_xo175_ec_cmd() warn: sleeping in atomic context
> drivers/scsi/lpfc/lpfc_hbadisc.c:5310 lpfc_unreg_rpi() warn: sleeping in atomic context
> drivers/scsi/lpfc/lpfc_hbadisc.c:5865 lpfc_issue_clear_la() warn: sleeping in atomic context
> drivers/scsi/lpfc/lpfc_hbadisc.c:5887 lpfc_issue_reg_vpi() warn: sleeping in atomic context
> drivers/scsi/lpfc/lpfc_hbadisc.c:6563 lpfc_nlp_init() warn: sleeping in atomic context
> drivers/scsi/lpfc/lpfc_init.c:8987 lpfc_sli4_create_rpi_hdr() warn: sleeping in atomic context
> drivers/scsi/lpfc/lpfc_sli.c:19541 lpfc_sli4_post_rpi_hdr() warn: sleeping in atomic context
> drivers/spi/spi.c:4245 spi_sync() warn: sleeping in atomic context
> drivers/staging/rtl8723bs/core/rtw_pwrctrl.c:560 LeaveAllPowerSaveMode() warn: sleeping in atomic context
> drivers/staging/rtl8723bs/core/rtw_pwrctrl.c:563 LeaveAllPowerSaveMode() warn: sleeping in atomic context
> drivers/staging/rtl8723bs/core/rtw_pwrctrl.c:80 ips_leave() warn: sleeping in atomic context
> drivers/staging/rtl8723bs/core/rtw_xmit.c:2520 rtw_sctx_wait() warn: sleeping in atomic context
> drivers/usb/cdns3/core.c:64 cdns_role_stop() warn: sleeping in atomic context
> drivers/usb/gadget/udc/core.c:740 usb_gadget_disconnect() warn: sleeping in atomic context
> drivers/usb/gadget/udc/max3420_udc.c:1308 max3420_remove() warn: sleeping in atomic context
> drivers/usb/host/max3421-hcd.c:1934 max3421_remove() warn: sleeping in atomic context
> drivers/video/fbdev/omap2/omapfb/dss/dispc-compat.c:538 dispc_mgr_enable_digit_out() warn: sleeping in atomic context
> drivers/xen/events/events_base.c:1213 bind_evtchn_to_irq_chip() warn: sleeping in atomic context
> drivers/xen/events/events_base.c:1427 unbind_from_irq() warn: sleeping in atomic context
> drivers/xen/xen-scsiback.c:1016 __scsiback_del_translation_entry() warn: sleeping in atomic context
> drivers/xen/xen-scsiback.c:276 scsiback_free_translation_entry() warn: sleeping in atomic context
> fs/debugfs/inode.c:765 debugfs_remove() warn: sleeping in atomic context
> fs/fscache/volume.c:420 fscache_put_volume() warn: sleeping in atomic context
> fs/gfs2/util.c:356 gfs2_withdraw() warn: sleeping in atomic context
> fs/notify/mark.c:496 fsnotify_destroy_mark() warn: sleeping in atomic context
> fs/notify/mark.c:854 fsnotify_destroy_marks() warn: sleeping in atomic context
> fs/proc/inode.c:267 proc_entry_rundown() warn: sleeping in atomic context
> ./include/linux/fsnotify_backend.h:266 fsnotify_group_lock() warn: sleeping in atomic context
> ./include/net/sock.h:1725 lock_sock() warn: sleeping in atomic context
> kernel/cpu.c:310 cpus_read_lock() warn: sleeping in atomic context
> kernel/irq/msi.c:334 msi_lock_descs() warn: sleeping in atomic context
> kernel/jump_label.c:217 static_key_enable() warn: sleeping in atomic context
> kernel/kthread.c:709 kthread_stop() warn: sleeping in atomic context
> kernel/locking/semaphore.c:58 down() warn: sleeping in atomic context
> kernel/relay.c:798 relay_flush() warn: sleeping in atomic context
> kernel/sched/completion.c:117 wait_for_common() warn: sleeping in atomic context
> kernel/sched/completion.c:138 wait_for_completion() warn: sleeping in atomic context
> kernel/trace/fgraph.c:63 ftrace_graph_stop() warn: sleeping in atomic context
> kernel/trace/trace_selftest.c:769 trace_graph_entry_watchdog() warn: sleeping in atomic context
> kernel/workqueue.c:2913 __flush_workqueue() warn: sleeping in atomic context
> net/core/stream.c:145 sk_stream_wait_memory() warn: sleeping in atomic context
> net/core/stream.c:75 sk_stream_wait_connect() warn: sleeping in atomic context
> net/ipv6/addrconf.c:2192 addrconf_leave_solict() warn: sleeping in atomic context
> net/ipv6/mcast.c:970 __ipv6_dev_mc_dec() warn: sleeping in atomic context
> net/sunrpc/xprt.c:2134 xprt_destroy_kref() warn: sleeping in atomic context
> net/sunrpc/xprt.c:2158 xprt_put() warn: sleeping in atomic context
> sound/core/jack.c:673 snd_jack_report() warn: sleeping in atomic context
> sound/core/pcm_native.c:1226 snd_pcm_action_group() warn: sleeping in atomic context


2023-02-06 18:25:28

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH v4] pipe: use __pipe_{lock,unlock} instead of spinlock

On Mon, Feb 6, 2023 at 8:45 AM Dan Carpenter <[email protected]> wrote:
>
> You need the cross function database to review these warnings. [...]
>
> hl_device_set_debug_mode() take a mutex. Then you do
> `smdb.py preempt hl_ctx_fini` and it prints out the call tree which
> disables preemption.
>
> cs_ioctl_unreserve_signals() <- disables preempt
> -> hl_ctx_put()
> -> hl_ctx_do_release()
> -> hl_ctx_fini()
>
> And so on.

Hmm. Do you have automation to do that at least for the non-driver (ie
"core kernel code") ones?

They are *hopefully* false positives, but if not they are obviously
the most interesting.

And they are presumably not quite as overwhelming as all the driver
ones, so even if they *are* false positives, maybe they would then be
the point to start looking at why the tool gives the wrong answer?

Linus

2023-02-07 07:03:04

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v4] pipe: use __pipe_{lock,unlock} instead of spinlock

On Mon, Feb 06, 2023 at 09:54:47AM -0800, Luis Chamberlain wrote:
> > block/blk-mq.c:4083 blk_mq_destroy_queue() warn: sleeping in atomic context
>
> Let's see as an example.
>
> blk_mq_exit_hctx() can spin_lock() and so could disable preemption but I
> can't see why this is sleeping in atomic context.
>

I should have said, the lines are from linux-next.

block/blk-mq.c
4078 void blk_mq_destroy_queue(struct request_queue *q)
4079 {
4080 WARN_ON_ONCE(!queue_is_mq(q));
4081 WARN_ON_ONCE(blk_queue_registered(q));
4082
4083 might_sleep();
^^^^^^^^^^^^^^

This is a weird example because today's cross function DB doesn't say
which function disables preemption. The output from `smdb.py preempt
blk_mq_destroy_queue` says:

nvme_remove_admin_tag_set()
nvme_remove_io_tag_set()
-> blk_mq_destroy_queue()

I would have assumed that nothing is disabling preempt and the
information just hasn't propagated through the call tree yet. However
yesterday's DB has enough information to show why the warning is
generated.

nvme_fc_match_disconn_ls() takes spin_lock_irqsave(&rport->lock, flags);
-> nvme_fc_ctrl_put(ctrl);
-> kref_put(&ctrl->ref, nvme_fc_ctrl_free);
-> nvme_remove_admin_tag_set(&ctrl->ctrl);
-> blk_mq_destroy_queue(ctrl->admin_q);
-> blk_mq_destroy_queue() <-- sleeps

It's the link between kref_put() and nvme_fc_ctrl_free() where the data
gets lost in today's DB. kref_put() is tricky to handle. I'm just
puzzled why it worked yesterday.

regards,
dan carpenter


2023-02-07 13:37:52

by Dan Carpenter

[permalink] [raw]
Subject: xen: sleeping in atomic warnings

These are static checker warnings from Smatch. The line numbers are
based on next-20230207. To reproduce these warnings then you need to
have the latest Smatch from git and you need to rebuild the cross
function probably four times. I have reviewed most of these and they
all seem valid to me. I remember I reported some a while back but never
heard back. https://lore.kernel.org/all/20210802144037.GA29540@kili/

regards,
dan carpenter

arch/x86/xen/p2m.c:189 alloc_p2m_page() warn: sleeping in atomic context
xen_create_contiguous_region() <- disables preempt
xen_destroy_contiguous_region() <- disables preempt
-> xen_remap_exchanged_ptes()
-> set_phys_to_machine()
-> xen_alloc_p2m_entry()
-> alloc_p2m_pmd()
xen_alloc_p2m_entry() <duplicate>
-> alloc_p2m_page()

drivers/xen/events/events_base.c:1213 bind_evtchn_to_irq_chip() warn: sleeping in atomic context
pvcalls_front_connect() <- disables preempt
pvcalls_front_accept() <- disables preempt
-> create_active()
-> bind_evtchn_to_irqhandler()
-> bind_evtchn_to_irqhandler_chip()
-> bind_evtchn_to_irq_chip()

drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c:303 netxen_pcie_sem_lock() warn: sleeping in atomic context
netxen_nic_hw_write_wx_2M() <- disables preempt
netxen_nic_hw_read_wx_2M() <- disables preempt
-> netxen_pcie_sem_lock()

drivers/xen/xen-pciback/pci_stub.c:110 pcistub_device_release() warn: sleeping in atomic context
pcistub_get_pci_dev_by_slot() <- disables preempt
pcistub_get_pci_dev() <- disables preempt
-> pcistub_device_get_pci_dev()
-> pcistub_device_put()
-> pcistub_device_release()

drivers/xen/xen-scsiback.c:1016 __scsiback_del_translation_entry() warn: sleeping in atomic context
scsiback_del_translation_entry() <- disables preempt
scsiback_release_translation_entry() <- disables preempt
-> __scsiback_del_translation_entry()

drivers/xen/xen-scsiback.c:276 scsiback_free_translation_entry() warn: sleeping in atomic context
scsiback_del_translation_entry() <- disables preempt
scsiback_release_translation_entry() <- disables preempt
-> __scsiback_del_translation_entry()
-> scsiback_free_translation_entry()

drivers/xen/events/events_base.c:1427 unbind_from_irq() warn: sleeping in atomic context
pvcalls_front_connect() <- disables preempt
pvcalls_front_accept() <- disables preempt
-> create_active()
-> bind_evtchn_to_irqhandler()
-> bind_evtchn_to_irqhandler_chip()
-> unbind_from_irq()

drivers/xen/xenbus/xenbus_client.c:473 xenbus_alloc_evtchn() warn: sleeping in atomic context
pvcalls_front_connect() <- disables preempt
pvcalls_front_accept() <- disables preempt
-> create_active()
-> xenbus_alloc_evtchn()

drivers/xen/xenbus/xenbus_client.c:321 xenbus_dev_error() warn: sleeping in atomic context
pvcalls_front_connect() <- disables preempt
pvcalls_front_accept() <- disables preempt
-> create_active()
-> xenbus_free_evtchn()
-> xenbus_dev_error()

drivers/xen/xenbus/xenbus_client.c:342 xenbus_dev_fatal() warn: sleeping in atomic context
pvcalls_front_connect() <- disables preempt
pvcalls_front_accept() <- disables preempt
-> create_active()
-> xenbus_alloc_evtchn()
-> xenbus_dev_fatal()

drivers/xen/xenbus/xenbus_client.c:494 xenbus_free_evtchn() warn: sleeping in atomic context
pvcalls_front_connect() <- disables preempt
pvcalls_front_accept() <- disables preempt
-> create_active()
-> xenbus_free_evtchn()

drivers/xen/xenbus/xenbus_client.c:290 xenbus_va_dev_error() warn: sleeping in atomic context
pvcalls_front_connect() <- disables preempt
pvcalls_front_accept() <- disables preempt
-> create_active()
-> xenbus_free_evtchn()
-> xenbus_dev_error()
create_active() <duplicate>
-> xenbus_alloc_evtchn()
-> xenbus_dev_fatal()
-> xenbus_va_dev_error()


2023-02-07 14:03:27

by Jürgen Groß

[permalink] [raw]
Subject: Re: xen: sleeping in atomic warnings

On 07.02.23 14:37, Dan Carpenter wrote:
> These are static checker warnings from Smatch. The line numbers are
> based on next-20230207. To reproduce these warnings then you need to
> have the latest Smatch from git and you need to rebuild the cross
> function probably four times. I have reviewed most of these and they
> all seem valid to me. I remember I reported some a while back but never
> heard back. https://lore.kernel.org/all/20210802144037.GA29540@kili/
>
> regards,
> dan carpenter
>
> arch/x86/xen/p2m.c:189 alloc_p2m_page() warn: sleeping in atomic context
> xen_create_contiguous_region() <- disables preempt
> xen_destroy_contiguous_region() <- disables preempt
> -> xen_remap_exchanged_ptes()
> -> set_phys_to_machine()
> -> xen_alloc_p2m_entry()
> -> alloc_p2m_pmd()
> xen_alloc_p2m_entry() <duplicate>
> -> alloc_p2m_page()

Those allocations can't be reached after early boot.

> drivers/xen/events/events_base.c:1213 bind_evtchn_to_irq_chip() warn: sleeping in atomic context
> pvcalls_front_connect() <- disables preempt
> pvcalls_front_accept() <- disables preempt
> -> create_active()
> -> bind_evtchn_to_irqhandler()
> -> bind_evtchn_to_irqhandler_chip()
> -> bind_evtchn_to_irq_chip()

Yes, that one looks very suspicious. Basically the same problem as all
the other pvcalls issues below.

> drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c:303 netxen_pcie_sem_lock() warn: sleeping in atomic context
> netxen_nic_hw_write_wx_2M() <- disables preempt
> netxen_nic_hw_read_wx_2M() <- disables preempt
> -> netxen_pcie_sem_lock()

This is not Xen related.

> drivers/xen/xen-pciback/pci_stub.c:110 pcistub_device_release() warn: sleeping in atomic context
> pcistub_get_pci_dev_by_slot() <- disables preempt
> pcistub_get_pci_dev() <- disables preempt
> -> pcistub_device_get_pci_dev()
> -> pcistub_device_put()
> -> pcistub_device_release()

Seems top be problematic, too.

> drivers/xen/xen-scsiback.c:1016 __scsiback_del_translation_entry() warn: sleeping in atomic context
> scsiback_del_translation_entry() <- disables preempt
> scsiback_release_translation_entry() <- disables preempt
> -> __scsiback_del_translation_entry()

Needs fixing (same fix as the other scsiback issue).

Thanks for the reports,


Juergen


Attachments:
OpenPGP_0xB0DE9DD628BF132F.asc (3.03 kB)
OpenPGP public key
OpenPGP_signature (495.00 B)
OpenPGP digital signature
Download all attachments

2023-02-07 14:06:53

by Dan Carpenter

[permalink] [raw]
Subject: block: sleeping in atomic warnings

These are static checker warnings from Smatch. The line numbers are
based on next-20230203. To reproduce these warnings then you need to
have the latest Smatch from git and you need to rebuild the cross
function probably four times. I have reviewed these. The first few
seem like real issues. I can't make heads or tails out of the
__blk_mq_run_hw_queue() warning. I suspect that the last warning is a
false positive. I remember I reported some a while back but never
heard back. https://lore.kernel.org/all/YNx1r8Jr3+t4bch%2F@mwanda/

regards,
dan carpenter

block/blk-crypto-profile.c:382 __blk_crypto_evict_key() warn: sleeping in atomic context
block/blk-crypto-profile.c:390 __blk_crypto_evict_key() warn: sleeping in atomic context
put_super() <- disables preempt
__iterate_supers() <- disables preempt
iterate_supers() <- disables preempt
iterate_supers_type() <- disables preempt
get_super() <- disables preempt
user_get_super() <- disables preempt
-> __put_super()
-> fscrypt_destroy_keyring()
-> fscrypt_put_master_key_activeref()
-> fscrypt_destroy_prepared_key()
-> fscrypt_destroy_inline_crypt_key()
-> blk_crypto_evict_key()
blk_crypto_evict_key() <duplicate>
-> blk_crypto_fallback_evict_key()
-> __blk_crypto_evict_key()

block/blk-mq.c:206 blk_freeze_queue() warn: sleeping in atomic context
rexmit_timer() <- disables preempt
-> aoedev_downdev()
-> blk_mq_freeze_queue()
-> blk_freeze_queue()

block/blk-mq.c:4083 blk_mq_destroy_queue() warn: sleeping in atomic context
nvme_fc_match_disconn_ls() <- disables preempt
-> nvme_fc_ctrl_put()
-> nvme_fc_ctrl_free()
-> nvme_remove_admin_tag_set()
nvme_fc_ctrl_free() <duplicate>
-> nvme_remove_io_tag_set()
-> blk_mq_destroy_queue()

block/blk-mq.c:2174 __blk_mq_run_hw_queue() warn: sleeping in atomic context
__blk_mq_run_hw_queue() <duplicate>
-> blk_mq_sched_dispatch_requests()
-> __blk_mq_sched_dispatch_requests()
-> blk_mq_do_dispatch_sched()
blk_mq_do_dispatch_sched() <duplicate>
-> __blk_mq_do_dispatch_sched()
-> blk_mq_dispatch_hctx_list()
__blk_mq_do_dispatch_sched() <duplicate>
__blk_mq_sched_dispatch_requests() <duplicate>
-> blk_mq_do_dispatch_ctx()
__blk_mq_sched_dispatch_requests() <duplicate>
-> blk_mq_dispatch_rq_list()
__blk_mq_do_dispatch_sched() <duplicate>
blk_mq_do_dispatch_ctx() <duplicate>
-> blk_mq_delay_run_hw_queues()
-> blk_mq_delay_run_hw_queue()
sg_remove_sfp_usercontext() <- disables preempt
-> sg_finish_rem_req()
dd_insert_requests() <- disables preempt
-> dd_insert_request()
-> blk_mq_free_requests()
mspro_block_complete_req() <- disables preempt
mspro_queue_rq() <- disables preempt
-> mspro_block_issue_req()
mspro_block_complete_req() <- disables preempt <duplicate>
aoe_flush_iocq_by_index() <- disables preempt
rexmit_timer() <- disables preempt
-> aoedev_downdev()
-> aoe_failip()
aoedev_downdev() <duplicate>
-> downdev_frame()
-> aoe_failbuf()
-> aoe_end_buf()
aoe_failip() <duplicate>
-> aoe_end_request()
-> __blk_mq_end_request()
-> blk_mq_free_request()
-> __blk_mq_free_request()
-> blk_mq_sched_restart()
-> __blk_mq_sched_restart()
blk_mq_sched_dispatch_requests() <duplicate>
blk_mq_dispatch_rq_list() <duplicate>
rexmit_timer() <- disables preempt <duplicate>
aoe_end_request() <duplicate>
bfq_finish_requeue_request() <- disables preempt
-> bfq_completed_request()
bfq_idle_slice_timer_body() <- disables preempt
bfq_pd_offline() <- disables preempt
-> bfq_put_async_queues()
-> __bfq_put_async_bfqq()
bfq_bio_merge() <- disables preempt
bfq_insert_request() <- disables preempt
-> bfq_init_rq()
-> bfq_bic_update_cgroup()
-> __bfq_bic_change_cgroup()
-> bfq_sync_bfqq_move()
bfq_pd_offline() <- disables preempt <duplicate>
-> bfq_reparent_active_queues()
-> bfq_reparent_leaf_entity()
-> bfq_bfqq_move()
-> bfq_schedule_dispatch()
nvme_fc_match_disconn_ls() <- disables preempt
-> nvme_fc_ctrl_put()
-> nvme_fc_ctrl_free()
-> nvme_unquiesce_admin_queue()
-> blk_mq_unquiesce_queue()
-> blk_mq_run_hw_queues()
virtblk_done() <- disables preempt
virtblk_poll() <- disables preempt
-> blk_mq_start_stopped_hw_queues()
-> blk_mq_start_stopped_hw_queue()
-> blk_mq_run_hw_queue()
-> __blk_mq_delay_run_hw_queue()
-> __blk_mq_run_hw_queue()

block/blk-wbt.c:843 wbt_init() warn: sleeping in atomic context
ioc_qos_write() <- disables preempt
-> wbt_enable_default()
-> wbt_init()


2023-02-07 16:15:42

by Linus Torvalds

[permalink] [raw]
Subject: Re: block: sleeping in atomic warnings

On Tue, Feb 7, 2023 at 6:06 AM Dan Carpenter <[email protected]> wrote:
>
> block/blk-crypto-profile.c:382 __blk_crypto_evict_key() warn: sleeping in atomic context
> block/blk-crypto-profile.c:390 __blk_crypto_evict_key() warn: sleeping in atomic context

Yeah, that looks very real, but doesn't really seem to be a block bug.

__put_super() has a big comment that it's called under the sb_lock
spinlock, so it's all in atomic context, but then:

> -> __put_super()
> -> fscrypt_destroy_keyring()
> -> fscrypt_put_master_key_activeref()
> -> fscrypt_destroy_prepared_key()
> -> fscrypt_destroy_inline_crypt_key()
> -> blk_crypto_evict_key()

and we have a comment in __blk_crypto_evict_key() that it must be
called in "process context".

However, the *normal* unmount sequence does all the cleanup *before*
it gets sb_lock, and calls fscrypt_destroy_keyring() in process
context, which is probably why it never triggers in practice, because
the "last put" is normally there, not in __put_super.

Eric? Al?

It smells like __put_super() may need to do some parts delayed, not
under sb_lock.

Linus

2023-02-07 17:53:44

by Eric Biggers

[permalink] [raw]
Subject: Re: block: sleeping in atomic warnings

On Tue, Feb 07, 2023 at 08:15:04AM -0800, Linus Torvalds wrote:
> On Tue, Feb 7, 2023 at 6:06 AM Dan Carpenter <[email protected]> wrote:
> >
> > block/blk-crypto-profile.c:382 __blk_crypto_evict_key() warn: sleeping in atomic context
> > block/blk-crypto-profile.c:390 __blk_crypto_evict_key() warn: sleeping in atomic context
>
> Yeah, that looks very real, but doesn't really seem to be a block bug.
>
> __put_super() has a big comment that it's called under the sb_lock
> spinlock, so it's all in atomic context, but then:
>
> > -> __put_super()
> > -> fscrypt_destroy_keyring()
> > -> fscrypt_put_master_key_activeref()
> > -> fscrypt_destroy_prepared_key()
> > -> fscrypt_destroy_inline_crypt_key()
> > -> blk_crypto_evict_key()
>
> and we have a comment in __blk_crypto_evict_key() that it must be
> called in "process context".
>
> However, the *normal* unmount sequence does all the cleanup *before*
> it gets sb_lock, and calls fscrypt_destroy_keyring() in process
> context, which is probably why it never triggers in practice, because
> the "last put" is normally there, not in __put_super.
>
> Eric? Al?
>
> It smells like __put_super() may need to do some parts delayed, not
> under sb_lock.
>

It's a false positive. See the comment above fscrypt_destroy_keyring(), which
is meant to explain this, though I can update the comment to be clearer. If the
filesystem has been mounted, then fscrypt_destroy_keyring() is called from
generic_shutdown_super(), which can sleep, and the call from __put_super() is a
no-op. If the filesystem has not been mounted, then the call from __put_super()
is needed, but blk_crypto_evict_key() can never be executed in that case.

- Eric

2023-02-07 18:25:56

by Linus Torvalds

[permalink] [raw]
Subject: Re: block: sleeping in atomic warnings

On Tue, Feb 7, 2023 at 9:53 AM Eric Biggers <[email protected]> wrote:
>
> It's a false positive. See the comment above fscrypt_destroy_keyring()

Hmm. Ok. Unfortunate.

> If the filesystem has not been mounted, then the call from __put_super()
> is needed, but blk_crypto_evict_key() can never be executed in that case.

It's not all that clear that some *other* error might not have
happened to keep the mount from actually succeeding, but after the
keys have been instantiated?

IOW, what's the thing that makes "blk_crypto_evict_key() can never be
executed in that case" be obvious?

I think _that_ is what might want a comment, about how we always call
generic_shutdown_super() before the last put_super() happens.

It does seem like Dan's automated checks could be useful, but if
there's no sane way to avoid the false positives, it's always going to
be a lot of noise ;(

Linus

2023-02-07 18:32:22

by Jens Axboe

[permalink] [raw]
Subject: Re: block: sleeping in atomic warnings

On 2/7/23 7:06?AM, Dan Carpenter wrote:
> block/blk-mq.c:206 blk_freeze_queue() warn: sleeping in atomic context
> rexmit_timer() <- disables preempt
> -> aoedev_downdev()
> -> blk_mq_freeze_queue()
> -> blk_freeze_queue()

That is definitely a legit bug, aoe should punt to a workqueue or
similar.

> block/blk-mq.c:4083 blk_mq_destroy_queue() warn: sleeping in atomic context
> nvme_fc_match_disconn_ls() <- disables preempt
> -> nvme_fc_ctrl_put()
> -> nvme_fc_ctrl_free()
> -> nvme_remove_admin_tag_set()
> nvme_fc_ctrl_free() <duplicate>
> -> nvme_remove_io_tag_set()
> -> blk_mq_destroy_queue()

Also looks like a legitimate bug.

> block/blk-mq.c:2174 __blk_mq_run_hw_queue() warn: sleeping in atomic context
> __blk_mq_run_hw_queue() <duplicate>
> -> blk_mq_sched_dispatch_requests()
> -> __blk_mq_sched_dispatch_requests()
> -> blk_mq_do_dispatch_sched()
> blk_mq_do_dispatch_sched() <duplicate>
> -> __blk_mq_do_dispatch_sched()
> -> blk_mq_dispatch_hctx_list()
> __blk_mq_do_dispatch_sched() <duplicate>
> __blk_mq_sched_dispatch_requests() <duplicate>
> -> blk_mq_do_dispatch_ctx()
> __blk_mq_sched_dispatch_requests() <duplicate>
> -> blk_mq_dispatch_rq_list()
> __blk_mq_do_dispatch_sched() <duplicate>
> blk_mq_do_dispatch_ctx() <duplicate>
> -> blk_mq_delay_run_hw_queues()
> -> blk_mq_delay_run_hw_queue()

This one I'm not really following... Would it be possible to expand the
reporting to be a bit more verbose?

> sg_remove_sfp_usercontext() <- disables preempt
> block/blk-wbt.c:843 wbt_init() warn: sleeping in atomic context
> ioc_qos_write() <- disables preempt
> -> wbt_enable_default()
> -> wbt_init()

Also definitely a bug.

--
Jens Axboe


2023-02-07 18:36:46

by Eric Biggers

[permalink] [raw]
Subject: Re: block: sleeping in atomic warnings

On Tue, Feb 07, 2023 at 10:24:45AM -0800, Linus Torvalds wrote:
> On Tue, Feb 7, 2023 at 9:53 AM Eric Biggers <[email protected]> wrote:
> >
> > It's a false positive. See the comment above fscrypt_destroy_keyring()
>
> Hmm. Ok. Unfortunate.
>
> > If the filesystem has not been mounted, then the call from __put_super()
> > is needed, but blk_crypto_evict_key() can never be executed in that case.
>
> It's not all that clear that some *other* error might not have
> happened to keep the mount from actually succeeding, but after the
> keys have been instantiated?
>
> IOW, what's the thing that makes "blk_crypto_evict_key() can never be
> executed in that case" be obvious?
>
> I think _that_ is what might want a comment, about how we always call
> generic_shutdown_super() before the last put_super() happens.
>
> It does seem like Dan's automated checks could be useful, but if
> there's no sane way to avoid the false positives, it's always going to
> be a lot of noise ;(
>

blk_crypto_evict_key() only runs if a key was prepared for inline encryption,
which can only happen if a user does I/O to an encrypted file. That can only
happen after the filesystem was successfully mounted.

Also note that keys are normally added using an ioctl, which can only be
executed after the filesystem was mounted. The only exception is the key
associated with the "test_dummy_encryption" mount option.

By the way, the following code is in generic_shutdown_super(), and not in
__put_super(), for a very similar reason:

if (sb->s_dio_done_wq) {
destroy_workqueue(sb->s_dio_done_wq);
sb->s_dio_done_wq = NULL;
}

That code is only needed if there has been user I/O to the filesystem, which
again can only have happened if the filesystem was successfully mounted.

- Eric

2023-02-07 18:57:35

by Linus Torvalds

[permalink] [raw]
Subject: Re: block: sleeping in atomic warnings

On Tue, Feb 7, 2023 at 10:36 AM Eric Biggers <[email protected]> wrote:
>
> Also note that keys are normally added using an ioctl, which can only be
> executed after the filesystem was mounted. The only exception is the key
> associated with the "test_dummy_encryption" mount option.

Could we perhaps then replace the

fscrypt_destroy_keyring(s);

with a more specific

fscrypt_destroy_dummy_keyring(s);

thing, that would only handle the dummy encryption case?

Ot could we just *fix* the dummy encryption test to actually work like
real encryption cases, so that it doesn't have this bogus case?


> By the way, the following code is in generic_shutdown_super(), and not in
> __put_super(), for a very similar reason:

Well, but that isn't a problem, exactly because that code isn't in
__put_super().

Put another way: the problem with the dummy encryption appears to be
exactly that it doesn't actually act like any real encryption would,
and then triggers that "this whole sequence gets called even under the
spinlock, even though it's documented to not be valid for that case,
because we added a bogus test-case that doesn't actually match
reality".

Looking at Jens' reply to the other cases, Dan's tool seems to be on
the money here except for this self-inflicted bogus crypto key thing.

Linus

2023-02-07 19:10:01

by Linus Torvalds

[permalink] [raw]
Subject: Re: block: sleeping in atomic warnings

On Tue, Feb 7, 2023 at 10:57 AM Linus Torvalds
<[email protected]> wrote:
>
> Looking at Jens' reply to the other cases, Dan's tool seems to be on
> the money here except for this self-inflicted bogus crypto key thing.

Oh, except you probably wouldn't have seen it, because Jens replied to
the original message where you weren't cc'd.

So here's the context for you wrt the other parts of the report:

https://lore.kernel.org/all/[email protected]/

where there isn't that same indirection through the super-block, but
more of a clear-cut "this is all in the block layer".

Linus

2023-02-07 19:36:02

by Eric Biggers

[permalink] [raw]
Subject: Re: block: sleeping in atomic warnings

On Tue, Feb 07, 2023 at 10:57:08AM -0800, Linus Torvalds wrote:
> On Tue, Feb 7, 2023 at 10:36 AM Eric Biggers <[email protected]> wrote:
> >
> > Also note that keys are normally added using an ioctl, which can only be
> > executed after the filesystem was mounted. The only exception is the key
> > associated with the "test_dummy_encryption" mount option.
>
> Could we perhaps then replace the
>
> fscrypt_destroy_keyring(s);
>
> with a more specific
>
> fscrypt_destroy_dummy_keyring(s);
>
> thing, that would only handle the dummy encryption case?


Sure, they would still need to do most of the same things though.

> Or could we just *fix* the dummy encryption test to actually work like
> real encryption cases, so that it doesn't have this bogus case?

We've wanted to do that for a very long time, but there never has been a way to
actually do it. Especially with the filesystem-level keyring now, if the kernel
doesn't automatically add the key for test_dummy_encryption, then userspace
would have to do it *every time it mounts the filesystem*.

The point of the "test_dummy_encryption" mount option is that you can just add
it to the mount options and run existing tests, such as a full run of xfstests,
and test all the encrypted I/O paths that way. Which is extremely useful; it
wouldn't really be possible to properly test the encryption feature without it.

So that's why we've gone through some pain to keep "test_dummy_encryption"
working over time.

Now, it's possible that "the kernel automatically adds the key for
test_dummy_encryption" could be implemented a bit differently. It maybe could
be done at the last minute, when the key is being looked for due to a user
filesystem operation, instead of during the mount itself. That would eliminate
the need to call fscrypt_destroy_keyring() from __put_super(), which would avoid
the issue being discussed here. I'll see if there's a good way to do that.

- Eric

2023-02-07 19:50:04

by Linus Torvalds

[permalink] [raw]
Subject: Re: block: sleeping in atomic warnings

On Tue, Feb 7, 2023 at 11:35 AM Eric Biggers <[email protected]> wrote:
>
> The point of the "test_dummy_encryption" mount option is that you can just add
> it to the mount options and run existing tests, such as a full run of xfstests,
> and test all the encrypted I/O paths that way. Which is extremely useful; it
> wouldn't really be possible to properly test the encryption feature without it.

Yes, I see how useful that is, but:

> Now, it's possible that "the kernel automatically adds the key for
> test_dummy_encryption" could be implemented a bit differently. It maybe could
> be done at the last minute, when the key is being looked for due to a user
> filesystem operation, instead of during the mount itself.

Yeah, that sounds like it would be the right solution, and get rid of
the fscrypt_destroy_keyring() case in __put_super().

Hmm.

I guess the filesystem only ever sees the '->get_tree()' call, and
then never gets any "this mount succeeded" callback.

And we do have at least that

error = security_sb_set_mnt_opts(sb, fc->security, 0, NULL);
if (unlikely(error)) {
fc_drop_locked(fc);
return error;
}

error case that does fc_drop_locked() -> deactivate_locked_super() ->
put_super().

Hmm. It does get "kill_sb()", if that happens, so if

(a) the filesystem registers the keys late only in the success case

and

(b) ->kill_sb() does the fscrypt_destroy_keyring(s)

then I *think* everything would be fine, and put_super() doesn't need to do it.

Or am I missing some case?

Linus

2023-02-08 03:15:36

by Yu Kuai

[permalink] [raw]
Subject: Re: block: sleeping in atomic warnings

Hi,

在 2023/02/08 2:31, Jens Axboe 写道:

>> block/blk-wbt.c:843 wbt_init() warn: sleeping in atomic context
>> ioc_qos_write() <- disables preempt
>> -> wbt_enable_default()
>> -> wbt_init()
>
> Also definitely a bug.
>

This won't happen currently, wbt_init() will be called while
initializing device, and later wbt_enable_devault() from iocost won't
call wbt_init().

However, we might support rq_qos destruction dynamically in the
future, I'll fix this warning.

Thanks,
Kuai


2023-02-08 06:54:05

by Eric Biggers

[permalink] [raw]
Subject: Re: block: sleeping in atomic warnings

On Tue, Feb 07, 2023 at 07:35:54PM +0000, Eric Biggers wrote:
> Now, it's possible that "the kernel automatically adds the key for
> test_dummy_encryption" could be implemented a bit differently. It maybe could
> be done at the last minute, when the key is being looked for due to a user
> filesystem operation, instead of during the mount itself. That would eliminate
> the need to call fscrypt_destroy_keyring() from __put_super(), which would avoid
> the issue being discussed here. I'll see if there's a good way to do that.

"[PATCH 0/5] Add the test_dummy_encryption key on-demand"
(https://lore.kernel.org/linux-fscrypt/[email protected]/T/#u)
implements this.

- Eric