* [PATCH 0/9] extent buffer indexing fixes
@ 2011-05-11 15:04 Christoph Hellwig
2011-05-11 15:04 ` [PATCH 1/9] xfs: remove the unused XFS_BMAPI_RSVBLOCKS flag Christoph Hellwig
` (8 more replies)
0 siblings, 9 replies; 30+ messages in thread
From: Christoph Hellwig @ 2011-05-11 15:04 UTC (permalink / raw)
To: xfs
I recently ran into some extent buffer indexing issue which turned to be
my fault in code I was working on. But while looking into these I found
an old patch from Lachlan McIlroy that tried to fix various issue in
that area. I went through them slowly to understand what's going on and
ended up with this series. The first patch is not actually related but
touches the area and was in my queue so I've decided to include it.
The second patch removes the if_lastex field in struct xfs_ifork as it's
not actually needed and just makes the code using it confusing. The
following patches fixes various places that feed too large indices into
xfs_iext_get_ext or xfs_iext_idx_to_irec, and the last patch finally
adds asserts into these to catch the incorrect accesses.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH 1/9] xfs: remove the unused XFS_BMAPI_RSVBLOCKS flag 2011-05-11 15:04 [PATCH 0/9] extent buffer indexing fixes Christoph Hellwig @ 2011-05-11 15:04 ` Christoph Hellwig 2011-05-20 20:17 ` Alex Elder 2011-05-11 15:04 ` [PATCH 2/9] xfs: remove if_lastex Christoph Hellwig ` (7 subsequent siblings) 8 siblings, 1 reply; 30+ messages in thread From: Christoph Hellwig @ 2011-05-11 15:04 UTC (permalink / raw) To: xfs [-- Attachment #1: xfs-kill-XFS_BMAPI_RSVBLOCKS --] [-- Type: text/plain, Size: 11861 bytes --] The XFS_BMAPI_RSVBLOCKS is unused, and as far as I can see has always been. Remove it to simplify the bmapi implementation and conserve stack space. Signed-off-by: Christoph Hellwig <hch@lst.de> Index: xfs/fs/xfs/xfs_bmap.c =================================================================== --- xfs.orig/fs/xfs/xfs_bmap.c 2011-05-10 12:05:00.662952211 +0200 +++ xfs/fs/xfs/xfs_bmap.c 2011-05-10 12:05:19.866950315 +0200 @@ -101,8 +101,7 @@ xfs_bmap_add_extent( xfs_fsblock_t *first, /* pointer to firstblock variable */ xfs_bmap_free_t *flist, /* list of extents to be freed */ int *logflagsp, /* inode logging flags */ - int whichfork, /* data or attr fork */ - int rsvd); /* OK to allocate reserved blocks */ + int whichfork); /* data or attr fork */ /* * Called by xfs_bmap_add_extent to handle cases converting a delayed @@ -117,8 +116,7 @@ xfs_bmap_add_extent_delay_real( xfs_filblks_t *dnew, /* new delayed-alloc indirect blocks */ xfs_fsblock_t *first, /* pointer to firstblock variable */ xfs_bmap_free_t *flist, /* list of extents to be freed */ - int *logflagsp, /* inode logging flags */ - int rsvd); /* OK to allocate reserved blocks */ + int *logflagsp); /* inode logging flags */ /* * Called by xfs_bmap_add_extent to handle cases converting a hole @@ -129,8 +127,7 @@ xfs_bmap_add_extent_hole_delay( xfs_inode_t *ip, /* incore inode pointer */ xfs_extnum_t idx, /* extent number to update/insert */ xfs_bmbt_irec_t *new, /* new data to add to file extents */ - int *logflagsp,/* inode logging flags */ - int rsvd); /* OK to allocate reserved blocks */ + int *logflagsp); /* inode logging flags */ /* * Called by xfs_bmap_add_extent to handle cases converting a hole @@ -180,22 +177,6 @@ xfs_bmap_btree_to_extents( int whichfork); /* data or attr fork */ /* - * Called by xfs_bmapi to update file extent records and the btree - * after removing space (or undoing a delayed allocation). - */ -STATIC int /* error */ -xfs_bmap_del_extent( - xfs_inode_t *ip, /* incore inode pointer */ - xfs_trans_t *tp, /* current trans pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ - xfs_bmap_free_t *flist, /* list of extents to be freed */ - xfs_btree_cur_t *cur, /* if null, not a btree */ - xfs_bmbt_irec_t *new, /* new data to add to file extents */ - int *logflagsp,/* inode logging flags */ - int whichfork, /* data or attr fork */ - int rsvd); /* OK to allocate reserved blocks */ - -/* * Remove the entry "free" from the free item list. Prev points to the * previous entry, unless "free" is the head of the list. */ @@ -480,8 +461,7 @@ xfs_bmap_add_extent( xfs_fsblock_t *first, /* pointer to firstblock variable */ xfs_bmap_free_t *flist, /* list of extents to be freed */ int *logflagsp, /* inode logging flags */ - int whichfork, /* data or attr fork */ - int rsvd) /* OK to use reserved data blocks */ + int whichfork) /* data or attr fork */ { xfs_btree_cur_t *cur; /* btree cursor or null */ xfs_filblks_t da_new; /* new count del alloc blocks used */ @@ -522,8 +502,8 @@ xfs_bmap_add_extent( if (cur) ASSERT((cur->bc_private.b.flags & XFS_BTCUR_BPRV_WASDEL) == 0); - if ((error = xfs_bmap_add_extent_hole_delay(ip, idx, new, - &logflags, rsvd))) + error = xfs_bmap_add_extent_hole_delay(ip, idx, new, &logflags); + if (error) goto done; } /* @@ -557,9 +537,10 @@ xfs_bmap_add_extent( if (cur) ASSERT(cur->bc_private.b.flags & XFS_BTCUR_BPRV_WASDEL); - if ((error = xfs_bmap_add_extent_delay_real(ip, - idx, &cur, new, &da_new, first, flist, - &logflags, rsvd))) + error = xfs_bmap_add_extent_delay_real(ip, idx, + &cur, new, &da_new, first, + flist, &logflags); + if (error) goto done; } else if (new->br_state == XFS_EXT_NORM) { ASSERT(new->br_state == XFS_EXT_NORM); @@ -615,7 +596,7 @@ xfs_bmap_add_extent( ASSERT(nblks <= da_old); if (nblks < da_old) xfs_icsb_modify_counters(ip->i_mount, XFS_SBS_FDBLOCKS, - (int64_t)(da_old - nblks), rsvd); + (int64_t)(da_old - nblks), 0); } /* * Clear out the allocated field, done with it now in any case. @@ -646,8 +627,7 @@ xfs_bmap_add_extent_delay_real( xfs_filblks_t *dnew, /* new delayed-alloc indirect blocks */ xfs_fsblock_t *first, /* pointer to firstblock variable */ xfs_bmap_free_t *flist, /* list of extents to be freed */ - int *logflagsp, /* inode logging flags */ - int rsvd) /* OK to use reserved data block allocation */ + int *logflagsp) /* inode logging flags */ { xfs_btree_cur_t *cur; /* btree cursor */ int diff; /* temp value */ @@ -1097,7 +1077,7 @@ xfs_bmap_add_extent_delay_real( (cur ? cur->bc_private.b.allocated : 0)); if (diff > 0 && xfs_icsb_modify_counters(ip->i_mount, XFS_SBS_FDBLOCKS, - -((int64_t)diff), rsvd)) { + -((int64_t)diff), 0)) { /* * Ick gross gag me with a spoon. */ @@ -1109,7 +1089,7 @@ xfs_bmap_add_extent_delay_real( if (!diff || !xfs_icsb_modify_counters(ip->i_mount, XFS_SBS_FDBLOCKS, - -((int64_t)diff), rsvd)) + -((int64_t)diff), 0)) break; } if (temp2) { @@ -1118,7 +1098,7 @@ xfs_bmap_add_extent_delay_real( if (!diff || !xfs_icsb_modify_counters(ip->i_mount, XFS_SBS_FDBLOCKS, - -((int64_t)diff), rsvd)) + -((int64_t)diff), 0)) break; } } @@ -1652,8 +1632,7 @@ xfs_bmap_add_extent_hole_delay( xfs_inode_t *ip, /* incore inode pointer */ xfs_extnum_t idx, /* extent number to update/insert */ xfs_bmbt_irec_t *new, /* new data to add to file extents */ - int *logflagsp, /* inode logging flags */ - int rsvd) /* OK to allocate reserved blocks */ + int *logflagsp) /* inode logging flags */ { xfs_bmbt_rec_host_t *ep; /* extent record for idx */ xfs_ifork_t *ifp; /* inode fork pointer */ @@ -1787,7 +1766,7 @@ xfs_bmap_add_extent_hole_delay( if (oldlen != newlen) { ASSERT(oldlen > newlen); xfs_icsb_modify_counters(ip->i_mount, XFS_SBS_FDBLOCKS, - (int64_t)(oldlen - newlen), rsvd); + (int64_t)(oldlen - newlen), 0); /* * Nothing to do for disk quota accounting here. */ @@ -2838,8 +2817,7 @@ xfs_bmap_del_extent( xfs_btree_cur_t *cur, /* if null, not a btree */ xfs_bmbt_irec_t *del, /* data to remove from extents */ int *logflagsp, /* inode logging flags */ - int whichfork, /* data or attr fork */ - int rsvd) /* OK to allocate reserved blocks */ + int whichfork) /* data or attr fork */ { xfs_filblks_t da_new; /* new delay-alloc indirect blocks */ xfs_filblks_t da_old; /* old delay-alloc indirect blocks */ @@ -3142,7 +3120,7 @@ xfs_bmap_del_extent( ASSERT(da_old >= da_new); if (da_old > da_new) { xfs_icsb_modify_counters(mp, XFS_SBS_FDBLOCKS, - (int64_t)(da_old - da_new), rsvd); + (int64_t)(da_old - da_new), 0); } done: *logflagsp = flags; @@ -4562,29 +4540,24 @@ xfs_bmapi( if (rt) { error = xfs_mod_incore_sb(mp, XFS_SBS_FREXTENTS, - -((int64_t)extsz), (flags & - XFS_BMAPI_RSVBLOCKS)); + -((int64_t)extsz), 0); } else { error = xfs_icsb_modify_counters(mp, XFS_SBS_FDBLOCKS, - -((int64_t)alen), (flags & - XFS_BMAPI_RSVBLOCKS)); + -((int64_t)alen), 0); } if (!error) { error = xfs_icsb_modify_counters(mp, XFS_SBS_FDBLOCKS, - -((int64_t)indlen), (flags & - XFS_BMAPI_RSVBLOCKS)); + -((int64_t)indlen), 0); if (error && rt) xfs_mod_incore_sb(mp, XFS_SBS_FREXTENTS, - (int64_t)extsz, (flags & - XFS_BMAPI_RSVBLOCKS)); + (int64_t)extsz, 0); else if (error) xfs_icsb_modify_counters(mp, XFS_SBS_FDBLOCKS, - (int64_t)alen, (flags & - XFS_BMAPI_RSVBLOCKS)); + (int64_t)alen, 0); } if (error) { @@ -4703,7 +4676,7 @@ xfs_bmapi( } error = xfs_bmap_add_extent(ip, lastx, &cur, &got, firstblock, flist, &tmp_logflags, - whichfork, (flags & XFS_BMAPI_RSVBLOCKS)); + whichfork); logflags |= tmp_logflags; if (error) goto error0; @@ -4805,7 +4778,7 @@ xfs_bmapi( : XFS_EXT_UNWRITTEN; error = xfs_bmap_add_extent(ip, lastx, &cur, mval, firstblock, flist, &tmp_logflags, - whichfork, (flags & XFS_BMAPI_RSVBLOCKS)); + whichfork); logflags |= tmp_logflags; if (error) goto error0; @@ -5026,7 +4999,6 @@ xfs_bunmapi( int tmp_logflags; /* partial logging flags */ int wasdel; /* was a delayed alloc extent */ int whichfork; /* data or attribute fork */ - int rsvd; /* OK to allocate reserved blocks */ xfs_fsblock_t sum; trace_xfs_bunmap(ip, bno, len, flags, _RET_IP_); @@ -5044,7 +5016,7 @@ xfs_bunmapi( mp = ip->i_mount; if (XFS_FORCED_SHUTDOWN(mp)) return XFS_ERROR(EIO); - rsvd = (flags & XFS_BMAPI_RSVBLOCKS) != 0; + ASSERT(len > 0); ASSERT(nexts >= 0); ASSERT(ifp->if_ext_max == @@ -5162,7 +5134,7 @@ xfs_bunmapi( del.br_state = XFS_EXT_UNWRITTEN; error = xfs_bmap_add_extent(ip, lastx, &cur, &del, firstblock, flist, &logflags, - XFS_DATA_FORK, 0); + XFS_DATA_FORK); if (error) goto error0; goto nodelete; @@ -5216,7 +5188,7 @@ xfs_bunmapi( prev.br_state = XFS_EXT_UNWRITTEN; error = xfs_bmap_add_extent(ip, lastx - 1, &cur, &prev, firstblock, flist, &logflags, - XFS_DATA_FORK, 0); + XFS_DATA_FORK); if (error) goto error0; goto nodelete; @@ -5225,7 +5197,7 @@ xfs_bunmapi( del.br_state = XFS_EXT_UNWRITTEN; error = xfs_bmap_add_extent(ip, lastx, &cur, &del, firstblock, flist, &logflags, - XFS_DATA_FORK, 0); + XFS_DATA_FORK); if (error) goto error0; goto nodelete; @@ -5240,13 +5212,13 @@ xfs_bunmapi( rtexts = XFS_FSB_TO_B(mp, del.br_blockcount); do_div(rtexts, mp->m_sb.sb_rextsize); xfs_mod_incore_sb(mp, XFS_SBS_FREXTENTS, - (int64_t)rtexts, rsvd); + (int64_t)rtexts, 0); (void)xfs_trans_reserve_quota_nblks(NULL, ip, -((long)del.br_blockcount), 0, XFS_QMOPT_RES_RTBLKS); } else { xfs_icsb_modify_counters(mp, XFS_SBS_FDBLOCKS, - (int64_t)del.br_blockcount, rsvd); + (int64_t)del.br_blockcount, 0); (void)xfs_trans_reserve_quota_nblks(NULL, ip, -((long)del.br_blockcount), 0, XFS_QMOPT_RES_REGBLKS); @@ -5278,7 +5250,7 @@ xfs_bunmapi( goto error0; } error = xfs_bmap_del_extent(ip, tp, lastx, flist, cur, &del, - &tmp_logflags, whichfork, rsvd); + &tmp_logflags, whichfork); logflags |= tmp_logflags; if (error) goto error0; Index: xfs/fs/xfs/xfs_bmap.h =================================================================== --- xfs.orig/fs/xfs/xfs_bmap.h 2011-04-24 20:52:34.171232490 +0200 +++ xfs/fs/xfs/xfs_bmap.h 2011-05-10 12:05:19.870955293 +0200 @@ -69,7 +69,6 @@ typedef struct xfs_bmap_free #define XFS_BMAPI_ENTIRE 0x004 /* return entire extent, not trimmed */ #define XFS_BMAPI_METADATA 0x008 /* mapping metadata not user data */ #define XFS_BMAPI_ATTRFORK 0x010 /* use attribute fork not data */ -#define XFS_BMAPI_RSVBLOCKS 0x020 /* OK to alloc. reserved data blocks */ #define XFS_BMAPI_PREALLOC 0x040 /* preallocation op: unwritten space */ #define XFS_BMAPI_IGSTATE 0x080 /* Ignore state - */ /* combine contig. space */ @@ -87,7 +86,6 @@ typedef struct xfs_bmap_free { XFS_BMAPI_ENTIRE, "ENTIRE" }, \ { XFS_BMAPI_METADATA, "METADATA" }, \ { XFS_BMAPI_ATTRFORK, "ATTRFORK" }, \ - { XFS_BMAPI_RSVBLOCKS, "RSVBLOCKS" }, \ { XFS_BMAPI_PREALLOC, "PREALLOC" }, \ { XFS_BMAPI_IGSTATE, "IGSTATE" }, \ { XFS_BMAPI_CONTIG, "CONTIG" }, \ _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/9] xfs: remove the unused XFS_BMAPI_RSVBLOCKS flag 2011-05-11 15:04 ` [PATCH 1/9] xfs: remove the unused XFS_BMAPI_RSVBLOCKS flag Christoph Hellwig @ 2011-05-20 20:17 ` Alex Elder 0 siblings, 0 replies; 30+ messages in thread From: Alex Elder @ 2011-05-20 20:17 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Wed, 2011-05-11 at 11:04 -0400, Christoph Hellwig wrote: > plain text document attachment (xfs-kill-XFS_BMAPI_RSVBLOCKS) > The XFS_BMAPI_RSVBLOCKS is unused, and as far as I can see has always been. > Remove it to simplify the bmapi implementation and conserve stack space. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Looks good. Reviewed-by: Alex Elder <aelder@sgi.com> _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2/9] xfs: remove if_lastex 2011-05-11 15:04 [PATCH 0/9] extent buffer indexing fixes Christoph Hellwig 2011-05-11 15:04 ` [PATCH 1/9] xfs: remove the unused XFS_BMAPI_RSVBLOCKS flag Christoph Hellwig @ 2011-05-11 15:04 ` Christoph Hellwig 2011-05-20 20:17 ` Alex Elder 2011-05-23 8:52 ` [PATCH 2/9 v2] " Christoph Hellwig 2011-05-11 15:04 ` [PATCH 3/9] xfs: correctly decrement the extent buffer index in xfs_bmap_del_extent Christoph Hellwig ` (6 subsequent siblings) 8 siblings, 2 replies; 30+ messages in thread From: Christoph Hellwig @ 2011-05-11 15:04 UTC (permalink / raw) To: xfs [-- Attachment #1: xfs-kill-if_lastex --] [-- Type: text/plain, Size: 44547 bytes --] The if_lastex field in struct xfs_ifork is only used as a temporary index during xfs_bmapi and xfs_bmapi. Instead of using the inode fork to store it keep it local in the callchain. Fortunately this is very easy as we already pass a stack copy of it down the whole chain which can simplify be changed to be passed by reference. Signed-off-by: Christoph Hellwig <hch@lst.de> Index: xfs/fs/xfs/xfs_bmap.c =================================================================== --- xfs.orig/fs/xfs/xfs_bmap.c 2011-05-10 13:20:21.182349200 +0200 +++ xfs/fs/xfs/xfs_bmap.c 2011-05-10 13:29:40.618349159 +0200 @@ -89,28 +89,13 @@ xfs_bmap_add_attrfork_local( int *flags); /* inode logging flags */ /* - * Called by xfs_bmapi to update file extent records and the btree - * after allocating space (or doing a delayed allocation). - */ -STATIC int /* error */ -xfs_bmap_add_extent( - xfs_inode_t *ip, /* incore inode pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ - xfs_btree_cur_t **curp, /* if *curp is null, not a btree */ - xfs_bmbt_irec_t *new, /* new data to add to file extents */ - xfs_fsblock_t *first, /* pointer to firstblock variable */ - xfs_bmap_free_t *flist, /* list of extents to be freed */ - int *logflagsp, /* inode logging flags */ - int whichfork); /* data or attr fork */ - -/* * Called by xfs_bmap_add_extent to handle cases converting a delayed * allocation to a real allocation. */ STATIC int /* error */ xfs_bmap_add_extent_delay_real( xfs_inode_t *ip, /* incore inode pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ + xfs_extnum_t *idx, /* extent number to update/insert */ xfs_btree_cur_t **curp, /* if *curp is null, not a btree */ xfs_bmbt_irec_t *new, /* new data to add to file extents */ xfs_filblks_t *dnew, /* new delayed-alloc indirect blocks */ @@ -125,7 +110,7 @@ xfs_bmap_add_extent_delay_real( STATIC int /* error */ xfs_bmap_add_extent_hole_delay( xfs_inode_t *ip, /* incore inode pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ + xfs_extnum_t *idx, /* extent number to update/insert */ xfs_bmbt_irec_t *new, /* new data to add to file extents */ int *logflagsp); /* inode logging flags */ @@ -136,7 +121,7 @@ xfs_bmap_add_extent_hole_delay( STATIC int /* error */ xfs_bmap_add_extent_hole_real( xfs_inode_t *ip, /* incore inode pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ + xfs_extnum_t *idx, /* extent number to update/insert */ xfs_btree_cur_t *cur, /* if null, not a btree */ xfs_bmbt_irec_t *new, /* new data to add to file extents */ int *logflagsp, /* inode logging flags */ @@ -149,7 +134,7 @@ xfs_bmap_add_extent_hole_real( STATIC int /* error */ xfs_bmap_add_extent_unwritten_real( xfs_inode_t *ip, /* incore inode pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ + xfs_extnum_t *idx, /* extent number to update/insert */ xfs_btree_cur_t **curp, /* if *curp is null, not a btree */ xfs_bmbt_irec_t *new, /* new data to add to file extents */ int *logflagsp); /* inode logging flags */ @@ -455,7 +440,7 @@ xfs_bmap_add_attrfork_local( STATIC int /* error */ xfs_bmap_add_extent( xfs_inode_t *ip, /* incore inode pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ + xfs_extnum_t *idx, /* extent number to update/insert */ xfs_btree_cur_t **curp, /* if *curp is null, not a btree */ xfs_bmbt_irec_t *new, /* new data to add to file extents */ xfs_fsblock_t *first, /* pointer to firstblock variable */ @@ -472,23 +457,27 @@ xfs_bmap_add_extent( xfs_extnum_t nextents; /* number of extents in file now */ XFS_STATS_INC(xs_add_exlist); + cur = *curp; ifp = XFS_IFORK_PTR(ip, whichfork); nextents = ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t); - ASSERT(idx <= nextents); da_old = da_new = 0; error = 0; + + ASSERT(*idx >= 0); + ASSERT(*idx <= nextents); + /* * This is the first extent added to a new/empty file. * Special case this one, so other routines get to assume there are * already extents in the list. */ if (nextents == 0) { - xfs_iext_insert(ip, 0, 1, new, + xfs_iext_insert(ip, *idx, 1, new, whichfork == XFS_ATTR_FORK ? BMAP_ATTRFORK : 0); ASSERT(cur == NULL); - ifp->if_lastex = 0; + if (!isnullstartblock(new->br_startblock)) { XFS_IFORK_NEXT_SET(ip, whichfork, 1); logflags = XFS_ILOG_CORE | xfs_ilog_fext(whichfork); @@ -502,27 +491,25 @@ xfs_bmap_add_extent( if (cur) ASSERT((cur->bc_private.b.flags & XFS_BTCUR_BPRV_WASDEL) == 0); - error = xfs_bmap_add_extent_hole_delay(ip, idx, new, &logflags); - if (error) - goto done; + error = xfs_bmap_add_extent_hole_delay(ip, idx, new, + &logflags); } /* * Real allocation off the end of the file. */ - else if (idx == nextents) { + else if (*idx == nextents) { if (cur) ASSERT((cur->bc_private.b.flags & XFS_BTCUR_BPRV_WASDEL) == 0); - if ((error = xfs_bmap_add_extent_hole_real(ip, idx, cur, new, - &logflags, whichfork))) - goto done; + error = xfs_bmap_add_extent_hole_real(ip, idx, cur, new, + &logflags, whichfork); } else { xfs_bmbt_irec_t prev; /* old extent at offset idx */ /* * Get the record referred to by idx. */ - xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx), &prev); + xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx), &prev); /* * If it's a real allocation record, and the new allocation ends * after the start of the referred to record, then we're filling @@ -537,23 +524,18 @@ xfs_bmap_add_extent( if (cur) ASSERT(cur->bc_private.b.flags & XFS_BTCUR_BPRV_WASDEL); - error = xfs_bmap_add_extent_delay_real(ip, idx, - &cur, new, &da_new, first, - flist, &logflags); - if (error) - goto done; - } else if (new->br_state == XFS_EXT_NORM) { - ASSERT(new->br_state == XFS_EXT_NORM); - if ((error = xfs_bmap_add_extent_unwritten_real( - ip, idx, &cur, new, &logflags))) - goto done; + error = xfs_bmap_add_extent_delay_real(ip, + idx, &cur, new, &da_new, + first, flist, &logflags); } else { - ASSERT(new->br_state == XFS_EXT_UNWRITTEN); - if ((error = xfs_bmap_add_extent_unwritten_real( - ip, idx, &cur, new, &logflags))) + ASSERT(new->br_state == XFS_EXT_NORM || + new->br_state == XFS_EXT_UNWRITTEN); + + error = xfs_bmap_add_extent_unwritten_real(ip, + idx, &cur, new, &logflags); + if (error) goto done; } - ASSERT(*curp == cur || *curp == NULL); } /* * Otherwise we're filling in a hole with an allocation. @@ -562,13 +544,15 @@ xfs_bmap_add_extent( if (cur) ASSERT((cur->bc_private.b.flags & XFS_BTCUR_BPRV_WASDEL) == 0); - if ((error = xfs_bmap_add_extent_hole_real(ip, idx, cur, - new, &logflags, whichfork))) - goto done; + error = xfs_bmap_add_extent_hole_real(ip, idx, cur, + new, &logflags, whichfork); } } + if (error) + goto done; ASSERT(*curp == cur || *curp == NULL); + /* * Convert to a btree if necessary. */ @@ -621,7 +605,7 @@ done: STATIC int /* error */ xfs_bmap_add_extent_delay_real( xfs_inode_t *ip, /* incore inode pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ + xfs_extnum_t *idx, /* extent number to update/insert */ xfs_btree_cur_t **curp, /* if *curp is null, not a btree */ xfs_bmbt_irec_t *new, /* new data to add to file extents */ xfs_filblks_t *dnew, /* new delayed-alloc indirect blocks */ @@ -653,7 +637,7 @@ xfs_bmap_add_extent_delay_real( */ cur = *curp; ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK); - ep = xfs_iext_get_ext(ifp, idx); + ep = xfs_iext_get_ext(ifp, *idx); xfs_bmbt_get_all(ep, &PREV); new_endoff = new->br_startoff + new->br_blockcount; ASSERT(PREV.br_startoff <= new->br_startoff); @@ -672,9 +656,9 @@ xfs_bmap_add_extent_delay_real( * Check and set flags if this segment has a left neighbor. * Don't set contiguous if the combined extent would be too large. */ - if (idx > 0) { + if (*idx > 0) { state |= BMAP_LEFT_VALID; - xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx - 1), &LEFT); + xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx - 1), &LEFT); if (isnullstartblock(LEFT.br_startblock)) state |= BMAP_LEFT_DELAY; @@ -692,9 +676,9 @@ xfs_bmap_add_extent_delay_real( * Don't set contiguous if the combined extent would be too large. * Also check for all-three-contiguous being too large. */ - if (idx < ip->i_df.if_bytes / (uint)sizeof(xfs_bmbt_rec_t) - 1) { + if (*idx < ip->i_df.if_bytes / (uint)sizeof(xfs_bmbt_rec_t) - 1) { state |= BMAP_RIGHT_VALID; - xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx + 1), &RIGHT); + xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx + 1), &RIGHT); if (isnullstartblock(RIGHT.br_startblock)) state |= BMAP_RIGHT_DELAY; @@ -725,14 +709,13 @@ xfs_bmap_add_extent_delay_real( * Filling in all of a previously delayed allocation extent. * The left and right neighbors are both contiguous with new. */ - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), + trace_xfs_bmap_pre_update(ip, *idx - 1, state, _THIS_IP_); + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx - 1), LEFT.br_blockcount + PREV.br_blockcount + RIGHT.br_blockcount); - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx - 1, state, _THIS_IP_); - xfs_iext_remove(ip, idx, 2, state); - ip->i_df.if_lastex = idx - 1; + xfs_iext_remove(ip, *idx, 2, state); ip->i_d.di_nextents--; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -756,6 +739,8 @@ xfs_bmap_add_extent_delay_real( RIGHT.br_blockcount, LEFT.br_state))) goto done; } + + --*idx; *dnew = 0; break; @@ -764,13 +749,12 @@ xfs_bmap_add_extent_delay_real( * Filling in all of a previously delayed allocation extent. * The left neighbor is contiguous, the right is not. */ - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), + trace_xfs_bmap_pre_update(ip, *idx - 1, state, _THIS_IP_); + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx - 1), LEFT.br_blockcount + PREV.br_blockcount); - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx - 1, state, _THIS_IP_); - ip->i_df.if_lastex = idx - 1; - xfs_iext_remove(ip, idx, 1, state); + xfs_iext_remove(ip, *idx, 1, state); if (cur == NULL) rval = XFS_ILOG_DEXT; else { @@ -786,6 +770,8 @@ xfs_bmap_add_extent_delay_real( PREV.br_blockcount, LEFT.br_state))) goto done; } + + --*idx; *dnew = 0; break; @@ -794,14 +780,13 @@ xfs_bmap_add_extent_delay_real( * Filling in all of a previously delayed allocation extent. * The right neighbor is contiguous, the left is not. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_startblock(ep, new->br_startblock); xfs_bmbt_set_blockcount(ep, PREV.br_blockcount + RIGHT.br_blockcount); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - ip->i_df.if_lastex = idx; - xfs_iext_remove(ip, idx + 1, 1, state); + xfs_iext_remove(ip, *idx + 1, 1, state); if (cur == NULL) rval = XFS_ILOG_DEXT; else { @@ -817,6 +802,7 @@ xfs_bmap_add_extent_delay_real( RIGHT.br_blockcount, PREV.br_state))) goto done; } + *dnew = 0; break; @@ -826,11 +812,10 @@ xfs_bmap_add_extent_delay_real( * Neither the left nor right neighbors are contiguous with * the new one. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_startblock(ep, new->br_startblock); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - ip->i_df.if_lastex = idx; ip->i_d.di_nextents++; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -846,6 +831,7 @@ xfs_bmap_add_extent_delay_real( goto done; XFS_WANT_CORRUPTED_GOTO(i == 1, done); } + *dnew = 0; break; @@ -854,17 +840,16 @@ xfs_bmap_add_extent_delay_real( * Filling in the first part of a previous delayed allocation. * The left neighbor is contiguous. */ - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), + trace_xfs_bmap_pre_update(ip, *idx - 1, state, _THIS_IP_); + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx - 1), LEFT.br_blockcount + new->br_blockcount); xfs_bmbt_set_startoff(ep, PREV.br_startoff + new->br_blockcount); - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx - 1, state, _THIS_IP_); temp = PREV.br_blockcount - new->br_blockcount; - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_blockcount(ep, temp); - ip->i_df.if_lastex = idx - 1; if (cur == NULL) rval = XFS_ILOG_DEXT; else { @@ -884,7 +869,9 @@ xfs_bmap_add_extent_delay_real( temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp), startblockval(PREV.br_startblock)); xfs_bmbt_set_startblock(ep, nullstartblock((int)temp)); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); + + --*idx; *dnew = temp; break; @@ -893,12 +880,11 @@ xfs_bmap_add_extent_delay_real( * Filling in the first part of a previous delayed allocation. * The left neighbor is not contiguous. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_startoff(ep, new_endoff); temp = PREV.br_blockcount - new->br_blockcount; xfs_bmbt_set_blockcount(ep, temp); - xfs_iext_insert(ip, idx, 1, new, state); - ip->i_df.if_lastex = idx; + xfs_iext_insert(ip, *idx, 1, new, state); ip->i_d.di_nextents++; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -926,9 +912,10 @@ xfs_bmap_add_extent_delay_real( temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp), startblockval(PREV.br_startblock) - (cur ? cur->bc_private.b.allocated : 0)); - ep = xfs_iext_get_ext(ifp, idx + 1); + ep = xfs_iext_get_ext(ifp, *idx + 1); xfs_bmbt_set_startblock(ep, nullstartblock((int)temp)); - trace_xfs_bmap_post_update(ip, idx + 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx + 1, state, _THIS_IP_); + *dnew = temp; break; @@ -938,15 +925,14 @@ xfs_bmap_add_extent_delay_real( * The right neighbor is contiguous with the new allocation. */ temp = PREV.br_blockcount - new->br_blockcount; - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); - trace_xfs_bmap_pre_update(ip, idx + 1, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx + 1, state, _THIS_IP_); xfs_bmbt_set_blockcount(ep, temp); - xfs_bmbt_set_allf(xfs_iext_get_ext(ifp, idx + 1), + xfs_bmbt_set_allf(xfs_iext_get_ext(ifp, *idx + 1), new->br_startoff, new->br_startblock, new->br_blockcount + RIGHT.br_blockcount, RIGHT.br_state); - trace_xfs_bmap_post_update(ip, idx + 1, state, _THIS_IP_); - ip->i_df.if_lastex = idx + 1; + trace_xfs_bmap_post_update(ip, *idx + 1, state, _THIS_IP_); if (cur == NULL) rval = XFS_ILOG_DEXT; else { @@ -966,7 +952,9 @@ xfs_bmap_add_extent_delay_real( temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp), startblockval(PREV.br_startblock)); xfs_bmbt_set_startblock(ep, nullstartblock((int)temp)); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); + + ++*idx; *dnew = temp; break; @@ -976,10 +964,9 @@ xfs_bmap_add_extent_delay_real( * The right neighbor is not contiguous. */ temp = PREV.br_blockcount - new->br_blockcount; - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_blockcount(ep, temp); - xfs_iext_insert(ip, idx + 1, 1, new, state); - ip->i_df.if_lastex = idx + 1; + xfs_iext_insert(ip, *idx + 1, 1, new, state); ip->i_d.di_nextents++; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -1007,9 +994,11 @@ xfs_bmap_add_extent_delay_real( temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp), startblockval(PREV.br_startblock) - (cur ? cur->bc_private.b.allocated : 0)); - ep = xfs_iext_get_ext(ifp, idx); + ep = xfs_iext_get_ext(ifp, *idx); xfs_bmbt_set_startblock(ep, nullstartblock((int)temp)); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); + + ++*idx; *dnew = temp; break; @@ -1036,7 +1025,7 @@ xfs_bmap_add_extent_delay_real( */ temp = new->br_startoff - PREV.br_startoff; temp2 = PREV.br_startoff + PREV.br_blockcount - new_endoff; - trace_xfs_bmap_pre_update(ip, idx, 0, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, 0, _THIS_IP_); xfs_bmbt_set_blockcount(ep, temp); /* truncate PREV */ LEFT = *new; RIGHT.br_state = PREV.br_state; @@ -1045,8 +1034,7 @@ xfs_bmap_add_extent_delay_real( RIGHT.br_startoff = new_endoff; RIGHT.br_blockcount = temp2; /* insert LEFT (r[0]) and RIGHT (r[1]) at the same time */ - xfs_iext_insert(ip, idx + 1, 2, &LEFT, state); - ip->i_df.if_lastex = idx + 1; + xfs_iext_insert(ip, *idx + 1, 2, &LEFT, state); ip->i_d.di_nextents++; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -1103,13 +1091,15 @@ xfs_bmap_add_extent_delay_real( } } } - ep = xfs_iext_get_ext(ifp, idx); + ep = xfs_iext_get_ext(ifp, *idx); xfs_bmbt_set_startblock(ep, nullstartblock((int)temp)); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); - trace_xfs_bmap_pre_update(ip, idx + 2, state, _THIS_IP_); - xfs_bmbt_set_startblock(xfs_iext_get_ext(ifp, idx + 2), + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx + 2, state, _THIS_IP_); + xfs_bmbt_set_startblock(xfs_iext_get_ext(ifp, *idx + 2), nullstartblock((int)temp2)); - trace_xfs_bmap_post_update(ip, idx + 2, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx + 2, state, _THIS_IP_); + + ++*idx; *dnew = temp + temp2; break; @@ -1141,7 +1131,7 @@ done: STATIC int /* error */ xfs_bmap_add_extent_unwritten_real( xfs_inode_t *ip, /* incore inode pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ + xfs_extnum_t *idx, /* extent number to update/insert */ xfs_btree_cur_t **curp, /* if *curp is null, not a btree */ xfs_bmbt_irec_t *new, /* new data to add to file extents */ int *logflagsp) /* inode logging flags */ @@ -1168,7 +1158,7 @@ xfs_bmap_add_extent_unwritten_real( error = 0; cur = *curp; ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK); - ep = xfs_iext_get_ext(ifp, idx); + ep = xfs_iext_get_ext(ifp, *idx); xfs_bmbt_get_all(ep, &PREV); newext = new->br_state; oldext = (newext == XFS_EXT_UNWRITTEN) ? @@ -1191,9 +1181,9 @@ xfs_bmap_add_extent_unwritten_real( * Check and set flags if this segment has a left neighbor. * Don't set contiguous if the combined extent would be too large. */ - if (idx > 0) { + if (*idx > 0) { state |= BMAP_LEFT_VALID; - xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx - 1), &LEFT); + xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx - 1), &LEFT); if (isnullstartblock(LEFT.br_startblock)) state |= BMAP_LEFT_DELAY; @@ -1211,9 +1201,9 @@ xfs_bmap_add_extent_unwritten_real( * Don't set contiguous if the combined extent would be too large. * Also check for all-three-contiguous being too large. */ - if (idx < ip->i_df.if_bytes / (uint)sizeof(xfs_bmbt_rec_t) - 1) { + if (*idx < ip->i_df.if_bytes / (uint)sizeof(xfs_bmbt_rec_t) - 1) { state |= BMAP_RIGHT_VALID; - xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx + 1), &RIGHT); + xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx + 1), &RIGHT); if (isnullstartblock(RIGHT.br_startblock)) state |= BMAP_RIGHT_DELAY; } @@ -1242,14 +1232,15 @@ xfs_bmap_add_extent_unwritten_real( * Setting all of a previous oldext extent to newext. * The left and right neighbors are both contiguous with new. */ - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), + --*idx; + + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx), LEFT.br_blockcount + PREV.br_blockcount + RIGHT.br_blockcount); - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - xfs_iext_remove(ip, idx, 2, state); - ip->i_df.if_lastex = idx - 1; + xfs_iext_remove(ip, *idx + 1, 2, state); ip->i_d.di_nextents -= 2; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -1285,13 +1276,14 @@ xfs_bmap_add_extent_unwritten_real( * Setting all of a previous oldext extent to newext. * The left neighbor is contiguous, the right is not. */ - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), + --*idx; + + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx), LEFT.br_blockcount + PREV.br_blockcount); - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - ip->i_df.if_lastex = idx - 1; - xfs_iext_remove(ip, idx, 1, state); + xfs_iext_remove(ip, *idx + 1, 1, state); ip->i_d.di_nextents--; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -1321,13 +1313,12 @@ xfs_bmap_add_extent_unwritten_real( * Setting all of a previous oldext extent to newext. * The right neighbor is contiguous, the left is not. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_blockcount(ep, PREV.br_blockcount + RIGHT.br_blockcount); xfs_bmbt_set_state(ep, newext); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); - ip->i_df.if_lastex = idx; - xfs_iext_remove(ip, idx + 1, 1, state); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); + xfs_iext_remove(ip, *idx + 1, 1, state); ip->i_d.di_nextents--; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -1358,11 +1349,10 @@ xfs_bmap_add_extent_unwritten_real( * Neither the left nor right neighbors are contiguous with * the new one. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_state(ep, newext); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - ip->i_df.if_lastex = idx; if (cur == NULL) rval = XFS_ILOG_DEXT; else { @@ -1384,21 +1374,22 @@ xfs_bmap_add_extent_unwritten_real( * Setting the first part of a previous oldext extent to newext. * The left neighbor is contiguous. */ - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), + trace_xfs_bmap_pre_update(ip, *idx - 1, state, _THIS_IP_); + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx - 1), LEFT.br_blockcount + new->br_blockcount); xfs_bmbt_set_startoff(ep, PREV.br_startoff + new->br_blockcount); - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx - 1, state, _THIS_IP_); - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_startblock(ep, new->br_startblock + new->br_blockcount); xfs_bmbt_set_blockcount(ep, PREV.br_blockcount - new->br_blockcount); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); + + --*idx; - ip->i_df.if_lastex = idx - 1; if (cur == NULL) rval = XFS_ILOG_DEXT; else { @@ -1429,17 +1420,16 @@ xfs_bmap_add_extent_unwritten_real( * Setting the first part of a previous oldext extent to newext. * The left neighbor is not contiguous. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); ASSERT(ep && xfs_bmbt_get_state(ep) == oldext); xfs_bmbt_set_startoff(ep, new_endoff); xfs_bmbt_set_blockcount(ep, PREV.br_blockcount - new->br_blockcount); xfs_bmbt_set_startblock(ep, new->br_startblock + new->br_blockcount); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - xfs_iext_insert(ip, idx, 1, new, state); - ip->i_df.if_lastex = idx; + xfs_iext_insert(ip, *idx, 1, new, state); ip->i_d.di_nextents++; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -1468,17 +1458,19 @@ xfs_bmap_add_extent_unwritten_real( * Setting the last part of a previous oldext extent to newext. * The right neighbor is contiguous with the new allocation. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); - trace_xfs_bmap_pre_update(ip, idx + 1, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_blockcount(ep, PREV.br_blockcount - new->br_blockcount); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); - xfs_bmbt_set_allf(xfs_iext_get_ext(ifp, idx + 1), + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); + + ++*idx; + + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); + xfs_bmbt_set_allf(xfs_iext_get_ext(ifp, *idx), new->br_startoff, new->br_startblock, new->br_blockcount + RIGHT.br_blockcount, newext); - trace_xfs_bmap_post_update(ip, idx + 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - ip->i_df.if_lastex = idx + 1; if (cur == NULL) rval = XFS_ILOG_DEXT; else { @@ -1508,13 +1500,14 @@ xfs_bmap_add_extent_unwritten_real( * Setting the last part of a previous oldext extent to newext. * The right neighbor is not contiguous. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_blockcount(ep, PREV.br_blockcount - new->br_blockcount); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); + + ++*idx; + xfs_iext_insert(ip, *idx, 1, new, state); - xfs_iext_insert(ip, idx + 1, 1, new, state); - ip->i_df.if_lastex = idx + 1; ip->i_d.di_nextents++; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -1548,10 +1541,10 @@ xfs_bmap_add_extent_unwritten_real( * newext. Contiguity is impossible here. * One extent becomes three extents. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_blockcount(ep, new->br_startoff - PREV.br_startoff); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); r[0] = *new; r[1].br_startoff = new_endoff; @@ -1559,8 +1552,10 @@ xfs_bmap_add_extent_unwritten_real( PREV.br_startoff + PREV.br_blockcount - new_endoff; r[1].br_startblock = new->br_startblock + new->br_blockcount; r[1].br_state = oldext; - xfs_iext_insert(ip, idx + 1, 2, &r[0], state); - ip->i_df.if_lastex = idx + 1; + + ++*idx; + xfs_iext_insert(ip, *idx, 2, &r[0], state); + ip->i_d.di_nextents += 2; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -1630,7 +1625,7 @@ done: STATIC int /* error */ xfs_bmap_add_extent_hole_delay( xfs_inode_t *ip, /* incore inode pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ + xfs_extnum_t *idx, /* extent number to update/insert */ xfs_bmbt_irec_t *new, /* new data to add to file extents */ int *logflagsp) /* inode logging flags */ { @@ -1644,16 +1639,16 @@ xfs_bmap_add_extent_hole_delay( xfs_filblks_t temp=0; /* temp for indirect calculations */ ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK); - ep = xfs_iext_get_ext(ifp, idx); + ep = xfs_iext_get_ext(ifp, *idx); state = 0; ASSERT(isnullstartblock(new->br_startblock)); /* * Check and set flags if this segment has a left neighbor */ - if (idx > 0) { + if (*idx > 0) { state |= BMAP_LEFT_VALID; - xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx - 1), &left); + xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx - 1), &left); if (isnullstartblock(left.br_startblock)) state |= BMAP_LEFT_DELAY; @@ -1663,7 +1658,7 @@ xfs_bmap_add_extent_hole_delay( * Check and set flags if the current (right) segment exists. * If it doesn't exist, we're converting the hole at end-of-file. */ - if (idx < ip->i_df.if_bytes / (uint)sizeof(xfs_bmbt_rec_t)) { + if (*idx < ip->i_df.if_bytes / (uint)sizeof(xfs_bmbt_rec_t)) { state |= BMAP_RIGHT_VALID; xfs_bmbt_get_all(ep, &right); @@ -1698,21 +1693,21 @@ xfs_bmap_add_extent_hole_delay( * on the left and on the right. * Merge all three into a single extent record. */ + --*idx; temp = left.br_blockcount + new->br_blockcount + right.br_blockcount; - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), temp); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx), temp); oldlen = startblockval(left.br_startblock) + startblockval(new->br_startblock) + startblockval(right.br_startblock); newlen = xfs_bmap_worst_indlen(ip, temp); - xfs_bmbt_set_startblock(xfs_iext_get_ext(ifp, idx - 1), + xfs_bmbt_set_startblock(xfs_iext_get_ext(ifp, *idx), nullstartblock((int)newlen)); - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - xfs_iext_remove(ip, idx, 1, state); - ip->i_df.if_lastex = idx - 1; + xfs_iext_remove(ip, *idx + 1, 1, state); break; case BMAP_LEFT_CONTIG: @@ -1721,17 +1716,17 @@ xfs_bmap_add_extent_hole_delay( * on the left. * Merge the new allocation with the left neighbor. */ + --*idx; temp = left.br_blockcount + new->br_blockcount; - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), temp); + + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx), temp); oldlen = startblockval(left.br_startblock) + startblockval(new->br_startblock); newlen = xfs_bmap_worst_indlen(ip, temp); - xfs_bmbt_set_startblock(xfs_iext_get_ext(ifp, idx - 1), + xfs_bmbt_set_startblock(xfs_iext_get_ext(ifp, *idx), nullstartblock((int)newlen)); - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); - - ip->i_df.if_lastex = idx - 1; + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); break; case BMAP_RIGHT_CONTIG: @@ -1740,16 +1735,14 @@ xfs_bmap_add_extent_hole_delay( * on the right. * Merge the new allocation with the right neighbor. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); temp = new->br_blockcount + right.br_blockcount; oldlen = startblockval(new->br_startblock) + startblockval(right.br_startblock); newlen = xfs_bmap_worst_indlen(ip, temp); xfs_bmbt_set_allf(ep, new->br_startoff, nullstartblock((int)newlen), temp, right.br_state); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); - - ip->i_df.if_lastex = idx; + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); break; case 0: @@ -1759,8 +1752,7 @@ xfs_bmap_add_extent_hole_delay( * Insert a new entry. */ oldlen = newlen = 0; - xfs_iext_insert(ip, idx, 1, new, state); - ip->i_df.if_lastex = idx; + xfs_iext_insert(ip, *idx, 1, new, state); break; } if (oldlen != newlen) { @@ -1782,7 +1774,7 @@ xfs_bmap_add_extent_hole_delay( STATIC int /* error */ xfs_bmap_add_extent_hole_real( xfs_inode_t *ip, /* incore inode pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ + xfs_extnum_t *idx, /* extent number to update/insert */ xfs_btree_cur_t *cur, /* if null, not a btree */ xfs_bmbt_irec_t *new, /* new data to add to file extents */ int *logflagsp, /* inode logging flags */ @@ -1798,8 +1790,8 @@ xfs_bmap_add_extent_hole_real( int state; /* state bits, accessed thru macros */ ifp = XFS_IFORK_PTR(ip, whichfork); - ASSERT(idx <= ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t)); - ep = xfs_iext_get_ext(ifp, idx); + ASSERT(*idx <= ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t)); + ep = xfs_iext_get_ext(ifp, *idx); state = 0; if (whichfork == XFS_ATTR_FORK) @@ -1808,9 +1800,9 @@ xfs_bmap_add_extent_hole_real( /* * Check and set flags if this segment has a left neighbor. */ - if (idx > 0) { + if (*idx > 0) { state |= BMAP_LEFT_VALID; - xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx - 1), &left); + xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx - 1), &left); if (isnullstartblock(left.br_startblock)) state |= BMAP_LEFT_DELAY; } @@ -1819,7 +1811,7 @@ xfs_bmap_add_extent_hole_real( * Check and set flags if this segment has a current value. * Not true if we're inserting into the "hole" at eof. */ - if (idx < ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t)) { + if (*idx < ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t)) { state |= BMAP_RIGHT_VALID; xfs_bmbt_get_all(ep, &right); if (isnullstartblock(right.br_startblock)) @@ -1858,14 +1850,15 @@ xfs_bmap_add_extent_hole_real( * left and on the right. * Merge all three into a single extent record. */ - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), + --*idx; + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx), left.br_blockcount + new->br_blockcount + right.br_blockcount); - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); + + xfs_iext_remove(ip, *idx + 1, 1, state); - xfs_iext_remove(ip, idx, 1, state); - ifp->if_lastex = idx - 1; XFS_IFORK_NEXT_SET(ip, whichfork, XFS_IFORK_NEXTENTS(ip, whichfork) - 1); if (cur == NULL) { @@ -1900,12 +1893,12 @@ xfs_bmap_add_extent_hole_real( * on the left. * Merge the new allocation with the left neighbor. */ - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), + --*idx; + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx), left.br_blockcount + new->br_blockcount); - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - ifp->if_lastex = idx - 1; if (cur == NULL) { rval = xfs_ilog_fext(whichfork); } else { @@ -1931,13 +1924,12 @@ xfs_bmap_add_extent_hole_real( * on the right. * Merge the new allocation with the right neighbor. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_allf(ep, new->br_startoff, new->br_startblock, new->br_blockcount + right.br_blockcount, right.br_state); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - ifp->if_lastex = idx; if (cur == NULL) { rval = xfs_ilog_fext(whichfork); } else { @@ -1963,8 +1955,7 @@ xfs_bmap_add_extent_hole_real( * real allocation. * Insert a new entry. */ - xfs_iext_insert(ip, idx, 1, new, state); - ifp->if_lastex = idx; + xfs_iext_insert(ip, *idx, 1, new, state); XFS_IFORK_NEXT_SET(ip, whichfork, XFS_IFORK_NEXTENTS(ip, whichfork) + 1); if (cur == NULL) { @@ -2812,7 +2803,7 @@ STATIC int /* error */ xfs_bmap_del_extent( xfs_inode_t *ip, /* incore inode pointer */ xfs_trans_t *tp, /* current transaction pointer */ - xfs_extnum_t idx, /* extent number to update/delete */ + xfs_extnum_t *idx, /* extent number to update/delete */ xfs_bmap_free_t *flist, /* list of extents to be freed */ xfs_btree_cur_t *cur, /* if null, not a btree */ xfs_bmbt_irec_t *del, /* data to remove from extents */ @@ -2848,10 +2839,10 @@ xfs_bmap_del_extent( mp = ip->i_mount; ifp = XFS_IFORK_PTR(ip, whichfork); - ASSERT((idx >= 0) && (idx < ifp->if_bytes / + ASSERT((*idx >= 0) && (*idx < ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t))); ASSERT(del->br_blockcount > 0); - ep = xfs_iext_get_ext(ifp, idx); + ep = xfs_iext_get_ext(ifp, *idx); xfs_bmbt_get_all(ep, &got); ASSERT(got.br_startoff <= del->br_startoff); del_endoff = del->br_startoff + del->br_blockcount; @@ -2925,9 +2916,8 @@ xfs_bmap_del_extent( /* * Matches the whole extent. Delete the entry. */ - xfs_iext_remove(ip, idx, 1, + xfs_iext_remove(ip, *idx, 1, whichfork == XFS_ATTR_FORK ? BMAP_ATTRFORK : 0); - ifp->if_lastex = idx; if (delay) break; XFS_IFORK_NEXT_SET(ip, whichfork, @@ -2946,21 +2936,20 @@ xfs_bmap_del_extent( /* * Deleting the first part of the extent. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_startoff(ep, del_endoff); temp = got.br_blockcount - del->br_blockcount; xfs_bmbt_set_blockcount(ep, temp); - ifp->if_lastex = idx; if (delay) { temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp), da_old); xfs_bmbt_set_startblock(ep, nullstartblock((int)temp)); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); da_new = temp; break; } xfs_bmbt_set_startblock(ep, del_endblock); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); if (!cur) { flags |= xfs_ilog_fext(whichfork); break; @@ -2976,18 +2965,17 @@ xfs_bmap_del_extent( * Deleting the last part of the extent. */ temp = got.br_blockcount - del->br_blockcount; - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_blockcount(ep, temp); - ifp->if_lastex = idx; if (delay) { temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp), da_old); xfs_bmbt_set_startblock(ep, nullstartblock((int)temp)); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); da_new = temp; break; } - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); if (!cur) { flags |= xfs_ilog_fext(whichfork); break; @@ -3004,7 +2992,7 @@ xfs_bmap_del_extent( * Deleting the middle of the extent. */ temp = del->br_startoff - got.br_startoff; - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_blockcount(ep, temp); new.br_startoff = del_endoff; temp2 = got_endoff - del_endoff; @@ -3091,9 +3079,9 @@ xfs_bmap_del_extent( } } } - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); - xfs_iext_insert(ip, idx + 1, 1, &new, state); - ifp->if_lastex = idx + 1; + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); + xfs_iext_insert(ip, *idx + 1, 1, &new, state); + ++*idx; break; } /* @@ -4674,13 +4662,12 @@ xfs_bmapi( if (!wasdelay && (flags & XFS_BMAPI_PREALLOC)) got.br_state = XFS_EXT_UNWRITTEN; } - error = xfs_bmap_add_extent(ip, lastx, &cur, &got, + error = xfs_bmap_add_extent(ip, &lastx, &cur, &got, firstblock, flist, &tmp_logflags, whichfork); logflags |= tmp_logflags; if (error) goto error0; - lastx = ifp->if_lastex; ep = xfs_iext_get_ext(ifp, lastx); nextents = ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t); xfs_bmbt_get_all(ep, &got); @@ -4776,13 +4763,12 @@ xfs_bmapi( mval->br_state = (mval->br_state == XFS_EXT_UNWRITTEN) ? XFS_EXT_NORM : XFS_EXT_UNWRITTEN; - error = xfs_bmap_add_extent(ip, lastx, &cur, mval, + error = xfs_bmap_add_extent(ip, &lastx, &cur, mval, firstblock, flist, &tmp_logflags, whichfork); logflags |= tmp_logflags; if (error) goto error0; - lastx = ifp->if_lastex; ep = xfs_iext_get_ext(ifp, lastx); nextents = ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t); xfs_bmbt_get_all(ep, &got); @@ -4848,7 +4834,6 @@ xfs_bmapi( else xfs_bmbt_get_all(ep, &got); } - ifp->if_lastex = lastx; *nmap = n; /* * Transform from btree to extents, give it cur. @@ -4957,7 +4942,6 @@ xfs_bmapi_single( ASSERT(!isnullstartblock(got.br_startblock)); ASSERT(bno < got.br_startoff + got.br_blockcount); *fsb = got.br_startblock + (bno - got.br_startoff); - ifp->if_lastex = lastx; return 0; } @@ -5132,7 +5116,7 @@ xfs_bunmapi( del.br_blockcount = mod; } del.br_state = XFS_EXT_UNWRITTEN; - error = xfs_bmap_add_extent(ip, lastx, &cur, &del, + error = xfs_bmap_add_extent(ip, &lastx, &cur, &del, firstblock, flist, &logflags, XFS_DATA_FORK); if (error) @@ -5186,7 +5170,8 @@ xfs_bunmapi( prev.br_startoff = start; } prev.br_state = XFS_EXT_UNWRITTEN; - error = xfs_bmap_add_extent(ip, lastx - 1, &cur, + lastx--; + error = xfs_bmap_add_extent(ip, &lastx, &cur, &prev, firstblock, flist, &logflags, XFS_DATA_FORK); if (error) @@ -5195,7 +5180,7 @@ xfs_bunmapi( } else { ASSERT(del.br_state == XFS_EXT_NORM); del.br_state = XFS_EXT_UNWRITTEN; - error = xfs_bmap_add_extent(ip, lastx, &cur, + error = xfs_bmap_add_extent(ip, &lastx, &cur, &del, firstblock, flist, &logflags, XFS_DATA_FORK); if (error) @@ -5249,14 +5234,13 @@ xfs_bunmapi( error = XFS_ERROR(ENOSPC); goto error0; } - error = xfs_bmap_del_extent(ip, tp, lastx, flist, cur, &del, + error = xfs_bmap_del_extent(ip, tp, &lastx, flist, cur, &del, &tmp_logflags, whichfork); logflags |= tmp_logflags; if (error) goto error0; bno = del.br_startoff - 1; nodelete: - lastx = ifp->if_lastex; /* * If not done go on to the next (previous) record. * Reset ep in case the extents array was re-alloced. @@ -5273,7 +5257,6 @@ nodelete: extno++; } } - ifp->if_lastex = lastx; *done = bno == (xfs_fileoff_t)-1 || bno < start || lastx < 0; ASSERT(ifp->if_ext_max == XFS_IFORK_SIZE(ip, whichfork) / (uint)sizeof(xfs_bmbt_rec_t)); Index: xfs/fs/xfs/xfs_inode.c =================================================================== --- xfs.orig/fs/xfs/xfs_inode.c 2011-05-10 13:29:36.446348941 +0200 +++ xfs/fs/xfs/xfs_inode.c 2011-05-10 13:29:40.622349777 +0200 @@ -920,7 +920,6 @@ xfs_iread_extents( /* * We know that the size is valid (it's checked in iformat_btree) */ - ifp->if_lastex = NULLEXTNUM; ifp->if_bytes = ifp->if_real_bytes = 0; ifp->if_flags |= XFS_IFEXTENTS; xfs_iext_add(ifp, 0, nextents); @@ -3191,7 +3190,6 @@ xfs_iext_add( } ifp->if_u1.if_extents = ifp->if_u2.if_inline_ext; ifp->if_real_bytes = 0; - ifp->if_lastex = nextents + ext_diff; } /* * Otherwise use a linear (direct) extent list. Index: xfs/fs/xfs/xfs_inode.h =================================================================== --- xfs.orig/fs/xfs/xfs_inode.h 2011-05-10 13:29:36.462348766 +0200 +++ xfs/fs/xfs/xfs_inode.h 2011-05-10 13:29:40.622349777 +0200 @@ -67,7 +67,6 @@ typedef struct xfs_ifork { short if_broot_bytes; /* bytes allocated for root */ unsigned char if_flags; /* per-fork flags */ unsigned char if_ext_max; /* max # of extent records */ - xfs_extnum_t if_lastex; /* last if_extents used */ union { xfs_bmbt_rec_host_t *if_extents;/* linear map file exts */ xfs_ext_irec_t *if_ext_irec; /* irec map file exts */ _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/9] xfs: remove if_lastex 2011-05-11 15:04 ` [PATCH 2/9] xfs: remove if_lastex Christoph Hellwig @ 2011-05-20 20:17 ` Alex Elder 2011-05-23 8:52 ` [PATCH 2/9 v2] " Christoph Hellwig 1 sibling, 0 replies; 30+ messages in thread From: Alex Elder @ 2011-05-20 20:17 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Wed, 2011-05-11 at 11:04 -0400, Christoph Hellwig wrote: > plain text document attachment (xfs-kill-if_lastex) > The if_lastex field in struct xfs_ifork is only used as a temporary index > during xfs_bmapi and xfs_bmapi. Instead of using the inode fork to store xfs_bunmapi (I'll fix this for you when I commit the change.) > it keep it local in the callchain. Fortunately this is very easy as > we already pass a stack copy of it down the whole chain which can simplify > be changed to be passed by reference. > > Signed-off-by: Christoph Hellwig <hch@lst.de> This looks good to me. I have a few comments below but nothing looks incorrect. Reviewed-by: Alex Elder <aelder@sgi.com> I will continue with patches 3-9 a little later on. > Index: xfs/fs/xfs/xfs_bmap.c > =================================================================== > --- xfs.orig/fs/xfs/xfs_bmap.c 2011-05-10 13:20:21.182349200 +0200 > +++ xfs/fs/xfs/xfs_bmap.c 2011-05-10 13:29:40.618349159 +0200 . . . > @@ -725,14 +709,13 @@ xfs_bmap_add_extent_delay_real( > * Filling in all of a previously delayed allocation extent. > * The left and right neighbors are both contiguous with new. > */ Couldn't you decrement *idx here, and use its value (without subtracting 1) in almost all spots below... > - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); > - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), > + trace_xfs_bmap_pre_update(ip, *idx - 1, state, _THIS_IP_); > + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx - 1), > LEFT.br_blockcount + PREV.br_blockcount + > RIGHT.br_blockcount); > - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); > + trace_xfs_bmap_post_update(ip, *idx - 1, state, _THIS_IP_); > > - xfs_iext_remove(ip, idx, 2, state); > - ip->i_df.if_lastex = idx - 1; > + xfs_iext_remove(ip, *idx, 2, state); ...(but pass *idx + 1 here)... > ip->i_d.di_nextents--; > if (cur == NULL) > rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; > @@ -756,6 +739,8 @@ xfs_bmap_add_extent_delay_real( > RIGHT.br_blockcount, LEFT.br_state))) > goto done; > } > + > + --*idx; ...rather than waiting until here to do the decrement? > *dnew = 0; > break; > > @@ -764,13 +749,12 @@ xfs_bmap_add_extent_delay_real( > * Filling in all of a previously delayed allocation extent. > * The left neighbor is contiguous, the right is not. > */ Same general comment in this section (and in some others that follow). This is not a big deal at all, but I did notice that you did the decrement (or increment) earlier in some spots below, but not in these. I see nothing incorrect, just saw what seemed like inconsistency and wondered if there was a reasaon. > - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); > - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), > + trace_xfs_bmap_pre_update(ip, *idx - 1, state, _THIS_IP_); > + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx - 1), > LEFT.br_blockcount + PREV.br_blockcount); > - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); > + trace_xfs_bmap_post_update(ip, *idx - 1, state, _THIS_IP_); > > - ip->i_df.if_lastex = idx - 1; > - xfs_iext_remove(ip, idx, 1, state); > + xfs_iext_remove(ip, *idx, 1, state); > if (cur == NULL) > rval = XFS_ILOG_DEXT; > else { . . . > @@ -1468,17 +1458,19 @@ xfs_bmap_add_extent_unwritten_real( > * Setting the last part of a previous oldext extent to newext. > * The right neighbor is contiguous with the new allocation. > */ > - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); > - trace_xfs_bmap_pre_update(ip, idx + 1, state, _THIS_IP_); This one was a bit weird (second trace call being out of place). > + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); > xfs_bmbt_set_blockcount(ep, > PREV.br_blockcount - new->br_blockcount); > - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); > - xfs_bmbt_set_allf(xfs_iext_get_ext(ifp, idx + 1), > + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); > + > + ++*idx; > + > + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); > + xfs_bmbt_set_allf(xfs_iext_get_ext(ifp, *idx), > new->br_startoff, new->br_startblock, > new->br_blockcount + RIGHT.br_blockcount, newext); > - trace_xfs_bmap_post_update(ip, idx + 1, state, _THIS_IP_); > + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); > > - ip->i_df.if_lastex = idx + 1; > if (cur == NULL) > rval = XFS_ILOG_DEXT; > else { . . . _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2/9 v2] xfs: remove if_lastex 2011-05-11 15:04 ` [PATCH 2/9] xfs: remove if_lastex Christoph Hellwig 2011-05-20 20:17 ` Alex Elder @ 2011-05-23 8:52 ` Christoph Hellwig 2011-05-25 1:14 ` Alex Elder 1 sibling, 1 reply; 30+ messages in thread From: Christoph Hellwig @ 2011-05-23 8:52 UTC (permalink / raw) To: xfs The if_lastex field in struct xfs_ifork is only used as a temporary index during xfs_bmapi and xfs_bunmapi. Instead of using the inode fork to store it keep it local in the callchain. Fortunately this is very easy as we already pass a stack copy of it down the whole chain which can simplify be changed to be passed by reference. Signed-off-by: Christoph Hellwig <hch@lst.de> Index: xfs/fs/xfs/xfs_bmap.c =================================================================== --- xfs.orig/fs/xfs/xfs_bmap.c 2011-05-22 12:45:44.944448378 +0200 +++ xfs/fs/xfs/xfs_bmap.c 2011-05-22 12:54:37.379971757 +0200 @@ -89,28 +89,13 @@ xfs_bmap_add_attrfork_local( int *flags); /* inode logging flags */ /* - * Called by xfs_bmapi to update file extent records and the btree - * after allocating space (or doing a delayed allocation). - */ -STATIC int /* error */ -xfs_bmap_add_extent( - xfs_inode_t *ip, /* incore inode pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ - xfs_btree_cur_t **curp, /* if *curp is null, not a btree */ - xfs_bmbt_irec_t *new, /* new data to add to file extents */ - xfs_fsblock_t *first, /* pointer to firstblock variable */ - xfs_bmap_free_t *flist, /* list of extents to be freed */ - int *logflagsp, /* inode logging flags */ - int whichfork); /* data or attr fork */ - -/* * Called by xfs_bmap_add_extent to handle cases converting a delayed * allocation to a real allocation. */ STATIC int /* error */ xfs_bmap_add_extent_delay_real( xfs_inode_t *ip, /* incore inode pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ + xfs_extnum_t *idx, /* extent number to update/insert */ xfs_btree_cur_t **curp, /* if *curp is null, not a btree */ xfs_bmbt_irec_t *new, /* new data to add to file extents */ xfs_filblks_t *dnew, /* new delayed-alloc indirect blocks */ @@ -125,7 +110,7 @@ xfs_bmap_add_extent_delay_real( STATIC int /* error */ xfs_bmap_add_extent_hole_delay( xfs_inode_t *ip, /* incore inode pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ + xfs_extnum_t *idx, /* extent number to update/insert */ xfs_bmbt_irec_t *new, /* new data to add to file extents */ int *logflagsp); /* inode logging flags */ @@ -136,7 +121,7 @@ xfs_bmap_add_extent_hole_delay( STATIC int /* error */ xfs_bmap_add_extent_hole_real( xfs_inode_t *ip, /* incore inode pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ + xfs_extnum_t *idx, /* extent number to update/insert */ xfs_btree_cur_t *cur, /* if null, not a btree */ xfs_bmbt_irec_t *new, /* new data to add to file extents */ int *logflagsp, /* inode logging flags */ @@ -149,7 +134,7 @@ xfs_bmap_add_extent_hole_real( STATIC int /* error */ xfs_bmap_add_extent_unwritten_real( xfs_inode_t *ip, /* incore inode pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ + xfs_extnum_t *idx, /* extent number to update/insert */ xfs_btree_cur_t **curp, /* if *curp is null, not a btree */ xfs_bmbt_irec_t *new, /* new data to add to file extents */ int *logflagsp); /* inode logging flags */ @@ -455,7 +440,7 @@ xfs_bmap_add_attrfork_local( STATIC int /* error */ xfs_bmap_add_extent( xfs_inode_t *ip, /* incore inode pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ + xfs_extnum_t *idx, /* extent number to update/insert */ xfs_btree_cur_t **curp, /* if *curp is null, not a btree */ xfs_bmbt_irec_t *new, /* new data to add to file extents */ xfs_fsblock_t *first, /* pointer to firstblock variable */ @@ -472,23 +457,27 @@ xfs_bmap_add_extent( xfs_extnum_t nextents; /* number of extents in file now */ XFS_STATS_INC(xs_add_exlist); + cur = *curp; ifp = XFS_IFORK_PTR(ip, whichfork); nextents = ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t); - ASSERT(idx <= nextents); da_old = da_new = 0; error = 0; + + ASSERT(*idx >= 0); + ASSERT(*idx <= nextents); + /* * This is the first extent added to a new/empty file. * Special case this one, so other routines get to assume there are * already extents in the list. */ if (nextents == 0) { - xfs_iext_insert(ip, 0, 1, new, + xfs_iext_insert(ip, *idx, 1, new, whichfork == XFS_ATTR_FORK ? BMAP_ATTRFORK : 0); ASSERT(cur == NULL); - ifp->if_lastex = 0; + if (!isnullstartblock(new->br_startblock)) { XFS_IFORK_NEXT_SET(ip, whichfork, 1); logflags = XFS_ILOG_CORE | xfs_ilog_fext(whichfork); @@ -502,27 +491,25 @@ xfs_bmap_add_extent( if (cur) ASSERT((cur->bc_private.b.flags & XFS_BTCUR_BPRV_WASDEL) == 0); - error = xfs_bmap_add_extent_hole_delay(ip, idx, new, &logflags); - if (error) - goto done; + error = xfs_bmap_add_extent_hole_delay(ip, idx, new, + &logflags); } /* * Real allocation off the end of the file. */ - else if (idx == nextents) { + else if (*idx == nextents) { if (cur) ASSERT((cur->bc_private.b.flags & XFS_BTCUR_BPRV_WASDEL) == 0); - if ((error = xfs_bmap_add_extent_hole_real(ip, idx, cur, new, - &logflags, whichfork))) - goto done; + error = xfs_bmap_add_extent_hole_real(ip, idx, cur, new, + &logflags, whichfork); } else { xfs_bmbt_irec_t prev; /* old extent at offset idx */ /* * Get the record referred to by idx. */ - xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx), &prev); + xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx), &prev); /* * If it's a real allocation record, and the new allocation ends * after the start of the referred to record, then we're filling @@ -537,23 +524,18 @@ xfs_bmap_add_extent( if (cur) ASSERT(cur->bc_private.b.flags & XFS_BTCUR_BPRV_WASDEL); - error = xfs_bmap_add_extent_delay_real(ip, idx, - &cur, new, &da_new, first, - flist, &logflags); - if (error) - goto done; - } else if (new->br_state == XFS_EXT_NORM) { - ASSERT(new->br_state == XFS_EXT_NORM); - if ((error = xfs_bmap_add_extent_unwritten_real( - ip, idx, &cur, new, &logflags))) - goto done; + error = xfs_bmap_add_extent_delay_real(ip, + idx, &cur, new, &da_new, + first, flist, &logflags); } else { - ASSERT(new->br_state == XFS_EXT_UNWRITTEN); - if ((error = xfs_bmap_add_extent_unwritten_real( - ip, idx, &cur, new, &logflags))) + ASSERT(new->br_state == XFS_EXT_NORM || + new->br_state == XFS_EXT_UNWRITTEN); + + error = xfs_bmap_add_extent_unwritten_real(ip, + idx, &cur, new, &logflags); + if (error) goto done; } - ASSERT(*curp == cur || *curp == NULL); } /* * Otherwise we're filling in a hole with an allocation. @@ -562,13 +544,15 @@ xfs_bmap_add_extent( if (cur) ASSERT((cur->bc_private.b.flags & XFS_BTCUR_BPRV_WASDEL) == 0); - if ((error = xfs_bmap_add_extent_hole_real(ip, idx, cur, - new, &logflags, whichfork))) - goto done; + error = xfs_bmap_add_extent_hole_real(ip, idx, cur, + new, &logflags, whichfork); } } + if (error) + goto done; ASSERT(*curp == cur || *curp == NULL); + /* * Convert to a btree if necessary. */ @@ -621,7 +605,7 @@ done: STATIC int /* error */ xfs_bmap_add_extent_delay_real( xfs_inode_t *ip, /* incore inode pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ + xfs_extnum_t *idx, /* extent number to update/insert */ xfs_btree_cur_t **curp, /* if *curp is null, not a btree */ xfs_bmbt_irec_t *new, /* new data to add to file extents */ xfs_filblks_t *dnew, /* new delayed-alloc indirect blocks */ @@ -653,7 +637,7 @@ xfs_bmap_add_extent_delay_real( */ cur = *curp; ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK); - ep = xfs_iext_get_ext(ifp, idx); + ep = xfs_iext_get_ext(ifp, *idx); xfs_bmbt_get_all(ep, &PREV); new_endoff = new->br_startoff + new->br_blockcount; ASSERT(PREV.br_startoff <= new->br_startoff); @@ -672,9 +656,9 @@ xfs_bmap_add_extent_delay_real( * Check and set flags if this segment has a left neighbor. * Don't set contiguous if the combined extent would be too large. */ - if (idx > 0) { + if (*idx > 0) { state |= BMAP_LEFT_VALID; - xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx - 1), &LEFT); + xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx - 1), &LEFT); if (isnullstartblock(LEFT.br_startblock)) state |= BMAP_LEFT_DELAY; @@ -692,9 +676,9 @@ xfs_bmap_add_extent_delay_real( * Don't set contiguous if the combined extent would be too large. * Also check for all-three-contiguous being too large. */ - if (idx < ip->i_df.if_bytes / (uint)sizeof(xfs_bmbt_rec_t) - 1) { + if (*idx < ip->i_df.if_bytes / (uint)sizeof(xfs_bmbt_rec_t) - 1) { state |= BMAP_RIGHT_VALID; - xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx + 1), &RIGHT); + xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx + 1), &RIGHT); if (isnullstartblock(RIGHT.br_startblock)) state |= BMAP_RIGHT_DELAY; @@ -725,14 +709,14 @@ xfs_bmap_add_extent_delay_real( * Filling in all of a previously delayed allocation extent. * The left and right neighbors are both contiguous with new. */ - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), + --*idx; + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx), LEFT.br_blockcount + PREV.br_blockcount + RIGHT.br_blockcount); - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - xfs_iext_remove(ip, idx, 2, state); - ip->i_df.if_lastex = idx - 1; + xfs_iext_remove(ip, *idx + 1, 2, state); ip->i_d.di_nextents--; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -764,13 +748,14 @@ xfs_bmap_add_extent_delay_real( * Filling in all of a previously delayed allocation extent. * The left neighbor is contiguous, the right is not. */ - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), + --*idx; + + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx), LEFT.br_blockcount + PREV.br_blockcount); - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - ip->i_df.if_lastex = idx - 1; - xfs_iext_remove(ip, idx, 1, state); + xfs_iext_remove(ip, *idx + 1, 1, state); if (cur == NULL) rval = XFS_ILOG_DEXT; else { @@ -794,14 +779,13 @@ xfs_bmap_add_extent_delay_real( * Filling in all of a previously delayed allocation extent. * The right neighbor is contiguous, the left is not. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_startblock(ep, new->br_startblock); xfs_bmbt_set_blockcount(ep, PREV.br_blockcount + RIGHT.br_blockcount); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - ip->i_df.if_lastex = idx; - xfs_iext_remove(ip, idx + 1, 1, state); + xfs_iext_remove(ip, *idx + 1, 1, state); if (cur == NULL) rval = XFS_ILOG_DEXT; else { @@ -817,6 +801,7 @@ xfs_bmap_add_extent_delay_real( RIGHT.br_blockcount, PREV.br_state))) goto done; } + *dnew = 0; break; @@ -826,11 +811,10 @@ xfs_bmap_add_extent_delay_real( * Neither the left nor right neighbors are contiguous with * the new one. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_startblock(ep, new->br_startblock); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - ip->i_df.if_lastex = idx; ip->i_d.di_nextents++; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -846,6 +830,7 @@ xfs_bmap_add_extent_delay_real( goto done; XFS_WANT_CORRUPTED_GOTO(i == 1, done); } + *dnew = 0; break; @@ -854,17 +839,16 @@ xfs_bmap_add_extent_delay_real( * Filling in the first part of a previous delayed allocation. * The left neighbor is contiguous. */ - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), + trace_xfs_bmap_pre_update(ip, *idx - 1, state, _THIS_IP_); + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx - 1), LEFT.br_blockcount + new->br_blockcount); xfs_bmbt_set_startoff(ep, PREV.br_startoff + new->br_blockcount); - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx - 1, state, _THIS_IP_); temp = PREV.br_blockcount - new->br_blockcount; - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_blockcount(ep, temp); - ip->i_df.if_lastex = idx - 1; if (cur == NULL) rval = XFS_ILOG_DEXT; else { @@ -884,7 +868,9 @@ xfs_bmap_add_extent_delay_real( temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp), startblockval(PREV.br_startblock)); xfs_bmbt_set_startblock(ep, nullstartblock((int)temp)); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); + + --*idx; *dnew = temp; break; @@ -893,12 +879,11 @@ xfs_bmap_add_extent_delay_real( * Filling in the first part of a previous delayed allocation. * The left neighbor is not contiguous. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_startoff(ep, new_endoff); temp = PREV.br_blockcount - new->br_blockcount; xfs_bmbt_set_blockcount(ep, temp); - xfs_iext_insert(ip, idx, 1, new, state); - ip->i_df.if_lastex = idx; + xfs_iext_insert(ip, *idx, 1, new, state); ip->i_d.di_nextents++; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -926,9 +911,10 @@ xfs_bmap_add_extent_delay_real( temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp), startblockval(PREV.br_startblock) - (cur ? cur->bc_private.b.allocated : 0)); - ep = xfs_iext_get_ext(ifp, idx + 1); + ep = xfs_iext_get_ext(ifp, *idx + 1); xfs_bmbt_set_startblock(ep, nullstartblock((int)temp)); - trace_xfs_bmap_post_update(ip, idx + 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx + 1, state, _THIS_IP_); + *dnew = temp; break; @@ -938,15 +924,13 @@ xfs_bmap_add_extent_delay_real( * The right neighbor is contiguous with the new allocation. */ temp = PREV.br_blockcount - new->br_blockcount; - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); - trace_xfs_bmap_pre_update(ip, idx + 1, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx + 1, state, _THIS_IP_); xfs_bmbt_set_blockcount(ep, temp); - xfs_bmbt_set_allf(xfs_iext_get_ext(ifp, idx + 1), + xfs_bmbt_set_allf(xfs_iext_get_ext(ifp, *idx + 1), new->br_startoff, new->br_startblock, new->br_blockcount + RIGHT.br_blockcount, RIGHT.br_state); - trace_xfs_bmap_post_update(ip, idx + 1, state, _THIS_IP_); - ip->i_df.if_lastex = idx + 1; + trace_xfs_bmap_post_update(ip, *idx + 1, state, _THIS_IP_); if (cur == NULL) rval = XFS_ILOG_DEXT; else { @@ -963,10 +947,14 @@ xfs_bmap_add_extent_delay_real( RIGHT.br_state))) goto done; } + temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp), startblockval(PREV.br_startblock)); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_startblock(ep, nullstartblock((int)temp)); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); + + ++*idx; *dnew = temp; break; @@ -976,10 +964,9 @@ xfs_bmap_add_extent_delay_real( * The right neighbor is not contiguous. */ temp = PREV.br_blockcount - new->br_blockcount; - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_blockcount(ep, temp); - xfs_iext_insert(ip, idx + 1, 1, new, state); - ip->i_df.if_lastex = idx + 1; + xfs_iext_insert(ip, *idx + 1, 1, new, state); ip->i_d.di_nextents++; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -1007,9 +994,11 @@ xfs_bmap_add_extent_delay_real( temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp), startblockval(PREV.br_startblock) - (cur ? cur->bc_private.b.allocated : 0)); - ep = xfs_iext_get_ext(ifp, idx); + ep = xfs_iext_get_ext(ifp, *idx); xfs_bmbt_set_startblock(ep, nullstartblock((int)temp)); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); + + ++*idx; *dnew = temp; break; @@ -1036,7 +1025,7 @@ xfs_bmap_add_extent_delay_real( */ temp = new->br_startoff - PREV.br_startoff; temp2 = PREV.br_startoff + PREV.br_blockcount - new_endoff; - trace_xfs_bmap_pre_update(ip, idx, 0, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, 0, _THIS_IP_); xfs_bmbt_set_blockcount(ep, temp); /* truncate PREV */ LEFT = *new; RIGHT.br_state = PREV.br_state; @@ -1045,8 +1034,7 @@ xfs_bmap_add_extent_delay_real( RIGHT.br_startoff = new_endoff; RIGHT.br_blockcount = temp2; /* insert LEFT (r[0]) and RIGHT (r[1]) at the same time */ - xfs_iext_insert(ip, idx + 1, 2, &LEFT, state); - ip->i_df.if_lastex = idx + 1; + xfs_iext_insert(ip, *idx + 1, 2, &LEFT, state); ip->i_d.di_nextents++; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -1103,13 +1091,15 @@ xfs_bmap_add_extent_delay_real( } } } - ep = xfs_iext_get_ext(ifp, idx); + ep = xfs_iext_get_ext(ifp, *idx); xfs_bmbt_set_startblock(ep, nullstartblock((int)temp)); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); - trace_xfs_bmap_pre_update(ip, idx + 2, state, _THIS_IP_); - xfs_bmbt_set_startblock(xfs_iext_get_ext(ifp, idx + 2), + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx + 2, state, _THIS_IP_); + xfs_bmbt_set_startblock(xfs_iext_get_ext(ifp, *idx + 2), nullstartblock((int)temp2)); - trace_xfs_bmap_post_update(ip, idx + 2, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx + 2, state, _THIS_IP_); + + ++*idx; *dnew = temp + temp2; break; @@ -1141,7 +1131,7 @@ done: STATIC int /* error */ xfs_bmap_add_extent_unwritten_real( xfs_inode_t *ip, /* incore inode pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ + xfs_extnum_t *idx, /* extent number to update/insert */ xfs_btree_cur_t **curp, /* if *curp is null, not a btree */ xfs_bmbt_irec_t *new, /* new data to add to file extents */ int *logflagsp) /* inode logging flags */ @@ -1168,7 +1158,7 @@ xfs_bmap_add_extent_unwritten_real( error = 0; cur = *curp; ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK); - ep = xfs_iext_get_ext(ifp, idx); + ep = xfs_iext_get_ext(ifp, *idx); xfs_bmbt_get_all(ep, &PREV); newext = new->br_state; oldext = (newext == XFS_EXT_UNWRITTEN) ? @@ -1191,9 +1181,9 @@ xfs_bmap_add_extent_unwritten_real( * Check and set flags if this segment has a left neighbor. * Don't set contiguous if the combined extent would be too large. */ - if (idx > 0) { + if (*idx > 0) { state |= BMAP_LEFT_VALID; - xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx - 1), &LEFT); + xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx - 1), &LEFT); if (isnullstartblock(LEFT.br_startblock)) state |= BMAP_LEFT_DELAY; @@ -1211,9 +1201,9 @@ xfs_bmap_add_extent_unwritten_real( * Don't set contiguous if the combined extent would be too large. * Also check for all-three-contiguous being too large. */ - if (idx < ip->i_df.if_bytes / (uint)sizeof(xfs_bmbt_rec_t) - 1) { + if (*idx < ip->i_df.if_bytes / (uint)sizeof(xfs_bmbt_rec_t) - 1) { state |= BMAP_RIGHT_VALID; - xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx + 1), &RIGHT); + xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx + 1), &RIGHT); if (isnullstartblock(RIGHT.br_startblock)) state |= BMAP_RIGHT_DELAY; } @@ -1242,14 +1232,15 @@ xfs_bmap_add_extent_unwritten_real( * Setting all of a previous oldext extent to newext. * The left and right neighbors are both contiguous with new. */ - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), + --*idx; + + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx), LEFT.br_blockcount + PREV.br_blockcount + RIGHT.br_blockcount); - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - xfs_iext_remove(ip, idx, 2, state); - ip->i_df.if_lastex = idx - 1; + xfs_iext_remove(ip, *idx + 1, 2, state); ip->i_d.di_nextents -= 2; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -1285,13 +1276,14 @@ xfs_bmap_add_extent_unwritten_real( * Setting all of a previous oldext extent to newext. * The left neighbor is contiguous, the right is not. */ - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), + --*idx; + + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx), LEFT.br_blockcount + PREV.br_blockcount); - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - ip->i_df.if_lastex = idx - 1; - xfs_iext_remove(ip, idx, 1, state); + xfs_iext_remove(ip, *idx + 1, 1, state); ip->i_d.di_nextents--; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -1321,13 +1313,12 @@ xfs_bmap_add_extent_unwritten_real( * Setting all of a previous oldext extent to newext. * The right neighbor is contiguous, the left is not. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_blockcount(ep, PREV.br_blockcount + RIGHT.br_blockcount); xfs_bmbt_set_state(ep, newext); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); - ip->i_df.if_lastex = idx; - xfs_iext_remove(ip, idx + 1, 1, state); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); + xfs_iext_remove(ip, *idx + 1, 1, state); ip->i_d.di_nextents--; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -1358,11 +1349,10 @@ xfs_bmap_add_extent_unwritten_real( * Neither the left nor right neighbors are contiguous with * the new one. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_state(ep, newext); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - ip->i_df.if_lastex = idx; if (cur == NULL) rval = XFS_ILOG_DEXT; else { @@ -1384,21 +1374,22 @@ xfs_bmap_add_extent_unwritten_real( * Setting the first part of a previous oldext extent to newext. * The left neighbor is contiguous. */ - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), + trace_xfs_bmap_pre_update(ip, *idx - 1, state, _THIS_IP_); + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx - 1), LEFT.br_blockcount + new->br_blockcount); xfs_bmbt_set_startoff(ep, PREV.br_startoff + new->br_blockcount); - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx - 1, state, _THIS_IP_); - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_startblock(ep, new->br_startblock + new->br_blockcount); xfs_bmbt_set_blockcount(ep, PREV.br_blockcount - new->br_blockcount); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); + + --*idx; - ip->i_df.if_lastex = idx - 1; if (cur == NULL) rval = XFS_ILOG_DEXT; else { @@ -1429,17 +1420,16 @@ xfs_bmap_add_extent_unwritten_real( * Setting the first part of a previous oldext extent to newext. * The left neighbor is not contiguous. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); ASSERT(ep && xfs_bmbt_get_state(ep) == oldext); xfs_bmbt_set_startoff(ep, new_endoff); xfs_bmbt_set_blockcount(ep, PREV.br_blockcount - new->br_blockcount); xfs_bmbt_set_startblock(ep, new->br_startblock + new->br_blockcount); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - xfs_iext_insert(ip, idx, 1, new, state); - ip->i_df.if_lastex = idx; + xfs_iext_insert(ip, *idx, 1, new, state); ip->i_d.di_nextents++; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -1468,17 +1458,19 @@ xfs_bmap_add_extent_unwritten_real( * Setting the last part of a previous oldext extent to newext. * The right neighbor is contiguous with the new allocation. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); - trace_xfs_bmap_pre_update(ip, idx + 1, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_blockcount(ep, PREV.br_blockcount - new->br_blockcount); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); - xfs_bmbt_set_allf(xfs_iext_get_ext(ifp, idx + 1), + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); + + ++*idx; + + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); + xfs_bmbt_set_allf(xfs_iext_get_ext(ifp, *idx), new->br_startoff, new->br_startblock, new->br_blockcount + RIGHT.br_blockcount, newext); - trace_xfs_bmap_post_update(ip, idx + 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - ip->i_df.if_lastex = idx + 1; if (cur == NULL) rval = XFS_ILOG_DEXT; else { @@ -1508,13 +1500,14 @@ xfs_bmap_add_extent_unwritten_real( * Setting the last part of a previous oldext extent to newext. * The right neighbor is not contiguous. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_blockcount(ep, PREV.br_blockcount - new->br_blockcount); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); + + ++*idx; + xfs_iext_insert(ip, *idx, 1, new, state); - xfs_iext_insert(ip, idx + 1, 1, new, state); - ip->i_df.if_lastex = idx + 1; ip->i_d.di_nextents++; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -1548,10 +1541,10 @@ xfs_bmap_add_extent_unwritten_real( * newext. Contiguity is impossible here. * One extent becomes three extents. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_blockcount(ep, new->br_startoff - PREV.br_startoff); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); r[0] = *new; r[1].br_startoff = new_endoff; @@ -1559,8 +1552,10 @@ xfs_bmap_add_extent_unwritten_real( PREV.br_startoff + PREV.br_blockcount - new_endoff; r[1].br_startblock = new->br_startblock + new->br_blockcount; r[1].br_state = oldext; - xfs_iext_insert(ip, idx + 1, 2, &r[0], state); - ip->i_df.if_lastex = idx + 1; + + ++*idx; + xfs_iext_insert(ip, *idx, 2, &r[0], state); + ip->i_d.di_nextents += 2; if (cur == NULL) rval = XFS_ILOG_CORE | XFS_ILOG_DEXT; @@ -1630,7 +1625,7 @@ done: STATIC int /* error */ xfs_bmap_add_extent_hole_delay( xfs_inode_t *ip, /* incore inode pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ + xfs_extnum_t *idx, /* extent number to update/insert */ xfs_bmbt_irec_t *new, /* new data to add to file extents */ int *logflagsp) /* inode logging flags */ { @@ -1644,16 +1639,16 @@ xfs_bmap_add_extent_hole_delay( xfs_filblks_t temp=0; /* temp for indirect calculations */ ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK); - ep = xfs_iext_get_ext(ifp, idx); + ep = xfs_iext_get_ext(ifp, *idx); state = 0; ASSERT(isnullstartblock(new->br_startblock)); /* * Check and set flags if this segment has a left neighbor */ - if (idx > 0) { + if (*idx > 0) { state |= BMAP_LEFT_VALID; - xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx - 1), &left); + xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx - 1), &left); if (isnullstartblock(left.br_startblock)) state |= BMAP_LEFT_DELAY; @@ -1663,7 +1658,7 @@ xfs_bmap_add_extent_hole_delay( * Check and set flags if the current (right) segment exists. * If it doesn't exist, we're converting the hole at end-of-file. */ - if (idx < ip->i_df.if_bytes / (uint)sizeof(xfs_bmbt_rec_t)) { + if (*idx < ip->i_df.if_bytes / (uint)sizeof(xfs_bmbt_rec_t)) { state |= BMAP_RIGHT_VALID; xfs_bmbt_get_all(ep, &right); @@ -1698,21 +1693,21 @@ xfs_bmap_add_extent_hole_delay( * on the left and on the right. * Merge all three into a single extent record. */ + --*idx; temp = left.br_blockcount + new->br_blockcount + right.br_blockcount; - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), temp); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx), temp); oldlen = startblockval(left.br_startblock) + startblockval(new->br_startblock) + startblockval(right.br_startblock); newlen = xfs_bmap_worst_indlen(ip, temp); - xfs_bmbt_set_startblock(xfs_iext_get_ext(ifp, idx - 1), + xfs_bmbt_set_startblock(xfs_iext_get_ext(ifp, *idx), nullstartblock((int)newlen)); - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - xfs_iext_remove(ip, idx, 1, state); - ip->i_df.if_lastex = idx - 1; + xfs_iext_remove(ip, *idx + 1, 1, state); break; case BMAP_LEFT_CONTIG: @@ -1721,17 +1716,17 @@ xfs_bmap_add_extent_hole_delay( * on the left. * Merge the new allocation with the left neighbor. */ + --*idx; temp = left.br_blockcount + new->br_blockcount; - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), temp); + + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx), temp); oldlen = startblockval(left.br_startblock) + startblockval(new->br_startblock); newlen = xfs_bmap_worst_indlen(ip, temp); - xfs_bmbt_set_startblock(xfs_iext_get_ext(ifp, idx - 1), + xfs_bmbt_set_startblock(xfs_iext_get_ext(ifp, *idx), nullstartblock((int)newlen)); - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); - - ip->i_df.if_lastex = idx - 1; + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); break; case BMAP_RIGHT_CONTIG: @@ -1740,16 +1735,14 @@ xfs_bmap_add_extent_hole_delay( * on the right. * Merge the new allocation with the right neighbor. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); temp = new->br_blockcount + right.br_blockcount; oldlen = startblockval(new->br_startblock) + startblockval(right.br_startblock); newlen = xfs_bmap_worst_indlen(ip, temp); xfs_bmbt_set_allf(ep, new->br_startoff, nullstartblock((int)newlen), temp, right.br_state); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); - - ip->i_df.if_lastex = idx; + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); break; case 0: @@ -1759,8 +1752,7 @@ xfs_bmap_add_extent_hole_delay( * Insert a new entry. */ oldlen = newlen = 0; - xfs_iext_insert(ip, idx, 1, new, state); - ip->i_df.if_lastex = idx; + xfs_iext_insert(ip, *idx, 1, new, state); break; } if (oldlen != newlen) { @@ -1782,7 +1774,7 @@ xfs_bmap_add_extent_hole_delay( STATIC int /* error */ xfs_bmap_add_extent_hole_real( xfs_inode_t *ip, /* incore inode pointer */ - xfs_extnum_t idx, /* extent number to update/insert */ + xfs_extnum_t *idx, /* extent number to update/insert */ xfs_btree_cur_t *cur, /* if null, not a btree */ xfs_bmbt_irec_t *new, /* new data to add to file extents */ int *logflagsp, /* inode logging flags */ @@ -1798,8 +1790,8 @@ xfs_bmap_add_extent_hole_real( int state; /* state bits, accessed thru macros */ ifp = XFS_IFORK_PTR(ip, whichfork); - ASSERT(idx <= ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t)); - ep = xfs_iext_get_ext(ifp, idx); + ASSERT(*idx <= ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t)); + ep = xfs_iext_get_ext(ifp, *idx); state = 0; if (whichfork == XFS_ATTR_FORK) @@ -1808,9 +1800,9 @@ xfs_bmap_add_extent_hole_real( /* * Check and set flags if this segment has a left neighbor. */ - if (idx > 0) { + if (*idx > 0) { state |= BMAP_LEFT_VALID; - xfs_bmbt_get_all(xfs_iext_get_ext(ifp, idx - 1), &left); + xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx - 1), &left); if (isnullstartblock(left.br_startblock)) state |= BMAP_LEFT_DELAY; } @@ -1819,7 +1811,7 @@ xfs_bmap_add_extent_hole_real( * Check and set flags if this segment has a current value. * Not true if we're inserting into the "hole" at eof. */ - if (idx < ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t)) { + if (*idx < ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t)) { state |= BMAP_RIGHT_VALID; xfs_bmbt_get_all(ep, &right); if (isnullstartblock(right.br_startblock)) @@ -1858,14 +1850,15 @@ xfs_bmap_add_extent_hole_real( * left and on the right. * Merge all three into a single extent record. */ - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), + --*idx; + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx), left.br_blockcount + new->br_blockcount + right.br_blockcount); - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); + + xfs_iext_remove(ip, *idx + 1, 1, state); - xfs_iext_remove(ip, idx, 1, state); - ifp->if_lastex = idx - 1; XFS_IFORK_NEXT_SET(ip, whichfork, XFS_IFORK_NEXTENTS(ip, whichfork) - 1); if (cur == NULL) { @@ -1900,12 +1893,12 @@ xfs_bmap_add_extent_hole_real( * on the left. * Merge the new allocation with the left neighbor. */ - trace_xfs_bmap_pre_update(ip, idx - 1, state, _THIS_IP_); - xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, idx - 1), + --*idx; + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); + xfs_bmbt_set_blockcount(xfs_iext_get_ext(ifp, *idx), left.br_blockcount + new->br_blockcount); - trace_xfs_bmap_post_update(ip, idx - 1, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - ifp->if_lastex = idx - 1; if (cur == NULL) { rval = xfs_ilog_fext(whichfork); } else { @@ -1931,13 +1924,12 @@ xfs_bmap_add_extent_hole_real( * on the right. * Merge the new allocation with the right neighbor. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_allf(ep, new->br_startoff, new->br_startblock, new->br_blockcount + right.br_blockcount, right.br_state); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); - ifp->if_lastex = idx; if (cur == NULL) { rval = xfs_ilog_fext(whichfork); } else { @@ -1963,8 +1955,7 @@ xfs_bmap_add_extent_hole_real( * real allocation. * Insert a new entry. */ - xfs_iext_insert(ip, idx, 1, new, state); - ifp->if_lastex = idx; + xfs_iext_insert(ip, *idx, 1, new, state); XFS_IFORK_NEXT_SET(ip, whichfork, XFS_IFORK_NEXTENTS(ip, whichfork) + 1); if (cur == NULL) { @@ -2812,7 +2803,7 @@ STATIC int /* error */ xfs_bmap_del_extent( xfs_inode_t *ip, /* incore inode pointer */ xfs_trans_t *tp, /* current transaction pointer */ - xfs_extnum_t idx, /* extent number to update/delete */ + xfs_extnum_t *idx, /* extent number to update/delete */ xfs_bmap_free_t *flist, /* list of extents to be freed */ xfs_btree_cur_t *cur, /* if null, not a btree */ xfs_bmbt_irec_t *del, /* data to remove from extents */ @@ -2848,10 +2839,10 @@ xfs_bmap_del_extent( mp = ip->i_mount; ifp = XFS_IFORK_PTR(ip, whichfork); - ASSERT((idx >= 0) && (idx < ifp->if_bytes / + ASSERT((*idx >= 0) && (*idx < ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t))); ASSERT(del->br_blockcount > 0); - ep = xfs_iext_get_ext(ifp, idx); + ep = xfs_iext_get_ext(ifp, *idx); xfs_bmbt_get_all(ep, &got); ASSERT(got.br_startoff <= del->br_startoff); del_endoff = del->br_startoff + del->br_blockcount; @@ -2925,9 +2916,8 @@ xfs_bmap_del_extent( /* * Matches the whole extent. Delete the entry. */ - xfs_iext_remove(ip, idx, 1, + xfs_iext_remove(ip, *idx, 1, whichfork == XFS_ATTR_FORK ? BMAP_ATTRFORK : 0); - ifp->if_lastex = idx; if (delay) break; XFS_IFORK_NEXT_SET(ip, whichfork, @@ -2946,21 +2936,20 @@ xfs_bmap_del_extent( /* * Deleting the first part of the extent. */ - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_startoff(ep, del_endoff); temp = got.br_blockcount - del->br_blockcount; xfs_bmbt_set_blockcount(ep, temp); - ifp->if_lastex = idx; if (delay) { temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp), da_old); xfs_bmbt_set_startblock(ep, nullstartblock((int)temp)); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); da_new = temp; break; } xfs_bmbt_set_startblock(ep, del_endblock); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); if (!cur) { flags |= xfs_ilog_fext(whichfork); break; @@ -2976,18 +2965,17 @@ xfs_bmap_del_extent( * Deleting the last part of the extent. */ temp = got.br_blockcount - del->br_blockcount; - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_blockcount(ep, temp); - ifp->if_lastex = idx; if (delay) { temp = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp), da_old); xfs_bmbt_set_startblock(ep, nullstartblock((int)temp)); - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); da_new = temp; break; } - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); if (!cur) { flags |= xfs_ilog_fext(whichfork); break; @@ -3004,7 +2992,7 @@ xfs_bmap_del_extent( * Deleting the middle of the extent. */ temp = del->br_startoff - got.br_startoff; - trace_xfs_bmap_pre_update(ip, idx, state, _THIS_IP_); + trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); xfs_bmbt_set_blockcount(ep, temp); new.br_startoff = del_endoff; temp2 = got_endoff - del_endoff; @@ -3091,9 +3079,9 @@ xfs_bmap_del_extent( } } } - trace_xfs_bmap_post_update(ip, idx, state, _THIS_IP_); - xfs_iext_insert(ip, idx + 1, 1, &new, state); - ifp->if_lastex = idx + 1; + trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); + xfs_iext_insert(ip, *idx + 1, 1, &new, state); + ++*idx; break; } /* @@ -4674,13 +4662,12 @@ xfs_bmapi( if (!wasdelay && (flags & XFS_BMAPI_PREALLOC)) got.br_state = XFS_EXT_UNWRITTEN; } - error = xfs_bmap_add_extent(ip, lastx, &cur, &got, + error = xfs_bmap_add_extent(ip, &lastx, &cur, &got, firstblock, flist, &tmp_logflags, whichfork); logflags |= tmp_logflags; if (error) goto error0; - lastx = ifp->if_lastex; ep = xfs_iext_get_ext(ifp, lastx); nextents = ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t); xfs_bmbt_get_all(ep, &got); @@ -4776,13 +4763,12 @@ xfs_bmapi( mval->br_state = (mval->br_state == XFS_EXT_UNWRITTEN) ? XFS_EXT_NORM : XFS_EXT_UNWRITTEN; - error = xfs_bmap_add_extent(ip, lastx, &cur, mval, + error = xfs_bmap_add_extent(ip, &lastx, &cur, mval, firstblock, flist, &tmp_logflags, whichfork); logflags |= tmp_logflags; if (error) goto error0; - lastx = ifp->if_lastex; ep = xfs_iext_get_ext(ifp, lastx); nextents = ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t); xfs_bmbt_get_all(ep, &got); @@ -4848,7 +4834,6 @@ xfs_bmapi( else xfs_bmbt_get_all(ep, &got); } - ifp->if_lastex = lastx; *nmap = n; /* * Transform from btree to extents, give it cur. @@ -4957,7 +4942,6 @@ xfs_bmapi_single( ASSERT(!isnullstartblock(got.br_startblock)); ASSERT(bno < got.br_startoff + got.br_blockcount); *fsb = got.br_startblock + (bno - got.br_startoff); - ifp->if_lastex = lastx; return 0; } @@ -5132,7 +5116,7 @@ xfs_bunmapi( del.br_blockcount = mod; } del.br_state = XFS_EXT_UNWRITTEN; - error = xfs_bmap_add_extent(ip, lastx, &cur, &del, + error = xfs_bmap_add_extent(ip, &lastx, &cur, &del, firstblock, flist, &logflags, XFS_DATA_FORK); if (error) @@ -5186,7 +5170,8 @@ xfs_bunmapi( prev.br_startoff = start; } prev.br_state = XFS_EXT_UNWRITTEN; - error = xfs_bmap_add_extent(ip, lastx - 1, &cur, + lastx--; + error = xfs_bmap_add_extent(ip, &lastx, &cur, &prev, firstblock, flist, &logflags, XFS_DATA_FORK); if (error) @@ -5195,7 +5180,7 @@ xfs_bunmapi( } else { ASSERT(del.br_state == XFS_EXT_NORM); del.br_state = XFS_EXT_UNWRITTEN; - error = xfs_bmap_add_extent(ip, lastx, &cur, + error = xfs_bmap_add_extent(ip, &lastx, &cur, &del, firstblock, flist, &logflags, XFS_DATA_FORK); if (error) @@ -5249,14 +5234,13 @@ xfs_bunmapi( error = XFS_ERROR(ENOSPC); goto error0; } - error = xfs_bmap_del_extent(ip, tp, lastx, flist, cur, &del, + error = xfs_bmap_del_extent(ip, tp, &lastx, flist, cur, &del, &tmp_logflags, whichfork); logflags |= tmp_logflags; if (error) goto error0; bno = del.br_startoff - 1; nodelete: - lastx = ifp->if_lastex; /* * If not done go on to the next (previous) record. * Reset ep in case the extents array was re-alloced. @@ -5273,7 +5257,6 @@ nodelete: extno++; } } - ifp->if_lastex = lastx; *done = bno == (xfs_fileoff_t)-1 || bno < start || lastx < 0; ASSERT(ifp->if_ext_max == XFS_IFORK_SIZE(ip, whichfork) / (uint)sizeof(xfs_bmbt_rec_t)); Index: xfs/fs/xfs/xfs_inode.c =================================================================== --- xfs.orig/fs/xfs/xfs_inode.c 2011-05-20 15:25:52.409233306 +0200 +++ xfs/fs/xfs/xfs_inode.c 2011-05-22 12:45:45.824962083 +0200 @@ -920,7 +920,6 @@ xfs_iread_extents( /* * We know that the size is valid (it's checked in iformat_btree) */ - ifp->if_lastex = NULLEXTNUM; ifp->if_bytes = ifp->if_real_bytes = 0; ifp->if_flags |= XFS_IFEXTENTS; xfs_iext_add(ifp, 0, nextents); @@ -3191,7 +3190,6 @@ xfs_iext_add( } ifp->if_u1.if_extents = ifp->if_u2.if_inline_ext; ifp->if_real_bytes = 0; - ifp->if_lastex = nextents + ext_diff; } /* * Otherwise use a linear (direct) extent list. Index: xfs/fs/xfs/xfs_inode.h =================================================================== --- xfs.orig/fs/xfs/xfs_inode.h 2011-05-20 13:32:26.512306370 +0200 +++ xfs/fs/xfs/xfs_inode.h 2011-05-22 12:45:45.832948481 +0200 @@ -67,7 +67,6 @@ typedef struct xfs_ifork { short if_broot_bytes; /* bytes allocated for root */ unsigned char if_flags; /* per-fork flags */ unsigned char if_ext_max; /* max # of extent records */ - xfs_extnum_t if_lastex; /* last if_extents used */ union { xfs_bmbt_rec_host_t *if_extents;/* linear map file exts */ xfs_ext_irec_t *if_ext_irec; /* irec map file exts */ _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/9 v2] xfs: remove if_lastex 2011-05-23 8:52 ` [PATCH 2/9 v2] " Christoph Hellwig @ 2011-05-25 1:14 ` Alex Elder 0 siblings, 0 replies; 30+ messages in thread From: Alex Elder @ 2011-05-25 1:14 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Mon, 2011-05-23 at 04:52 -0400, Christoph Hellwig wrote: > The if_lastex field in struct xfs_ifork is only used as a temporary index > during xfs_bmapi and xfs_bunmapi. Instead of using the inode fork to store > it keep it local in the callchain. Fortunately this is very easy as > we already pass a stack copy of it down the whole chain which can simplify > be changed to be passed by reference. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Looks good. Reviewed-by: Alex Elder <aelder@sgi.com> _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 3/9] xfs: correctly decrement the extent buffer index in xfs_bmap_del_extent 2011-05-11 15:04 [PATCH 0/9] extent buffer indexing fixes Christoph Hellwig 2011-05-11 15:04 ` [PATCH 1/9] xfs: remove the unused XFS_BMAPI_RSVBLOCKS flag Christoph Hellwig 2011-05-11 15:04 ` [PATCH 2/9] xfs: remove if_lastex Christoph Hellwig @ 2011-05-11 15:04 ` Christoph Hellwig 2011-05-12 6:50 ` Lachlan McIlroy ` (2 more replies) 2011-05-11 15:04 ` [PATCH 4/9] xfs: do not use unchecked extent indices in xfs_bmap_add_extent_* Christoph Hellwig ` (5 subsequent siblings) 8 siblings, 3 replies; 30+ messages in thread From: Christoph Hellwig @ 2011-05-11 15:04 UTC (permalink / raw) To: xfs [-- Attachment #1: xfs-fix-ep-access --] [-- Type: text/plain, Size: 979 bytes --] The code in xfs_bmap_del_extent does not correctly decrement the extent buffer index when deleting a whole extent. Most of the time this gets caught by checks in xfs_bmapi that work around it and decrement it manually and thus wasn't noticed so far. Based on an earlier patch from Lachlan McIlroy. Signed-off-by: Christoph Hellwig <hch@lst.de> Index: xfs/fs/xfs/xfs_bmap.c =================================================================== --- xfs.orig/fs/xfs/xfs_bmap.c 2011-05-10 17:11:21.212901236 +0200 +++ xfs/fs/xfs/xfs_bmap.c 2011-05-10 17:13:36.177399627 +0200 @@ -2916,8 +2916,10 @@ xfs_bmap_del_extent( */ xfs_iext_remove(ip, *idx, 1, whichfork == XFS_ATTR_FORK ? BMAP_ATTRFORK : 0); + --*idx; if (delay) break; + XFS_IFORK_NEXT_SET(ip, whichfork, XFS_IFORK_NEXTENTS(ip, whichfork) - 1); flags |= XFS_ILOG_CORE; _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 3/9] xfs: correctly decrement the extent buffer index in xfs_bmap_del_extent 2011-05-11 15:04 ` [PATCH 3/9] xfs: correctly decrement the extent buffer index in xfs_bmap_del_extent Christoph Hellwig @ 2011-05-12 6:50 ` Lachlan McIlroy 2011-05-12 6:54 ` Lachlan McIlroy 2011-05-12 7:17 ` Lachlan McIlroy 2011-05-25 0:27 ` Alex Elder 2 siblings, 1 reply; 30+ messages in thread From: Lachlan McIlroy @ 2011-05-12 6:50 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs ----- Original Message ----- > The code in xfs_bmap_del_extent does not correctly decrement the > extent buffer > index when deleting a whole extent. Most of the time this gets caught > by > checks in xfs_bmapi that work around it and decrement it manually and > thus > wasn't noticed so far. > > Based on an earlier patch from Lachlan McIlroy. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Index: xfs/fs/xfs/xfs_bmap.c > =================================================================== > --- xfs.orig/fs/xfs/xfs_bmap.c 2011-05-10 17:11:21.212901236 +0200 > +++ xfs/fs/xfs/xfs_bmap.c 2011-05-10 17:13:36.177399627 +0200 > @@ -2916,8 +2916,10 @@ xfs_bmap_del_extent( > */ > xfs_iext_remove(ip, *idx, 1, > whichfork == XFS_ATTR_FORK ? BMAP_ATTRFORK : 0); > + --*idx; I can see why this is needed but if we remove extent at idx 0 then wont this go negative and confuse the next call to xfs_iext_get_ext()? > if (delay) > break; > + > XFS_IFORK_NEXT_SET(ip, whichfork, > XFS_IFORK_NEXTENTS(ip, whichfork) - 1); > flags |= XFS_ILOG_CORE; > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 3/9] xfs: correctly decrement the extent buffer index in xfs_bmap_del_extent 2011-05-12 6:50 ` Lachlan McIlroy @ 2011-05-12 6:54 ` Lachlan McIlroy 0 siblings, 0 replies; 30+ messages in thread From: Lachlan McIlroy @ 2011-05-12 6:54 UTC (permalink / raw) To: Lachlan McIlroy; +Cc: Christoph Hellwig, xfs ----- Original Message ----- > ----- Original Message ----- > > The code in xfs_bmap_del_extent does not correctly decrement the > > extent buffer > > index when deleting a whole extent. Most of the time this gets > > caught > > by > > checks in xfs_bmapi that work around it and decrement it manually > > and > > thus > > wasn't noticed so far. > > > > Based on an earlier patch from Lachlan McIlroy. > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > > > Index: xfs/fs/xfs/xfs_bmap.c > > =================================================================== > > --- xfs.orig/fs/xfs/xfs_bmap.c 2011-05-10 17:11:21.212901236 +0200 > > +++ xfs/fs/xfs/xfs_bmap.c 2011-05-10 17:13:36.177399627 +0200 > > @@ -2916,8 +2916,10 @@ xfs_bmap_del_extent( > > */ > > xfs_iext_remove(ip, *idx, 1, > > whichfork == XFS_ATTR_FORK ? BMAP_ATTRFORK : 0); > > + --*idx; > > I can see why this is needed but if we remove extent at > idx 0 then wont this go negative and confuse the next > call to xfs_iext_get_ext()? Ignore this comment - I see you've fixed that case in patch 6. Guess I should have looked ahead. > > > if (delay) > > break; > > + > > XFS_IFORK_NEXT_SET(ip, whichfork, > > XFS_IFORK_NEXTENTS(ip, whichfork) - 1); > > flags |= XFS_ILOG_CORE; > > > > _______________________________________________ > > xfs mailing list > > xfs@oss.sgi.com > > http://oss.sgi.com/mailman/listinfo/xfs > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 3/9] xfs: correctly decrement the extent buffer index in xfs_bmap_del_extent 2011-05-11 15:04 ` [PATCH 3/9] xfs: correctly decrement the extent buffer index in xfs_bmap_del_extent Christoph Hellwig 2011-05-12 6:50 ` Lachlan McIlroy @ 2011-05-12 7:17 ` Lachlan McIlroy 2011-05-25 0:27 ` Alex Elder 2 siblings, 0 replies; 30+ messages in thread From: Lachlan McIlroy @ 2011-05-12 7:17 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs Looks good. ----- Original Message ----- > The code in xfs_bmap_del_extent does not correctly decrement the > extent buffer > index when deleting a whole extent. Most of the time this gets caught > by > checks in xfs_bmapi that work around it and decrement it manually and > thus > wasn't noticed so far. > > Based on an earlier patch from Lachlan McIlroy. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Index: xfs/fs/xfs/xfs_bmap.c > =================================================================== > --- xfs.orig/fs/xfs/xfs_bmap.c 2011-05-10 17:11:21.212901236 +0200 > +++ xfs/fs/xfs/xfs_bmap.c 2011-05-10 17:13:36.177399627 +0200 > @@ -2916,8 +2916,10 @@ xfs_bmap_del_extent( > */ > xfs_iext_remove(ip, *idx, 1, > whichfork == XFS_ATTR_FORK ? BMAP_ATTRFORK : 0); > + --*idx; > if (delay) > break; > + > XFS_IFORK_NEXT_SET(ip, whichfork, > XFS_IFORK_NEXTENTS(ip, whichfork) - 1); > flags |= XFS_ILOG_CORE; > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 3/9] xfs: correctly decrement the extent buffer index in xfs_bmap_del_extent 2011-05-11 15:04 ` [PATCH 3/9] xfs: correctly decrement the extent buffer index in xfs_bmap_del_extent Christoph Hellwig 2011-05-12 6:50 ` Lachlan McIlroy 2011-05-12 7:17 ` Lachlan McIlroy @ 2011-05-25 0:27 ` Alex Elder 2 siblings, 0 replies; 30+ messages in thread From: Alex Elder @ 2011-05-25 0:27 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Wed, 2011-05-11 at 11:04 -0400, Christoph Hellwig wrote: > plain text document attachment (xfs-fix-ep-access) > The code in xfs_bmap_del_extent does not correctly decrement the extent buffer > index when deleting a whole extent. Most of the time this gets caught by > checks in xfs_bmapi that work around it and decrement it manually and thus > wasn't noticed so far. > > Based on an earlier patch from Lachlan McIlroy. > > Signed-off-by: Christoph Hellwig <hch@lst.de> This one crashes in test 008. I suspect it's due to what Lachlan pointed out--the index can go negative and we aren't (yet) ensuring we don't call into xfs_iext_get_ext() in that case--but I have not absolutely verified this. It would be better if this change were combined with the portion of patch 6 that pulls out the if (lastx >= XFS_IFORK_NEXTENTS() check in xfs_bunmapi(). But it still would need to go later in the series. This does look like it's a reasonable thing to do, it moves "left" in extent list because we know we've exhausted the current one, and the only caller--xfs_bunmapi()--is going from the end backward. I think the same decrement would be appropriate for "case 2" below where you put this change though (because there will be no more of the current extent remaining after deleting the left portion). In fact, it looks to me like the only one that got it right was "case 1" because if you're splitting an extent (case 0) the next one to examine will be the left one of the two resulting from the split. (I'm a bit unsure about these observations though, you or someone else should verify this and set me straight if I'm wrong.) It's all a bit confusing though. It's not obvious here what the value of *idx should be after one of these operations, certainly not independent of knowledge of whether the extents are being scanned from the beginning (as xfs_bmapi() does) or from the end (as xfs_bunmap() does). I moved this one change to the end of the series and I no longer hit the problem I had with test 008 (presumably because the index checking in later patches avoids the problem). I will keep testing with the whole series, with this one at the end. Meanwhile I'm interested to hear back on whether other spots in xfs_bmap_del_extent() should adjust the index value. -Alex > Index: xfs/fs/xfs/xfs_bmap.c > =================================================================== > --- xfs.orig/fs/xfs/xfs_bmap.c 2011-05-10 17:11:21.212901236 +0200 > +++ xfs/fs/xfs/xfs_bmap.c 2011-05-10 17:13:36.177399627 +0200 > @@ -2916,8 +2916,10 @@ xfs_bmap_del_extent( > */ > xfs_iext_remove(ip, *idx, 1, > whichfork == XFS_ATTR_FORK ? BMAP_ATTRFORK : 0); > + --*idx; > if (delay) > break; > + > XFS_IFORK_NEXT_SET(ip, whichfork, > XFS_IFORK_NEXTENTS(ip, whichfork) - 1); > flags |= XFS_ILOG_CORE; > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 4/9] xfs: do not use unchecked extent indices in xfs_bmap_add_extent_* 2011-05-11 15:04 [PATCH 0/9] extent buffer indexing fixes Christoph Hellwig ` (2 preceding siblings ...) 2011-05-11 15:04 ` [PATCH 3/9] xfs: correctly decrement the extent buffer index in xfs_bmap_del_extent Christoph Hellwig @ 2011-05-11 15:04 ` Christoph Hellwig 2011-05-12 7:31 ` Lachlan McIlroy 2011-05-25 0:30 ` Alex Elder 2011-05-11 15:04 ` [PATCH 5/9] xfs: do not use unchecked extent indices in xfs_bmapi Christoph Hellwig ` (4 subsequent siblings) 8 siblings, 2 replies; 30+ messages in thread From: Christoph Hellwig @ 2011-05-11 15:04 UTC (permalink / raw) To: xfs [-- Attachment #1: xfs-fix-ep-access-1 --] [-- Type: text/plain, Size: 3302 bytes --] Make sure to only call xfs_iext_get_ext after we've validate the extent index in the various xfs_bmap_add_extent_* helpers. Based on an earlier patch from Lachlan McIlroy. Signed-off-by: Christoph Hellwig <hch@lst.de> Index: xfs/fs/xfs/xfs_bmap.c =================================================================== --- xfs.orig/fs/xfs/xfs_bmap.c 2011-05-10 13:57:12.297088697 +0200 +++ xfs/fs/xfs/xfs_bmap.c 2011-05-10 14:00:16.405087271 +0200 @@ -1629,7 +1629,6 @@ xfs_bmap_add_extent_hole_delay( xfs_bmbt_irec_t *new, /* new data to add to file extents */ int *logflagsp) /* inode logging flags */ { - xfs_bmbt_rec_host_t *ep; /* extent record for idx */ xfs_ifork_t *ifp; /* inode fork pointer */ xfs_bmbt_irec_t left; /* left neighbor extent entry */ xfs_filblks_t newlen=0; /* new indirect size */ @@ -1639,7 +1638,6 @@ xfs_bmap_add_extent_hole_delay( xfs_filblks_t temp=0; /* temp for indirect calculations */ ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK); - ep = xfs_iext_get_ext(ifp, *idx); state = 0; ASSERT(isnullstartblock(new->br_startblock)); @@ -1660,7 +1658,7 @@ xfs_bmap_add_extent_hole_delay( */ if (*idx < ip->i_df.if_bytes / (uint)sizeof(xfs_bmbt_rec_t)) { state |= BMAP_RIGHT_VALID; - xfs_bmbt_get_all(ep, &right); + xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx), &right); if (isnullstartblock(right.br_startblock)) state |= BMAP_RIGHT_DELAY; @@ -1740,7 +1738,8 @@ xfs_bmap_add_extent_hole_delay( oldlen = startblockval(new->br_startblock) + startblockval(right.br_startblock); newlen = xfs_bmap_worst_indlen(ip, temp); - xfs_bmbt_set_allf(ep, new->br_startoff, + xfs_bmbt_set_allf(xfs_iext_get_ext(ifp, *idx), + new->br_startoff, nullstartblock((int)newlen), temp, right.br_state); trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); break; @@ -1780,7 +1779,6 @@ xfs_bmap_add_extent_hole_real( int *logflagsp, /* inode logging flags */ int whichfork) /* data or attr fork */ { - xfs_bmbt_rec_host_t *ep; /* pointer to extent entry ins. point */ int error; /* error return value */ int i; /* temp state */ xfs_ifork_t *ifp; /* inode fork pointer */ @@ -1791,7 +1789,6 @@ xfs_bmap_add_extent_hole_real( ifp = XFS_IFORK_PTR(ip, whichfork); ASSERT(*idx <= ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t)); - ep = xfs_iext_get_ext(ifp, *idx); state = 0; if (whichfork == XFS_ATTR_FORK) @@ -1813,7 +1810,7 @@ xfs_bmap_add_extent_hole_real( */ if (*idx < ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t)) { state |= BMAP_RIGHT_VALID; - xfs_bmbt_get_all(ep, &right); + xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx), &right); if (isnullstartblock(right.br_startblock)) state |= BMAP_RIGHT_DELAY; } @@ -1925,7 +1922,8 @@ xfs_bmap_add_extent_hole_real( * Merge the new allocation with the right neighbor. */ trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); - xfs_bmbt_set_allf(ep, new->br_startoff, new->br_startblock, + xfs_bmbt_set_allf(xfs_iext_get_ext(ifp, *idx), + new->br_startoff, new->br_startblock, new->br_blockcount + right.br_blockcount, right.br_state); trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 4/9] xfs: do not use unchecked extent indices in xfs_bmap_add_extent_* 2011-05-11 15:04 ` [PATCH 4/9] xfs: do not use unchecked extent indices in xfs_bmap_add_extent_* Christoph Hellwig @ 2011-05-12 7:31 ` Lachlan McIlroy 2011-05-25 0:30 ` Alex Elder 1 sibling, 0 replies; 30+ messages in thread From: Lachlan McIlroy @ 2011-05-12 7:31 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs Looks good. ----- Original Message ----- > Make sure to only call xfs_iext_get_ext after we've validate the > extent index > in the various xfs_bmap_add_extent_* helpers. > > Based on an earlier patch from Lachlan McIlroy. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Index: xfs/fs/xfs/xfs_bmap.c > =================================================================== > --- xfs.orig/fs/xfs/xfs_bmap.c 2011-05-10 13:57:12.297088697 +0200 > +++ xfs/fs/xfs/xfs_bmap.c 2011-05-10 14:00:16.405087271 +0200 > @@ -1629,7 +1629,6 @@ xfs_bmap_add_extent_hole_delay( > xfs_bmbt_irec_t *new, /* new data to add to file extents */ > int *logflagsp) /* inode logging flags */ > { > - xfs_bmbt_rec_host_t *ep; /* extent record for idx */ > xfs_ifork_t *ifp; /* inode fork pointer */ > xfs_bmbt_irec_t left; /* left neighbor extent entry */ > xfs_filblks_t newlen=0; /* new indirect size */ > @@ -1639,7 +1638,6 @@ xfs_bmap_add_extent_hole_delay( > xfs_filblks_t temp=0; /* temp for indirect calculations */ > > ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK); > - ep = xfs_iext_get_ext(ifp, *idx); > state = 0; > ASSERT(isnullstartblock(new->br_startblock)); > > @@ -1660,7 +1658,7 @@ xfs_bmap_add_extent_hole_delay( > */ > if (*idx < ip->i_df.if_bytes / (uint)sizeof(xfs_bmbt_rec_t)) { > state |= BMAP_RIGHT_VALID; > - xfs_bmbt_get_all(ep, &right); > + xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx), &right); > > if (isnullstartblock(right.br_startblock)) > state |= BMAP_RIGHT_DELAY; > @@ -1740,7 +1738,8 @@ xfs_bmap_add_extent_hole_delay( > oldlen = startblockval(new->br_startblock) + > startblockval(right.br_startblock); > newlen = xfs_bmap_worst_indlen(ip, temp); > - xfs_bmbt_set_allf(ep, new->br_startoff, > + xfs_bmbt_set_allf(xfs_iext_get_ext(ifp, *idx), > + new->br_startoff, > nullstartblock((int)newlen), temp, right.br_state); > trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); > break; > @@ -1780,7 +1779,6 @@ xfs_bmap_add_extent_hole_real( > int *logflagsp, /* inode logging flags */ > int whichfork) /* data or attr fork */ > { > - xfs_bmbt_rec_host_t *ep; /* pointer to extent entry ins. point */ > int error; /* error return value */ > int i; /* temp state */ > xfs_ifork_t *ifp; /* inode fork pointer */ > @@ -1791,7 +1789,6 @@ xfs_bmap_add_extent_hole_real( > > ifp = XFS_IFORK_PTR(ip, whichfork); > ASSERT(*idx <= ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t)); > - ep = xfs_iext_get_ext(ifp, *idx); > state = 0; > > if (whichfork == XFS_ATTR_FORK) > @@ -1813,7 +1810,7 @@ xfs_bmap_add_extent_hole_real( > */ > if (*idx < ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t)) { > state |= BMAP_RIGHT_VALID; > - xfs_bmbt_get_all(ep, &right); > + xfs_bmbt_get_all(xfs_iext_get_ext(ifp, *idx), &right); > if (isnullstartblock(right.br_startblock)) > state |= BMAP_RIGHT_DELAY; > } > @@ -1925,7 +1922,8 @@ xfs_bmap_add_extent_hole_real( > * Merge the new allocation with the right neighbor. > */ > trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_); > - xfs_bmbt_set_allf(ep, new->br_startoff, new->br_startblock, > + xfs_bmbt_set_allf(xfs_iext_get_ext(ifp, *idx), > + new->br_startoff, new->br_startblock, > new->br_blockcount + right.br_blockcount, > right.br_state); > trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_); > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 4/9] xfs: do not use unchecked extent indices in xfs_bmap_add_extent_* 2011-05-11 15:04 ` [PATCH 4/9] xfs: do not use unchecked extent indices in xfs_bmap_add_extent_* Christoph Hellwig 2011-05-12 7:31 ` Lachlan McIlroy @ 2011-05-25 0:30 ` Alex Elder 1 sibling, 0 replies; 30+ messages in thread From: Alex Elder @ 2011-05-25 0:30 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Wed, 2011-05-11 at 11:04 -0400, Christoph Hellwig wrote: > Make sure to only call xfs_iext_get_ext after we've validate the extent index > in the various xfs_bmap_add_extent_* helpers. > > Based on an earlier patch from Lachlan McIlroy. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Looks good. Reviewed-by: Alex Elder <aelder@sgi.com> _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 5/9] xfs: do not use unchecked extent indices in xfs_bmapi 2011-05-11 15:04 [PATCH 0/9] extent buffer indexing fixes Christoph Hellwig ` (3 preceding siblings ...) 2011-05-11 15:04 ` [PATCH 4/9] xfs: do not use unchecked extent indices in xfs_bmap_add_extent_* Christoph Hellwig @ 2011-05-11 15:04 ` Christoph Hellwig 2011-05-12 7:20 ` Lachlan McIlroy 2011-05-25 0:31 ` Alex Elder 2011-05-11 15:04 ` [PATCH 6/9] xfs: do not use unchecked extent indices in xfs_bunmapi Christoph Hellwig ` (3 subsequent siblings) 8 siblings, 2 replies; 30+ messages in thread From: Christoph Hellwig @ 2011-05-11 15:04 UTC (permalink / raw) To: xfs [-- Attachment #1: xfs-fix-ep-access-2 --] [-- Type: text/plain, Size: 909 bytes --] Make sure to only call xfs_iext_get_ext after we've validate the extent index when moving on to the next index in xfs_bmapi. Based on an earlier patch from Lachlan McIlroy. Signed-off-by: Christoph Hellwig <hch@lst.de> Index: xfs/fs/xfs/xfs_bmap.c =================================================================== --- xfs.orig/fs/xfs/xfs_bmap.c 2011-05-11 10:16:58.831733512 +0200 +++ xfs/fs/xfs/xfs_bmap.c 2011-05-11 10:16:58.847733078 +0200 @@ -4827,12 +4827,13 @@ xfs_bmapi( /* * Else go on to the next record. */ - ep = xfs_iext_get_ext(ifp, ++lastx); prev = got; - if (lastx >= nextents) - eof = 1; - else + if (++lastx < nextents) { + ep = xfs_iext_get_ext(ifp, lastx); xfs_bmbt_get_all(ep, &got); + } else { + eof = 1; + } } *nmap = n; /* _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 5/9] xfs: do not use unchecked extent indices in xfs_bmapi 2011-05-11 15:04 ` [PATCH 5/9] xfs: do not use unchecked extent indices in xfs_bmapi Christoph Hellwig @ 2011-05-12 7:20 ` Lachlan McIlroy 2011-05-25 0:31 ` Alex Elder 1 sibling, 0 replies; 30+ messages in thread From: Lachlan McIlroy @ 2011-05-12 7:20 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs Looks good. ----- Original Message ----- > Make sure to only call xfs_iext_get_ext after we've validate the > extent index > when moving on to the next index in xfs_bmapi. > > Based on an earlier patch from Lachlan McIlroy. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Index: xfs/fs/xfs/xfs_bmap.c > =================================================================== > --- xfs.orig/fs/xfs/xfs_bmap.c 2011-05-11 10:16:58.831733512 +0200 > +++ xfs/fs/xfs/xfs_bmap.c 2011-05-11 10:16:58.847733078 +0200 > @@ -4827,12 +4827,13 @@ xfs_bmapi( > /* > * Else go on to the next record. > */ > - ep = xfs_iext_get_ext(ifp, ++lastx); > prev = got; > - if (lastx >= nextents) > - eof = 1; > - else > + if (++lastx < nextents) { > + ep = xfs_iext_get_ext(ifp, lastx); > xfs_bmbt_get_all(ep, &got); > + } else { > + eof = 1; > + } > } > *nmap = n; > /* > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 5/9] xfs: do not use unchecked extent indices in xfs_bmapi 2011-05-11 15:04 ` [PATCH 5/9] xfs: do not use unchecked extent indices in xfs_bmapi Christoph Hellwig 2011-05-12 7:20 ` Lachlan McIlroy @ 2011-05-25 0:31 ` Alex Elder 1 sibling, 0 replies; 30+ messages in thread From: Alex Elder @ 2011-05-25 0:31 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Wed, 2011-05-11 at 11:04 -0400, Christoph Hellwig wrote: > plain text document attachment (xfs-fix-ep-access-2) > Make sure to only call xfs_iext_get_ext after we've validate the extent index > when moving on to the next index in xfs_bmapi. > > Based on an earlier patch from Lachlan McIlroy. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Looks good. Reviewed-by: Alex Elder <aelder@sgi.com> _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 6/9] xfs: do not use unchecked extent indices in xfs_bunmapi 2011-05-11 15:04 [PATCH 0/9] extent buffer indexing fixes Christoph Hellwig ` (4 preceding siblings ...) 2011-05-11 15:04 ` [PATCH 5/9] xfs: do not use unchecked extent indices in xfs_bmapi Christoph Hellwig @ 2011-05-11 15:04 ` Christoph Hellwig 2011-05-12 7:22 ` Lachlan McIlroy 2011-05-25 0:31 ` Alex Elder 2011-05-11 15:04 ` [PATCH 7/9] xfs: do not do pointer arithmetics on extent records Christoph Hellwig ` (2 subsequent siblings) 8 siblings, 2 replies; 30+ messages in thread From: Christoph Hellwig @ 2011-05-11 15:04 UTC (permalink / raw) To: xfs [-- Attachment #1: xfs-fix-ep-access-4 --] [-- Type: text/plain, Size: 1398 bytes --] Make sure to only call xfs_iext_get_ext after we've validate the extent index when moving on to the next index in xfs_bunmapi. Also remove the old workaround for too large indices that has been superceeded by the proper fix in xfs_bmap_del_extent. Based on an earlier patch from Lachlan McIlroy. Signed-off-by: Christoph Hellwig <hch@lst.de> Index: xfs/fs/xfs/xfs_bmap.c =================================================================== --- xfs.orig/fs/xfs/xfs_bmap.c 2011-05-11 10:17:04.803235692 +0200 +++ xfs/fs/xfs/xfs_bmap.c 2011-05-11 10:17:06.432734169 +0200 @@ -5247,17 +5247,17 @@ xfs_bunmapi( nodelete: /* * If not done go on to the next (previous) record. - * Reset ep in case the extents array was re-alloced. */ - ep = xfs_iext_get_ext(ifp, lastx); if (bno != (xfs_fileoff_t)-1 && bno >= start) { - if (lastx >= XFS_IFORK_NEXTENTS(ip, whichfork) || - xfs_bmbt_get_startoff(ep) > bno) { - if (--lastx >= 0) - ep = xfs_iext_get_ext(ifp, lastx); - } - if (lastx >= 0) + if (lastx >= 0) { + ep = xfs_iext_get_ext(ifp, lastx); + if (xfs_bmbt_get_startoff(ep) > bno) { + if (--lastx >= 0) + ep = xfs_iext_get_ext(ifp, + lastx); + } xfs_bmbt_get_all(ep, &got); + } extno++; } } _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 6/9] xfs: do not use unchecked extent indices in xfs_bunmapi 2011-05-11 15:04 ` [PATCH 6/9] xfs: do not use unchecked extent indices in xfs_bunmapi Christoph Hellwig @ 2011-05-12 7:22 ` Lachlan McIlroy 2011-05-25 0:31 ` Alex Elder 1 sibling, 0 replies; 30+ messages in thread From: Lachlan McIlroy @ 2011-05-12 7:22 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs Looks good. ----- Original Message ----- > Make sure to only call xfs_iext_get_ext after we've validate the > extent index > when moving on to the next index in xfs_bunmapi. Also remove the old > workaround for too large indices that has been superceeded by the > proper > fix in xfs_bmap_del_extent. > > Based on an earlier patch from Lachlan McIlroy. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Index: xfs/fs/xfs/xfs_bmap.c > =================================================================== > --- xfs.orig/fs/xfs/xfs_bmap.c 2011-05-11 10:17:04.803235692 +0200 > +++ xfs/fs/xfs/xfs_bmap.c 2011-05-11 10:17:06.432734169 +0200 > @@ -5247,17 +5247,17 @@ xfs_bunmapi( > nodelete: > /* > * If not done go on to the next (previous) record. > - * Reset ep in case the extents array was re-alloced. > */ > - ep = xfs_iext_get_ext(ifp, lastx); > if (bno != (xfs_fileoff_t)-1 && bno >= start) { > - if (lastx >= XFS_IFORK_NEXTENTS(ip, whichfork) || > - xfs_bmbt_get_startoff(ep) > bno) { > - if (--lastx >= 0) > - ep = xfs_iext_get_ext(ifp, lastx); > - } > - if (lastx >= 0) > + if (lastx >= 0) { > + ep = xfs_iext_get_ext(ifp, lastx); > + if (xfs_bmbt_get_startoff(ep) > bno) { > + if (--lastx >= 0) > + ep = xfs_iext_get_ext(ifp, > + lastx); > + } > xfs_bmbt_get_all(ep, &got); > + } > extno++; > } > } > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 6/9] xfs: do not use unchecked extent indices in xfs_bunmapi 2011-05-11 15:04 ` [PATCH 6/9] xfs: do not use unchecked extent indices in xfs_bunmapi Christoph Hellwig 2011-05-12 7:22 ` Lachlan McIlroy @ 2011-05-25 0:31 ` Alex Elder 1 sibling, 0 replies; 30+ messages in thread From: Alex Elder @ 2011-05-25 0:31 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Wed, 2011-05-11 at 11:04 -0400, Christoph Hellwig wrote: > Make sure to only call xfs_iext_get_ext after we've validate the extent index > when moving on to the next index in xfs_bunmapi. Also remove the old > workaround for too large indices that has been superceeded by the proper > fix in xfs_bmap_del_extent. > > Based on an earlier patch from Lachlan McIlroy. This looks good, but I'd like to see this patch and patch 3/9 re-done to more cleanly separate their purposes (as I mentioned in my comments on patch 3). Reviewed-by: Alex Elder <aelder@sgi.com> > Signed-off-by: Christoph Hellwig <hch@lst.de> _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 7/9] xfs: do not do pointer arithmetics on extent records 2011-05-11 15:04 [PATCH 0/9] extent buffer indexing fixes Christoph Hellwig ` (5 preceding siblings ...) 2011-05-11 15:04 ` [PATCH 6/9] xfs: do not use unchecked extent indices in xfs_bunmapi Christoph Hellwig @ 2011-05-11 15:04 ` Christoph Hellwig 2011-05-12 7:23 ` Lachlan McIlroy 2011-05-25 0:31 ` Alex Elder 2011-05-11 15:04 ` [PATCH 8/9] xfs: fix up asserts in xfs_iflush_fork Christoph Hellwig 2011-05-11 15:04 ` [PATCH 9/9] xfs: check for valid indices in xfs_iext_get_ext and xfs_iext_idx_to_irec Christoph Hellwig 8 siblings, 2 replies; 30+ messages in thread From: Christoph Hellwig @ 2011-05-11 15:04 UTC (permalink / raw) To: xfs [-- Attachment #1: xfs-fix-ep-access-3 --] [-- Type: text/plain, Size: 996 bytes --] We need to call xfs_iext_get_ext for the previous extent to get a valid pointer, and can't just do pointer arithmetics as they might be in different pages. Signed-off-by: Christoph Hellwig <hch@lst.de> Index: xfs/fs/xfs/xfs_bmap.c =================================================================== --- xfs.orig/fs/xfs/xfs_bmap.c 2011-05-11 10:16:58.847733078 +0200 +++ xfs/fs/xfs/xfs_bmap.c 2011-05-11 10:17:04.803235692 +0200 @@ -5145,9 +5145,12 @@ xfs_bunmapi( */ ASSERT(bno >= del.br_blockcount); bno -= del.br_blockcount; - if (bno < got.br_startoff) { - if (--lastx >= 0) - xfs_bmbt_get_all(--ep, &got); + if (got.br_startoff > bno) { + if (--lastx >= 0) { + ep = xfs_iext_get_ext(ifp, + lastx); + xfs_bmbt_get_all(ep, &got); + } } continue; } else if (del.br_state == XFS_EXT_UNWRITTEN) { _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 7/9] xfs: do not do pointer arithmetics on extent records 2011-05-11 15:04 ` [PATCH 7/9] xfs: do not do pointer arithmetics on extent records Christoph Hellwig @ 2011-05-12 7:23 ` Lachlan McIlroy 2011-05-25 0:31 ` Alex Elder 1 sibling, 0 replies; 30+ messages in thread From: Lachlan McIlroy @ 2011-05-12 7:23 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs Looks good. Nice catch too. ----- Original Message ----- > We need to call xfs_iext_get_ext for the previous extent to get a > valid > pointer, and can't just do pointer arithmetics as they might be in > different pages. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Index: xfs/fs/xfs/xfs_bmap.c > =================================================================== > --- xfs.orig/fs/xfs/xfs_bmap.c 2011-05-11 10:16:58.847733078 +0200 > +++ xfs/fs/xfs/xfs_bmap.c 2011-05-11 10:17:04.803235692 +0200 > @@ -5145,9 +5145,12 @@ xfs_bunmapi( > */ > ASSERT(bno >= del.br_blockcount); > bno -= del.br_blockcount; > - if (bno < got.br_startoff) { > - if (--lastx >= 0) > - xfs_bmbt_get_all(--ep, &got); > + if (got.br_startoff > bno) { > + if (--lastx >= 0) { > + ep = xfs_iext_get_ext(ifp, > + lastx); > + xfs_bmbt_get_all(ep, &got); > + } > } > continue; > } else if (del.br_state == XFS_EXT_UNWRITTEN) { > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 7/9] xfs: do not do pointer arithmetics on extent records 2011-05-11 15:04 ` [PATCH 7/9] xfs: do not do pointer arithmetics on extent records Christoph Hellwig 2011-05-12 7:23 ` Lachlan McIlroy @ 2011-05-25 0:31 ` Alex Elder 1 sibling, 0 replies; 30+ messages in thread From: Alex Elder @ 2011-05-25 0:31 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Wed, 2011-05-11 at 11:04 -0400, Christoph Hellwig wrote: > We need to call xfs_iext_get_ext for the previous extent to get a valid > pointer, and can't just do pointer arithmetics as they might be in > different pages. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Wow, that one's nasty. Did someone find this by hitting it? Looks good. Reviewed-by: Alex Elder <aelder@sgi.com> > Index: xfs/fs/xfs/xfs_bmap.c > =================================================================== > --- xfs.orig/fs/xfs/xfs_bmap.c 2011-05-11 10:16:58.847733078 +0200 > +++ xfs/fs/xfs/xfs_bmap.c 2011-05-11 10:17:04.803235692 +0200 > @@ -5145,9 +5145,12 @@ xfs_bunmapi( > */ > ASSERT(bno >= del.br_blockcount); > bno -= del.br_blockcount; > - if (bno < got.br_startoff) { > - if (--lastx >= 0) > - xfs_bmbt_get_all(--ep, &got); > + if (got.br_startoff > bno) { > + if (--lastx >= 0) { > + ep = xfs_iext_get_ext(ifp, > + lastx); > + xfs_bmbt_get_all(ep, &got); > + } > } > continue; > } else if (del.br_state == XFS_EXT_UNWRITTEN) { > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 8/9] xfs: fix up asserts in xfs_iflush_fork 2011-05-11 15:04 [PATCH 0/9] extent buffer indexing fixes Christoph Hellwig ` (6 preceding siblings ...) 2011-05-11 15:04 ` [PATCH 7/9] xfs: do not do pointer arithmetics on extent records Christoph Hellwig @ 2011-05-11 15:04 ` Christoph Hellwig 2011-05-12 7:24 ` Lachlan McIlroy 2011-05-25 0:32 ` Alex Elder 2011-05-11 15:04 ` [PATCH 9/9] xfs: check for valid indices in xfs_iext_get_ext and xfs_iext_idx_to_irec Christoph Hellwig 8 siblings, 2 replies; 30+ messages in thread From: Christoph Hellwig @ 2011-05-11 15:04 UTC (permalink / raw) To: xfs [-- Attachment #1: xfs-fix-ep-access-5 --] [-- Type: text/plain, Size: 1141 bytes --] Remove asserts in xfs_iflush_fork that would call xfs_iext_get_ext with a potentially invalid extent buffer index. Based on an earlier patch from Lachlan McIlroy. Signed-off-by: Christoph Hellwig <hch@lst.de> Index: xfs/fs/xfs/xfs_inode.c =================================================================== --- xfs.orig/fs/xfs/xfs_inode.c 2011-05-11 10:18:39.555233397 +0200 +++ xfs/fs/xfs/xfs_inode.c 2011-05-11 12:04:24.099733330 +0200 @@ -2557,12 +2557,9 @@ xfs_iflush_fork( case XFS_DINODE_FMT_EXTENTS: ASSERT((ifp->if_flags & XFS_IFEXTENTS) || !(iip->ili_format.ilf_fields & extflag[whichfork])); - ASSERT((xfs_iext_get_ext(ifp, 0) != NULL) || - (ifp->if_bytes == 0)); - ASSERT((xfs_iext_get_ext(ifp, 0) == NULL) || - (ifp->if_bytes > 0)); if ((iip->ili_format.ilf_fields & extflag[whichfork]) && (ifp->if_bytes > 0)) { + ASSERT(xfs_iext_get_ext(ifp, 0)); ASSERT(XFS_IFORK_NEXTENTS(ip, whichfork) > 0); (void)xfs_iextents_copy(ip, (xfs_bmbt_rec_t *)cp, whichfork); _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 8/9] xfs: fix up asserts in xfs_iflush_fork 2011-05-11 15:04 ` [PATCH 8/9] xfs: fix up asserts in xfs_iflush_fork Christoph Hellwig @ 2011-05-12 7:24 ` Lachlan McIlroy 2011-05-25 0:32 ` Alex Elder 1 sibling, 0 replies; 30+ messages in thread From: Lachlan McIlroy @ 2011-05-12 7:24 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs Looks good. ----- Original Message ----- > Remove asserts in xfs_iflush_fork that would call xfs_iext_get_ext > with > a potentially invalid extent buffer index. > > Based on an earlier patch from Lachlan McIlroy. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Index: xfs/fs/xfs/xfs_inode.c > =================================================================== > --- xfs.orig/fs/xfs/xfs_inode.c 2011-05-11 10:18:39.555233397 +0200 > +++ xfs/fs/xfs/xfs_inode.c 2011-05-11 12:04:24.099733330 +0200 > @@ -2557,12 +2557,9 @@ xfs_iflush_fork( > case XFS_DINODE_FMT_EXTENTS: > ASSERT((ifp->if_flags & XFS_IFEXTENTS) || > !(iip->ili_format.ilf_fields & extflag[whichfork])); > - ASSERT((xfs_iext_get_ext(ifp, 0) != NULL) || > - (ifp->if_bytes == 0)); > - ASSERT((xfs_iext_get_ext(ifp, 0) == NULL) || > - (ifp->if_bytes > 0)); > if ((iip->ili_format.ilf_fields & extflag[whichfork]) && > (ifp->if_bytes > 0)) { > + ASSERT(xfs_iext_get_ext(ifp, 0)); > ASSERT(XFS_IFORK_NEXTENTS(ip, whichfork) > 0); > (void)xfs_iextents_copy(ip, (xfs_bmbt_rec_t *)cp, > whichfork); > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 8/9] xfs: fix up asserts in xfs_iflush_fork 2011-05-11 15:04 ` [PATCH 8/9] xfs: fix up asserts in xfs_iflush_fork Christoph Hellwig 2011-05-12 7:24 ` Lachlan McIlroy @ 2011-05-25 0:32 ` Alex Elder 1 sibling, 0 replies; 30+ messages in thread From: Alex Elder @ 2011-05-25 0:32 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Wed, 2011-05-11 at 11:04 -0400, Christoph Hellwig wrote: > Remove asserts in xfs_iflush_fork that would call xfs_iext_get_ext with > a potentially invalid extent buffer index. > > Based on an earlier patch from Lachlan McIlroy. > > Signed-off-by: Christoph Hellwig <hch@lst.de> This states that it's invalid to attempt to request the first (0th) extent, because it's invalid if the fork has no extents, right? Looks good. Reviewed-by: Alex Elder <aelder@sgi.com> > Index: xfs/fs/xfs/xfs_inode.c > =================================================================== > --- xfs.orig/fs/xfs/xfs_inode.c 2011-05-11 10:18:39.555233397 +0200 > +++ xfs/fs/xfs/xfs_inode.c 2011-05-11 12:04:24.099733330 +0200 > @@ -2557,12 +2557,9 @@ xfs_iflush_fork( > case XFS_DINODE_FMT_EXTENTS: > ASSERT((ifp->if_flags & XFS_IFEXTENTS) || > !(iip->ili_format.ilf_fields & extflag[whichfork])); > - ASSERT((xfs_iext_get_ext(ifp, 0) != NULL) || > - (ifp->if_bytes == 0)); > - ASSERT((xfs_iext_get_ext(ifp, 0) == NULL) || > - (ifp->if_bytes > 0)); > if ((iip->ili_format.ilf_fields & extflag[whichfork]) && > (ifp->if_bytes > 0)) { > + ASSERT(xfs_iext_get_ext(ifp, 0)); > ASSERT(XFS_IFORK_NEXTENTS(ip, whichfork) > 0); > (void)xfs_iextents_copy(ip, (xfs_bmbt_rec_t *)cp, > whichfork); > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 9/9] xfs: check for valid indices in xfs_iext_get_ext and xfs_iext_idx_to_irec 2011-05-11 15:04 [PATCH 0/9] extent buffer indexing fixes Christoph Hellwig ` (7 preceding siblings ...) 2011-05-11 15:04 ` [PATCH 8/9] xfs: fix up asserts in xfs_iflush_fork Christoph Hellwig @ 2011-05-11 15:04 ` Christoph Hellwig 2011-05-12 7:26 ` Lachlan McIlroy 2011-05-25 0:32 ` Alex Elder 8 siblings, 2 replies; 30+ messages in thread From: Christoph Hellwig @ 2011-05-11 15:04 UTC (permalink / raw) To: xfs [-- Attachment #1: xfs-iext-asserts-1 --] [-- Type: text/plain, Size: 1242 bytes --] Based on an earlier patch from Lachlan McIlroy. Signed-off-by: Christoph Hellwig <hch@lst.de> Index: xfs/fs/xfs/xfs_inode.c =================================================================== --- xfs.orig/fs/xfs/xfs_inode.c 2011-05-11 12:05:12.943735034 +0200 +++ xfs/fs/xfs/xfs_inode.c 2011-05-11 12:05:28.327733646 +0200 @@ -3108,6 +3108,8 @@ xfs_iext_get_ext( xfs_extnum_t idx) /* index of target extent */ { ASSERT(idx >= 0); + ASSERT(idx < ifp->if_bytes / sizeof(xfs_bmbt_rec_t)); + if ((ifp->if_flags & XFS_IFEXTIREC) && (idx == 0)) { return ifp->if_u1.if_ext_irec->er_extbuf; } else if (ifp->if_flags & XFS_IFEXTIREC) { @@ -3881,8 +3883,10 @@ xfs_iext_idx_to_irec( xfs_extnum_t page_idx = *idxp; /* extent index in target list */ ASSERT(ifp->if_flags & XFS_IFEXTIREC); - ASSERT(page_idx >= 0 && page_idx <= - ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t)); + ASSERT(page_idx >= 0); + ASSERT(page_idx <= ifp->if_bytes / sizeof(xfs_bmbt_rec_t)); + ASSERT(page_idx < ifp->if_bytes / sizeof(xfs_bmbt_rec_t) || realloc); + nlists = ifp->if_real_bytes / XFS_IEXT_BUFSZ; erp_idx = 0; low = 0; _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 9/9] xfs: check for valid indices in xfs_iext_get_ext and xfs_iext_idx_to_irec 2011-05-11 15:04 ` [PATCH 9/9] xfs: check for valid indices in xfs_iext_get_ext and xfs_iext_idx_to_irec Christoph Hellwig @ 2011-05-12 7:26 ` Lachlan McIlroy 2011-05-25 0:32 ` Alex Elder 1 sibling, 0 replies; 30+ messages in thread From: Lachlan McIlroy @ 2011-05-12 7:26 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs Looks good too. Christoph, thanks for following up on these fixes - I didn't know they didn't make it in. ----- Original Message ----- > Based on an earlier patch from Lachlan McIlroy. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Index: xfs/fs/xfs/xfs_inode.c > =================================================================== > --- xfs.orig/fs/xfs/xfs_inode.c 2011-05-11 12:05:12.943735034 +0200 > +++ xfs/fs/xfs/xfs_inode.c 2011-05-11 12:05:28.327733646 +0200 > @@ -3108,6 +3108,8 @@ xfs_iext_get_ext( > xfs_extnum_t idx) /* index of target extent */ > { > ASSERT(idx >= 0); > + ASSERT(idx < ifp->if_bytes / sizeof(xfs_bmbt_rec_t)); > + > if ((ifp->if_flags & XFS_IFEXTIREC) && (idx == 0)) { > return ifp->if_u1.if_ext_irec->er_extbuf; > } else if (ifp->if_flags & XFS_IFEXTIREC) { > @@ -3881,8 +3883,10 @@ xfs_iext_idx_to_irec( > xfs_extnum_t page_idx = *idxp; /* extent index in target list */ > > ASSERT(ifp->if_flags & XFS_IFEXTIREC); > - ASSERT(page_idx >= 0 && page_idx <= > - ifp->if_bytes / (uint)sizeof(xfs_bmbt_rec_t)); > + ASSERT(page_idx >= 0); > + ASSERT(page_idx <= ifp->if_bytes / sizeof(xfs_bmbt_rec_t)); > + ASSERT(page_idx < ifp->if_bytes / sizeof(xfs_bmbt_rec_t) || > realloc); > + > nlists = ifp->if_real_bytes / XFS_IEXT_BUFSZ; > erp_idx = 0; > low = 0; > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 9/9] xfs: check for valid indices in xfs_iext_get_ext and xfs_iext_idx_to_irec 2011-05-11 15:04 ` [PATCH 9/9] xfs: check for valid indices in xfs_iext_get_ext and xfs_iext_idx_to_irec Christoph Hellwig 2011-05-12 7:26 ` Lachlan McIlroy @ 2011-05-25 0:32 ` Alex Elder 1 sibling, 0 replies; 30+ messages in thread From: Alex Elder @ 2011-05-25 0:32 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Wed, 2011-05-11 at 11:04 -0400, Christoph Hellwig wrote: > Based on an earlier patch from Lachlan McIlroy. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Looks good. Reviewed-by: Alex Elder <aelder@sgi.com> _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2011-05-25 1:15 UTC | newest] Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-05-11 15:04 [PATCH 0/9] extent buffer indexing fixes Christoph Hellwig 2011-05-11 15:04 ` [PATCH 1/9] xfs: remove the unused XFS_BMAPI_RSVBLOCKS flag Christoph Hellwig 2011-05-20 20:17 ` Alex Elder 2011-05-11 15:04 ` [PATCH 2/9] xfs: remove if_lastex Christoph Hellwig 2011-05-20 20:17 ` Alex Elder 2011-05-23 8:52 ` [PATCH 2/9 v2] " Christoph Hellwig 2011-05-25 1:14 ` Alex Elder 2011-05-11 15:04 ` [PATCH 3/9] xfs: correctly decrement the extent buffer index in xfs_bmap_del_extent Christoph Hellwig 2011-05-12 6:50 ` Lachlan McIlroy 2011-05-12 6:54 ` Lachlan McIlroy 2011-05-12 7:17 ` Lachlan McIlroy 2011-05-25 0:27 ` Alex Elder 2011-05-11 15:04 ` [PATCH 4/9] xfs: do not use unchecked extent indices in xfs_bmap_add_extent_* Christoph Hellwig 2011-05-12 7:31 ` Lachlan McIlroy 2011-05-25 0:30 ` Alex Elder 2011-05-11 15:04 ` [PATCH 5/9] xfs: do not use unchecked extent indices in xfs_bmapi Christoph Hellwig 2011-05-12 7:20 ` Lachlan McIlroy 2011-05-25 0:31 ` Alex Elder 2011-05-11 15:04 ` [PATCH 6/9] xfs: do not use unchecked extent indices in xfs_bunmapi Christoph Hellwig 2011-05-12 7:22 ` Lachlan McIlroy 2011-05-25 0:31 ` Alex Elder 2011-05-11 15:04 ` [PATCH 7/9] xfs: do not do pointer arithmetics on extent records Christoph Hellwig 2011-05-12 7:23 ` Lachlan McIlroy 2011-05-25 0:31 ` Alex Elder 2011-05-11 15:04 ` [PATCH 8/9] xfs: fix up asserts in xfs_iflush_fork Christoph Hellwig 2011-05-12 7:24 ` Lachlan McIlroy 2011-05-25 0:32 ` Alex Elder 2011-05-11 15:04 ` [PATCH 9/9] xfs: check for valid indices in xfs_iext_get_ext and xfs_iext_idx_to_irec Christoph Hellwig 2011-05-12 7:26 ` Lachlan McIlroy 2011-05-25 0:32 ` Alex Elder
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.