>
> In ext2 and most other file systems it is enough to hold the inode semaphore
to pin
> blocks returned by bmap (because truncate and unlink are the only functions t
hat
> could steal blocks from under you). This seems to be mostly true for XFS too
because it is
> enforced in the VFS above XFS, but it has some loopholes with the space manag
ement
> ioctls and the fsr (swapext). The following patch plugs them by grabbing the
> respective linux inode semaphores. Did I forget any case where bmap could be
invalidated?
You are correct, except for the fact that xfs is doing its own locking under
the covers internally. These should be protecting XFS from stamping on itself.
Steve
>
> -Andi
>
> --- ./fs/xfs/linux/xfs_ioctl.c-o Wed Aug 9 02:18:40 2000
> +++ ./fs/xfs/linux/xfs_ioctl.c Sun Aug 20 20:55:00 2000
> @@ -751,7 +751,8 @@
> xfs_flock64_t bf;
> xfs_inode_t *ip;
> xfs_mount_t *mp;
> -
> + int need_isem;
> +
> vp = LINVFS_GET_VP(inode);
>
> ASSERT(vp);
> @@ -769,15 +770,24 @@
> return (EIO);
>
>
> + /*
> + * Linux code relies on i_sem protecing bmap mappings, so grab it
> + * for all operations that may shrink the file or free space.
> + * XXX: is this correct for inode mapped files?
> + */
> + need_isem = 0;
> switch (cmd) {
> - case XFS_IOC_ALLOCSP:
> case XFS_IOC_FREESP:
> + case XFS_IOC_FREESP64:
> + need_isem = 1;
> + /*FALL THROUGH*/
> +
> + case XFS_IOC_ALLOCSP:
>
> case XFS_IOC_RESVSP:
> case XFS_IOC_UNRESVSP:
>
> case XFS_IOC_ALLOCSP64:
> - case XFS_IOC_FREESP64:
>
> case XFS_IOC_RESVSP64:
> case XFS_IOC_UNRESVSP64: {
> @@ -798,13 +808,15 @@
>
> /* if (filp->f_flags & FINVIS) XXXjtk - need O_INVISIBLE? */
> attr_flags |= ATTR_DMI;
> -
> +
> + if (need_isem)
> + down(&inode->i_sem);
> error = xfs_change_file_space(bdp, cmd, &bf, filp->f_pos,
> - &cred, attr_flags);
> - if (error)
> - return -error;
> + &cred, attr_flags);
> + if (need_isem)
> + up(&inode->i_sem);
>
> - return 0;
> + return -error;
> }
>
> case XFS_IOC_DIOINFO:
> --- ./fs/xfs/xfs_dfrag.c-o Wed Aug 9 02:18:50 2000
> +++ ./fs/xfs/xfs_dfrag.c Sun Aug 20 19:57:16 2000
> @@ -293,6 +293,9 @@
> xfs_trans_cancel(tp, 0);
> return error;
> }
> +
> + double_down(fp->f_dentry->d_inode, tfp->f_dentry->d_inode);
> +
> xfs_lock_inodes(ips, 2, 0, XFS_ILOCK_EXCL);
>
> /*
> @@ -304,6 +307,8 @@
> if (error) {
> xfs_iunlock(ip, lock_flags);
> xfs_iunlock(tip, lock_flags);
> + up(&fp->f_dentry->d_inode);
> + up(&tfp->f_dentry->d_inode);
> xfs_trans_cancel(tp, 0);
> return error;
> }
> @@ -396,6 +401,9 @@
> error = xfs_trans_commit(tp, XFS_TRANS_SWAPEXT, NULL);
>
> CELL_ONLY(cfs_end_defrag(vp, cxfs_val));
> +
> + up(fp->f_dentry->d_inode);
> + up(tfp->f_dentry->d_inode);
>
> fput(fp);
> fput(tfp);
|