* [PATCH] factor out inode has attrs check
@ 2008-06-03 11:39 Christoph Hellwig
2008-06-05 7:30 ` Timothy Shimmin
0 siblings, 1 reply; 2+ messages in thread
From: Christoph Hellwig @ 2008-06-03 11:39 UTC (permalink / raw)
To: xfs
Note that xfs_attr_fetch did the check twice while keeping the inode
locked and we can just remove the superflous check.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6-xfs/fs/xfs/xfs_attr.c
===================================================================
--- linux-2.6-xfs.orig/fs/xfs/xfs_attr.c 2008-06-01 14:32:06.000000000 +0200
+++ linux-2.6-xfs/fs/xfs/xfs_attr.c 2008-06-01 14:40:04.000000000 +0200
@@ -111,6 +111,17 @@ xfs_attr_name_to_xname(
return 0;
}
+STATIC int
+xfs_inode_hasattr(
+ struct xfs_inode *ip)
+{
+ if (!XFS_IFORK_Q(ip) ||
+ (ip->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
+ ip->i_d.di_anextents == 0))
+ return 0;
+ return 1;
+}
+
/*========================================================================
* Overall external interface routines.
*========================================================================*/
@@ -122,10 +133,8 @@ xfs_attr_fetch(xfs_inode_t *ip, struct x
xfs_da_args_t args;
int error;
- if ((XFS_IFORK_Q(ip) == 0) ||
- (ip->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
- ip->i_d.di_anextents == 0))
- return(ENOATTR);
+ if (!xfs_inode_hasattr(ip))
+ return ENOATTR;
/*
* Fill in the arg structure for this request.
@@ -143,11 +152,7 @@ xfs_attr_fetch(xfs_inode_t *ip, struct x
/*
* Decide on what work routines to call based on the inode size.
*/
- if (XFS_IFORK_Q(ip) == 0 ||
- (ip->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
- ip->i_d.di_anextents == 0)) {
- error = XFS_ERROR(ENOATTR);
- } else if (ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
+ if (ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
error = xfs_attr_shortform_getvalue(&args);
} else if (xfs_bmap_one_block(ip, XFS_ATTR_FORK)) {
error = xfs_attr_leaf_get(&args);
@@ -523,9 +528,7 @@ xfs_attr_remove_int(xfs_inode_t *dp, str
/*
* Decide on what work routines to call based on the inode size.
*/
- if (XFS_IFORK_Q(dp) == 0 ||
- (dp->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
- dp->i_d.di_anextents == 0)) {
+ if (!xfs_inode_hasattr(dp)) {
error = XFS_ERROR(ENOATTR);
goto out;
}
@@ -595,11 +598,9 @@ xfs_attr_remove(
return error;
xfs_ilock(dp, XFS_ILOCK_SHARED);
- if (XFS_IFORK_Q(dp) == 0 ||
- (dp->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
- dp->i_d.di_anextents == 0)) {
+ if (!xfs_inode_hasattr(dp)) {
xfs_iunlock(dp, XFS_ILOCK_SHARED);
- return(XFS_ERROR(ENOATTR));
+ return XFS_ERROR(ENOATTR);
}
xfs_iunlock(dp, XFS_ILOCK_SHARED);
@@ -615,9 +616,7 @@ xfs_attr_list_int(xfs_attr_list_context_
/*
* Decide on what work routines to call based on the inode size.
*/
- if (XFS_IFORK_Q(dp) == 0 ||
- (dp->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
- dp->i_d.di_anextents == 0)) {
+ if (!xfs_inode_hasattr(dp)) {
error = 0;
} else if (dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
error = xfs_attr_shortform_list(context);
@@ -810,12 +809,10 @@ xfs_attr_inactive(xfs_inode_t *dp)
ASSERT(! XFS_NOT_DQATTACHED(mp, dp));
xfs_ilock(dp, XFS_ILOCK_SHARED);
- if ((XFS_IFORK_Q(dp) == 0) ||
- (dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) ||
- (dp->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
- dp->i_d.di_anextents == 0)) {
+ if (!xfs_inode_hasattr(dp) ||
+ dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
xfs_iunlock(dp, XFS_ILOCK_SHARED);
- return(0);
+ return 0;
}
xfs_iunlock(dp, XFS_ILOCK_SHARED);
@@ -848,10 +845,8 @@ xfs_attr_inactive(xfs_inode_t *dp)
/*
* Decide on what work routines to call based on the inode size.
*/
- if ((XFS_IFORK_Q(dp) == 0) ||
- (dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) ||
- (dp->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
- dp->i_d.di_anextents == 0)) {
+ if (!xfs_inode_hasattr(dp) ||
+ dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
error = 0;
goto out;
}
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] factor out inode has attrs check
2008-06-03 11:39 [PATCH] factor out inode has attrs check Christoph Hellwig
@ 2008-06-05 7:30 ` Timothy Shimmin
0 siblings, 0 replies; 2+ messages in thread
From: Timothy Shimmin @ 2008-06-05 7:30 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
Looks good.
I'll run thru qa and check in.
So there are a couple of cases where XFS_DINODE_FMT_LOCAL
is used in xfs_attr_inactive() as well as the noattr check.
Okay, that's because the EAs would be in shortform so we
don't have to do anything for inactive here (the inode inactive
will suffice).
Apart from that they are all replacements (callouts) except
for the superfluous check Christoph mentioned. Cool.
--Tim
Christoph Hellwig wrote:
> Note that xfs_attr_fetch did the check twice while keeping the inode
> locked and we can just remove the superflous check.
>
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> Index: linux-2.6-xfs/fs/xfs/xfs_attr.c
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/xfs_attr.c 2008-06-01 14:32:06.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/xfs_attr.c 2008-06-01 14:40:04.000000000 +0200
> @@ -111,6 +111,17 @@ xfs_attr_name_to_xname(
> return 0;
> }
>
> +STATIC int
> +xfs_inode_hasattr(
> + struct xfs_inode *ip)
> +{
> + if (!XFS_IFORK_Q(ip) ||
> + (ip->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
> + ip->i_d.di_anextents == 0))
> + return 0;
> + return 1;
> +}
> +
> /*========================================================================
> * Overall external interface routines.
> *========================================================================*/
> @@ -122,10 +133,8 @@ xfs_attr_fetch(xfs_inode_t *ip, struct x
> xfs_da_args_t args;
> int error;
>
> - if ((XFS_IFORK_Q(ip) == 0) ||
> - (ip->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
> - ip->i_d.di_anextents == 0))
> - return(ENOATTR);
> + if (!xfs_inode_hasattr(ip))
> + return ENOATTR;
>
> /*
> * Fill in the arg structure for this request.
> @@ -143,11 +152,7 @@ xfs_attr_fetch(xfs_inode_t *ip, struct x
> /*
> * Decide on what work routines to call based on the inode size.
> */
> - if (XFS_IFORK_Q(ip) == 0 ||
> - (ip->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
> - ip->i_d.di_anextents == 0)) {
> - error = XFS_ERROR(ENOATTR);
> - } else if (ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
> + if (ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
> error = xfs_attr_shortform_getvalue(&args);
> } else if (xfs_bmap_one_block(ip, XFS_ATTR_FORK)) {
> error = xfs_attr_leaf_get(&args);
> @@ -523,9 +528,7 @@ xfs_attr_remove_int(xfs_inode_t *dp, str
> /*
> * Decide on what work routines to call based on the inode size.
> */
> - if (XFS_IFORK_Q(dp) == 0 ||
> - (dp->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
> - dp->i_d.di_anextents == 0)) {
> + if (!xfs_inode_hasattr(dp)) {
> error = XFS_ERROR(ENOATTR);
> goto out;
> }
> @@ -595,11 +598,9 @@ xfs_attr_remove(
> return error;
>
> xfs_ilock(dp, XFS_ILOCK_SHARED);
> - if (XFS_IFORK_Q(dp) == 0 ||
> - (dp->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
> - dp->i_d.di_anextents == 0)) {
> + if (!xfs_inode_hasattr(dp)) {
> xfs_iunlock(dp, XFS_ILOCK_SHARED);
> - return(XFS_ERROR(ENOATTR));
> + return XFS_ERROR(ENOATTR);
> }
> xfs_iunlock(dp, XFS_ILOCK_SHARED);
>
> @@ -615,9 +616,7 @@ xfs_attr_list_int(xfs_attr_list_context_
> /*
> * Decide on what work routines to call based on the inode size.
> */
> - if (XFS_IFORK_Q(dp) == 0 ||
> - (dp->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
> - dp->i_d.di_anextents == 0)) {
> + if (!xfs_inode_hasattr(dp)) {
> error = 0;
> } else if (dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
> error = xfs_attr_shortform_list(context);
> @@ -810,12 +809,10 @@ xfs_attr_inactive(xfs_inode_t *dp)
> ASSERT(! XFS_NOT_DQATTACHED(mp, dp));
>
> xfs_ilock(dp, XFS_ILOCK_SHARED);
> - if ((XFS_IFORK_Q(dp) == 0) ||
> - (dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) ||
> - (dp->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
> - dp->i_d.di_anextents == 0)) {
> + if (!xfs_inode_hasattr(dp) ||
> + dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
> xfs_iunlock(dp, XFS_ILOCK_SHARED);
> - return(0);
> + return 0;
> }
> xfs_iunlock(dp, XFS_ILOCK_SHARED);
>
> @@ -848,10 +845,8 @@ xfs_attr_inactive(xfs_inode_t *dp)
> /*
> * Decide on what work routines to call based on the inode size.
> */
> - if ((XFS_IFORK_Q(dp) == 0) ||
> - (dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) ||
> - (dp->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
> - dp->i_d.di_anextents == 0)) {
> + if (!xfs_inode_hasattr(dp) ||
> + dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
> error = 0;
> goto out;
> }
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2008-06-05 7:30 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-06-03 11:39 [PATCH] factor out inode has attrs check Christoph Hellwig
2008-06-05 7:30 ` Timothy Shimmin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox