Commit f3508bcd authored by Dave Chinner's avatar Dave Chinner Committed by Ben Myers

xfs: remove local fork format handling from xfs_bmapi_write()

The conversion from local format to extent format requires
interpretation of the data in the fork being converted, so it cannot
be done in a generic way. It is up to the caller to convert the fork
format to extent format before calling into xfs_bmapi_write() so
format conversion can be done correctly.

The code in xfs_bmapi_write() to convert the format is used
implicitly by the attribute and directory code, but they
specifically zero the fork size so that the conversion does not do
any allocation or manipulation. Move this conversion into the
shortform to leaf functions for the dir/attr code so the conversions
are explicitly controlled by all callers.

Now we can remove the conversion code in xfs_bmapi_write.
Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
Signed-off-by: default avatarBen Myers <bpm@sgi.com>
parent 3e5b7d8b
...@@ -690,6 +690,8 @@ xfs_attr_shortform_to_leaf(xfs_da_args_t *args) ...@@ -690,6 +690,8 @@ xfs_attr_shortform_to_leaf(xfs_da_args_t *args)
sf = (xfs_attr_shortform_t *)tmpbuffer; sf = (xfs_attr_shortform_t *)tmpbuffer;
xfs_idata_realloc(dp, -size, XFS_ATTR_FORK); xfs_idata_realloc(dp, -size, XFS_ATTR_FORK);
xfs_bmap_local_to_extents_empty(dp, XFS_ATTR_FORK);
bp = NULL; bp = NULL;
error = xfs_da_grow_inode(args, &blkno); error = xfs_da_grow_inode(args, &blkno);
if (error) { if (error) {
......
...@@ -1161,6 +1161,24 @@ xfs_bmap_extents_to_btree( ...@@ -1161,6 +1161,24 @@ xfs_bmap_extents_to_btree(
* since the file data needs to get logged so things will stay consistent. * since the file data needs to get logged so things will stay consistent.
* (The bmap-level manipulations are ok, though). * (The bmap-level manipulations are ok, though).
*/ */
void
xfs_bmap_local_to_extents_empty(
struct xfs_inode *ip,
int whichfork)
{
struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
ASSERT(XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL);
ASSERT(ifp->if_bytes == 0);
ASSERT(XFS_IFORK_NEXTENTS(ip, whichfork) == 0);
xfs_bmap_forkoff_reset(ip->i_mount, ip, whichfork);
ifp->if_flags &= ~XFS_IFINLINE;
ifp->if_flags |= XFS_IFEXTENTS;
XFS_IFORK_FMT_SET(ip, whichfork, XFS_DINODE_FMT_EXTENTS);
}
STATIC int /* error */ STATIC int /* error */
xfs_bmap_local_to_extents( xfs_bmap_local_to_extents(
xfs_trans_t *tp, /* transaction pointer */ xfs_trans_t *tp, /* transaction pointer */
...@@ -1174,9 +1192,12 @@ xfs_bmap_local_to_extents( ...@@ -1174,9 +1192,12 @@ xfs_bmap_local_to_extents(
struct xfs_inode *ip, struct xfs_inode *ip,
struct xfs_ifork *ifp)) struct xfs_ifork *ifp))
{ {
int error; /* error return value */ int error = 0;
int flags; /* logging flags returned */ int flags; /* logging flags returned */
xfs_ifork_t *ifp; /* inode fork pointer */ xfs_ifork_t *ifp; /* inode fork pointer */
xfs_alloc_arg_t args; /* allocation arguments */
xfs_buf_t *bp; /* buffer for extent block */
xfs_bmbt_rec_host_t *ep; /* extent record pointer */
/* /*
* We don't want to deal with the case of keeping inode data inline yet. * We don't want to deal with the case of keeping inode data inline yet.
...@@ -1185,15 +1206,17 @@ xfs_bmap_local_to_extents( ...@@ -1185,15 +1206,17 @@ xfs_bmap_local_to_extents(
ASSERT(!(S_ISREG(ip->i_d.di_mode) && whichfork == XFS_DATA_FORK)); ASSERT(!(S_ISREG(ip->i_d.di_mode) && whichfork == XFS_DATA_FORK));
ifp = XFS_IFORK_PTR(ip, whichfork); ifp = XFS_IFORK_PTR(ip, whichfork);
ASSERT(XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL); ASSERT(XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL);
if (!ifp->if_bytes) {
xfs_bmap_local_to_extents_empty(ip, whichfork);
flags = XFS_ILOG_CORE;
goto done;
}
flags = 0; flags = 0;
error = 0; error = 0;
if (ifp->if_bytes) { ASSERT((ifp->if_flags & (XFS_IFINLINE|XFS_IFEXTENTS|XFS_IFEXTIREC)) ==
xfs_alloc_arg_t args; /* allocation arguments */ XFS_IFINLINE);
xfs_buf_t *bp; /* buffer for extent block */
xfs_bmbt_rec_host_t *ep;/* extent record pointer */
ASSERT((ifp->if_flags &
(XFS_IFINLINE|XFS_IFEXTENTS|XFS_IFEXTIREC)) == XFS_IFINLINE);
memset(&args, 0, sizeof(args)); memset(&args, 0, sizeof(args));
args.tp = tp; args.tp = tp;
args.mp = ip->i_mount; args.mp = ip->i_mount;
...@@ -1226,8 +1249,10 @@ xfs_bmap_local_to_extents( ...@@ -1226,8 +1249,10 @@ xfs_bmap_local_to_extents(
/* account for the change in fork size and log everything */ /* account for the change in fork size and log everything */
xfs_trans_log_buf(tp, bp, 0, ifp->if_bytes - 1); xfs_trans_log_buf(tp, bp, 0, ifp->if_bytes - 1);
xfs_bmap_forkoff_reset(args.mp, ip, whichfork);
xfs_idata_realloc(ip, -ifp->if_bytes, whichfork); xfs_idata_realloc(ip, -ifp->if_bytes, whichfork);
xfs_bmap_local_to_extents_empty(ip, whichfork);
flags |= XFS_ILOG_CORE;
xfs_iext_add(ifp, 0, 1); xfs_iext_add(ifp, 0, 1);
ep = xfs_iext_get_ext(ifp, 0); ep = xfs_iext_get_ext(ifp, 0);
xfs_bmbt_set_allf(ep, 0, args.fsbno, 1, XFS_EXT_NORM); xfs_bmbt_set_allf(ep, 0, args.fsbno, 1, XFS_EXT_NORM);
...@@ -1239,14 +1264,7 @@ xfs_bmap_local_to_extents( ...@@ -1239,14 +1264,7 @@ xfs_bmap_local_to_extents(
xfs_trans_mod_dquot_byino(tp, ip, xfs_trans_mod_dquot_byino(tp, ip,
XFS_TRANS_DQ_BCOUNT, 1L); XFS_TRANS_DQ_BCOUNT, 1L);
flags |= xfs_ilog_fext(whichfork); flags |= xfs_ilog_fext(whichfork);
} else {
ASSERT(XFS_IFORK_NEXTENTS(ip, whichfork) == 0);
xfs_bmap_forkoff_reset(ip->i_mount, ip, whichfork);
}
ifp->if_flags &= ~XFS_IFINLINE;
ifp->if_flags |= XFS_IFEXTENTS;
XFS_IFORK_FMT_SET(ip, whichfork, XFS_DINODE_FMT_EXTENTS);
flags |= XFS_ILOG_CORE;
done: done:
*logflagsp = flags; *logflagsp = flags;
return error; return error;
...@@ -1322,25 +1340,6 @@ xfs_bmap_add_attrfork_extents( ...@@ -1322,25 +1340,6 @@ xfs_bmap_add_attrfork_extents(
return error; return error;
} }
/*
* Block initialisation function for local to extent format conversion.
*
* This shouldn't actually be called by anyone, so make sure debug kernels cause
* a noticable failure.
*/
STATIC void
xfs_bmap_local_to_extents_init_fn(
struct xfs_trans *tp,
struct xfs_buf *bp,
struct xfs_inode *ip,
struct xfs_ifork *ifp)
{
ASSERT(0);
bp->b_ops = &xfs_bmbt_buf_ops;
memcpy(bp->b_addr, ifp->if_u1.if_data, ifp->if_bytes);
xfs_trans_buf_set_type(tp, bp, XFS_BLFT_BTREE_BUF);
}
/* /*
* Called from xfs_bmap_add_attrfork to handle local format files. Each * Called from xfs_bmap_add_attrfork to handle local format files. Each
* different data fork content type needs a different callout to do the * different data fork content type needs a different callout to do the
...@@ -1381,9 +1380,9 @@ xfs_bmap_add_attrfork_local( ...@@ -1381,9 +1380,9 @@ xfs_bmap_add_attrfork_local(
flags, XFS_DATA_FORK, flags, XFS_DATA_FORK,
xfs_symlink_local_to_remote); xfs_symlink_local_to_remote);
return xfs_bmap_local_to_extents(tp, ip, firstblock, 1, flags, /* should only be called for types that support local format data */
XFS_DATA_FORK, ASSERT(0);
xfs_bmap_local_to_extents_init_fn); return EFSCORRUPTED;
} }
/* /*
...@@ -4907,20 +4906,19 @@ xfs_bmapi_write( ...@@ -4907,20 +4906,19 @@ xfs_bmapi_write(
orig_mval = mval; orig_mval = mval;
orig_nmap = *nmap; orig_nmap = *nmap;
#endif #endif
whichfork = (flags & XFS_BMAPI_ATTRFORK) ?
XFS_ATTR_FORK : XFS_DATA_FORK;
ASSERT(*nmap >= 1); ASSERT(*nmap >= 1);
ASSERT(*nmap <= XFS_BMAP_MAX_NMAP); ASSERT(*nmap <= XFS_BMAP_MAX_NMAP);
ASSERT(!(flags & XFS_BMAPI_IGSTATE)); ASSERT(!(flags & XFS_BMAPI_IGSTATE));
ASSERT(tp != NULL); ASSERT(tp != NULL);
ASSERT(len > 0); ASSERT(len > 0);
ASSERT(XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_LOCAL);
whichfork = (flags & XFS_BMAPI_ATTRFORK) ?
XFS_ATTR_FORK : XFS_DATA_FORK;
if (unlikely(XFS_TEST_ERROR( if (unlikely(XFS_TEST_ERROR(
(XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS && (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE && XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE),
XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_LOCAL),
mp, XFS_ERRTAG_BMAPIFORMAT, XFS_RANDOM_BMAPIFORMAT))) { mp, XFS_ERRTAG_BMAPIFORMAT, XFS_RANDOM_BMAPIFORMAT))) {
XFS_ERROR_REPORT("xfs_bmapi_write", XFS_ERRLEVEL_LOW, mp); XFS_ERROR_REPORT("xfs_bmapi_write", XFS_ERRLEVEL_LOW, mp);
return XFS_ERROR(EFSCORRUPTED); return XFS_ERROR(EFSCORRUPTED);
...@@ -4933,37 +4931,6 @@ xfs_bmapi_write( ...@@ -4933,37 +4931,6 @@ xfs_bmapi_write(
XFS_STATS_INC(xs_blk_mapw); XFS_STATS_INC(xs_blk_mapw);
if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL) {
/*
* XXX (dgc): This assumes we are only called for inodes that
* contain content neutral data in local format. Anything that
* contains caller-specific data in local format that needs
* transformation to move to a block format needs to do the
* conversion to extent format itself.
*
* Directory data forks and attribute forks handle this
* themselves, but with the addition of metadata verifiers every
* data fork in local format now contains caller specific data
* and as such conversion through this function is likely to be
* broken.
*
* The only likely user of this branch is for remote symlinks,
* but we cannot overwrite the data fork contents of the symlink
* (EEXIST occurs higher up the stack) and so it will never go
* from local format to extent format here. Hence I don't think
* this branch is ever executed intentionally and we should
* consider removing it and asserting that xfs_bmapi_write()
* cannot be called directly on local format forks. i.e. callers
* are completely responsible for local to extent format
* conversion, not xfs_bmapi_write().
*/
error = xfs_bmap_local_to_extents(tp, ip, firstblock, total,
&bma.logflags, whichfork,
xfs_bmap_local_to_extents_init_fn);
if (error)
goto error0;
}
if (*firstblock == NULLFSBLOCK) { if (*firstblock == NULLFSBLOCK) {
if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_BTREE) if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_BTREE)
bma.minleft = be16_to_cpu(ifp->if_broot->bb_level) + 1; bma.minleft = be16_to_cpu(ifp->if_broot->bb_level) + 1;
......
...@@ -172,6 +172,7 @@ void xfs_bmap_trace_exlist(struct xfs_inode *ip, xfs_extnum_t cnt, ...@@ -172,6 +172,7 @@ void xfs_bmap_trace_exlist(struct xfs_inode *ip, xfs_extnum_t cnt,
#endif #endif
int xfs_bmap_add_attrfork(struct xfs_inode *ip, int size, int rsvd); int xfs_bmap_add_attrfork(struct xfs_inode *ip, int size, int rsvd);
void xfs_bmap_local_to_extents_empty(struct xfs_inode *ip, int whichfork);
void xfs_bmap_add_free(xfs_fsblock_t bno, xfs_filblks_t len, void xfs_bmap_add_free(xfs_fsblock_t bno, xfs_filblks_t len,
struct xfs_bmap_free *flist, struct xfs_mount *mp); struct xfs_bmap_free *flist, struct xfs_mount *mp);
void xfs_bmap_cancel(struct xfs_bmap_free *flist); void xfs_bmap_cancel(struct xfs_bmap_free *flist);
......
...@@ -29,6 +29,7 @@ ...@@ -29,6 +29,7 @@
#include "xfs_dinode.h" #include "xfs_dinode.h"
#include "xfs_inode.h" #include "xfs_inode.h"
#include "xfs_inode_item.h" #include "xfs_inode_item.h"
#include "xfs_bmap.h"
#include "xfs_buf_item.h" #include "xfs_buf_item.h"
#include "xfs_dir2.h" #include "xfs_dir2.h"
#include "xfs_dir2_format.h" #include "xfs_dir2_format.h"
...@@ -1167,13 +1168,15 @@ xfs_dir2_sf_to_block( ...@@ -1167,13 +1168,15 @@ xfs_dir2_sf_to_block(
__be16 *tagp; /* end of data entry */ __be16 *tagp; /* end of data entry */
xfs_trans_t *tp; /* transaction pointer */ xfs_trans_t *tp; /* transaction pointer */
struct xfs_name name; struct xfs_name name;
struct xfs_ifork *ifp;
trace_xfs_dir2_sf_to_block(args); trace_xfs_dir2_sf_to_block(args);
dp = args->dp; dp = args->dp;
tp = args->trans; tp = args->trans;
mp = dp->i_mount; mp = dp->i_mount;
ASSERT(dp->i_df.if_flags & XFS_IFINLINE); ifp = XFS_IFORK_PTR(dp, XFS_DATA_FORK);
ASSERT(ifp->if_flags & XFS_IFINLINE);
/* /*
* Bomb out if the shortform directory is way too short. * Bomb out if the shortform directory is way too short.
*/ */
...@@ -1182,22 +1185,23 @@ xfs_dir2_sf_to_block( ...@@ -1182,22 +1185,23 @@ xfs_dir2_sf_to_block(
return XFS_ERROR(EIO); return XFS_ERROR(EIO);
} }
oldsfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data; oldsfp = (xfs_dir2_sf_hdr_t *)ifp->if_u1.if_data;
ASSERT(dp->i_df.if_bytes == dp->i_d.di_size); ASSERT(ifp->if_bytes == dp->i_d.di_size);
ASSERT(dp->i_df.if_u1.if_data != NULL); ASSERT(ifp->if_u1.if_data != NULL);
ASSERT(dp->i_d.di_size >= xfs_dir2_sf_hdr_size(oldsfp->i8count)); ASSERT(dp->i_d.di_size >= xfs_dir2_sf_hdr_size(oldsfp->i8count));
ASSERT(dp->i_d.di_nextents == 0);
/* /*
* Copy the directory into a temporary buffer. * Copy the directory into a temporary buffer.
* Then pitch the incore inode data so we can make extents. * Then pitch the incore inode data so we can make extents.
*/ */
sfp = kmem_alloc(dp->i_df.if_bytes, KM_SLEEP); sfp = kmem_alloc(ifp->if_bytes, KM_SLEEP);
memcpy(sfp, oldsfp, dp->i_df.if_bytes); memcpy(sfp, oldsfp, ifp->if_bytes);
xfs_idata_realloc(dp, -dp->i_df.if_bytes, XFS_DATA_FORK); xfs_idata_realloc(dp, -ifp->if_bytes, XFS_DATA_FORK);
xfs_bmap_local_to_extents_empty(dp, XFS_DATA_FORK);
dp->i_d.di_size = 0; dp->i_d.di_size = 0;
xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
/* /*
* Add block 0 to the inode. * Add block 0 to the inode.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment