Skip to content

Commit

Permalink
xfs: introduce xfs_bmapi_read()
Browse files Browse the repository at this point in the history
xfs_bmapi() currently handles both extent map reading and
allocation. As a result, the code is littered with "if (wr)"
branches to conditionally do allocation operations if required.
This makes the code much harder to follow and causes significant
indent issues with the code.

Given that read mapping is much simpler than allocation, we can
split out read mapping from xfs_bmapi() and reuse the logic that
we have already factored out do do all the hard work of handling the
extent map manipulations. The results in a much simpler function for
the common extent read operations, and will allow the allocation
code to be simplified in another commit.

Once xfs_bmapi_read() is implemented, convert all the callers of
xfs_bmapi() that are only reading extents to use the new function.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Alex Elder <aelder@sgi.com>
  • Loading branch information
Dave Chinner authored and Alex Elder committed Oct 12, 2011
1 parent aef9a89 commit 5c8ed20
Show file tree
Hide file tree
Showing 13 changed files with 151 additions and 77 deletions.
8 changes: 4 additions & 4 deletions fs/xfs/xfs_aops.c
Original file line number Diff line number Diff line change
Expand Up @@ -315,8 +315,8 @@ xfs_map_blocks(
count = mp->m_maxioffset - offset;
end_fsb = XFS_B_TO_FSB(mp, (xfs_ufsize_t)offset + count);
offset_fsb = XFS_B_TO_FSBT(mp, offset);
error = xfs_bmapi(NULL, ip, offset_fsb, end_fsb - offset_fsb,
bmapi_flags, NULL, 0, imap, &nimaps, NULL);
error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb,
imap, &nimaps, bmapi_flags);
xfs_iunlock(ip, XFS_ILOCK_SHARED);

if (error)
Expand Down Expand Up @@ -1138,8 +1138,8 @@ __xfs_get_blocks(
end_fsb = XFS_B_TO_FSB(mp, (xfs_ufsize_t)offset + size);
offset_fsb = XFS_B_TO_FSBT(mp, offset);

error = xfs_bmapi(NULL, ip, offset_fsb, end_fsb - offset_fsb,
XFS_BMAPI_ENTIRE, NULL, 0, &imap, &nimaps, NULL);
error = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb,
&imap, &nimaps, XFS_BMAPI_ENTIRE);
if (error)
goto out_unlock;

Expand Down
30 changes: 11 additions & 19 deletions fs/xfs/xfs_attr.c
Original file line number Diff line number Diff line change
Expand Up @@ -1963,10 +1963,9 @@ xfs_attr_rmtval_get(xfs_da_args_t *args)
lblkno = args->rmtblkno;
while (valuelen > 0) {
nmap = ATTR_RMTVALUE_MAPSIZE;
error = xfs_bmapi(args->trans, args->dp, (xfs_fileoff_t)lblkno,
args->rmtblkcnt,
XFS_BMAPI_ATTRFORK | XFS_BMAPI_METADATA,
NULL, 0, map, &nmap, NULL);
error = xfs_bmapi_read(args->dp, (xfs_fileoff_t)lblkno,
args->rmtblkcnt, map, &nmap,
XFS_BMAPI_ATTRFORK);
if (error)
return(error);
ASSERT(nmap >= 1);
Expand Down Expand Up @@ -2092,14 +2091,11 @@ xfs_attr_rmtval_set(xfs_da_args_t *args)
*/
xfs_bmap_init(args->flist, args->firstblock);
nmap = 1;
error = xfs_bmapi(NULL, dp, (xfs_fileoff_t)lblkno,
args->rmtblkcnt,
XFS_BMAPI_ATTRFORK | XFS_BMAPI_METADATA,
args->firstblock, 0, &map, &nmap,
NULL);
if (error) {
error = xfs_bmapi_read(dp, (xfs_fileoff_t)lblkno,
args->rmtblkcnt, &map, &nmap,
XFS_BMAPI_ATTRFORK);
if (error)
return(error);
}
ASSERT(nmap == 1);
ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
(map.br_startblock != HOLESTARTBLOCK));
Expand Down Expand Up @@ -2155,16 +2151,12 @@ xfs_attr_rmtval_remove(xfs_da_args_t *args)
/*
* Try to remember where we decided to put the value.
*/
xfs_bmap_init(args->flist, args->firstblock);
nmap = 1;
error = xfs_bmapi(NULL, args->dp, (xfs_fileoff_t)lblkno,
args->rmtblkcnt,
XFS_BMAPI_ATTRFORK | XFS_BMAPI_METADATA,
args->firstblock, 0, &map, &nmap,
args->flist);
if (error) {
error = xfs_bmapi_read(args->dp, (xfs_fileoff_t)lblkno,
args->rmtblkcnt, &map, &nmap,
XFS_BMAPI_ATTRFORK);
if (error)
return(error);
}
ASSERT(nmap == 1);
ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
(map.br_startblock != HOLESTARTBLOCK));
Expand Down
5 changes: 2 additions & 3 deletions fs/xfs/xfs_attr_leaf.c
Original file line number Diff line number Diff line change
Expand Up @@ -2926,9 +2926,8 @@ xfs_attr_leaf_freextent(xfs_trans_t **trans, xfs_inode_t *dp,
* Try to remember where we decided to put the value.
*/
nmap = 1;
error = xfs_bmapi(*trans, dp, (xfs_fileoff_t)tblkno, tblkcnt,
XFS_BMAPI_ATTRFORK | XFS_BMAPI_METADATA,
NULL, 0, &map, &nmap, NULL);
error = xfs_bmapi_read(dp, (xfs_fileoff_t)tblkno, tblkcnt,
&map, &nmap, XFS_BMAPI_ATTRFORK);
if (error) {
return(error);
}
Expand Down
104 changes: 97 additions & 7 deletions fs/xfs/xfs_bmap.c
Original file line number Diff line number Diff line change
Expand Up @@ -4349,6 +4349,98 @@ xfs_bmapi_update_map(
*map = mval;
}

/*
* Map file blocks to filesystem blocks without allocation.
*/
int
xfs_bmapi_read(
struct xfs_inode *ip,
xfs_fileoff_t bno,
xfs_filblks_t len,
struct xfs_bmbt_irec *mval,
int *nmap,
int flags)
{
struct xfs_mount *mp = ip->i_mount;
struct xfs_ifork *ifp;
struct xfs_bmbt_irec got;
struct xfs_bmbt_irec prev;
xfs_fileoff_t obno;
xfs_fileoff_t end;
xfs_extnum_t lastx;
int error;
int eof;
int n = 0;
int whichfork = (flags & XFS_BMAPI_ATTRFORK) ?
XFS_ATTR_FORK : XFS_DATA_FORK;

ASSERT(*nmap >= 1);
ASSERT(!(flags & ~(XFS_BMAPI_ATTRFORK|XFS_BMAPI_ENTIRE|
XFS_BMAPI_IGSTATE)));

if (unlikely(XFS_TEST_ERROR(
(XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE),
mp, XFS_ERRTAG_BMAPIFORMAT, XFS_RANDOM_BMAPIFORMAT))) {
XFS_ERROR_REPORT("xfs_bmapi_read", XFS_ERRLEVEL_LOW, mp);
return XFS_ERROR(EFSCORRUPTED);
}

if (XFS_FORCED_SHUTDOWN(mp))
return XFS_ERROR(EIO);

XFS_STATS_INC(xs_blk_mapr);

ifp = XFS_IFORK_PTR(ip, whichfork);
ASSERT(ifp->if_ext_max ==
XFS_IFORK_SIZE(ip, whichfork) / (uint)sizeof(xfs_bmbt_rec_t));

if (!(ifp->if_flags & XFS_IFEXTENTS)) {
error = xfs_iread_extents(NULL, ip, whichfork);
if (error)
return error;
}

xfs_bmap_search_extents(ip, bno, whichfork, &eof, &lastx, &got, &prev);
end = bno + len;
obno = bno;

while (bno < end && n < *nmap) {
/* Reading past eof, act as though there's a hole up to end. */
if (eof)
got.br_startoff = end;
if (got.br_startoff > bno) {
/* Reading in a hole. */
mval->br_startoff = bno;
mval->br_startblock = HOLESTARTBLOCK;
mval->br_blockcount =
XFS_FILBLKS_MIN(len, got.br_startoff - bno);
mval->br_state = XFS_EXT_NORM;
bno += mval->br_blockcount;
len -= mval->br_blockcount;
mval++;
n++;
continue;
}

/* set up the extent map to return. */
xfs_bmapi_trim_map(mval, &got, &bno, len, obno, end, n, flags);
xfs_bmapi_update_map(&mval, &bno, &len, obno, end, &n, flags);

/* If we're done, stop now. */
if (bno >= end || n >= *nmap)
break;

/* Else go on to the next record. */
if (++lastx < ifp->if_bytes / sizeof(xfs_bmbt_rec_t))
xfs_bmbt_get_all(xfs_iext_get_ext(ifp, lastx), &got);
else
eof = 1;
}
*nmap = n;
return 0;
}

/*
* Map file blocks to filesystem blocks.
* File range is given by the bno/len pair.
Expand Down Expand Up @@ -5490,10 +5582,9 @@ xfs_getbmap(

do {
nmap = (nexleft > subnex) ? subnex : nexleft;
error = xfs_bmapi(NULL, ip, XFS_BB_TO_FSBT(mp, bmv->bmv_offset),
XFS_BB_TO_FSB(mp, bmv->bmv_length),
bmapi_flags, NULL, 0, map, &nmap,
NULL);
error = xfs_bmapi_read(ip, XFS_BB_TO_FSBT(mp, bmv->bmv_offset),
XFS_BB_TO_FSB(mp, bmv->bmv_length),
map, &nmap, bmapi_flags);
if (error)
goto out_free_map;
ASSERT(nmap <= subnex);
Expand Down Expand Up @@ -6084,9 +6175,8 @@ xfs_bmap_punch_delalloc_range(
* trying to remove a real extent (which requires a
* transaction) or a hole, which is probably a bad idea...
*/
error = xfs_bmapi(NULL, ip, start_fsb, 1,
XFS_BMAPI_ENTIRE, NULL, 0, &imap,
&nimaps, NULL);
error = xfs_bmapi_read(ip, start_fsb, 1, &imap, &nimaps,
XFS_BMAPI_ENTIRE);

if (error) {
/* something screwed, just bail */
Expand Down
4 changes: 4 additions & 0 deletions fs/xfs/xfs_bmap.h
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,10 @@ xfs_bmapi(
int *nmap, /* i/o: mval size/count */
xfs_bmap_free_t *flist); /* i/o: list extents to free */

int xfs_bmapi_read(struct xfs_inode *ip, xfs_fileoff_t bno,
xfs_filblks_t len, struct xfs_bmbt_irec *mval,
int *nmap, int flags);

/*
* Map file blocks to filesystem blocks, simple version.
* One block only, read-only.
Expand Down
9 changes: 4 additions & 5 deletions fs/xfs/xfs_da_btree.c
Original file line number Diff line number Diff line change
Expand Up @@ -1995,11 +1995,10 @@ xfs_da_do_buf(
} else {
mapp = kmem_alloc(sizeof(*mapp) * nfsb, KM_SLEEP);
nmap = nfsb;
if ((error = xfs_bmapi(trans, dp, (xfs_fileoff_t)bno,
nfsb,
XFS_BMAPI_METADATA |
xfs_bmapi_aflag(whichfork),
NULL, 0, mapp, &nmap, NULL)))
error = xfs_bmapi_read(dp, (xfs_fileoff_t)bno, nfsb,
mapp, &nmap,
xfs_bmapi_aflag(whichfork));
if (error)
goto exit0;
}
} else {
Expand Down
6 changes: 2 additions & 4 deletions fs/xfs/xfs_dir2_leaf.c
Original file line number Diff line number Diff line change
Expand Up @@ -888,12 +888,10 @@ xfs_dir2_leaf_getdents(
* we already have in the table.
*/
nmap = map_size - map_valid;
error = xfs_bmapi(NULL, dp,
map_off,
error = xfs_bmapi_read(dp, map_off,
xfs_dir2_byte_to_da(mp,
XFS_DIR2_LEAF_OFFSET) - map_off,
XFS_BMAPI_METADATA, NULL, 0,
&map[map_valid], &nmap, NULL);
&map[map_valid], &nmap, 0);
/*
* Don't know if we should ignore this or
* try to return an error.
Expand Down
5 changes: 2 additions & 3 deletions fs/xfs/xfs_dquot.c
Original file line number Diff line number Diff line change
Expand Up @@ -488,9 +488,8 @@ xfs_qm_dqtobp(
/*
* Find the block map; no allocations yet
*/
error = xfs_bmapi(NULL, quotip, dqp->q_fileoffset,
XFS_DQUOT_CLUSTER_SIZE_FSB, XFS_BMAPI_METADATA,
NULL, 0, &map, &nmaps, NULL);
error = xfs_bmapi_read(quotip, dqp->q_fileoffset,
XFS_DQUOT_CLUSTER_SIZE_FSB, &map, &nmaps, 0);

xfs_iunlock(quotip, XFS_ILOCK_SHARED);
if (error)
Expand Down
10 changes: 4 additions & 6 deletions fs/xfs/xfs_file.c
Original file line number Diff line number Diff line change
Expand Up @@ -509,11 +509,9 @@ xfs_zero_last_block(

last_fsb = XFS_B_TO_FSBT(mp, isize);
nimaps = 1;
error = xfs_bmapi(NULL, ip, last_fsb, 1, 0, NULL, 0, &imap,
&nimaps, NULL);
if (error) {
error = xfs_bmapi_read(ip, last_fsb, 1, &imap, &nimaps, 0);
if (error)
return error;
}
ASSERT(nimaps > 0);
/*
* If the block underlying isize is just a hole, then there
Expand Down Expand Up @@ -604,8 +602,8 @@ xfs_zero_eof(
while (start_zero_fsb <= end_zero_fsb) {
nimaps = 1;
zero_count_fsb = end_zero_fsb - start_zero_fsb + 1;
error = xfs_bmapi(NULL, ip, start_zero_fsb, zero_count_fsb,
0, NULL, 0, &imap, &nimaps, NULL);
error = xfs_bmapi_read(ip, start_zero_fsb, zero_count_fsb,
&imap, &nimaps, 0);
if (error) {
ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL|XFS_IOLOCK_EXCL));
return error;
Expand Down
12 changes: 6 additions & 6 deletions fs/xfs/xfs_inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -1187,6 +1187,7 @@ xfs_isize_check(
xfs_fileoff_t map_first;
int nimaps;
xfs_bmbt_irec_t imaps[2];
int error;

if (!S_ISREG(ip->i_d.di_mode))
return;
Expand All @@ -1203,13 +1204,12 @@ xfs_isize_check(
* The filesystem could be shutting down, so bmapi may return
* an error.
*/
if (xfs_bmapi(NULL, ip, map_first,
error = xfs_bmapi_read(ip, map_first,
(XFS_B_TO_FSB(mp,
(xfs_ufsize_t)XFS_MAXIOFFSET(mp)) -
map_first),
XFS_BMAPI_ENTIRE, NULL, 0, imaps, &nimaps,
NULL))
return;
(xfs_ufsize_t)XFS_MAXIOFFSET(mp)) - map_first),
imaps, &nimaps, XFS_BMAPI_ENTIRE);
if (error)
return;
ASSERT(nimaps == 1);
ASSERT(imaps[0].br_startblock == HOLESTARTBLOCK);
}
Expand Down
4 changes: 2 additions & 2 deletions fs/xfs/xfs_iomap.c
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,8 @@ xfs_iomap_eof_want_preallocate(
while (count_fsb > 0) {
imaps = nimaps;
firstblock = NULLFSBLOCK;
error = xfs_bmapi(NULL, ip, start_fsb, count_fsb, 0,
&firstblock, 0, imap, &imaps, NULL);
error = xfs_bmapi_read(ip, start_fsb, count_fsb, imap, &imaps,
0);
if (error)
return error;
for (n = 0; n < imaps; n++) {
Expand Down
7 changes: 2 additions & 5 deletions fs/xfs/xfs_qm.c
Original file line number Diff line number Diff line change
Expand Up @@ -1347,11 +1347,8 @@ xfs_qm_dqiterate(
* the inode is never added to the transaction.
*/
xfs_ilock(qip, XFS_ILOCK_SHARED);
error = xfs_bmapi(NULL, qip, lblkno,
maxlblkcnt - lblkno,
XFS_BMAPI_METADATA,
NULL,
0, map, &nmaps, NULL);
error = xfs_bmapi_read(qip, lblkno, maxlblkcnt - lblkno,
map, &nmaps, 0);
xfs_iunlock(qip, XFS_ILOCK_SHARED);
if (error)
break;
Expand Down
Loading

0 comments on commit 5c8ed20

Please sign in to comment.