| Message ID | 1470637351-27933-3-git-send-email-david@fromorbit.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Aug 08, 2016 at 04:22:31PM +1000, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > Prior to the iomap conversion, XFS supported the FIEMAP_FLAG_XATTR > flag for mapping the attribute fork block map. This flag was not > added to the iomap fiemap support so we have regressed fiemap > functionality with this change. > > Add an iomap control flag to indicate that we should be operating > on the attribute map rather than the file data map and pass it from > iomap_fiemap() as appropriate. Add the appropriate flags to the XFS > code to switch to the attribute fork lookup, and ensure we return > EINVAL if anyone attempts a write mapping of the attribute fork. I'm a little worried about this for a few reasons: > - ret = fiemap_check_flags(fi, FIEMAP_FLAG_SYNC); > + ret = fiemap_check_flags(fi, FIEMAP_FLAG_SYNC | FIEMAP_FLAG_XATTR); FIEMAP_FLAG_XATTR is a magic thing only supported by ext4 and historically XFS. By claiming general support here we make iomap_fiemap unsuitable for general use. At least we should pass in a flag if it's supported or not into the generic helper. Or see below for another idea. > diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c > index 4398932..17b5b82 100644 > --- a/fs/xfs/xfs_iomap.c > +++ b/fs/xfs/xfs_iomap.c > @@ -993,10 +993,22 @@ xfs_file_iomap_begin( > struct xfs_bmbt_irec imap; > xfs_fileoff_t offset_fsb, end_fsb; > int nimaps = 1, error = 0; > + int bmflags = XFS_BMAPI_ENTIRE; > > if (XFS_FORCED_SHUTDOWN(mp)) > return -EIO; > > + /* Attribute fork can only be read via this interface */ > + if (flags & IOMAP_ATTR) { > + if (flags & ~IOMAP_ATTR) > + return -EINVAL; > + /* if there are no attribute fork or extents, return ENOENT */ > + if (!XFS_IFORK_Q(ip) || !ip->i_d.di_anextents) > + return -ENOENT; > + ASSERT(ip->i_d.di_aformat != XFS_DINODE_FMT_LOCAL); > + bmflags |= XFS_BMAPI_ATTRFORK; > + } And this adds a special case for totally rare attr fork operation to the fast path that we'll soon be using for all file I/O. I'd suggest to just have a separate stub set of iomap_ops for the xattr case. xfs_vn_fiemap would then look something like: xfs_ilock(XFS_I(inode), XFS_IOLOCK_SHARED); if (fi->fi_flags & FIEMAP_FLAG_XATTR) { fi->fi_flags &= ~FIEMAP_FLAG_XATTR; error = iomap_fiemap(inode, fieinfo, start, length, &xfs_attr_iomap_ops); } else { error = iomap_fiemap(inode, fieinfo, start, length, &xfs_iomap_ops); } xfs_iunlock(XFS_I(inode), XFS_IOLOCK_SHARED); return error; -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" 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/iomap.c b/fs/iomap.c index 189742b..2a04e5e 100644 --- a/fs/iomap.c +++ b/fs/iomap.c @@ -461,12 +461,13 @@ int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fi, { struct fiemap_ctx ctx; loff_t ret; + int flags = 0; memset(&ctx, 0, sizeof(ctx)); ctx.fi = fi; ctx.prev.type = IOMAP_HOLE; - ret = fiemap_check_flags(fi, FIEMAP_FLAG_SYNC); + ret = fiemap_check_flags(fi, FIEMAP_FLAG_SYNC | FIEMAP_FLAG_XATTR); if (ret) return ret; @@ -476,9 +477,15 @@ int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fi, return ret; } + if (fi->fi_flags & FIEMAP_FLAG_XATTR) + flags |= IOMAP_ATTR; + while (len > 0) { - ret = iomap_apply(inode, start, len, 0, ops, &ctx, + ret = iomap_apply(inode, start, len, flags, ops, &ctx, iomap_fiemap_actor); + /* inode with no attribute mapping will give ENOENT */ + if (ret == -ENOENT) + break; if (ret < 0) return ret; if (ret == 0) diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c index 4398932..17b5b82 100644 --- a/fs/xfs/xfs_iomap.c +++ b/fs/xfs/xfs_iomap.c @@ -993,10 +993,22 @@ xfs_file_iomap_begin( struct xfs_bmbt_irec imap; xfs_fileoff_t offset_fsb, end_fsb; int nimaps = 1, error = 0; + int bmflags = XFS_BMAPI_ENTIRE; if (XFS_FORCED_SHUTDOWN(mp)) return -EIO; + /* Attribute fork can only be read via this interface */ + if (flags & IOMAP_ATTR) { + if (flags & ~IOMAP_ATTR) + return -EINVAL; + /* if there are no attribute fork or extents, return ENOENT */ + if (!XFS_IFORK_Q(ip) || !ip->i_d.di_anextents) + return -ENOENT; + ASSERT(ip->i_d.di_aformat != XFS_DINODE_FMT_LOCAL); + bmflags |= XFS_BMAPI_ATTRFORK; + } + xfs_ilock(ip, XFS_ILOCK_EXCL); ASSERT(offset <= mp->m_super->s_maxbytes); @@ -1006,7 +1018,7 @@ xfs_file_iomap_begin( end_fsb = XFS_B_TO_FSB(mp, offset + length); error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb, &imap, - &nimaps, XFS_BMAPI_ENTIRE); + &nimaps, bmflags); if (error) { xfs_iunlock(ip, XFS_ILOCK_EXCL); return error; diff --git a/include/linux/iomap.h b/include/linux/iomap.h index 3267df4..00a5477 100644 --- a/include/linux/iomap.h +++ b/include/linux/iomap.h @@ -36,6 +36,7 @@ struct iomap { */ #define IOMAP_WRITE (1 << 0) #define IOMAP_ZERO (1 << 1) +#define IOMAP_ATTR (1 << 2) /* operate on attributes */ struct iomap_ops { /*