On Tue, Nov 15, 2016 at 10:34:10AM -0500, Brian Foster wrote: > > @@ -1592,12 +1591,10 @@ xfs_vm_bmap( > > * that on reflinks inodes, so we have to skip out here. And yes, > > * 0 is the magic code for a bmap error.. > > */ > > - if (xfs_is_reflink_inode(ip)) { > > - xfs_iunlock(ip, XFS_IOLOCK_SHARED); > > + if (xfs_is_reflink_inode(ip)) > > return 0; > > - } > > + > > filemap_write_and_wait(mapping); > > - xfs_iunlock(ip, XFS_IOLOCK_SHARED); > > The commit log makes no mention of dropping lock usage, unless I missed > where the inode lock is taken elsewhere..? For swapon, the only case where the lock matter is is taken by the calller now. For the ioctl it simply doesn't matter as the result will be stale by the time we return. > > > > if (lock_flags & (XFS_IOLOCK_EXCL|XFS_IOLOCK_SHARED)) { > > if (!(lock_flags & XFS_IOLOCK_SHARED)) > > - return !!ip->i_iolock.mr_writer; > > - return rwsem_is_locked(&ip->i_iolock.mr_lock); > > + return !debug_locks || > > + lockdep_is_held_type(&VFS_I(ip)->i_rwsem, 0); > > + return rwsem_is_locked(&VFS_I(ip)->i_rwsem); > > So if I read this correctly, we can no longer safely assert that an > inode is not exclusively locked because the debug_locks == 0 case would > always tell us it is. It doesn't look like we do that today, but it > warrants a comment IMO. Ok, I can add a comment. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html