| Message ID | 1530846750-6686-7-git-send-email-shan.hai@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jul 06, 2018 at 11:12:27AM +0800, Shan Hai wrote: > The XFS_ILOCK_SHARED type lock is held in the xfs_ioc_fsgetxattr > but does not released when the inode is in local format, fix it by > eleasing the lock on the local format inode. > > Signed-off-by: Shan Hai <shan.hai@oracle.com> > --- > fs/xfs/xfs_ioctl.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c > index 0ef5ece5634c..246562615ccd 100644 > --- a/fs/xfs/xfs_ioctl.c > +++ b/fs/xfs/xfs_ioctl.c > @@ -907,7 +907,10 @@ xfs_ioc_fsgetxattr( > } else { > if (ip->i_df.if_flags & XFS_IFEXTENTS) > fa.fsx_nextents = xfs_iext_count(&ip->i_df); > - else > + else if (ip->i_df.if_flags & XFS_IFINLINE) { > + xfs_iunlock(ip, XFS_ILOCK_SHARED); > + return 0; > + } else > fa.fsx_nextents = ip->i_d.di_nextents; > } > xfs_iunlock(ip, XFS_ILOCK_SHARED); This looks odd, and the changelog confuses even more. Please avoid the early return, which means we miss the copy_to_user of the fa struct. It seems to me like this should be something like: if (ip->i_df.if_flags & XFS_IFEXTENTS) fa.fsx_nextents = xfs_iext_count(&ip->i_df); esle if (ip->i_df.if_flags & XFS_IFINLINE) fa.fsx_nextents = 0; else fa.fsx_nextents = ip->i_d.di_nextents; -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2018年07月08日 23:53, Christoph Hellwig wrote: > On Fri, Jul 06, 2018 at 11:12:27AM +0800, Shan Hai wrote: >> The XFS_ILOCK_SHARED type lock is held in the xfs_ioc_fsgetxattr >> but does not released when the inode is in local format, fix it by >> eleasing the lock on the local format inode. >> >> Signed-off-by: Shan Hai <shan.hai@oracle.com> >> --- >> fs/xfs/xfs_ioctl.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c >> index 0ef5ece5634c..246562615ccd 100644 >> --- a/fs/xfs/xfs_ioctl.c >> +++ b/fs/xfs/xfs_ioctl.c >> @@ -907,7 +907,10 @@ xfs_ioc_fsgetxattr( >> } else { >> if (ip->i_df.if_flags & XFS_IFEXTENTS) >> fa.fsx_nextents = xfs_iext_count(&ip->i_df); >> - else >> + else if (ip->i_df.if_flags & XFS_IFINLINE) { >> + xfs_iunlock(ip, XFS_ILOCK_SHARED); >> + return 0; >> + } else >> fa.fsx_nextents = ip->i_d.di_nextents; >> } >> xfs_iunlock(ip, XFS_ILOCK_SHARED); > This looks odd, and the changelog confuses even more. > > Please avoid the early return, which means we miss the copy_to_user > of the fa struct. It seems to me like this should be something like: > > if (ip->i_df.if_flags & XFS_IFEXTENTS) > fa.fsx_nextents = xfs_iext_count(&ip->i_df); > esle if (ip->i_df.if_flags & XFS_IFINLINE) > fa.fsx_nextents = 0; > else > fa.fsx_nextents = ip->i_d.di_nextents; Agreed, I will double check this, thanks for the review. Thanks Shan Hai -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c index 0ef5ece5634c..246562615ccd 100644 --- a/fs/xfs/xfs_ioctl.c +++ b/fs/xfs/xfs_ioctl.c @@ -907,7 +907,10 @@ xfs_ioc_fsgetxattr( } else { if (ip->i_df.if_flags & XFS_IFEXTENTS) fa.fsx_nextents = xfs_iext_count(&ip->i_df); - else + else if (ip->i_df.if_flags & XFS_IFINLINE) { + xfs_iunlock(ip, XFS_ILOCK_SHARED); + return 0; + } else fa.fsx_nextents = ip->i_d.di_nextents; } xfs_iunlock(ip, XFS_ILOCK_SHARED);
The XFS_ILOCK_SHARED type lock is held in the xfs_ioc_fsgetxattr but does not released when the inode is in local format, fix it by eleasing the lock on the local format inode. Signed-off-by: Shan Hai <shan.hai@oracle.com> --- fs/xfs/xfs_ioctl.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)