lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 13 Jan 2020 16:35:21 -0800 From: Ira Weiny <ira.weiny@...el.com> To: "Darrick J. Wong" <darrick.wong@...cle.com> Cc: linux-kernel@...r.kernel.org, Alexander Viro <viro@...iv.linux.org.uk>, Dan Williams <dan.j.williams@...el.com>, Dave Chinner <david@...morbit.com>, Christoph Hellwig <hch@....de>, "Theodore Y. Ts'o" <tytso@....edu>, Jan Kara <jack@...e.cz>, linux-ext4@...r.kernel.org, linux-xfs@...r.kernel.org, linux-fsdevel@...r.kernel.org Subject: Re: [RFC PATCH V2 08/12] fs/xfs: Add lock/unlock mode to xfs On Mon, Jan 13, 2020 at 02:19:57PM -0800, Darrick J. Wong wrote: > On Fri, Jan 10, 2020 at 11:29:38AM -0800, ira.weiny@...el.com wrote: > > From: Ira Weiny <ira.weiny@...el.com> [snip] > > > > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c > > index 401da197f012..e8fd95b75e5b 100644 > > --- a/fs/xfs/xfs_inode.c > > +++ b/fs/xfs/xfs_inode.c > > @@ -142,12 +142,12 @@ xfs_ilock_attr_map_shared( > > * > > * Basic locking order: > > * > > - * i_rwsem -> i_mmap_lock -> page_lock -> i_ilock > > + * i_rwsem -> i_dax_sem -> i_mmap_lock -> page_lock -> i_ilock > > Mmmmmm, more locks. Can we skip the extra lock if CONFIG_FSDAX=n or if > the filesystem devices don't support DAX at all? I'll look into it. > > Also, I don't think we're actually following the i_rwsem -> i_daxsem > order in fallocate, and possibly elsewhere too? I'll have to verify. It took a lot of iterations to get the order working so I'm not going to claim perfection. > > Does the vfs have to take the i_dax_sem to do remapping things like > reflink? (Pretend that reflink and dax are compatible for the moment) Honestly I can't say for sure. For this series I was careful to exclude reflink from the locking requirement. [snip] > > > > #define XFS_LOCK_FLAGS \ > > { XFS_IOLOCK_EXCL, "IOLOCK_EXCL" }, \ > > @@ -289,7 +295,9 @@ static inline void xfs_ifunlock(struct xfs_inode *ip) > > { XFS_ILOCK_EXCL, "ILOCK_EXCL" }, \ > > { XFS_ILOCK_SHARED, "ILOCK_SHARED" }, \ > > { XFS_MMAPLOCK_EXCL, "MMAPLOCK_EXCL" }, \ > > - { XFS_MMAPLOCK_SHARED, "MMAPLOCK_SHARED" } > > + { XFS_MMAPLOCK_SHARED, "MMAPLOCK_SHARED" }, \ > > + { XFS_DAX_EXCL, "DAX_EXCL" }, \ > > Whitespace between the comma & string. Fixed. [snip] > > + > > static const struct inode_operations xfs_dir_inode_operations = { > > .create = xfs_vn_create, > > .lookup = xfs_vn_lookup, > > @@ -1372,7 +1394,7 @@ xfs_setup_iops( > > > > switch (inode->i_mode & S_IFMT) { > > case S_IFREG: > > - inode->i_op = &xfs_inode_operations; > > + inode->i_op = &xfs_reg_inode_operations; > > xfs_file_inode_operations? Sounds better. Fixed. Ira
Powered by blists - more mailing lists