* [PATCH v3 1/4] xfs: push down inactive transaction mgmt for remote symlinks
2013-09-20 15:06 [PATCH v3 0/4] xfs: rework xfs_inactive() Brian Foster
@ 2013-09-20 15:06 ` Brian Foster
2013-09-30 0:29 ` Dave Chinner
2013-09-20 15:06 ` [PATCH v3 2/4] xfs: push down inactive transaction mgmt for truncate Brian Foster
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Brian Foster @ 2013-09-20 15:06 UTC (permalink / raw)
To: xfs
Push down the transaction management for remote symlinks from
xfs_inactive() down to xfs_inactive_symlink_rmt(). The latter is
cleaned up to avoid transaction management intended for the
calling context (i.e., trans duplication, reservation, item
attachment).
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/xfs_inode.c | 15 ++++-----
fs/xfs/xfs_symlink.c | 86 ++++++++++++++++++++++++----------------------------
fs/xfs/xfs_symlink.h | 2 +-
3 files changed, 49 insertions(+), 54 deletions(-)
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index e3d7538..30db70e 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1724,9 +1724,14 @@ xfs_inactive(
if (error)
return VN_INACTIVE_CACHE;
+ if (S_ISLNK(ip->i_d.di_mode)) {
+ error = xfs_inactive_symlink(ip);
+ if (error)
+ goto out;
+ }
+
tp = xfs_trans_alloc(mp, XFS_TRANS_INACTIVE);
- resp = (truncate || S_ISLNK(ip->i_d.di_mode)) ?
- &M_RES(mp)->tr_itruncate : &M_RES(mp)->tr_ifree;
+ resp = truncate ? &M_RES(mp)->tr_itruncate : &M_RES(mp)->tr_ifree;
error = xfs_trans_reserve(tp, resp, 0, 0);
if (error) {
@@ -1738,11 +1743,7 @@ xfs_inactive(
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, 0);
- if (S_ISLNK(ip->i_d.di_mode)) {
- error = xfs_inactive_symlink(ip, &tp);
- if (error)
- goto out_cancel;
- } else if (truncate) {
+ if (truncate) {
ip->i_d.di_size = 0;
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index f622a97..ded282b 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -424,8 +424,7 @@ xfs_symlink(
*/
STATIC int
xfs_inactive_symlink_rmt(
- xfs_inode_t *ip,
- xfs_trans_t **tpp)
+ struct xfs_inode *ip)
{
xfs_buf_t *bp;
int committed;
@@ -437,11 +436,9 @@ xfs_inactive_symlink_rmt(
xfs_mount_t *mp;
xfs_bmbt_irec_t mval[XFS_SYMLINK_MAPS];
int nmaps;
- xfs_trans_t *ntp;
int size;
xfs_trans_t *tp;
- tp = *tpp;
mp = ip->i_mount;
ASSERT(ip->i_df.if_flags & XFS_IFEXTENTS);
/*
@@ -453,6 +450,16 @@ xfs_inactive_symlink_rmt(
*/
ASSERT(ip->i_d.di_nextents > 0 && ip->i_d.di_nextents <= 2);
+ tp = xfs_trans_alloc(mp, XFS_TRANS_INACTIVE);
+ error = xfs_trans_reserve(tp, &M_RES(mp)->tr_itruncate, 0, 0);
+ if (error) {
+ xfs_trans_cancel(tp, 0);
+ return error;
+ }
+
+ xfs_ilock(ip, XFS_ILOCK_EXCL);
+ xfs_trans_ijoin(tp, ip, 0);
+
/*
* Lock the inode, fix the size, and join it to the transaction.
* Hold it so in the normal path, we still have it locked for
@@ -471,7 +478,7 @@ xfs_inactive_symlink_rmt(
error = xfs_bmapi_read(ip, 0, xfs_symlink_blocks(mp, size),
mval, &nmaps, 0);
if (error)
- goto error0;
+ goto error_trans_cancel;
/*
* Invalidate the block(s). No validation is done.
*/
@@ -481,22 +488,24 @@ xfs_inactive_symlink_rmt(
XFS_FSB_TO_BB(mp, mval[i].br_blockcount), 0);
if (!bp) {
error = ENOMEM;
- goto error1;
+ goto error_bmap_cancel;
}
xfs_trans_binval(tp, bp);
}
/*
* Unmap the dead block(s) to the free_list.
*/
- if ((error = xfs_bunmapi(tp, ip, 0, size, XFS_BMAPI_METADATA, nmaps,
- &first_block, &free_list, &done)))
- goto error1;
+ error = xfs_bunmapi(tp, ip, 0, size, XFS_BMAPI_METADATA, nmaps,
+ &first_block, &free_list, &done);
+ if (error)
+ goto error_bmap_cancel;
ASSERT(done);
/*
* Commit the first transaction. This logs the EFI and the inode.
*/
- if ((error = xfs_bmap_finish(&tp, &free_list, &committed)))
- goto error1;
+ error = xfs_bmap_finish(&tp, &free_list, &committed);
+ if (error)
+ goto error_bmap_cancel;
/*
* The transaction must have been committed, since there were
* actually extents freed by xfs_bunmapi. See xfs_bmap_finish.
@@ -511,26 +520,13 @@ xfs_inactive_symlink_rmt(
xfs_trans_ijoin(tp, ip, 0);
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
/*
- * Get a new, empty transaction to return to our caller.
- */
- ntp = xfs_trans_dup(tp);
- /*
* Commit the transaction containing extent freeing and EFDs.
- * If we get an error on the commit here or on the reserve below,
- * we need to unlock the inode since the new transaction doesn't
- * have the inode attached.
*/
- error = xfs_trans_commit(tp, 0);
- tp = ntp;
+ error = xfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES);
if (error) {
ASSERT(XFS_FORCED_SHUTDOWN(mp));
- goto error0;
+ goto error_unlock;
}
- /*
- * transaction commit worked ok so we can drop the extra ticket
- * reference that we gained in xfs_trans_dup()
- */
- xfs_log_ticket_put(tp->t_ticket);
/*
* Remove the memory for extent descriptions (just bookkeeping).
@@ -538,23 +534,16 @@ xfs_inactive_symlink_rmt(
if (ip->i_df.if_bytes)
xfs_idata_realloc(ip, -ip->i_df.if_bytes, XFS_DATA_FORK);
ASSERT(ip->i_df.if_bytes == 0);
- /*
- * Put an itruncate log reservation in the new transaction
- * for our caller.
- */
- error = xfs_trans_reserve(tp, &M_RES(mp)->tr_itruncate, 0, 0);
- if (error) {
- ASSERT(XFS_FORCED_SHUTDOWN(mp));
- goto error0;
- }
- xfs_trans_ijoin(tp, ip, 0);
- *tpp = tp;
+ xfs_iunlock(ip, XFS_ILOCK_EXCL);
return 0;
- error1:
+error_bmap_cancel:
xfs_bmap_cancel(&free_list);
- error0:
+error_trans_cancel:
+ xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES | XFS_TRANS_ABORT);
+error_unlock:
+ xfs_iunlock(ip, XFS_ILOCK_EXCL);
return error;
}
@@ -563,41 +552,46 @@ xfs_inactive_symlink_rmt(
*/
int
xfs_inactive_symlink(
- struct xfs_inode *ip,
- struct xfs_trans **tp)
+ struct xfs_inode *ip)
{
struct xfs_mount *mp = ip->i_mount;
int pathlen;
trace_xfs_inactive_symlink(ip);
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
-
if (XFS_FORCED_SHUTDOWN(mp))
return XFS_ERROR(EIO);
+ xfs_ilock(ip, XFS_ILOCK_EXCL);
+
/*
* Zero length symlinks _can_ exist.
*/
pathlen = (int)ip->i_d.di_size;
- if (!pathlen)
+ if (!pathlen) {
+ xfs_iunlock(ip, XFS_ILOCK_EXCL);
return 0;
+ }
if (pathlen < 0 || pathlen > MAXPATHLEN) {
xfs_alert(mp, "%s: inode (0x%llx) bad symlink length (%d)",
__func__, (unsigned long long)ip->i_ino, pathlen);
+ xfs_iunlock(ip, XFS_ILOCK_EXCL);
ASSERT(0);
return XFS_ERROR(EFSCORRUPTED);
}
if (ip->i_df.if_flags & XFS_IFINLINE) {
- if (ip->i_df.if_bytes > 0)
+ if (ip->i_df.if_bytes > 0)
xfs_idata_realloc(ip, -(ip->i_df.if_bytes),
XFS_DATA_FORK);
+ xfs_iunlock(ip, XFS_ILOCK_EXCL);
ASSERT(ip->i_df.if_bytes == 0);
return 0;
}
+ xfs_iunlock(ip, XFS_ILOCK_EXCL);
+
/* remove the remote symlink */
- return xfs_inactive_symlink_rmt(ip, tp);
+ return xfs_inactive_symlink_rmt(ip);
}
diff --git a/fs/xfs/xfs_symlink.h b/fs/xfs/xfs_symlink.h
index 99338ba..e75245d 100644
--- a/fs/xfs/xfs_symlink.h
+++ b/fs/xfs/xfs_symlink.h
@@ -22,6 +22,6 @@
int xfs_symlink(struct xfs_inode *dp, struct xfs_name *link_name,
const char *target_path, umode_t mode, struct xfs_inode **ipp);
int xfs_readlink(struct xfs_inode *ip, char *link);
-int xfs_inactive_symlink(struct xfs_inode *ip, struct xfs_trans **tpp);
+int xfs_inactive_symlink(struct xfs_inode *ip);
#endif /* __XFS_SYMLINK_H */
--
1.8.1.4
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3 1/4] xfs: push down inactive transaction mgmt for remote symlinks
2013-09-20 15:06 ` [PATCH v3 1/4] xfs: push down inactive transaction mgmt for remote symlinks Brian Foster
@ 2013-09-30 0:29 ` Dave Chinner
0 siblings, 0 replies; 9+ messages in thread
From: Dave Chinner @ 2013-09-30 0:29 UTC (permalink / raw)
To: Brian Foster; +Cc: xfs
On Fri, Sep 20, 2013 at 11:06:09AM -0400, Brian Foster wrote:
> Push down the transaction management for remote symlinks from
> xfs_inactive() down to xfs_inactive_symlink_rmt(). The latter is
> cleaned up to avoid transaction management intended for the
> calling context (i.e., trans duplication, reservation, item
> attachment).
>
> Signed-off-by: Brian Foster <bfoster@redhat.com>
Looks good. One minor quibble if you need to respin the patches
again, but otherwise:
Reviewed-by: Dave Chinner <dchinner@redhat.com>
>
> @@ -563,41 +552,46 @@ xfs_inactive_symlink_rmt(
> */
> int
> xfs_inactive_symlink(
> - struct xfs_inode *ip,
> - struct xfs_trans **tp)
> + struct xfs_inode *ip)
> {
> struct xfs_mount *mp = ip->i_mount;
> int pathlen;
>
> trace_xfs_inactive_symlink(ip);
>
> - ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> -
> if (XFS_FORCED_SHUTDOWN(mp))
> return XFS_ERROR(EIO);
>
> + xfs_ilock(ip, XFS_ILOCK_EXCL);
> +
> /*
> * Zero length symlinks _can_ exist.
> */
> pathlen = (int)ip->i_d.di_size;
> - if (!pathlen)
> + if (!pathlen) {
> + xfs_iunlock(ip, XFS_ILOCK_EXCL);
> return 0;
> + }
Minor quibble: this repeated "unlock, return error" pattern could
be done with:
....
if (!pathlen)
goto out_unlock;
....
/* remove the remote symlink */
return xfs_inactive_symlink_rmt(ip);
out_unlock:
xfs_iunlock(ip, XFS_ILOCK_EXCL);
return error;
}
But, like I said, there's no need to do this unless I find other
things in the rest of the series that require a respin...
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 2/4] xfs: push down inactive transaction mgmt for truncate
2013-09-20 15:06 [PATCH v3 0/4] xfs: rework xfs_inactive() Brian Foster
2013-09-20 15:06 ` [PATCH v3 1/4] xfs: push down inactive transaction mgmt for remote symlinks Brian Foster
@ 2013-09-20 15:06 ` Brian Foster
2013-09-30 0:32 ` Dave Chinner
2013-09-20 15:06 ` [PATCH v3 3/4] xfs: push down inactive transaction mgmt for ifree Brian Foster
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Brian Foster @ 2013-09-20 15:06 UTC (permalink / raw)
To: xfs
Create the new xfs_inactive_truncate() function to handle the
truncate portion of xfs_inactive(). Push the locking and
transaction management into the new function.
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/xfs_inode.c | 117 +++++++++++++++++++++++++++++++----------------------
1 file changed, 68 insertions(+), 49 deletions(-)
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 30db70e..ff4e729 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1663,6 +1663,58 @@ xfs_release(
}
/*
+ * xfs_inactive_truncate
+ *
+ * Called to perform a truncate when an inode becomes unlinked.
+ */
+STATIC int
+xfs_inactive_truncate(
+ struct xfs_inode *ip)
+{
+ struct xfs_mount *mp = ip->i_mount;
+ struct xfs_trans *tp;
+ int error;
+
+ tp = xfs_trans_alloc(mp, XFS_TRANS_INACTIVE);
+ error = xfs_trans_reserve(tp, &M_RES(mp)->tr_itruncate, 0, 0);
+ if (error) {
+ ASSERT(XFS_FORCED_SHUTDOWN(mp));
+ xfs_trans_cancel(tp, 0);
+ return error;
+ }
+
+ xfs_ilock(ip, XFS_ILOCK_EXCL);
+ xfs_trans_ijoin(tp, ip, 0);
+
+ /*
+ * Log the inode size first to prevent stale data exposure in the event
+ * of a system crash before the truncate completes. See the related
+ * comment in xfs_setattr_size() for details.
+ */
+ ip->i_d.di_size = 0;
+ xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
+
+ error = xfs_itruncate_extents(&tp, ip, XFS_DATA_FORK, 0);
+ if (error)
+ goto error_trans_cancel;
+
+ ASSERT(ip->i_d.di_nextents == 0);
+
+ error = xfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES);
+ if (error)
+ goto error_unlock;
+
+ xfs_iunlock(ip, XFS_ILOCK_EXCL);
+ return 0;
+
+error_trans_cancel:
+ xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES | XFS_TRANS_ABORT);
+error_unlock:
+ xfs_iunlock(ip, XFS_ILOCK_EXCL);
+ return error;
+}
+
+/*
* xfs_inactive
*
* This is called when the vnode reference count for the vnode
@@ -1679,7 +1731,6 @@ xfs_inactive(
int committed;
struct xfs_trans *tp;
struct xfs_mount *mp;
- struct xfs_trans_res *resp;
int error;
int truncate = 0;
@@ -1724,35 +1775,12 @@ xfs_inactive(
if (error)
return VN_INACTIVE_CACHE;
- if (S_ISLNK(ip->i_d.di_mode)) {
+ if (S_ISLNK(ip->i_d.di_mode))
error = xfs_inactive_symlink(ip);
- if (error)
- goto out;
- }
-
- tp = xfs_trans_alloc(mp, XFS_TRANS_INACTIVE);
- resp = truncate ? &M_RES(mp)->tr_itruncate : &M_RES(mp)->tr_ifree;
-
- error = xfs_trans_reserve(tp, resp, 0, 0);
- if (error) {
- ASSERT(XFS_FORCED_SHUTDOWN(mp));
- xfs_trans_cancel(tp, 0);
- return VN_INACTIVE_CACHE;
- }
-
- xfs_ilock(ip, XFS_ILOCK_EXCL);
- xfs_trans_ijoin(tp, ip, 0);
-
- if (truncate) {
- ip->i_d.di_size = 0;
- xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
-
- error = xfs_itruncate_extents(&tp, ip, XFS_DATA_FORK, 0);
- if (error)
- goto out_cancel;
-
- ASSERT(ip->i_d.di_nextents == 0);
- }
+ else if (truncate)
+ error = xfs_inactive_truncate(ip);
+ if (error)
+ goto out;
/*
* If there are attributes associated with the file then blow them away
@@ -1763,25 +1791,9 @@ xfs_inactive(
if (ip->i_d.di_anextents > 0) {
ASSERT(ip->i_d.di_forkoff != 0);
- error = xfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES);
- if (error)
- goto out_unlock;
-
- xfs_iunlock(ip, XFS_ILOCK_EXCL);
-
error = xfs_attr_inactive(ip);
if (error)
goto out;
-
- tp = xfs_trans_alloc(mp, XFS_TRANS_INACTIVE);
- error = xfs_trans_reserve(tp, &M_RES(mp)->tr_ifree, 0, 0);
- if (error) {
- xfs_trans_cancel(tp, 0);
- goto out;
- }
-
- xfs_ilock(ip, XFS_ILOCK_EXCL);
- xfs_trans_ijoin(tp, ip, 0);
}
if (ip->i_afp)
@@ -1789,6 +1801,17 @@ xfs_inactive(
ASSERT(ip->i_d.di_anextents == 0);
+ tp = xfs_trans_alloc(mp, XFS_TRANS_INACTIVE);
+ error = xfs_trans_reserve(tp, &M_RES(mp)->tr_ifree, 0, 0);
+ if (error) {
+ ASSERT(XFS_FORCED_SHUTDOWN(mp));
+ xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES);
+ goto out;
+ }
+
+ xfs_ilock(ip, XFS_ILOCK_EXCL);
+ xfs_trans_ijoin(tp, ip, 0);
+
/*
* Free the inode.
*/
@@ -1831,13 +1854,9 @@ xfs_inactive(
* Release the dquots held by inode, if any.
*/
xfs_qm_dqdetach(ip);
-out_unlock:
xfs_iunlock(ip, XFS_ILOCK_EXCL);
out:
return VN_INACTIVE_CACHE;
-out_cancel:
- xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES | XFS_TRANS_ABORT);
- goto out_unlock;
}
/*
--
1.8.1.4
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 3/4] xfs: push down inactive transaction mgmt for ifree
2013-09-20 15:06 [PATCH v3 0/4] xfs: rework xfs_inactive() Brian Foster
2013-09-20 15:06 ` [PATCH v3 1/4] xfs: push down inactive transaction mgmt for remote symlinks Brian Foster
2013-09-20 15:06 ` [PATCH v3 2/4] xfs: push down inactive transaction mgmt for truncate Brian Foster
@ 2013-09-20 15:06 ` Brian Foster
2013-09-30 0:34 ` Dave Chinner
2013-09-20 15:06 ` [PATCH v3 4/4] xfs: clean up xfs_inactive() error handling, kill VN_INACTIVE_[NO]CACHE Brian Foster
2013-10-09 0:25 ` [PATCH v3 0/4] xfs: rework xfs_inactive() Ben Myers
4 siblings, 1 reply; 9+ messages in thread
From: Brian Foster @ 2013-09-20 15:06 UTC (permalink / raw)
To: xfs
Push the inode free work performed during xfs_inactive() down into
a new xfs_inactive_ifree() helper. This clears xfs_inactive() from
all inode locking and transaction management more directly
associated with freeing the inode xattrs, extents and the inode
itself.
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/xfs_inode.c | 121 +++++++++++++++++++++++++++++++----------------------
1 file changed, 71 insertions(+), 50 deletions(-)
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index ff4e729..b7e9279 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1715,6 +1715,74 @@ error_unlock:
}
/*
+ * xfs_inactive_ifree()
+ *
+ * Perform the inode free when an inode is unlinked.
+ */
+STATIC int
+xfs_inactive_ifree(
+ struct xfs_inode *ip)
+{
+ xfs_bmap_free_t free_list;
+ xfs_fsblock_t first_block;
+ int committed;
+ struct xfs_mount *mp = ip->i_mount;
+ struct xfs_trans *tp;
+ int error;
+
+ tp = xfs_trans_alloc(mp, XFS_TRANS_INACTIVE);
+ error = xfs_trans_reserve(tp, &M_RES(mp)->tr_ifree, 0, 0);
+ if (error) {
+ ASSERT(XFS_FORCED_SHUTDOWN(mp));
+ xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES);
+ return error;
+ }
+
+ xfs_ilock(ip, XFS_ILOCK_EXCL);
+ xfs_trans_ijoin(tp, ip, 0);
+
+ xfs_bmap_init(&free_list, &first_block);
+ error = xfs_ifree(tp, ip, &free_list);
+ if (error) {
+ /*
+ * If we fail to free the inode, shut down. The cancel
+ * might do that, we need to make sure. Otherwise the
+ * inode might be lost for a long time or forever.
+ */
+ if (!XFS_FORCED_SHUTDOWN(mp)) {
+ xfs_notice(mp, "%s: xfs_ifree returned error %d",
+ __func__, error);
+ xfs_force_shutdown(mp, SHUTDOWN_META_IO_ERROR);
+ }
+ xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES|XFS_TRANS_ABORT);
+ xfs_iunlock(ip, XFS_ILOCK_EXCL);
+ return error;
+ }
+
+ /*
+ * Credit the quota account(s). The inode is gone.
+ */
+ xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_ICOUNT, -1);
+
+ /*
+ * Just ignore errors at this point. There is nothing we can
+ * do except to try to keep going. Make sure it's not a silent
+ * error.
+ */
+ error = xfs_bmap_finish(&tp, &free_list, &committed);
+ if (error)
+ xfs_notice(mp, "%s: xfs_bmap_finish returned error %d",
+ __func__, error);
+ error = xfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES);
+ if (error)
+ xfs_notice(mp, "%s: xfs_trans_commit returned error %d",
+ __func__, error);
+
+ xfs_iunlock(ip, XFS_ILOCK_EXCL);
+ return 0;
+}
+
+/*
* xfs_inactive
*
* This is called when the vnode reference count for the vnode
@@ -1726,10 +1794,6 @@ int
xfs_inactive(
xfs_inode_t *ip)
{
- xfs_bmap_free_t free_list;
- xfs_fsblock_t first_block;
- int committed;
- struct xfs_trans *tp;
struct xfs_mount *mp;
int error;
int truncate = 0;
@@ -1801,60 +1865,17 @@ xfs_inactive(
ASSERT(ip->i_d.di_anextents == 0);
- tp = xfs_trans_alloc(mp, XFS_TRANS_INACTIVE);
- error = xfs_trans_reserve(tp, &M_RES(mp)->tr_ifree, 0, 0);
- if (error) {
- ASSERT(XFS_FORCED_SHUTDOWN(mp));
- xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES);
- goto out;
- }
-
- xfs_ilock(ip, XFS_ILOCK_EXCL);
- xfs_trans_ijoin(tp, ip, 0);
-
/*
* Free the inode.
*/
- xfs_bmap_init(&free_list, &first_block);
- error = xfs_ifree(tp, ip, &free_list);
- if (error) {
- /*
- * If we fail to free the inode, shut down. The cancel
- * might do that, we need to make sure. Otherwise the
- * inode might be lost for a long time or forever.
- */
- if (!XFS_FORCED_SHUTDOWN(mp)) {
- xfs_notice(mp, "%s: xfs_ifree returned error %d",
- __func__, error);
- xfs_force_shutdown(mp, SHUTDOWN_META_IO_ERROR);
- }
- xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES|XFS_TRANS_ABORT);
- } else {
- /*
- * Credit the quota account(s). The inode is gone.
- */
- xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_ICOUNT, -1);
-
- /*
- * Just ignore errors at this point. There is nothing we can
- * do except to try to keep going. Make sure it's not a silent
- * error.
- */
- error = xfs_bmap_finish(&tp, &free_list, &committed);
- if (error)
- xfs_notice(mp, "%s: xfs_bmap_finish returned error %d",
- __func__, error);
- error = xfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES);
- if (error)
- xfs_notice(mp, "%s: xfs_trans_commit returned error %d",
- __func__, error);
- }
+ error = xfs_inactive_ifree(ip);
+ if (error)
+ goto out;
/*
* Release the dquots held by inode, if any.
*/
xfs_qm_dqdetach(ip);
- xfs_iunlock(ip, XFS_ILOCK_EXCL);
out:
return VN_INACTIVE_CACHE;
}
--
1.8.1.4
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 4/4] xfs: clean up xfs_inactive() error handling, kill VN_INACTIVE_[NO]CACHE
2013-09-20 15:06 [PATCH v3 0/4] xfs: rework xfs_inactive() Brian Foster
` (2 preceding siblings ...)
2013-09-20 15:06 ` [PATCH v3 3/4] xfs: push down inactive transaction mgmt for ifree Brian Foster
@ 2013-09-20 15:06 ` Brian Foster
2013-10-09 0:25 ` [PATCH v3 0/4] xfs: rework xfs_inactive() Ben Myers
4 siblings, 0 replies; 9+ messages in thread
From: Brian Foster @ 2013-09-20 15:06 UTC (permalink / raw)
To: xfs
The xfs_inactive() return value is meaningless. Turn xfs_inactive()
into a void function and clean up the error handling appropriately.
Kill the VN_INACTIVE_[NO]CACHE directives as they are not relevant
to Linux.
Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/xfs_inode.c | 28 +++++++++++-----------------
fs/xfs/xfs_inode.h | 2 +-
fs/xfs/xfs_vnode.h | 8 --------
3 files changed, 12 insertions(+), 26 deletions(-)
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index b7e9279..64f7733 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1790,7 +1790,7 @@ xfs_inactive_ifree(
* now be truncated. Also, we clear all of the read-ahead state
* kept for the inode here since the file is now closed.
*/
-int
+void
xfs_inactive(
xfs_inode_t *ip)
{
@@ -1805,16 +1805,14 @@ xfs_inactive(
if (ip->i_d.di_mode == 0 || is_bad_inode(VFS_I(ip))) {
ASSERT(ip->i_df.if_real_bytes == 0);
ASSERT(ip->i_df.if_broot_bytes == 0);
- return VN_INACTIVE_CACHE;
+ return;
}
mp = ip->i_mount;
- error = 0;
-
/* If this is a read-only mount, don't do this (would generate I/O) */
if (mp->m_flags & XFS_MOUNT_RDONLY)
- goto out;
+ return;
if (ip->i_d.di_nlink != 0) {
/*
@@ -1822,12 +1820,10 @@ xfs_inactive(
* cache. Post-eof blocks must be freed, lest we end up with
* broken free space accounting.
*/
- if (xfs_can_free_eofblocks(ip, true)) {
- error = xfs_free_eofblocks(mp, ip, false);
- if (error)
- return VN_INACTIVE_CACHE;
- }
- goto out;
+ if (xfs_can_free_eofblocks(ip, true))
+ xfs_free_eofblocks(mp, ip, false);
+
+ return;
}
if (S_ISREG(ip->i_d.di_mode) &&
@@ -1837,14 +1833,14 @@ xfs_inactive(
error = xfs_qm_dqattach(ip, 0);
if (error)
- return VN_INACTIVE_CACHE;
+ return;
if (S_ISLNK(ip->i_d.di_mode))
error = xfs_inactive_symlink(ip);
else if (truncate)
error = xfs_inactive_truncate(ip);
if (error)
- goto out;
+ return;
/*
* If there are attributes associated with the file then blow them away
@@ -1857,7 +1853,7 @@ xfs_inactive(
error = xfs_attr_inactive(ip);
if (error)
- goto out;
+ return;
}
if (ip->i_afp)
@@ -1870,14 +1866,12 @@ xfs_inactive(
*/
error = xfs_inactive_ifree(ip);
if (error)
- goto out;
+ return;
/*
* Release the dquots held by inode, if any.
*/
xfs_qm_dqdetach(ip);
-out:
- return VN_INACTIVE_CACHE;
}
/*
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 4a91358..cce62ce 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -316,7 +316,7 @@ static inline int xfs_isiflocked(struct xfs_inode *ip)
int xfs_release(struct xfs_inode *ip);
-int xfs_inactive(struct xfs_inode *ip);
+void xfs_inactive(struct xfs_inode *ip);
int xfs_lookup(struct xfs_inode *dp, struct xfs_name *name,
struct xfs_inode **ipp, struct xfs_name *ci_name);
int xfs_create(struct xfs_inode *dp, struct xfs_name *name,
diff --git a/fs/xfs/xfs_vnode.h b/fs/xfs/xfs_vnode.h
index db14d0c..3e8e797 100644
--- a/fs/xfs/xfs_vnode.h
+++ b/fs/xfs/xfs_vnode.h
@@ -25,14 +25,6 @@ struct xfs_inode;
struct attrlist_cursor_kern;
/*
- * Return values for xfs_inactive. A return value of
- * VN_INACTIVE_NOCACHE implies that the file system behavior
- * has disassociated its state and bhv_desc_t from the vnode.
- */
-#define VN_INACTIVE_CACHE 0
-#define VN_INACTIVE_NOCACHE 1
-
-/*
* Flags for read/write calls - same values as IRIX
*/
#define IO_ISDIRECT 0x00004 /* bypass page cache */
--
1.8.1.4
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3 0/4] xfs: rework xfs_inactive()
2013-09-20 15:06 [PATCH v3 0/4] xfs: rework xfs_inactive() Brian Foster
` (3 preceding siblings ...)
2013-09-20 15:06 ` [PATCH v3 4/4] xfs: clean up xfs_inactive() error handling, kill VN_INACTIVE_[NO]CACHE Brian Foster
@ 2013-10-09 0:25 ` Ben Myers
4 siblings, 0 replies; 9+ messages in thread
From: Ben Myers @ 2013-10-09 0:25 UTC (permalink / raw)
To: Brian Foster; +Cc: xfs
On Fri, Sep 20, 2013 at 11:06:08AM -0400, Brian Foster wrote:
> Hi all,
>
> This set reworks the xfs_inactive() path with the intent to clean up the
> transaction management overall. This is preparation work for the free
> inode btree set and subsequent work in the area. The patches clean up
> the remote symlink work, truncate work and ifree work respectively.
>
> This passes through a quick xfstests run (with debug and lockdep)
> without any major explosions. Thoughts appreciated.
>
> Brian
Applied these four.
Thanks,
Ben
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 9+ messages in thread