On Fri, Feb 21, 2020 at 02:14:31PM +0100, Kamil Rytarowski wrote:
> On 22.12.2019 20:47, Andrew Doran wrote:
> > Module Name: src
> > Committed By: ad
> > Date: Sun Dec 22 19:47:35 UTC 2019
> >
> > Modified Files:
> > src/external/cddl/osnet/dist/uts/common/fs/zfs: zfs_ctldir.c
> > src/sys/kern: vfs_mount.c vfs_subr.c vfs_syscalls.c
> > src/sys/miscfs/genfs: genfs_vfsops.c
> > src/sys/nfs: nfs_export.c
> > src/sys/sys: mount.h vnode.h vnode_impl.h
> > src/sys/ufs/lfs: ulfs_vfsops.c
> > src/sys/ufs/ufs: ufs_vfsops.c ufs_wapbl.c
> >
> > Log Message:
> > Make mntvnode_lock per-mount, and address false sharing of struct mount.
> >
>
> This change broke kUBSan syzbot.
>
> The sanitizer is now very noisy as struct mount requires 64 byte alignment.
>
> http://netbsd.org/~kamil/kubsan/mount-alignment.txt
I had a look this weekend. This is down to KMEM_SIZE messing up the
alignment, so is a DIAGNOSTIC thing. The align_offset parameter to
pool_cache() would be a nice easy way to solve this but it seems someone
killed that off, so I'll need to give this some more thought.
Andrew
> > diff -u src/sys/sys/mount.h:1.234 src/sys/sys/mount.h:1.235
> > --- src/sys/sys/mount.h:1.234 Tue Jan 1 10:06:54 2019
> > +++ src/sys/sys/mount.h Sun Dec 22 19:47:34 2019
> > @@ -1,4 +1,4 @@
> > -/* $NetBSD: mount.h,v 1.234 2019/01/01 10:06:54 hannken Exp $ */
> > +/* $NetBSD: mount.h,v 1.235 2019/12/22 19:47:34 ad Exp $ */
> >
> > /*
> > * Copyright (c) 1989, 1991, 1993
> > @@ -133,29 +133,38 @@ struct vattr;
> > * array of operations and an instance record.
> > */
> > struct mount {
> > - TAILQ_HEAD(, vnode_impl) mnt_vnodelist; /* list of vnodes this mount */
> > + /*
> > + * Mostly stable data.
> > + */
> > + kmutex_t *mnt_vnodelock; /* lock on mnt_vnodelist */
> > struct vfsops *mnt_op; /* operations on fs */
> > struct vnode *mnt_vnodecovered; /* vnode we mounted on */
> > struct mount *mnt_lower; /* fs mounted on */
> > - int mnt_synclist_slot; /* synclist slot index */
> > void *mnt_transinfo; /* for FS-internal use */
> > void *mnt_data; /* private data */
> > - kmutex_t mnt_renamelock; /* per-fs rename lock */
> > - int mnt_refcnt; /* ref count on this structure
> > */
> > + kmutex_t *mnt_renamelock; /* per-fs rename lock */
> > int mnt_flag; /* flags */
> > int mnt_iflag; /* internal flags */
> > int mnt_fs_bshift; /* offset shift for lblkno */
> > int mnt_dev_bshift; /* shift for device sectors */
> > - struct statvfs mnt_stat; /* cache of filesystem stats */
> > specificdata_reference
> > mnt_specdataref; /* subsystem specific data */
> > - kmutex_t mnt_updating; /* to serialize updates */
> > + kmutex_t *mnt_updating; /* to serialize updates */
> > const struct wapbl_ops
> > *mnt_wapbl_op; /* logging ops */
> > struct wapbl *mnt_wapbl; /* log info */
> > struct wapbl_replay
> > *mnt_wapbl_replay; /* replay support XXX: what? */
> > uint64_t mnt_gen;
> > +
> > + /*
> > + * Volatile data: pad to keep away from the stable items.
> > + */
> > + int mnt_refcnt /* ref count on this structure
> > */
> > + __aligned(COHERENCY_UNIT);
> > + int mnt_synclist_slot; /* synclist slot index */
> > + TAILQ_HEAD(, vnode_impl) mnt_vnodelist; /* list of vnodes this mount */
> > + struct statvfs mnt_stat; /* cache of filesystem stats */
> > };
> >
> > #endif /* defined(_KERNEL) || defined(__EXPOSE_MOUNT) */
> >
>
>
>
>
