Currently the spu coredump code triggers an RCU warning:
=============================
WARNING: suspicious RCU usage
5.7.0-rc3-01755-g7cd49f0b7ec7 #1 Not tainted
-----------------------------
include/linux/fdtable.h:95 suspicious rcu_dereference_check() usage!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
1 lock held by spu-coredump/1343:
#0: c0000007fa22f430 (sb_writers#2){.+.+}-{0:0}, at:
.do_coredump+0x1010/0x13c8
stack backtrace:
CPU: 0 PID: 1343 Comm: spu-coredump Not tainted 5.7.0-rc3-01755-g7cd49f0b7ec7
#1
Call Trace:
.dump_stack+0xec/0x15c (unreliable)
.lockdep_rcu_suspicious+0x120/0x144
.coredump_next_context+0x148/0x158
.spufs_coredump_extra_notes_size+0x54/0x190
.elf_coredump_extra_notes_size+0x34/0x50
.elf_core_dump+0xe48/0x19d0
.do_coredump+0xe50/0x13c8
.get_signal+0x864/0xd88
.do_notify_resume+0x158/0x3c8
.interrupt_exit_user_prepare+0x19c/0x208
interrupt_return+0x14/0x1c0
This comes from fcheck_files() via fcheck().
It's pretty clearly documented that fcheck() must be wrapped with
rcu_read_lock(), adding that fixes the RCU warning.
hch points out that once we've released the RCU read lock the file may
be closed and freed, which would leave us with a pointer to a freed
spu_context.
To avoid that, take a reference to the spu_context while we hold the
RCU read lock, and drop that reference later once we're done with the
context.
Signed-off-by: Michael Ellerman <[email protected]>
---
arch/powerpc/platforms/cell/spufs/coredump.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
v2: Take a reference and hold it until we're done.
diff --git a/arch/powerpc/platforms/cell/spufs/coredump.c
b/arch/powerpc/platforms/cell/spufs/coredump.c
index 8b3296b62f65..37c155254cd5 100644
--- a/arch/powerpc/platforms/cell/spufs/coredump.c
+++ b/arch/powerpc/platforms/cell/spufs/coredump.c
@@ -82,13 +82,20 @@ static int match_context(const void *v, struct file *file,
unsigned fd)
*/
static struct spu_context *coredump_next_context(int *fd)
{
+ struct spu_context *ctx;
struct file *file;
int n = iterate_fd(current->files, *fd, match_context, NULL);
if (!n)
return NULL;
*fd = n - 1;
+
+ rcu_read_lock();
file = fcheck(*fd);
- return SPUFS_I(file_inode(file))->i_ctx;
+ ctx = SPUFS_I(file_inode(file))->i_ctx;
+ get_spu_context(ctx);
+ rcu_read_unlock();
+
+ return ctx;
}
int spufs_coredump_extra_notes_size(void)
@@ -99,17 +106,23 @@ int spufs_coredump_extra_notes_size(void)
fd = 0;
while ((ctx = coredump_next_context(&fd)) != NULL) {
rc = spu_acquire_saved(ctx);
- if (rc)
+ if (rc) {
+ put_spu_context(ctx);
break;
+ }
+
rc = spufs_ctx_note_size(ctx, fd);
spu_release_saved(ctx);
- if (rc < 0)
+ if (rc < 0) {
+ put_spu_context(ctx);
break;
+ }
size += rc;
/* start searching the next fd next time */
fd++;
+ put_spu_context(ctx);
}
return size;
--
2.25.1
