[XFS] Ensure the inode is joined in xfs_itruncate_finish
On success, we still need to join the inode to the current transaction in xfs_itruncate_finish(). Fixes regression from error handling changes. SGI-PV: 980084 SGI-Modid: xfs-linux-melb:xfs-kern:30845a Signed-off-by: David Chinner <dgc@sgi.com> Signed-off-by: Christoph Hellwig <hch@infradead.org> Signed-off-by: Lachlan McIlroy <lachlan@sgi.com>
This commit is contained in:
parent
7e20694d91
commit
f6485057c5
|
@ -1464,51 +1464,50 @@ xfs_itruncate_start(
|
|||
}
|
||||
|
||||
/*
|
||||
* Shrink the file to the given new_size. The new
|
||||
* size must be smaller than the current size.
|
||||
* This will free up the underlying blocks
|
||||
* in the removed range after a call to xfs_itruncate_start()
|
||||
* or xfs_atruncate_start().
|
||||
* Shrink the file to the given new_size. The new size must be smaller than
|
||||
* the current size. This will free up the underlying blocks in the removed
|
||||
* range after a call to xfs_itruncate_start() or xfs_atruncate_start().
|
||||
*
|
||||
* The transaction passed to this routine must have made
|
||||
* a permanent log reservation of at least XFS_ITRUNCATE_LOG_RES.
|
||||
* This routine may commit the given transaction and
|
||||
* start new ones, so make sure everything involved in
|
||||
* the transaction is tidy before calling here.
|
||||
* Some transaction will be returned to the caller to be
|
||||
* committed. The incoming transaction must already include
|
||||
* the inode, and both inode locks must be held exclusively.
|
||||
* The inode must also be "held" within the transaction. On
|
||||
* return the inode will be "held" within the returned transaction.
|
||||
* This routine does NOT require any disk space to be reserved
|
||||
* for it within the transaction.
|
||||
* The transaction passed to this routine must have made a permanent log
|
||||
* reservation of at least XFS_ITRUNCATE_LOG_RES. This routine may commit the
|
||||
* given transaction and start new ones, so make sure everything involved in
|
||||
* the transaction is tidy before calling here. Some transaction will be
|
||||
* returned to the caller to be committed. The incoming transaction must
|
||||
* already include the inode, and both inode locks must be held exclusively.
|
||||
* The inode must also be "held" within the transaction. On return the inode
|
||||
* will be "held" within the returned transaction. This routine does NOT
|
||||
* require any disk space to be reserved for it within the transaction.
|
||||
*
|
||||
* The fork parameter must be either xfs_attr_fork or xfs_data_fork,
|
||||
* and it indicates the fork which is to be truncated. For the
|
||||
* attribute fork we only support truncation to size 0.
|
||||
* The fork parameter must be either xfs_attr_fork or xfs_data_fork, and it
|
||||
* indicates the fork which is to be truncated. For the attribute fork we only
|
||||
* support truncation to size 0.
|
||||
*
|
||||
* We use the sync parameter to indicate whether or not the first
|
||||
* transaction we perform might have to be synchronous. For the attr fork,
|
||||
* it needs to be so if the unlink of the inode is not yet known to be
|
||||
* permanent in the log. This keeps us from freeing and reusing the
|
||||
* blocks of the attribute fork before the unlink of the inode becomes
|
||||
* permanent.
|
||||
* We use the sync parameter to indicate whether or not the first transaction
|
||||
* we perform might have to be synchronous. For the attr fork, it needs to be
|
||||
* so if the unlink of the inode is not yet known to be permanent in the log.
|
||||
* This keeps us from freeing and reusing the blocks of the attribute fork
|
||||
* before the unlink of the inode becomes permanent.
|
||||
*
|
||||
* For the data fork, we normally have to run synchronously if we're
|
||||
* being called out of the inactive path or we're being called
|
||||
* out of the create path where we're truncating an existing file.
|
||||
* Either way, the truncate needs to be sync so blocks don't reappear
|
||||
* in the file with altered data in case of a crash. wsync filesystems
|
||||
* can run the first case async because anything that shrinks the inode
|
||||
* has to run sync so by the time we're called here from inactive, the
|
||||
* inode size is permanently set to 0.
|
||||
* For the data fork, we normally have to run synchronously if we're being
|
||||
* called out of the inactive path or we're being called out of the create path
|
||||
* where we're truncating an existing file. Either way, the truncate needs to
|
||||
* be sync so blocks don't reappear in the file with altered data in case of a
|
||||
* crash. wsync filesystems can run the first case async because anything that
|
||||
* shrinks the inode has to run sync so by the time we're called here from
|
||||
* inactive, the inode size is permanently set to 0.
|
||||
*
|
||||
* Calls from the truncate path always need to be sync unless we're
|
||||
* in a wsync filesystem and the file has already been unlinked.
|
||||
* Calls from the truncate path always need to be sync unless we're in a wsync
|
||||
* filesystem and the file has already been unlinked.
|
||||
*
|
||||
* The caller is responsible for correctly setting the sync parameter.
|
||||
* It gets too hard for us to guess here which path we're being called
|
||||
* out of just based on inode state.
|
||||
* The caller is responsible for correctly setting the sync parameter. It gets
|
||||
* too hard for us to guess here which path we're being called out of just
|
||||
* based on inode state.
|
||||
*
|
||||
* If we get an error, we must return with the inode locked and linked into the
|
||||
* current transaction. This keeps things simple for the higher level code,
|
||||
* because it always knows that the inode is locked and held in the transaction
|
||||
* that returns to it whether errors occur or not. We don't mark the inode
|
||||
* dirty on error so that transactions can be easily aborted if possible.
|
||||
*/
|
||||
int
|
||||
xfs_itruncate_finish(
|
||||
|
@ -1687,45 +1686,51 @@ xfs_itruncate_finish(
|
|||
*/
|
||||
error = xfs_bmap_finish(tp, &free_list, &committed);
|
||||
ntp = *tp;
|
||||
if (committed) {
|
||||
/* link the inode into the next xact in the chain */
|
||||
xfs_trans_ijoin(ntp, ip,
|
||||
XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
|
||||
xfs_trans_ihold(ntp, ip);
|
||||
}
|
||||
|
||||
if (error) {
|
||||
/*
|
||||
* If the bmap finish call encounters an error,
|
||||
* return to the caller where the transaction
|
||||
* can be properly aborted. We just need to
|
||||
* make sure we're not holding any resources
|
||||
* that we were not when we came in.
|
||||
* If the bmap finish call encounters an error, return
|
||||
* to the caller where the transaction can be properly
|
||||
* aborted. We just need to make sure we're not
|
||||
* holding any resources that we were not when we came
|
||||
* in.
|
||||
*
|
||||
* Aborting from this point might lose some
|
||||
* blocks in the file system, but oh well.
|
||||
* Aborting from this point might lose some blocks in
|
||||
* the file system, but oh well.
|
||||
*/
|
||||
xfs_bmap_cancel(&free_list);
|
||||
if (committed)
|
||||
goto error_join;
|
||||
return error;
|
||||
}
|
||||
|
||||
if (committed) {
|
||||
/*
|
||||
* The first xact was committed, so add the inode to
|
||||
* the new one. Mark it dirty so it will be logged and
|
||||
* Mark the inode dirty so it will be logged and
|
||||
* moved forward in the log as part of every commit.
|
||||
*/
|
||||
xfs_trans_ijoin(ntp, ip,
|
||||
XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
|
||||
xfs_trans_ihold(ntp, ip);
|
||||
xfs_trans_log_inode(ntp, ip, XFS_ILOG_CORE);
|
||||
}
|
||||
|
||||
ntp = xfs_trans_dup(ntp);
|
||||
error = xfs_trans_commit(*tp, 0);
|
||||
*tp = ntp;
|
||||
if (error)
|
||||
goto error_join;
|
||||
error = xfs_trans_reserve(ntp, 0, XFS_ITRUNCATE_LOG_RES(mp), 0,
|
||||
XFS_TRANS_PERM_LOG_RES,
|
||||
XFS_ITRUNCATE_LOG_COUNT);
|
||||
if (error)
|
||||
goto error_join;
|
||||
|
||||
/* link the inode into the next transaction in the chain */
|
||||
xfs_trans_ijoin(ntp, ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
|
||||
xfs_trans_ihold(ntp, ip);
|
||||
|
||||
if (!error)
|
||||
error = xfs_trans_reserve(ntp, 0,
|
||||
XFS_ITRUNCATE_LOG_RES(mp), 0,
|
||||
XFS_TRANS_PERM_LOG_RES,
|
||||
XFS_ITRUNCATE_LOG_COUNT);
|
||||
if (error)
|
||||
return error;
|
||||
}
|
||||
/*
|
||||
* Only update the size in the case of the data fork, but
|
||||
|
@ -1757,18 +1762,6 @@ xfs_itruncate_finish(
|
|||
(ip->i_d.di_nextents == 0));
|
||||
xfs_itrunc_trace(XFS_ITRUNC_FINISH2, ip, 0, new_size, 0, 0);
|
||||
return 0;
|
||||
|
||||
error_join:
|
||||
/*
|
||||
* Add the inode being truncated to the next chained transaction. This
|
||||
* keeps things simple for the higher level code, because it always
|
||||
* knows that the inode is locked and held in the transaction that
|
||||
* returns to it whether errors occur or not. We don't mark the inode
|
||||
* dirty so that this transaction can be easily aborted if possible.
|
||||
*/
|
||||
xfs_trans_ijoin(ntp, ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
|
||||
xfs_trans_ihold(ntp, ip);
|
||||
return error;
|
||||
}
|
||||
|
||||
|
||||
|
|
Loading…
Reference in New Issue