| From hch@infradead.org Tue Dec 6 14:02:17 2011 |
| From: Christoph Hellwig <hch@infradead.org> |
| Date: Tue, 6 Dec 2011 16:21:30 -0500 |
| Subject: xfs: fix attr2 vs large data fork assert |
| To: stable@vger.kernel.org |
| Cc: xfs@oss.sgi.com |
| Message-ID: <20111206212130.GC28459@infradead.org> |
| Content-Disposition: inline |
| |
| From: Christoph Hellwig <hch@infradead.org> |
| |
| commit 4c393a6059f8442a70512a48ce4639b882b6f6ad upstream. |
| |
| With Dmitry fsstress updates I've seen very reproducible crashes in |
| xfs_attr_shortform_remove because xfs_attr_shortform_bytesfit claims that |
| the attributes would not fit inline into the inode after removing an |
| attribute. It turns out that we were operating on an inode with lots |
| of delalloc extents, and thus an if_bytes values for the data fork that |
| is larger than biggest possible on-disk storage for it which utterly |
| confuses the code near the end of xfs_attr_shortform_bytesfit. |
| |
| Fix this by always allowing the current attribute fork, like we already |
| do for the attr1 format, given that delalloc conversion will take care |
| for moving either the data or attribute area out of line if it doesn't |
| fit at that point - or making the point moot by merging extents at this |
| point. |
| |
| Also document the function better, and clean up some loose bits. |
| |
| Reviewed-by: Dave Chinner <dchinner@redhat.com> |
| Signed-off-by: Christoph Hellwig <hch@lst.de> |
| Signed-off-by: Ben Myers <bpm@sgi.com> |
| Acked-by: Dave Chinner <dchinner@redhat.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> |
| |
| --- |
| fs/xfs/xfs_attr_leaf.c | 64 +++++++++++++++++++++++++++++-------------------- |
| 1 file changed, 39 insertions(+), 25 deletions(-) |
| |
| --- a/fs/xfs/xfs_attr_leaf.c |
| +++ b/fs/xfs/xfs_attr_leaf.c |
| @@ -110,6 +110,7 @@ xfs_attr_namesp_match(int arg_flags, int |
| /* |
| * Query whether the requested number of additional bytes of extended |
| * attribute space will be able to fit inline. |
| + * |
| * Returns zero if not, else the di_forkoff fork offset to be used in the |
| * literal area for attribute data once the new bytes have been added. |
| * |
| @@ -122,7 +123,7 @@ xfs_attr_shortform_bytesfit(xfs_inode_t |
| int offset; |
| int minforkoff; /* lower limit on valid forkoff locations */ |
| int maxforkoff; /* upper limit on valid forkoff locations */ |
| - int dsize; |
| + int dsize; |
| xfs_mount_t *mp = dp->i_mount; |
| |
| offset = (XFS_LITINO(mp) - bytes) >> 3; /* rounded down */ |
| @@ -136,47 +137,60 @@ xfs_attr_shortform_bytesfit(xfs_inode_t |
| return (offset >= minforkoff) ? minforkoff : 0; |
| } |
| |
| - if (!(mp->m_flags & XFS_MOUNT_ATTR2)) { |
| - if (bytes <= XFS_IFORK_ASIZE(dp)) |
| - return dp->i_d.di_forkoff; |
| + /* |
| + * If the requested numbers of bytes is smaller or equal to the |
| + * current attribute fork size we can always proceed. |
| + * |
| + * Note that if_bytes in the data fork might actually be larger than |
| + * the current data fork size is due to delalloc extents. In that |
| + * case either the extent count will go down when they are converted |
| + * to real extents, or the delalloc conversion will take care of the |
| + * literal area rebalancing. |
| + */ |
| + if (bytes <= XFS_IFORK_ASIZE(dp)) |
| + return dp->i_d.di_forkoff; |
| + |
| + /* |
| + * For attr2 we can try to move the forkoff if there is space in the |
| + * literal area, but for the old format we are done if there is no |
| + * space in the fixed attribute fork. |
| + */ |
| + if (!(mp->m_flags & XFS_MOUNT_ATTR2)) |
| return 0; |
| - } |
| |
| dsize = dp->i_df.if_bytes; |
| - |
| + |
| switch (dp->i_d.di_format) { |
| case XFS_DINODE_FMT_EXTENTS: |
| - /* |
| + /* |
| * If there is no attr fork and the data fork is extents, |
| - * determine if creating the default attr fork will result |
| - * in the extents form migrating to btree. If so, the |
| - * minimum offset only needs to be the space required for |
| + * determine if creating the default attr fork will result |
| + * in the extents form migrating to btree. If so, the |
| + * minimum offset only needs to be the space required for |
| * the btree root. |
| - */ |
| + */ |
| if (!dp->i_d.di_forkoff && dp->i_df.if_bytes > |
| xfs_default_attroffset(dp)) |
| dsize = XFS_BMDR_SPACE_CALC(MINDBTPTRS); |
| break; |
| - |
| case XFS_DINODE_FMT_BTREE: |
| /* |
| - * If have data btree then keep forkoff if we have one, |
| - * otherwise we are adding a new attr, so then we set |
| - * minforkoff to where the btree root can finish so we have |
| + * If we have a data btree then keep forkoff if we have one, |
| + * otherwise we are adding a new attr, so then we set |
| + * minforkoff to where the btree root can finish so we have |
| * plenty of room for attrs |
| */ |
| if (dp->i_d.di_forkoff) { |
| - if (offset < dp->i_d.di_forkoff) |
| + if (offset < dp->i_d.di_forkoff) |
| return 0; |
| - else |
| - return dp->i_d.di_forkoff; |
| - } else |
| - dsize = XFS_BMAP_BROOT_SPACE(dp->i_df.if_broot); |
| + return dp->i_d.di_forkoff; |
| + } |
| + dsize = XFS_BMAP_BROOT_SPACE(dp->i_df.if_broot); |
| break; |
| } |
| - |
| - /* |
| - * A data fork btree root must have space for at least |
| + |
| + /* |
| + * A data fork btree root must have space for at least |
| * MINDBTPTRS key/ptr pairs if the data fork is small or empty. |
| */ |
| minforkoff = MAX(dsize, XFS_BMDR_SPACE_CALC(MINDBTPTRS)); |
| @@ -186,10 +200,10 @@ xfs_attr_shortform_bytesfit(xfs_inode_t |
| maxforkoff = XFS_LITINO(mp) - XFS_BMDR_SPACE_CALC(MINABTPTRS); |
| maxforkoff = maxforkoff >> 3; /* rounded down */ |
| |
| - if (offset >= minforkoff && offset < maxforkoff) |
| - return offset; |
| if (offset >= maxforkoff) |
| return maxforkoff; |
| + if (offset >= minforkoff) |
| + return offset; |
| return 0; |
| } |
| |