On Fri, Mar 06, 2020 at 04:25:27PM +0100, David Sterba wrote:

On Fri, Mar 06, 2020 at 07:30:24PM +0530, Madhuparna Bhowmik wrote:

quoted

On Fri, Mar 06, 2020 at 03:16:53PM +0800, Qu Wenruo wrote:

quoted

On 2020/3/6 下午2:52, madhuparnabhowmik10@gmail.com wrote:

quoted

From: Madhuparna Bhowmik <redacted>

The space_info list is rcu protected.
Hence, it should be traversed with rcu_read_lock held.

Warning:
[   29.104591] =============================
[   29.104756] WARNING: suspicious RCU usage
[   29.105046] 5.6.0-rc4-next-20200305 #1 Not tainted
[   29.105231] -----------------------------
[   29.105401] fs/btrfs/block-group.c:2011 RCU-list traversed in non-reader section!!

Reported-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Madhuparna Bhowmik <redacted>
---
 fs/btrfs/block-group.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 404e050ce8ee..9cabeef66f5b 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c

@@ -1987,6 +1987,7 @@ int btrfs_read_block_groups(struct btrfs_fs_info *info)

This function is only triggered at mount time, where no other rcu
operation can happen.

Thanks Qu.

Joel and Paul, what should we do in this case?
Should we just pass cond = true or use list_for_each_entry instead?

I think we can afford to add rcu lock/unlock, even if it's not strictly
necessary due to the single threaded context where the function is run.
There are some lightweight operations inside and inc_block_group starts
with two spin locks so there's nothing we'd be losing with disabled
preemption from the caller.

I think use list_for_each_entry().

thanks,

 - Joel

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees