PR/52301 CVS commit: [netbsd-9] src

["Martin Husemann" <martin%netbsd.org@localhost>]

The following reply was made to PR kern/52301; it has been noted by GNATS.

From: "Martin Husemann" <martin%netbsd.org@localhost>
To: gnats-bugs%gnats.NetBSD.org@localhost
Cc: 
Subject: PR/52301 CVS commit: [netbsd-9] src
Date: Mon, 17 Aug 2020 10:30:23 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Mon Aug 17 10:30:23 UTC 2020
 
 Modified Files:
 	src/lib/libp2k [netbsd-9]: p2k.c
 	src/sbin/fsck_lfs [netbsd-9]: pass1.c
 	src/sys/arch/i386/stand/efiboot/bootx64 [netbsd-9]: Makefile
 	src/sys/rump/fs/lib/liblfs [netbsd-9]: Makefile
 	src/sys/ufs/lfs [netbsd-9]: lfs.h lfs_accessors.h lfs_alloc.c
 	    lfs_balloc.c lfs_bio.c lfs_debug.c lfs_extern.h lfs_inode.c
 	    lfs_inode.h lfs_pages.c lfs_rename.c lfs_segment.c lfs_subr.c
 	    lfs_vfsops.c lfs_vnops.c
 	src/usr.sbin/dumplfs [netbsd-9]: dumplfs.c
 
 Log Message:
 Pull up following revision(s) (requested by riastradh in ticket #1050):
 
 	sys/ufs/lfs/lfs_subr.c: revision 1.101
 	sys/ufs/lfs/lfs_subr.c: revision 1.102
 	sys/ufs/lfs/lfs_inode.c: revision 1.158
 	sys/ufs/lfs/lfs_inode.h: revision 1.25
 	sys/ufs/lfs/lfs_balloc.c: revision 1.95
 	sys/ufs/lfs/lfs_pages.c: revision 1.21
 	sys/ufs/lfs/lfs_vnops.c: revision 1.330
 	sys/ufs/lfs/lfs_alloc.c: revision 1.140 (patch)
 	sys/ufs/lfs/lfs_alloc.c: revision 1.141 (patch)
 	lib/libp2k/p2k.c: revision 1.72
 	sys/ufs/lfs/lfs.h: revision 1.205
 	sys/ufs/lfs/lfs.h: revision 1.206
 	sys/ufs/lfs/lfs_segment.c: revision 1.284
 	sys/ufs/lfs/lfs.h: revision 1.207
 	sys/ufs/lfs/lfs_segment.c: revision 1.285
 	sys/ufs/lfs/lfs_debug.c: revision 1.55
 	sys/ufs/lfs/lfs_rename.c: revision 1.23
 	usr.sbin/dumplfs/dumplfs.c: revision 1.65
 	sys/ufs/lfs/lfs_vfsops.c: revision 1.371
 	sys/arch/i386/stand/efiboot/bootx64/Makefile: revision 1.3
 	sys/ufs/lfs/lfs_vfsops.c: revision 1.372
 	sys/ufs/lfs/lfs_vfsops.c: revision 1.373
 	sbin/fsck_lfs/pass1.c: revision 1.46
 	sys/ufs/lfs/lfs_vnops.c: revision 1.326
 	sys/ufs/lfs/lfs_vnops.c: revision 1.327
 	sys/ufs/lfs/lfs_vfsops.c: revision 1.375 (patch)
 	sys/ufs/lfs/lfs_vnops.c: revision 1.328
 	sys/ufs/lfs/lfs_subr.c: revision 1.98
 	sys/ufs/lfs/lfs_extern.h: revision 1.116
 	sys/ufs/lfs/lfs_vnops.c: revision 1.329
 	sys/ufs/lfs/lfs_subr.c: revision 1.99
 	sys/ufs/lfs/lfs_extern.h: revision 1.117
 	sys/ufs/lfs/lfs_accessors.h: revision 1.49
 	sys/ufs/lfs/lfs_extern.h: revision 1.118
 	sys/rump/fs/lib/liblfs/Makefile: revision 1.15
 	sys/ufs/lfs/lfs_bio.c: revision 1.146 (patch)
 	sys/ufs/lfs/lfs_bio.c: revision 1.147
 	sys/ufs/lfs/lfs_subr.c: revision 1.100
 
 Fix kassert in lfs by initializing vp first.
 
 Use a marker node to iterate lfs_dchainhd / i_lfs_dchain.
 
 I believe elements can be removed while the lock is dropped,
 including the next node we're hanging on to.
 
 Just use VOP_BWRITE for lfs_bwrite_log.
 Hope this doesn't cause trouble with vfs_suspend.
 
 Teach lfs to transition ro<->rw.
 
 Prevent new dirops while we issue lfs_flush_dirops.
 
 lfs_flush_dirops assumes (by KASSERT((ip->i_state & IN_ADIROP) == 0))
 that vnodes on the dchain will not become involved in active dirops
 even while holding no other locks (lfs_lock, v_interlock), so we must
 set lfs_writer here.  All other callers already set lfs_writer.
 
 We set fs->lfs_writer++ without explicitly doing lfs_writer_enter
 because
 (a) we already waited for the dirops to drain, and
 (b) we hold lfs_lock and cannot drop it before setting lfs_writer.
 
 Assert lfs_writer where I think we can now prove it.
 
 Serialize access to the splay tree with lfs_lock.
 
 Change some cheap KDASSERT into KASSERT.
 
 Take a reference and fix assertions in lfs_flush_dirops.
 Fixes panic:
 KASSERT((ip->i_state & IN_ADIROP) == 0) at lfs_vnops.c:1670
 lfs_flush_dirops
 lfs_check
 lfs_setattr
 VOP_SETATTR
 change_mode
 sys_fchmod
 syscall
 
 This assertion -- and the assertion that vp->v_uflag has VU_DIROP set
 -- is valid only until we release lfs_lock, because we may race with
 lfs_unmark_dirop which will remove the nodes and change the flags.
 
 Further, vp itself is valid only as long as it is referenced, which it
 is as long as it's on the dchain, but lfs_unmark_dirop drops the
 dchain's reference.
 
 Don't lfs_writer_enter while holding v_interlock.
 
 There's no need to lfs_writer_enter at all here, as far as I can see.
 lfs_flush_fs will do it for us.
 
 Break deadlock in PR kern/52301.
 
 The lock order is lfs_writer -> lfs_seglock.  The problem in 52301 is
 that lfs_segwrite violates this lock order by sometimes doing
 lfs_seglock -> lfs_writer, either (a) when doing a checkpoint or (b),
 opportunistically, when there are no dirops pending.  Both cases can
 deadlock, because dirops sometimes take the seglock (lfs_truncate,
 lfs_valloc, lfs_vfree):
 (a) There may be dirops pending, and they may be waiting for the
 seglock, so we can't wait for them to complete while holding the
 seglock.
 (b) The test for fs->lfs_dirops == 0 happens unlocked, and the state
 may change by the time lfs_writer_enter acquires lfs_lock.
 
 To resolve this in each case:
 (a) Do lfs_writer_enter before lfs_seglock, since we will need it
 unconditionally anyway.  The worst performance impact of this should
 be that some dirops get delayed a little bit.
 (b) Create a new lfs_writer_tryenter to use at this point so that the
 test for fs->lfs_dirops == 0 and the acquisition of lfs_writer happen
 atomically under lfs_lock.
 
 Initialize/destroy lfs_allclean_wakeup in modcmd, not lfs_mountfs.
 
 Fixes reloading lfs.kmod.
 
 In lfs_update, hold lfs_writer around lfs_vflush.
 
 Otherwise, we might do
 lfs_vflush
 -> lfs_seglock
 -> lfs_segwait(SEGM_CKP)
    -> lfs_writer_enter
 which is the reverse of the lfs_writer -> lfs_seglock ordering.
 
 Call lfs_orphan in lfs_rename while we're still in the dirop.
 lfs_writer_enter can't fail; keep it simple and don't pretend it can.
 
 Assert that mtsleep can't fail either -- it doesn't catch signals and
 there's no timeout.
 
 Teach LFS_ORPHAN_NEXTFREE about lfs64.
 
 Dust off the orphan detection code and try to make it work.
 
 Fix !DIAGNOSTIC compile
 
 Fix userland references to LFS_ORPHAN_NEXTFREE.
 
 Forgot to grep for these or do a full distribution build, oops!
 
 Fix missing <sys/evcnt.h> by removing the evcnts instead.
 
 Just wanted to confirm that a race might happen, and indeed it did.
 These serve little diagnostic value otherwise.
 
 OR into bp->b_cflags; don't overwrite.
 
 CTASSERT lfs on-disk structure sizes.
 
 Avoid misaligned access to lfs64 on-disk records in memory.
 lfs64 directory entries are only 32-bit aligned in order to conserve
 space in directory blocks, and we had a hack to stuff a 64-bit inode
 in them.  This replaces the hack by __aligned(4) __packed, and goes
 further:
 
 1. It's not clear that all the other lfs64 data structures are 64-bit
    aligned on disk to begin with.  We can go through these later and
    upgrade them from
         struct foo64 {
                 ...
         } __aligned(4) __packed;
         union foo {
                 struct foo64 f64;
                 ...
         };
    to
         struct foo64 {
                 ...
         };
         union foo {
                 struct foo64 f64 __aligned(8);
                 ...
         } __aligned(4) __packed;
    if we really want to take advantage of 64-bit memory accesses.
    However, the __aligned(4) __packed must remain on the union
    because:
 2. We access even the lfs32 data structures via a union that has
    lfs64 members, and it turns out that compilers will assume access
    through a union with 64-bit aligned members implies the whole
    union has 64-bit alignment, even if we're only accessing a 32-bit
    aligned member.
 
 Fix clang build after packed lfs64 accessor change.
 
 Suppress spurious address-of-packed error in rump lfs too.
 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.70 -r1.70.14.1 src/lib/libp2k/p2k.c
 cvs rdiff -u -r1.45 -r1.45.18.1 src/sbin/fsck_lfs/pass1.c
 cvs rdiff -u -r1.1.26.1 -r1.1.26.2 \
     src/sys/arch/i386/stand/efiboot/bootx64/Makefile
 cvs rdiff -u -r1.14 -r1.14.22.1 src/sys/rump/fs/lib/liblfs/Makefile
 cvs rdiff -u -r1.204 -r1.204.4.1 src/sys/ufs/lfs/lfs.h
 cvs rdiff -u -r1.48 -r1.48.12.1 src/sys/ufs/lfs/lfs_accessors.h
 cvs rdiff -u -r1.137 -r1.137.8.1 src/sys/ufs/lfs/lfs_alloc.c
 cvs rdiff -u -r1.94 -r1.94.10.1 src/sys/ufs/lfs/lfs_balloc.c
 cvs rdiff -u -r1.142 -r1.142.6.1 src/sys/ufs/lfs/lfs_bio.c
 cvs rdiff -u -r1.54 -r1.54.22.1 src/sys/ufs/lfs/lfs_debug.c
 cvs rdiff -u -r1.114 -r1.114.4.1 src/sys/ufs/lfs/lfs_extern.h
 cvs rdiff -u -r1.157 -r1.157.10.1 src/sys/ufs/lfs/lfs_inode.c
 cvs rdiff -u -r1.23 -r1.23.10.1 src/sys/ufs/lfs/lfs_inode.h
 cvs rdiff -u -r1.15 -r1.15.8.1 src/sys/ufs/lfs/lfs_pages.c
 cvs rdiff -u -r1.22 -r1.22.10.1 src/sys/ufs/lfs/lfs_rename.c
 cvs rdiff -u -r1.278 -r1.278.4.1 src/sys/ufs/lfs/lfs_segment.c
 cvs rdiff -u -r1.97 -r1.97.8.1 src/sys/ufs/lfs/lfs_subr.c
 cvs rdiff -u -r1.365 -r1.365.2.1 src/sys/ufs/lfs/lfs_vfsops.c
 cvs rdiff -u -r1.324 -r1.324.2.1 src/sys/ufs/lfs/lfs_vnops.c
 cvs rdiff -u -r1.64 -r1.64.4.1 src/usr.sbin/dumplfs/dumplfs.c
 
 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.