xfs: consolidate xfs_bulkstat_single
PATCH v2 2/10 xfs: consolidate xfs_bulkstat_single
From: Jie Liu <[email protected]>
In xfs_bulkstat_single(), call xfs_bulkstat_one() and xfs_bulkstat() would return different error if either failed, we'd better return the proper error in this case. Moreover, the function argument done is useless in terms of xfs_ioc_bulkstat(), hence we can get rid of it.
--- fs/xfs/xfs_ioctl.c | 3 +-- fs/xfs/xfs_itable.c | 60 ++++++++++++++++++++++++++--------------------------- fs/xfs/xfs_itable.h | 5 ++--- 3 files changed, 33 insertions(+), 35 deletions(-)
fs/xfs/xfs_ioctl.c +1 -2
@@ -798,8 +798,7 @@ xfs_ioc_bulkstat( error =xfs_inumbers (mp, &inlast, &count, bulkreq.ubuffer, xfs_inumbers_fmt); else if (cmd == XFS_IOC_FSBULKSTAT_SINGLE) - error =xfs_bulkstat_single (mp, &inlast, - bulkreq.ubuffer, &done); + error =xfs_bulkstat_single (mp, &inlast, bulkreq.ubuffer); else /* XFS_IOC_FSBULKSTAT */ error = xfs_bulkstat(mp, &inlast, &count, xfs_bulkstat_one, sizeof(xfs_bstat_t), bulkreq.ubuffer,
fs/xfs/xfs_itable.c +30 -30
81 lines not on this page · raw message
fs/xfs/xfs_itable.h +2 -3
@@ -52,10 +52,9 @@ xfs_bulkstat(int xfs_bulkstat_single ( -xfs_mount_t *mp, +struct xfs_mount *mp,xfs_ino_t *lastinop, -char __user *buffer, -int *done); +char __user *buffer); typedef int (*bulkstat_one_fmt_pf)( /* used size in bytes or negative error */ void __user *ubuffer, /* buffer to write to */
Signature · 6 lines
1.8.3.2 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 6 lines · Jeff Liu
From: Jie Liu <[email protected]>
In
xfs_bulkstat_single(), callxfs_bulkstat_one()andxfs_bulkstat()would return different error if either failed, we'd better return the proper error in this case. Moreover, the function argument done is useless in terms ofxfs_ioc_bulkstat(), hence we can get rid of it.
I've looked at xfs_bulkstat_single I really can't see how falling back to the full xfs_bulkstat could fix any error. We probably should just get rid of the fallback and instead do something like the (lightly tested) patch below:
---
From: Christoph Hellwig <[email protected]>
Subject: xfs: remove xfs_bulkstat_single
xfs_bukstat_one doesn't have any failure case that would go away when called through xfs_bulkstat, so remove the fallback and the now unessecary xfs_bulkstat_single function.
--- fs/xfs/xfs_ioctl.c | 4 ++-- fs/xfs/xfs_itable.c | 48 ------------------------------------------------ fs/xfs/xfs_itable.h | 7 ------- 3 files changed, 2 insertions(+), 57 deletions(-)
fs/xfs/xfs_ioctl.c +2 -2
@@ -824,8 +824,8 @@ xfs_ioc_bulkstat( error =xfs_inumbers (mp, &inlast, &count, bulkreq.ubuffer, xfs_inumbers_fmt); else if (cmd == XFS_IOC_FSBULKSTAT_SINGLE) - error =xfs_bulkstat_single (mp, &inlast, - bulkreq.ubuffer, &done); + error =xfs_bulkstat_one (mp, inlast, bulkreq.ubuffer, +sizeof (xfs_bstat_t),NULL , &done); else /* XFS_IOC_FSBULKSTAT */ error = xfs_bulkstat(mp, &inlast, &count, xfs_bulkstat_one, sizeof(xfs_bstat_t), bulkreq.ubuffer,
fs/xfs/xfs_itable.c +0 -48
54 lines not on this page · raw message
fs/xfs/xfs_itable.h +0 -7
@@ -50,13 +50,6 @@ xfs_bulkstat(char __user *ubuffer,/* buffer with inode stats */ int *done); /* 1 if there are more stats to get */ -int -xfs_bulkstat_single ( -xfs_mount_t *mp, -xfs_ino_t *lastinop, -char __user *buffer, -int *done); - typedef int (*bulkstat_one_fmt_pf)( /* used size in bytes or negative error */ void __user *ubuffer, /* buffer to write to */ int ubsize, /* remaining user buffer sz */
Signature · 6 lines
1.7.10.4 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 17 lines · Christoph Hellwig
Quoted 6 lines · Jeff Liu
From: Jie Liu <[email protected]>
In
xfs_bulkstat_single(), callxfs_bulkstat_one()andxfs_bulkstat()would return different error if either failed, we'd better return the proper error in this case. Moreover, the function argument done is useless in terms ofxfs_ioc_bulkstat(), hence we can get rid of it.I've looked at
xfs_bulkstat_singleI really can't see how falling back to the fullxfs_bulkstatcould fix any error. We probably should just get rid of the fallback and instead do something like the (lightly tested) patch below:---
From: Christoph Hellwig <[email protected]>
Subject: xfs: removexfs_bulkstat_single
Quoted 5 lines
xfs_bukstat_onedoesn't have any failure case that would go away when called throughxfs_bulkstat, so remove the fallback and the now unessecaryxfs_bulkstat_singlefunction.Signed-off-by Christoph Hellwig <[email protected]>
*nod*
I like this approach :)
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 22 lines · Dave Chinner
Quoted 21 lines · Christoph Hellwig
Quoted 6 lines · Jeff Liu
From: Jie Liu <[email protected]>
In
xfs_bulkstat_single(), callxfs_bulkstat_one()andxfs_bulkstat()would return different error if either failed, we'd better return the proper error in this case. Moreover, the function argument done is useless in terms ofxfs_ioc_bulkstat(), hence we can get rid of it.I've looked at
xfs_bulkstat_singleI really can't see how falling back to the fullxfs_bulkstatcould fix any error. We probably should just get rid of the fallback and instead do something like the (lightly tested) patch below:---
From: Christoph Hellwig <[email protected]>
Subject: xfs: removexfs_bulkstat_single
xfs_bukstat_onedoesn't have any failure case that would go away when called throughxfs_bulkstat, so remove the fallback and the now unessecaryxfs_bulkstat_singlefunction.
Quoted 5 lines
Quoted 1 line
Signed-off-by Christoph Hellwig <[email protected]>*nod*
I like this approach :)
I reconsidered the call interface in xfs_fsr and xfs_io/parent. Yep, your idea is better and the patch looks good to me.
Thanks,
-Jeff
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs