Re: [GIT PULL] xfs: CIL and log scalability improvements

On Mon, Jun 07, 2021 at 08:11:19AM +1000, Dave Chinner wrote:

On Fri, Jun 04, 2021 at 07:15:33PM -0700, Darrick J. Wong wrote:

quoted

On Fri, Jun 04, 2021 at 07:03:54PM -0700, Darrick J. Wong wrote:

quoted

On Fri, Jun 04, 2021 at 01:29:28PM +1000, Dave Chinner wrote:

quoted

Hi Darrick,

Can you please pull the CIL and log improvements from the tag listed
below?

I tried that and threw the series at fstests, which crashed all VMs with
the following null pointer dereference:

BUG: kernel NULL pointer dereference, address: 0000000000000000
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 0 P4D 0 
Oops: 0000 [#1] PREEMPT SMP
CPU: 2 PID: 731060 Comm: mount Not tainted 5.13.0-rc4-djwx #rc4
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.13.0-1ubuntu1.1 04/01/2014
RIP: 0010:xlog_cil_init+0x2f7/0x370 [xfs]
Code: b4 7e a0 bf 1c 00 00 00 e8 c6 3b 8d e0 85 c0 78 0c c6 05 13 7f 12 00 01 e9 7b fd ff ff 4
2 48 c7 c6 f8 bf 7f a0 <48> 8b 39 e8 f0 24 04 00 31 ff b9 fc 05 00 00 48 c7 c2 d9 b3 7e a0
RSP: 0018:ffffc9000776bcd0 EFLAGS: 00010286
RAX: 00000000fffffff0 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 00000000fffffff0 RSI: ffffffffa07fbff8 RDI: 00000000ffffffff
RBP: ffff888004cf3c00 R08: ffffffffa078fb40 R09: 0000000000000000
R10: 000000000000000c R11: 0000000000000048 R12: ffff888052810000
R13: 0000607f81c0b0f8 R14: ffff888004a09c00 R15: ffff888004a09c00
FS:  00007fd2e4486840(0000) GS:ffff88807e000000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 00000000528e5004 CR4: 00000000001706a0
Call Trace:
 xlog_alloc_log+0x51f/0x5f0 [xfs]
 xfs_log_mount+0x55/0x340 [xfs]
 xfs_mountfs+0x4e4/0x9f0 [xfs]
 xfs_fs_fill_super+0x4dd/0x7a0 [xfs]
 ? suffix_kstrtoint.constprop.0+0xe0/0xe0 [xfs]
 get_tree_bdev+0x175/0x280
 vfs_get_tree+0x1a/0x80
 ? capable+0x2f/0x50
 path_mount+0x6fb/0xa90
 __x64_sys_mount+0x103/0x140
 do_syscall_64+0x3a/0x70
 entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7fd2e46e8dde
Code: 48 8b 0d b5 80 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0
f 1e fa 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 82 80 0c 00 f7 d8 64 89 01 48

I'm pretty sure that's due to:

	if (!xlog_cil_pcp_init) {
		int	ret;

		ret = cpuhp_setup_state_nocalls(CPUHP_XFS_CIL_DEAD,
						"xfs/cil_pcp:dead", NULL,
						xlog_cil_pcp_dead);
		if (ret < 0) {
			xfs_warn(cil->xc_log->l_mp,
	"Failed to initialise CIL hotplug, error %d. XFS is non-functional.",
				ret);

Because we haven't set cil->xc_log yet.

And having now fixed that, I get tons of:

XFS (sda): Failed to initialise CIL hotplug, error -16. XFS is non-functional.
XFS: Assertion failed: 0, file: fs/xfs/xfs_log_cil.c, line: 1532
------------[ cut here ]------------
WARNING: CPU: 2 PID: 113983 at fs/xfs/xfs_message.c:112 assfail+0x3c/0x40 [xfs]
Modules linked in: xfs libcrc32c ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 xt_REDIR

EBUSY??

Ok, so EBUSY implies a setup race - multiple filesystems are trying
to run the init code at the same time. That seems somewhat unlikely,
but regardless what I'm going to do is move this setup/teardown to
the XFS module init functions rather than do it in the CIL.

i.e. turn this into a generic XFS filesystem hotplug infrastructure
in xfs_super.c and call out to xlog_cil_pcp_dead() from there.

This way we are guaranteed a single init call when the module is
inserted, and a single destroy call when the module is removed. That
should solve all these issues.

How do you want me to handle these changes? Just send out the
replacement patches for the code that is alread in the branch for
review, then resend a rebased pull-req after review? Or something
else? I don't really want to keep bombing the mailing list with 40
emails every time I need to get fixes for a single patch reviewed...

I've tested the two patches that will appear as replies to this mail
as the replacement for the one hotplug infrastructure patch in the
original series. I've tested it against modular XFS builds, and it
doesn't cause any problems here.  Essentially the first patch splits
out the XFS hotplug infrastructure, the second is the CIL
functionality for handling dead CPU notifications.

If they work for you, I'll rebase the branch for you to reconstruct
the for-next tree with the fixes in it...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com