* [PATCH v3 00/11] xfs: refactor incore inode walking
@ 2020-05-20 1:45 Darrick J. Wong
2020-05-20 1:45 ` [PATCH 01/11] xfs: move eofblocks conversion function to xfs_ioctl.c Darrick J. Wong
` (10 more replies)
0 siblings, 11 replies; 28+ messages in thread
From: Darrick J. Wong @ 2020-05-20 1:45 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
Hi all,
This series prepares the incore inode walking code used by the
eofblocks/cowblocks scanner to handle deferred inode inactivation.
First we clean up the eofblocks/cowblocks incore inode walking code to
get rid of some of the warts left by reflink development. Next, we
rip out the many trivial wrapper functions that don't add much value.
Finally, we refactor the various helpers and predicate functions to
reduce open-coded logic.
If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.
This is an extraordinary way to destroy everything. Enjoy!
Comments and questions are, as always, welcome.
--D
kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=incore-inode-walk
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 01/11] xfs: move eofblocks conversion function to xfs_ioctl.c
2020-05-20 1:45 [PATCH v3 00/11] xfs: refactor incore inode walking Darrick J. Wong
@ 2020-05-20 1:45 ` Darrick J. Wong
2020-05-20 6:30 ` Christoph Hellwig
2020-05-20 1:45 ` [PATCH 02/11] xfs: replace open-coded XFS_ICI_NO_TAG Darrick J. Wong
` (9 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Darrick J. Wong @ 2020-05-20 1:45 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
From: Darrick J. Wong <darrick.wong@oracle.com>
Move xfs_fs_eofblocks_from_user into the only file that actually uses
it, so that we don't have this function cluttering up the header file.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/xfs_icache.h | 35 -----------------------------------
fs/xfs/xfs_ioctl.c | 35 +++++++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+), 35 deletions(-)
diff --git a/fs/xfs/xfs_icache.h b/fs/xfs/xfs_icache.h
index 48f1fd2bb6ad..c13bc8a3e02f 100644
--- a/fs/xfs/xfs_icache.h
+++ b/fs/xfs/xfs_icache.h
@@ -81,41 +81,6 @@ int xfs_inode_ag_iterator_tag(struct xfs_mount *mp,
int (*execute)(struct xfs_inode *ip, int flags, void *args),
int flags, void *args, int tag);
-static inline int
-xfs_fs_eofblocks_from_user(
- struct xfs_fs_eofblocks *src,
- struct xfs_eofblocks *dst)
-{
- if (src->eof_version != XFS_EOFBLOCKS_VERSION)
- return -EINVAL;
-
- if (src->eof_flags & ~XFS_EOF_FLAGS_VALID)
- return -EINVAL;
-
- if (memchr_inv(&src->pad32, 0, sizeof(src->pad32)) ||
- memchr_inv(src->pad64, 0, sizeof(src->pad64)))
- return -EINVAL;
-
- dst->eof_flags = src->eof_flags;
- dst->eof_prid = src->eof_prid;
- dst->eof_min_file_size = src->eof_min_file_size;
-
- dst->eof_uid = INVALID_UID;
- if (src->eof_flags & XFS_EOF_FLAGS_UID) {
- dst->eof_uid = make_kuid(current_user_ns(), src->eof_uid);
- if (!uid_valid(dst->eof_uid))
- return -EINVAL;
- }
-
- dst->eof_gid = INVALID_GID;
- if (src->eof_flags & XFS_EOF_FLAGS_GID) {
- dst->eof_gid = make_kgid(current_user_ns(), src->eof_gid);
- if (!gid_valid(dst->eof_gid))
- return -EINVAL;
- }
- return 0;
-}
-
int xfs_icache_inode_is_allocated(struct xfs_mount *mp, struct xfs_trans *tp,
xfs_ino_t ino, bool *inuse);
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 309958186d33..6a3c675a8aeb 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -2082,6 +2082,41 @@ xfs_ioc_setlabel(
return error;
}
+static inline int
+xfs_fs_eofblocks_from_user(
+ struct xfs_fs_eofblocks *src,
+ struct xfs_eofblocks *dst)
+{
+ if (src->eof_version != XFS_EOFBLOCKS_VERSION)
+ return -EINVAL;
+
+ if (src->eof_flags & ~XFS_EOF_FLAGS_VALID)
+ return -EINVAL;
+
+ if (memchr_inv(&src->pad32, 0, sizeof(src->pad32)) ||
+ memchr_inv(src->pad64, 0, sizeof(src->pad64)))
+ return -EINVAL;
+
+ dst->eof_flags = src->eof_flags;
+ dst->eof_prid = src->eof_prid;
+ dst->eof_min_file_size = src->eof_min_file_size;
+
+ dst->eof_uid = INVALID_UID;
+ if (src->eof_flags & XFS_EOF_FLAGS_UID) {
+ dst->eof_uid = make_kuid(current_user_ns(), src->eof_uid);
+ if (!uid_valid(dst->eof_uid))
+ return -EINVAL;
+ }
+
+ dst->eof_gid = INVALID_GID;
+ if (src->eof_flags & XFS_EOF_FLAGS_GID) {
+ dst->eof_gid = make_kgid(current_user_ns(), src->eof_gid);
+ if (!gid_valid(dst->eof_gid))
+ return -EINVAL;
+ }
+ return 0;
+}
+
/*
* Note: some of the ioctl's return positive numbers as a
* byte count indicating success, such as readlink_by_handle.
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 02/11] xfs: replace open-coded XFS_ICI_NO_TAG
2020-05-20 1:45 [PATCH v3 00/11] xfs: refactor incore inode walking Darrick J. Wong
2020-05-20 1:45 ` [PATCH 01/11] xfs: move eofblocks conversion function to xfs_ioctl.c Darrick J. Wong
@ 2020-05-20 1:45 ` Darrick J. Wong
2020-05-20 6:30 ` Christoph Hellwig
2020-05-20 1:45 ` [PATCH 03/11] xfs: remove unused xfs_inode_ag_iterator function Darrick J. Wong
` (8 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Darrick J. Wong @ 2020-05-20 1:45 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
From: Darrick J. Wong <darrick.wong@oracle.com>
Use XFS_ICI_NO_TAG instead of -1 when appropriate.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/xfs_icache.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 0ed904c2aa12..b1e2541810be 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -815,7 +815,7 @@ xfs_inode_ag_walk(
rcu_read_lock();
- if (tag == -1)
+ if (tag == XFS_ICI_NO_TAG)
nr_found = radix_tree_gang_lookup(&pag->pag_ici_root,
(void **)batch, first_index,
XFS_LOOKUP_BATCH);
@@ -973,8 +973,8 @@ xfs_inode_ag_iterator_flags(
ag = 0;
while ((pag = xfs_perag_get(mp, ag))) {
ag = pag->pag_agno + 1;
- error = xfs_inode_ag_walk(mp, pag, execute, flags, args, -1,
- iter_flags);
+ error = xfs_inode_ag_walk(mp, pag, execute, flags, args,
+ XFS_ICI_NO_TAG, iter_flags);
xfs_perag_put(pag);
if (error) {
last_error = error;
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 03/11] xfs: remove unused xfs_inode_ag_iterator function
2020-05-20 1:45 [PATCH v3 00/11] xfs: refactor incore inode walking Darrick J. Wong
2020-05-20 1:45 ` [PATCH 01/11] xfs: move eofblocks conversion function to xfs_ioctl.c Darrick J. Wong
2020-05-20 1:45 ` [PATCH 02/11] xfs: replace open-coded XFS_ICI_NO_TAG Darrick J. Wong
@ 2020-05-20 1:45 ` Darrick J. Wong
2020-05-20 6:30 ` Christoph Hellwig
2020-05-20 1:45 ` [PATCH 04/11] xfs: remove xfs_inode_ag_iterator_flags Darrick J. Wong
` (7 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Darrick J. Wong @ 2020-05-20 1:45 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
From: Darrick J. Wong <darrick.wong@oracle.com>
Not used by anyone, so get rid of it.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/xfs_icache.c | 11 -----------
fs/xfs/xfs_icache.h | 3 ---
2 files changed, 14 deletions(-)
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index b1e2541810be..6aafb547f21a 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -985,17 +985,6 @@ xfs_inode_ag_iterator_flags(
return last_error;
}
-int
-xfs_inode_ag_iterator(
- struct xfs_mount *mp,
- int (*execute)(struct xfs_inode *ip, int flags,
- void *args),
- int flags,
- void *args)
-{
- return xfs_inode_ag_iterator_flags(mp, execute, flags, args, 0);
-}
-
int
xfs_inode_ag_iterator_tag(
struct xfs_mount *mp,
diff --git a/fs/xfs/xfs_icache.h b/fs/xfs/xfs_icache.h
index c13bc8a3e02f..0556fa32074f 100644
--- a/fs/xfs/xfs_icache.h
+++ b/fs/xfs/xfs_icache.h
@@ -71,9 +71,6 @@ int xfs_inode_free_quota_cowblocks(struct xfs_inode *ip);
void xfs_cowblocks_worker(struct work_struct *);
void xfs_queue_cowblocks(struct xfs_mount *);
-int xfs_inode_ag_iterator(struct xfs_mount *mp,
- int (*execute)(struct xfs_inode *ip, int flags, void *args),
- int flags, void *args);
int xfs_inode_ag_iterator_flags(struct xfs_mount *mp,
int (*execute)(struct xfs_inode *ip, int flags, void *args),
int flags, void *args, int iter_flags);
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 04/11] xfs: remove xfs_inode_ag_iterator_flags
2020-05-20 1:45 [PATCH v3 00/11] xfs: refactor incore inode walking Darrick J. Wong
` (2 preceding siblings ...)
2020-05-20 1:45 ` [PATCH 03/11] xfs: remove unused xfs_inode_ag_iterator function Darrick J. Wong
@ 2020-05-20 1:45 ` Darrick J. Wong
2020-05-20 6:35 ` Christoph Hellwig
2020-05-20 1:45 ` [PATCH 05/11] xfs: remove flags argument from xfs_inode_ag_walk Darrick J. Wong
` (6 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Darrick J. Wong @ 2020-05-20 1:45 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
From: Darrick J. Wong <darrick.wong@oracle.com>
Combine xfs_inode_ag_iterator_flags and xfs_inode_ag_iterator_tag into a
single wrapper function since there's only one caller of the _flags
variant.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/xfs_icache.c | 43 +++++++++++++------------------------------
fs/xfs/xfs_icache.h | 5 +----
fs/xfs/xfs_qm_syscalls.c | 4 ++--
3 files changed, 16 insertions(+), 36 deletions(-)
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 6aafb547f21a..e716b19879c6 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -956,38 +956,22 @@ xfs_cowblocks_worker(
xfs_queue_cowblocks(mp);
}
-int
-xfs_inode_ag_iterator_flags(
+/* Fetch the next (possibly tagged) per-AG structure. */
+static inline struct xfs_perag *
+xfs_ici_walk_get_perag(
struct xfs_mount *mp,
- int (*execute)(struct xfs_inode *ip, int flags,
- void *args),
- int flags,
- void *args,
- int iter_flags)
+ xfs_agnumber_t agno,
+ int tag)
{
- struct xfs_perag *pag;
- int error = 0;
- int last_error = 0;
- xfs_agnumber_t ag;
-
- ag = 0;
- while ((pag = xfs_perag_get(mp, ag))) {
- ag = pag->pag_agno + 1;
- error = xfs_inode_ag_walk(mp, pag, execute, flags, args,
- XFS_ICI_NO_TAG, iter_flags);
- xfs_perag_put(pag);
- if (error) {
- last_error = error;
- if (error == -EFSCORRUPTED)
- break;
- }
- }
- return last_error;
+ if (tag == XFS_ICI_NO_TAG)
+ return xfs_perag_get(mp, agno);
+ return xfs_perag_get_tag(mp, agno, tag);
}
int
-xfs_inode_ag_iterator_tag(
+xfs_inode_ag_iterator(
struct xfs_mount *mp,
+ int iter_flags,
int (*execute)(struct xfs_inode *ip, int flags,
void *args),
int flags,
@@ -1000,10 +984,10 @@ xfs_inode_ag_iterator_tag(
xfs_agnumber_t ag;
ag = 0;
- while ((pag = xfs_perag_get_tag(mp, ag, tag))) {
+ while ((pag = xfs_ici_walk_get_perag(mp, ag, tag))) {
ag = pag->pag_agno + 1;
error = xfs_inode_ag_walk(mp, pag, execute, flags, args, tag,
- 0);
+ iter_flags);
xfs_perag_put(pag);
if (error) {
last_error = error;
@@ -1523,8 +1507,7 @@ __xfs_icache_free_eofblocks(
if (eofb && (eofb->eof_flags & XFS_EOF_FLAGS_SYNC))
flags = SYNC_WAIT;
- return xfs_inode_ag_iterator_tag(mp, execute, flags,
- eofb, tag);
+ return xfs_inode_ag_iterator(mp, 0, execute, flags, eofb, tag);
}
int
diff --git a/fs/xfs/xfs_icache.h b/fs/xfs/xfs_icache.h
index 0556fa32074f..2d5ab9957d9f 100644
--- a/fs/xfs/xfs_icache.h
+++ b/fs/xfs/xfs_icache.h
@@ -71,10 +71,7 @@ int xfs_inode_free_quota_cowblocks(struct xfs_inode *ip);
void xfs_cowblocks_worker(struct work_struct *);
void xfs_queue_cowblocks(struct xfs_mount *);
-int xfs_inode_ag_iterator_flags(struct xfs_mount *mp,
- int (*execute)(struct xfs_inode *ip, int flags, void *args),
- int flags, void *args, int iter_flags);
-int xfs_inode_ag_iterator_tag(struct xfs_mount *mp,
+int xfs_inode_ag_iterator(struct xfs_mount *mp, int iter_flags,
int (*execute)(struct xfs_inode *ip, int flags, void *args),
int flags, void *args, int tag);
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 5d5ac65aa1cc..a9460bdcca87 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -772,6 +772,6 @@ xfs_qm_dqrele_all_inodes(
uint flags)
{
ASSERT(mp->m_quotainfo);
- xfs_inode_ag_iterator_flags(mp, xfs_dqrele_inode, flags, NULL,
- XFS_AGITER_INEW_WAIT);
+ xfs_inode_ag_iterator(mp, XFS_AGITER_INEW_WAIT, xfs_dqrele_inode,
+ flags, NULL, XFS_ICI_NO_TAG);
}
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 05/11] xfs: remove flags argument from xfs_inode_ag_walk
2020-05-20 1:45 [PATCH v3 00/11] xfs: refactor incore inode walking Darrick J. Wong
` (3 preceding siblings ...)
2020-05-20 1:45 ` [PATCH 04/11] xfs: remove xfs_inode_ag_iterator_flags Darrick J. Wong
@ 2020-05-20 1:45 ` Darrick J. Wong
2020-05-20 6:38 ` Christoph Hellwig
2020-05-20 1:45 ` [PATCH 06/11] xfs: remove __xfs_icache_free_eofblocks Darrick J. Wong
` (5 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Darrick J. Wong @ 2020-05-20 1:45 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
From: Darrick J. Wong <darrick.wong@oracle.com>
The incore inode walk code passes a flags argument and a pointer from
the xfs_inode_ag_iterator caller all the way to the iteration function.
We can reduce the function complexity by passing flags through the
private pointer.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/xfs_icache.c | 38 ++++++++++++++------------------------
fs/xfs/xfs_icache.h | 4 ++--
fs/xfs/xfs_qm_syscalls.c | 25 +++++++++++++++++--------
3 files changed, 33 insertions(+), 34 deletions(-)
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index e716b19879c6..87b98bfdf27d 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -790,9 +790,7 @@ STATIC int
xfs_inode_ag_walk(
struct xfs_mount *mp,
struct xfs_perag *pag,
- int (*execute)(struct xfs_inode *ip, int flags,
- void *args),
- int flags,
+ int (*execute)(struct xfs_inode *ip, void *args),
void *args,
int tag,
int iter_flags)
@@ -868,7 +866,7 @@ xfs_inode_ag_walk(
if ((iter_flags & XFS_AGITER_INEW_WAIT) &&
xfs_iflags_test(batch[i], XFS_INEW))
xfs_inew_wait(batch[i]);
- error = execute(batch[i], flags, args);
+ error = execute(batch[i], args);
xfs_irele(batch[i]);
if (error == -EAGAIN) {
skipped++;
@@ -972,9 +970,7 @@ int
xfs_inode_ag_iterator(
struct xfs_mount *mp,
int iter_flags,
- int (*execute)(struct xfs_inode *ip, int flags,
- void *args),
- int flags,
+ int (*execute)(struct xfs_inode *ip, void *args),
void *args,
int tag)
{
@@ -986,7 +982,7 @@ xfs_inode_ag_iterator(
ag = 0;
while ((pag = xfs_ici_walk_get_perag(mp, ag, tag))) {
ag = pag->pag_agno + 1;
- error = xfs_inode_ag_walk(mp, pag, execute, flags, args, tag,
+ error = xfs_inode_ag_walk(mp, pag, execute, args, tag,
iter_flags);
xfs_perag_put(pag);
if (error) {
@@ -1443,12 +1439,14 @@ xfs_inode_match_id_union(
STATIC int
xfs_inode_free_eofblocks(
struct xfs_inode *ip,
- int flags,
void *args)
{
- int ret = 0;
- struct xfs_eofblocks *eofb = args;
- int match;
+ struct xfs_eofblocks *eofb = args;
+ bool wait;
+ int match;
+ int ret = 0;
+
+ wait = (eofb && (eofb->eof_flags & XFS_EOF_FLAGS_SYNC));
if (!xfs_can_free_eofblocks(ip, false)) {
/* inode could be preallocated or append-only */
@@ -1461,8 +1459,7 @@ xfs_inode_free_eofblocks(
* If the mapping is dirty the operation can block and wait for some
* time. Unless we are waiting, skip it.
*/
- if (!(flags & SYNC_WAIT) &&
- mapping_tagged(VFS_I(ip)->i_mapping, PAGECACHE_TAG_DIRTY))
+ if (!wait && mapping_tagged(VFS_I(ip)->i_mapping, PAGECACHE_TAG_DIRTY))
return 0;
if (eofb) {
@@ -1484,7 +1481,7 @@ xfs_inode_free_eofblocks(
* scanner moving and revisit the inode in a subsequent pass.
*/
if (!xfs_ilock_nowait(ip, XFS_IOLOCK_EXCL)) {
- if (flags & SYNC_WAIT)
+ if (wait)
ret = -EAGAIN;
return ret;
}
@@ -1498,16 +1495,10 @@ static int
__xfs_icache_free_eofblocks(
struct xfs_mount *mp,
struct xfs_eofblocks *eofb,
- int (*execute)(struct xfs_inode *ip, int flags,
- void *args),
+ int (*execute)(struct xfs_inode *ip, void *args),
int tag)
{
- int flags = SYNC_TRYLOCK;
-
- if (eofb && (eofb->eof_flags & XFS_EOF_FLAGS_SYNC))
- flags = SYNC_WAIT;
-
- return xfs_inode_ag_iterator(mp, 0, execute, flags, eofb, tag);
+ return xfs_inode_ag_iterator(mp, 0, execute, eofb, tag);
}
int
@@ -1732,7 +1723,6 @@ xfs_prep_free_cowblocks(
STATIC int
xfs_inode_free_cowblocks(
struct xfs_inode *ip,
- int flags,
void *args)
{
struct xfs_eofblocks *eofb = args;
diff --git a/fs/xfs/xfs_icache.h b/fs/xfs/xfs_icache.h
index 2d5ab9957d9f..e7f86ebd7b22 100644
--- a/fs/xfs/xfs_icache.h
+++ b/fs/xfs/xfs_icache.h
@@ -72,8 +72,8 @@ void xfs_cowblocks_worker(struct work_struct *);
void xfs_queue_cowblocks(struct xfs_mount *);
int xfs_inode_ag_iterator(struct xfs_mount *mp, int iter_flags,
- int (*execute)(struct xfs_inode *ip, int flags, void *args),
- int flags, void *args, int tag);
+ int (*execute)(struct xfs_inode *ip, void *args),
+ void *args, int tag);
int xfs_icache_inode_is_allocated(struct xfs_mount *mp, struct xfs_trans *tp,
xfs_ino_t ino, bool *inuse);
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index a9460bdcca87..571ecb17b3bf 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -726,12 +726,17 @@ xfs_qm_scall_getquota_next(
return error;
}
+struct xfs_dqrele {
+ uint flags;
+};
+
STATIC int
xfs_dqrele_inode(
struct xfs_inode *ip,
- int flags,
void *args)
{
+ struct xfs_dqrele *dqr = args;
+
/* skip quota inodes */
if (ip == ip->i_mount->m_quotainfo->qi_uquotaip ||
ip == ip->i_mount->m_quotainfo->qi_gquotaip ||
@@ -743,15 +748,15 @@ xfs_dqrele_inode(
}
xfs_ilock(ip, XFS_ILOCK_EXCL);
- if ((flags & XFS_UQUOTA_ACCT) && ip->i_udquot) {
+ if ((dqr->flags & XFS_UQUOTA_ACCT) && ip->i_udquot) {
xfs_qm_dqrele(ip->i_udquot);
ip->i_udquot = NULL;
}
- if ((flags & XFS_GQUOTA_ACCT) && ip->i_gdquot) {
+ if ((dqr->flags & XFS_GQUOTA_ACCT) && ip->i_gdquot) {
xfs_qm_dqrele(ip->i_gdquot);
ip->i_gdquot = NULL;
}
- if ((flags & XFS_PQUOTA_ACCT) && ip->i_pdquot) {
+ if ((dqr->flags & XFS_PQUOTA_ACCT) && ip->i_pdquot) {
xfs_qm_dqrele(ip->i_pdquot);
ip->i_pdquot = NULL;
}
@@ -768,10 +773,14 @@ xfs_dqrele_inode(
*/
void
xfs_qm_dqrele_all_inodes(
- struct xfs_mount *mp,
- uint flags)
+ struct xfs_mount *mp,
+ uint flags)
{
+ struct xfs_dqrele dqr = {
+ .flags = flags,
+ };
+
ASSERT(mp->m_quotainfo);
- xfs_inode_ag_iterator(mp, XFS_AGITER_INEW_WAIT, xfs_dqrele_inode,
- flags, NULL, XFS_ICI_NO_TAG);
+ xfs_inode_ag_iterator(mp, XFS_AGITER_INEW_WAIT, xfs_dqrele_inode, &dqr,
+ XFS_ICI_NO_TAG);
}
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 06/11] xfs: remove __xfs_icache_free_eofblocks
2020-05-20 1:45 [PATCH v3 00/11] xfs: refactor incore inode walking Darrick J. Wong
` (4 preceding siblings ...)
2020-05-20 1:45 ` [PATCH 05/11] xfs: remove flags argument from xfs_inode_ag_walk Darrick J. Wong
@ 2020-05-20 1:45 ` Darrick J. Wong
2020-05-20 6:38 ` Christoph Hellwig
2020-05-20 1:46 ` [PATCH 07/11] xfs: refactor eofb matching into a single helper Darrick J. Wong
` (4 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Darrick J. Wong @ 2020-05-20 1:45 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
From: Darrick J. Wong <darrick.wong@oracle.com>
This is now a pointless wrapper, so kill it.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/xfs_icache.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 87b98bfdf27d..ac66e7d8698d 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -1491,22 +1491,12 @@ xfs_inode_free_eofblocks(
return ret;
}
-static int
-__xfs_icache_free_eofblocks(
- struct xfs_mount *mp,
- struct xfs_eofblocks *eofb,
- int (*execute)(struct xfs_inode *ip, void *args),
- int tag)
-{
- return xfs_inode_ag_iterator(mp, 0, execute, eofb, tag);
-}
-
int
xfs_icache_free_eofblocks(
struct xfs_mount *mp,
struct xfs_eofblocks *eofb)
{
- return __xfs_icache_free_eofblocks(mp, eofb, xfs_inode_free_eofblocks,
+ return xfs_inode_ag_iterator(mp, 0, xfs_inode_free_eofblocks, eofb,
XFS_ICI_EOFBLOCKS_TAG);
}
@@ -1768,7 +1758,7 @@ xfs_icache_free_cowblocks(
struct xfs_mount *mp,
struct xfs_eofblocks *eofb)
{
- return __xfs_icache_free_eofblocks(mp, eofb, xfs_inode_free_cowblocks,
+ return xfs_inode_ag_iterator(mp, 0, xfs_inode_free_cowblocks, eofb,
XFS_ICI_COWBLOCKS_TAG);
}
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 07/11] xfs: refactor eofb matching into a single helper
2020-05-20 1:45 [PATCH v3 00/11] xfs: refactor incore inode walking Darrick J. Wong
` (5 preceding siblings ...)
2020-05-20 1:45 ` [PATCH 06/11] xfs: remove __xfs_icache_free_eofblocks Darrick J. Wong
@ 2020-05-20 1:46 ` Darrick J. Wong
2020-05-20 6:42 ` Christoph Hellwig
2020-05-20 1:46 ` [PATCH 08/11] xfs: fix inode ag walk predicate function return values Darrick J. Wong
` (3 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Darrick J. Wong @ 2020-05-20 1:46 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
From: Darrick J. Wong <darrick.wong@oracle.com>
Refactor the two eofb-matching logics into a single helper so that we
don't repeat ourselves.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/xfs_icache.c | 59 +++++++++++++++++++++++++++------------------------
1 file changed, 31 insertions(+), 28 deletions(-)
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index ac66e7d8698d..1f12c6a0c48e 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -1436,6 +1436,33 @@ xfs_inode_match_id_union(
return 0;
}
+/*
+ * Is this inode @ip eligible for eof/cow block reclamation, given some
+ * filtering parameters @eofb? The inode is eligible if @eofb is null or
+ * if the predicate functions match.
+ */
+static bool
+xfs_inode_matches_eofb(
+ struct xfs_inode *ip,
+ struct xfs_eofblocks *eofb)
+{
+ int match;
+
+ if (!eofb)
+ return true;
+
+ if (eofb->eof_flags & XFS_EOF_FLAGS_UNION)
+ match = xfs_inode_match_id_union(ip, eofb);
+ else
+ match = xfs_inode_match_id(ip, eofb);
+ if (match)
+ return true;
+
+ /* skip the inode if the file size is too small */
+ return !(eofb->eof_flags & XFS_EOF_FLAGS_MINFILESIZE &&
+ XFS_ISIZE(ip) < eofb->eof_min_file_size);
+}
+
STATIC int
xfs_inode_free_eofblocks(
struct xfs_inode *ip,
@@ -1443,7 +1470,6 @@ xfs_inode_free_eofblocks(
{
struct xfs_eofblocks *eofb = args;
bool wait;
- int match;
int ret = 0;
wait = (eofb && (eofb->eof_flags & XFS_EOF_FLAGS_SYNC));
@@ -1462,19 +1488,8 @@ xfs_inode_free_eofblocks(
if (!wait && mapping_tagged(VFS_I(ip)->i_mapping, PAGECACHE_TAG_DIRTY))
return 0;
- if (eofb) {
- if (eofb->eof_flags & XFS_EOF_FLAGS_UNION)
- match = xfs_inode_match_id_union(ip, eofb);
- else
- match = xfs_inode_match_id(ip, eofb);
- if (!match)
- return 0;
-
- /* skip the inode if the file size is too small */
- if (eofb->eof_flags & XFS_EOF_FLAGS_MINFILESIZE &&
- XFS_ISIZE(ip) < eofb->eof_min_file_size)
- return 0;
- }
+ if (!xfs_inode_matches_eofb(ip, eofb))
+ return 0;
/*
* If the caller is waiting, return -EAGAIN to keep the background
@@ -1716,25 +1731,13 @@ xfs_inode_free_cowblocks(
void *args)
{
struct xfs_eofblocks *eofb = args;
- int match;
int ret = 0;
if (!xfs_prep_free_cowblocks(ip))
return 0;
- if (eofb) {
- if (eofb->eof_flags & XFS_EOF_FLAGS_UNION)
- match = xfs_inode_match_id_union(ip, eofb);
- else
- match = xfs_inode_match_id(ip, eofb);
- if (!match)
- return 0;
-
- /* skip the inode if the file size is too small */
- if (eofb->eof_flags & XFS_EOF_FLAGS_MINFILESIZE &&
- XFS_ISIZE(ip) < eofb->eof_min_file_size)
- return 0;
- }
+ if (!xfs_inode_matches_eofb(ip, eofb))
+ return 0;
/* Free the CoW blocks */
xfs_ilock(ip, XFS_IOLOCK_EXCL);
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 08/11] xfs: fix inode ag walk predicate function return values
2020-05-20 1:45 [PATCH v3 00/11] xfs: refactor incore inode walking Darrick J. Wong
` (6 preceding siblings ...)
2020-05-20 1:46 ` [PATCH 07/11] xfs: refactor eofb matching into a single helper Darrick J. Wong
@ 2020-05-20 1:46 ` Darrick J. Wong
2020-05-20 6:42 ` Christoph Hellwig
2020-05-20 1:46 ` [PATCH 09/11] xfs: use bool for done in xfs_inode_ag_walk Darrick J. Wong
` (2 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Darrick J. Wong @ 2020-05-20 1:46 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
From: Darrick J. Wong <darrick.wong@oracle.com>
There are a number of predicate functions that help the incore inode
walking code decide if we really want to apply the iteration function to
the inode. These are boolean decisions, so change the return types to
boolean to match.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/xfs_icache.c | 39 ++++++++++++++++++++++-----------------
1 file changed, 22 insertions(+), 17 deletions(-)
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 1f12c6a0c48e..926927a5f42e 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -741,7 +741,12 @@ xfs_icache_inode_is_allocated(
*/
#define XFS_LOOKUP_BATCH 32
-STATIC int
+/*
+ * Decide if the given @ip is eligible to be a part of the inode walk, and
+ * grab it if so. Returns true if it's ready to go or false if we should just
+ * ignore it.
+ */
+STATIC bool
xfs_inode_ag_walk_grab(
struct xfs_inode *ip,
int flags)
@@ -772,18 +777,18 @@ xfs_inode_ag_walk_grab(
/* nothing to sync during shutdown */
if (XFS_FORCED_SHUTDOWN(ip->i_mount))
- return -EFSCORRUPTED;
+ return false;
/* If we can't grab the inode, it must on it's way to reclaim. */
if (!igrab(inode))
- return -ENOENT;
+ return false;
/* inode is valid */
- return 0;
+ return true;
out_unlock_noent:
spin_unlock(&ip->i_flags_lock);
- return -ENOENT;
+ return false;
}
STATIC int
@@ -835,7 +840,7 @@ xfs_inode_ag_walk(
for (i = 0; i < nr_found; i++) {
struct xfs_inode *ip = batch[i];
- if (done || xfs_inode_ag_walk_grab(ip, iter_flags))
+ if (done || !xfs_inode_ag_walk_grab(ip, iter_flags))
batch[i] = NULL;
/*
@@ -1392,48 +1397,48 @@ xfs_reclaim_inodes_count(
return reclaimable;
}
-STATIC int
+STATIC bool
xfs_inode_match_id(
struct xfs_inode *ip,
struct xfs_eofblocks *eofb)
{
if ((eofb->eof_flags & XFS_EOF_FLAGS_UID) &&
!uid_eq(VFS_I(ip)->i_uid, eofb->eof_uid))
- return 0;
+ return false;
if ((eofb->eof_flags & XFS_EOF_FLAGS_GID) &&
!gid_eq(VFS_I(ip)->i_gid, eofb->eof_gid))
- return 0;
+ return false;
if ((eofb->eof_flags & XFS_EOF_FLAGS_PRID) &&
ip->i_d.di_projid != eofb->eof_prid)
- return 0;
+ return false;
- return 1;
+ return true;
}
/*
* A union-based inode filtering algorithm. Process the inode if any of the
* criteria match. This is for global/internal scans only.
*/
-STATIC int
+STATIC bool
xfs_inode_match_id_union(
struct xfs_inode *ip,
struct xfs_eofblocks *eofb)
{
if ((eofb->eof_flags & XFS_EOF_FLAGS_UID) &&
uid_eq(VFS_I(ip)->i_uid, eofb->eof_uid))
- return 1;
+ return true;
if ((eofb->eof_flags & XFS_EOF_FLAGS_GID) &&
gid_eq(VFS_I(ip)->i_gid, eofb->eof_gid))
- return 1;
+ return true;
if ((eofb->eof_flags & XFS_EOF_FLAGS_PRID) &&
ip->i_d.di_projid == eofb->eof_prid)
- return 1;
+ return true;
- return 0;
+ return false;
}
/*
@@ -1446,7 +1451,7 @@ xfs_inode_matches_eofb(
struct xfs_inode *ip,
struct xfs_eofblocks *eofb)
{
- int match;
+ bool match;
if (!eofb)
return true;
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 09/11] xfs: use bool for done in xfs_inode_ag_walk
2020-05-20 1:45 [PATCH v3 00/11] xfs: refactor incore inode walking Darrick J. Wong
` (7 preceding siblings ...)
2020-05-20 1:46 ` [PATCH 08/11] xfs: fix inode ag walk predicate function return values Darrick J. Wong
@ 2020-05-20 1:46 ` Darrick J. Wong
2020-05-20 6:43 ` Christoph Hellwig
2020-05-20 1:46 ` [PATCH 10/11] xfs: move xfs_inode_ag_iterator to be closer to the perag walking code Darrick J. Wong
2020-05-20 1:46 ` [PATCH 11/11] xfs: hide most of the incore inode walk interface Darrick J. Wong
10 siblings, 1 reply; 28+ messages in thread
From: Darrick J. Wong @ 2020-05-20 1:46 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
From: Darrick J. Wong <darrick.wong@oracle.com>
This is a boolean variable, so use the bool type.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/xfs_icache.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 926927a5f42e..6c8c626e7ef1 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -803,11 +803,11 @@ xfs_inode_ag_walk(
uint32_t first_index;
int last_error = 0;
int skipped;
- int done;
+ bool done;
int nr_found;
restart:
- done = 0;
+ done = false;
skipped = 0;
first_index = 0;
nr_found = 0;
@@ -859,7 +859,7 @@ xfs_inode_ag_walk(
continue;
first_index = XFS_INO_TO_AGINO(mp, ip->i_ino + 1);
if (first_index < XFS_INO_TO_AGINO(mp, ip->i_ino))
- done = 1;
+ done = true;
}
/* unlock now we've grabbed the inodes. */
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 10/11] xfs: move xfs_inode_ag_iterator to be closer to the perag walking code
2020-05-20 1:45 [PATCH v3 00/11] xfs: refactor incore inode walking Darrick J. Wong
` (8 preceding siblings ...)
2020-05-20 1:46 ` [PATCH 09/11] xfs: use bool for done in xfs_inode_ag_walk Darrick J. Wong
@ 2020-05-20 1:46 ` Darrick J. Wong
2020-05-20 6:43 ` Christoph Hellwig
2020-05-20 1:46 ` [PATCH 11/11] xfs: hide most of the incore inode walk interface Darrick J. Wong
10 siblings, 1 reply; 28+ messages in thread
From: Darrick J. Wong @ 2020-05-20 1:46 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
From: Darrick J. Wong <darrick.wong@oracle.com>
Move the xfs_inode_ag_iterator function to be nearer xfs_inode_ag_walk
so that we don't have to scroll back and forth to figure out how the
incore inode walking function works. No functional changes.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/xfs_icache.c | 88 ++++++++++++++++++++++++++++-----------------------
1 file changed, 48 insertions(+), 40 deletions(-)
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 6c8c626e7ef1..05614758fb1e 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -791,6 +791,10 @@ xfs_inode_ag_walk_grab(
return false;
}
+/*
+ * For a given per-AG structure @pag, @grab, @execute, and @rele all incore
+ * inodes with the given radix tree @tag.
+ */
STATIC int
xfs_inode_ag_walk(
struct xfs_mount *mp,
@@ -896,6 +900,50 @@ xfs_inode_ag_walk(
return last_error;
}
+/* Fetch the next (possibly tagged) per-AG structure. */
+static inline struct xfs_perag *
+xfs_ici_walk_get_perag(
+ struct xfs_mount *mp,
+ xfs_agnumber_t agno,
+ int tag)
+{
+ if (tag == XFS_ICI_NO_TAG)
+ return xfs_perag_get(mp, agno);
+ return xfs_perag_get_tag(mp, agno, tag);
+}
+
+/*
+ * Call the @execute function on all incore inodes matching the radix tree
+ * @tag.
+ */
+int
+xfs_inode_ag_iterator(
+ struct xfs_mount *mp,
+ int iter_flags,
+ int (*execute)(struct xfs_inode *ip, void *args),
+ void *args,
+ int tag)
+{
+ struct xfs_perag *pag;
+ int error = 0;
+ int last_error = 0;
+ xfs_agnumber_t ag;
+
+ ag = 0;
+ while ((pag = xfs_ici_walk_get_perag(mp, ag, tag))) {
+ ag = pag->pag_agno + 1;
+ error = xfs_inode_ag_walk(mp, pag, execute, args, tag,
+ iter_flags);
+ xfs_perag_put(pag);
+ if (error) {
+ last_error = error;
+ if (error == -EFSCORRUPTED)
+ break;
+ }
+ }
+ return last_error;
+}
+
/*
* Background scanning to trim post-EOF preallocated space. This is queued
* based on the 'speculative_prealloc_lifetime' tunable (5m by default).
@@ -959,46 +1007,6 @@ xfs_cowblocks_worker(
xfs_queue_cowblocks(mp);
}
-/* Fetch the next (possibly tagged) per-AG structure. */
-static inline struct xfs_perag *
-xfs_ici_walk_get_perag(
- struct xfs_mount *mp,
- xfs_agnumber_t agno,
- int tag)
-{
- if (tag == XFS_ICI_NO_TAG)
- return xfs_perag_get(mp, agno);
- return xfs_perag_get_tag(mp, agno, tag);
-}
-
-int
-xfs_inode_ag_iterator(
- struct xfs_mount *mp,
- int iter_flags,
- int (*execute)(struct xfs_inode *ip, void *args),
- void *args,
- int tag)
-{
- struct xfs_perag *pag;
- int error = 0;
- int last_error = 0;
- xfs_agnumber_t ag;
-
- ag = 0;
- while ((pag = xfs_ici_walk_get_perag(mp, ag, tag))) {
- ag = pag->pag_agno + 1;
- error = xfs_inode_ag_walk(mp, pag, execute, args, tag,
- iter_flags);
- xfs_perag_put(pag);
- if (error) {
- last_error = error;
- if (error == -EFSCORRUPTED)
- break;
- }
- }
- return last_error;
-}
-
/*
* Grab the inode for reclaim exclusively.
* Return 0 if we grabbed it, non-zero otherwise.
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 11/11] xfs: hide most of the incore inode walk interface
2020-05-20 1:45 [PATCH v3 00/11] xfs: refactor incore inode walking Darrick J. Wong
` (9 preceding siblings ...)
2020-05-20 1:46 ` [PATCH 10/11] xfs: move xfs_inode_ag_iterator to be closer to the perag walking code Darrick J. Wong
@ 2020-05-20 1:46 ` Darrick J. Wong
2020-05-20 6:46 ` Christoph Hellwig
10 siblings, 1 reply; 28+ messages in thread
From: Darrick J. Wong @ 2020-05-20 1:46 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs, hch
From: Darrick J. Wong <darrick.wong@oracle.com>
Hide the incore inode walk interface because callers outside of the
icache code don't need to know about iter_flags and radix tags and other
implementation details of the incore inode cache.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/xfs_icache.c | 30 ++++++++++++++++++++++--------
fs/xfs/xfs_icache.h | 8 ++++----
fs/xfs/xfs_qm_syscalls.c | 3 +--
3 files changed, 27 insertions(+), 14 deletions(-)
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 05614758fb1e..73ac0e18498e 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -752,7 +752,7 @@ xfs_inode_ag_walk_grab(
int flags)
{
struct inode *inode = VFS_I(ip);
- bool newinos = !!(flags & XFS_AGITER_INEW_WAIT);
+ bool newinos = !!(flags & XFS_ICI_WALK_INEW_WAIT);
ASSERT(rcu_read_lock_held());
@@ -796,7 +796,7 @@ xfs_inode_ag_walk_grab(
* inodes with the given radix tree @tag.
*/
STATIC int
-xfs_inode_ag_walk(
+xfs_ici_walk_ag(
struct xfs_mount *mp,
struct xfs_perag *pag,
int (*execute)(struct xfs_inode *ip, void *args),
@@ -872,7 +872,7 @@ xfs_inode_ag_walk(
for (i = 0; i < nr_found; i++) {
if (!batch[i])
continue;
- if ((iter_flags & XFS_AGITER_INEW_WAIT) &&
+ if ((iter_flags & XFS_ICI_WALK_INEW_WAIT) &&
xfs_iflags_test(batch[i], XFS_INEW))
xfs_inew_wait(batch[i]);
error = execute(batch[i], args);
@@ -916,8 +916,8 @@ xfs_ici_walk_get_perag(
* Call the @execute function on all incore inodes matching the radix tree
* @tag.
*/
-int
-xfs_inode_ag_iterator(
+STATIC int
+xfs_ici_walk(
struct xfs_mount *mp,
int iter_flags,
int (*execute)(struct xfs_inode *ip, void *args),
@@ -932,7 +932,7 @@ xfs_inode_ag_iterator(
ag = 0;
while ((pag = xfs_ici_walk_get_perag(mp, ag, tag))) {
ag = pag->pag_agno + 1;
- error = xfs_inode_ag_walk(mp, pag, execute, args, tag,
+ error = xfs_ici_walk_ag(mp, pag, execute, args, tag,
iter_flags);
xfs_perag_put(pag);
if (error) {
@@ -944,6 +944,20 @@ xfs_inode_ag_iterator(
return last_error;
}
+/*
+ * Walk all incore inodes in the filesystem. Knowledge of radix tree tags
+ * is hidden and we always wait for INEW inodes.
+ */
+int
+xfs_ici_walk_all(
+ struct xfs_mount *mp,
+ int (*execute)(struct xfs_inode *ip, void *args),
+ void *args)
+{
+ return xfs_ici_walk(mp, XFS_ICI_WALK_INEW_WAIT, execute, args,
+ XFS_ICI_NO_TAG);
+}
+
/*
* Background scanning to trim post-EOF preallocated space. This is queued
* based on the 'speculative_prealloc_lifetime' tunable (5m by default).
@@ -1524,7 +1538,7 @@ xfs_icache_free_eofblocks(
struct xfs_mount *mp,
struct xfs_eofblocks *eofb)
{
- return xfs_inode_ag_iterator(mp, 0, xfs_inode_free_eofblocks, eofb,
+ return xfs_ici_walk(mp, 0, xfs_inode_free_eofblocks, eofb,
XFS_ICI_EOFBLOCKS_TAG);
}
@@ -1774,7 +1788,7 @@ xfs_icache_free_cowblocks(
struct xfs_mount *mp,
struct xfs_eofblocks *eofb)
{
- return xfs_inode_ag_iterator(mp, 0, xfs_inode_free_cowblocks, eofb,
+ return xfs_ici_walk(mp, 0, xfs_inode_free_cowblocks, eofb,
XFS_ICI_COWBLOCKS_TAG);
}
diff --git a/fs/xfs/xfs_icache.h b/fs/xfs/xfs_icache.h
index e7f86ebd7b22..0dc85a03dc6c 100644
--- a/fs/xfs/xfs_icache.h
+++ b/fs/xfs/xfs_icache.h
@@ -38,9 +38,9 @@ struct xfs_eofblocks {
#define XFS_IGET_INCORE 0x8 /* don't read from disk or reinit */
/*
- * flags for AG inode iterator
+ * flags for incore inode iterator
*/
-#define XFS_AGITER_INEW_WAIT 0x1 /* wait on new inodes */
+#define XFS_ICI_WALK_INEW_WAIT 0x1 /* wait on new inodes */
int xfs_iget(struct xfs_mount *mp, struct xfs_trans *tp, xfs_ino_t ino,
uint flags, uint lock_flags, xfs_inode_t **ipp);
@@ -71,9 +71,9 @@ int xfs_inode_free_quota_cowblocks(struct xfs_inode *ip);
void xfs_cowblocks_worker(struct work_struct *);
void xfs_queue_cowblocks(struct xfs_mount *);
-int xfs_inode_ag_iterator(struct xfs_mount *mp, int iter_flags,
+int xfs_ici_walk_all(struct xfs_mount *mp,
int (*execute)(struct xfs_inode *ip, void *args),
- void *args, int tag);
+ void *args);
int xfs_icache_inode_is_allocated(struct xfs_mount *mp, struct xfs_trans *tp,
xfs_ino_t ino, bool *inuse);
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 571ecb17b3bf..95c44d5c57a8 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -781,6 +781,5 @@ xfs_qm_dqrele_all_inodes(
};
ASSERT(mp->m_quotainfo);
- xfs_inode_ag_iterator(mp, XFS_AGITER_INEW_WAIT, xfs_dqrele_inode, &dqr,
- XFS_ICI_NO_TAG);
+ xfs_ici_walk_all(mp, xfs_dqrele_inode, &dqr);
}
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 01/11] xfs: move eofblocks conversion function to xfs_ioctl.c
2020-05-20 1:45 ` [PATCH 01/11] xfs: move eofblocks conversion function to xfs_ioctl.c Darrick J. Wong
@ 2020-05-20 6:30 ` Christoph Hellwig
0 siblings, 0 replies; 28+ messages in thread
From: Christoph Hellwig @ 2020-05-20 6:30 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Tue, May 19, 2020 at 06:45:24PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Move xfs_fs_eofblocks_from_user into the only file that actually uses
> it, so that we don't have this function cluttering up the header file.
>
Thanks, the strange inline function really irked me when looking over the
code:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 02/11] xfs: replace open-coded XFS_ICI_NO_TAG
2020-05-20 1:45 ` [PATCH 02/11] xfs: replace open-coded XFS_ICI_NO_TAG Darrick J. Wong
@ 2020-05-20 6:30 ` Christoph Hellwig
0 siblings, 0 replies; 28+ messages in thread
From: Christoph Hellwig @ 2020-05-20 6:30 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Tue, May 19, 2020 at 06:45:30PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Use XFS_ICI_NO_TAG instead of -1 when appropriate.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 03/11] xfs: remove unused xfs_inode_ag_iterator function
2020-05-20 1:45 ` [PATCH 03/11] xfs: remove unused xfs_inode_ag_iterator function Darrick J. Wong
@ 2020-05-20 6:30 ` Christoph Hellwig
0 siblings, 0 replies; 28+ messages in thread
From: Christoph Hellwig @ 2020-05-20 6:30 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Tue, May 19, 2020 at 06:45:37PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Not used by anyone, so get rid of it.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 04/11] xfs: remove xfs_inode_ag_iterator_flags
2020-05-20 1:45 ` [PATCH 04/11] xfs: remove xfs_inode_ag_iterator_flags Darrick J. Wong
@ 2020-05-20 6:35 ` Christoph Hellwig
0 siblings, 0 replies; 28+ messages in thread
From: Christoph Hellwig @ 2020-05-20 6:35 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Tue, May 19, 2020 at 06:45:43PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Combine xfs_inode_ag_iterator_flags and xfs_inode_ag_iterator_tag into a
> single wrapper function since there's only one caller of the _flags
> variant.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 05/11] xfs: remove flags argument from xfs_inode_ag_walk
2020-05-20 1:45 ` [PATCH 05/11] xfs: remove flags argument from xfs_inode_ag_walk Darrick J. Wong
@ 2020-05-20 6:38 ` Christoph Hellwig
2020-05-20 18:36 ` Darrick J. Wong
0 siblings, 1 reply; 28+ messages in thread
From: Christoph Hellwig @ 2020-05-20 6:38 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Tue, May 19, 2020 at 06:45:49PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> The incore inode walk code passes a flags argument and a pointer from
> the xfs_inode_ag_iterator caller all the way to the iteration function.
> We can reduce the function complexity by passing flags through the
> private pointer.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/xfs_icache.c | 38 ++++++++++++++------------------------
> fs/xfs/xfs_icache.h | 4 ++--
> fs/xfs/xfs_qm_syscalls.c | 25 +++++++++++++++++--------
> 3 files changed, 33 insertions(+), 34 deletions(-)
>
>
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index e716b19879c6..87b98bfdf27d 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -790,9 +790,7 @@ STATIC int
> xfs_inode_ag_walk(
> struct xfs_mount *mp,
> struct xfs_perag *pag,
> - int (*execute)(struct xfs_inode *ip, int flags,
> - void *args),
> - int flags,
> + int (*execute)(struct xfs_inode *ip, void *args),
> void *args,
> int tag,
> int iter_flags)
> @@ -868,7 +866,7 @@ xfs_inode_ag_walk(
> if ((iter_flags & XFS_AGITER_INEW_WAIT) &&
> xfs_iflags_test(batch[i], XFS_INEW))
> xfs_inew_wait(batch[i]);
> - error = execute(batch[i], flags, args);
> + error = execute(batch[i], args);
> xfs_irele(batch[i]);
> if (error == -EAGAIN) {
> skipped++;
> @@ -972,9 +970,7 @@ int
> xfs_inode_ag_iterator(
> struct xfs_mount *mp,
> int iter_flags,
> - int (*execute)(struct xfs_inode *ip, int flags,
> - void *args),
> - int flags,
> + int (*execute)(struct xfs_inode *ip, void *args),
> void *args,
> int tag)
> {
> @@ -986,7 +982,7 @@ xfs_inode_ag_iterator(
> ag = 0;
> while ((pag = xfs_ici_walk_get_perag(mp, ag, tag))) {
> ag = pag->pag_agno + 1;
> - error = xfs_inode_ag_walk(mp, pag, execute, flags, args, tag,
> + error = xfs_inode_ag_walk(mp, pag, execute, args, tag,
> iter_flags);
> xfs_perag_put(pag);
> if (error) {
> @@ -1443,12 +1439,14 @@ xfs_inode_match_id_union(
> STATIC int
> xfs_inode_free_eofblocks(
> struct xfs_inode *ip,
> - int flags,
> void *args)
> {
> - int ret = 0;
> - struct xfs_eofblocks *eofb = args;
> - int match;
> + struct xfs_eofblocks *eofb = args;
> + bool wait;
> + int match;
> + int ret = 0;
> +
> + wait = (eofb && (eofb->eof_flags & XFS_EOF_FLAGS_SYNC));
No need for the outer braces.
> @@ -1484,7 +1481,7 @@ xfs_inode_free_eofblocks(
> * scanner moving and revisit the inode in a subsequent pass.
> */
> if (!xfs_ilock_nowait(ip, XFS_IOLOCK_EXCL)) {
> - if (flags & SYNC_WAIT)
> + if (wait)
> ret = -EAGAIN;
> return ret;
Just me, but I'd prefer an explicit:
if (wait)
return -EAGAIN;
return 0;
here. Not really new in this patch, but if you touch this area anyway..
> index a9460bdcca87..571ecb17b3bf 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -726,12 +726,17 @@ xfs_qm_scall_getquota_next(
> return error;
> }
>
> +struct xfs_dqrele {
> + uint flags;
> +};
> + struct xfs_dqrele dqr = {
> + .flags = flags,
> + };
Instead of a new structure we could just take the address of flags and
pass that to simplify the code a bit.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 06/11] xfs: remove __xfs_icache_free_eofblocks
2020-05-20 1:45 ` [PATCH 06/11] xfs: remove __xfs_icache_free_eofblocks Darrick J. Wong
@ 2020-05-20 6:38 ` Christoph Hellwig
0 siblings, 0 replies; 28+ messages in thread
From: Christoph Hellwig @ 2020-05-20 6:38 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Tue, May 19, 2020 at 06:45:55PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> This is now a pointless wrapper, so kill it.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 07/11] xfs: refactor eofb matching into a single helper
2020-05-20 1:46 ` [PATCH 07/11] xfs: refactor eofb matching into a single helper Darrick J. Wong
@ 2020-05-20 6:42 ` Christoph Hellwig
2020-05-20 18:35 ` Darrick J. Wong
0 siblings, 1 reply; 28+ messages in thread
From: Christoph Hellwig @ 2020-05-20 6:42 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Tue, May 19, 2020 at 06:46:02PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Refactor the two eofb-matching logics into a single helper so that we
> don't repeat ourselves.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/xfs_icache.c | 59 +++++++++++++++++++++++++++------------------------
> 1 file changed, 31 insertions(+), 28 deletions(-)
>
>
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index ac66e7d8698d..1f12c6a0c48e 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -1436,6 +1436,33 @@ xfs_inode_match_id_union(
> return 0;
> }
>
> +/*
> + * Is this inode @ip eligible for eof/cow block reclamation, given some
> + * filtering parameters @eofb? The inode is eligible if @eofb is null or
> + * if the predicate functions match.
> + */
> +static bool
> +xfs_inode_matches_eofb(
> + struct xfs_inode *ip,
> + struct xfs_eofblocks *eofb)
> +{
> + int match;
> +
> + if (!eofb)
> + return true;
> +
> + if (eofb->eof_flags & XFS_EOF_FLAGS_UNION)
> + match = xfs_inode_match_id_union(ip, eofb);
> + else
> + match = xfs_inode_match_id(ip, eofb);
> + if (match)
> + return true;
> +
> + /* skip the inode if the file size is too small */
> + return !(eofb->eof_flags & XFS_EOF_FLAGS_MINFILESIZE &&
> + XFS_ISIZE(ip) < eofb->eof_min_file_size);
This looks wrong - the size check should be applied if we did already
find a match and not override it based on the current code, e.g.:
if (eofb->eof_flags & XFS_EOF_FLAGS_UNION)
match = xfs_inode_match_id_union(ip, eofb);
else
match = xfs_inode_match_id(ip, eofb);
if (match) {
/* skip the inode if the file size is too small */
if ((eofb->eof_flags & XFS_EOF_FLAGS_MINFILESIZE) &&
XFS_ISIZE(ip) < eofb->eof_min_file_size)
return false;
}
return match;
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 08/11] xfs: fix inode ag walk predicate function return values
2020-05-20 1:46 ` [PATCH 08/11] xfs: fix inode ag walk predicate function return values Darrick J. Wong
@ 2020-05-20 6:42 ` Christoph Hellwig
0 siblings, 0 replies; 28+ messages in thread
From: Christoph Hellwig @ 2020-05-20 6:42 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 09/11] xfs: use bool for done in xfs_inode_ag_walk
2020-05-20 1:46 ` [PATCH 09/11] xfs: use bool for done in xfs_inode_ag_walk Darrick J. Wong
@ 2020-05-20 6:43 ` Christoph Hellwig
0 siblings, 0 replies; 28+ messages in thread
From: Christoph Hellwig @ 2020-05-20 6:43 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Tue, May 19, 2020 at 06:46:14PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> This is a boolean variable, so use the bool type.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 10/11] xfs: move xfs_inode_ag_iterator to be closer to the perag walking code
2020-05-20 1:46 ` [PATCH 10/11] xfs: move xfs_inode_ag_iterator to be closer to the perag walking code Darrick J. Wong
@ 2020-05-20 6:43 ` Christoph Hellwig
0 siblings, 0 replies; 28+ messages in thread
From: Christoph Hellwig @ 2020-05-20 6:43 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 11/11] xfs: hide most of the incore inode walk interface
2020-05-20 1:46 ` [PATCH 11/11] xfs: hide most of the incore inode walk interface Darrick J. Wong
@ 2020-05-20 6:46 ` Christoph Hellwig
2020-05-20 18:54 ` Darrick J. Wong
0 siblings, 1 reply; 28+ messages in thread
From: Christoph Hellwig @ 2020-05-20 6:46 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, hch
On Tue, May 19, 2020 at 06:46:27PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Hide the incore inode walk interface because callers outside of the
> icache code don't need to know about iter_flags and radix tags and other
> implementation details of the incore inode cache.
I don't really see the point here. It isn't hiding much, and only from
a single caller. I have to say I also prefer the old naming over _ici_
and find the _all postfix not exactly descriptive.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 07/11] xfs: refactor eofb matching into a single helper
2020-05-20 6:42 ` Christoph Hellwig
@ 2020-05-20 18:35 ` Darrick J. Wong
0 siblings, 0 replies; 28+ messages in thread
From: Darrick J. Wong @ 2020-05-20 18:35 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Wed, May 20, 2020 at 08:42:11AM +0200, Christoph Hellwig wrote:
> On Tue, May 19, 2020 at 06:46:02PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Refactor the two eofb-matching logics into a single helper so that we
> > don't repeat ourselves.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > fs/xfs/xfs_icache.c | 59 +++++++++++++++++++++++++++------------------------
> > 1 file changed, 31 insertions(+), 28 deletions(-)
> >
> >
> > diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> > index ac66e7d8698d..1f12c6a0c48e 100644
> > --- a/fs/xfs/xfs_icache.c
> > +++ b/fs/xfs/xfs_icache.c
> > @@ -1436,6 +1436,33 @@ xfs_inode_match_id_union(
> > return 0;
> > }
> >
> > +/*
> > + * Is this inode @ip eligible for eof/cow block reclamation, given some
> > + * filtering parameters @eofb? The inode is eligible if @eofb is null or
> > + * if the predicate functions match.
> > + */
> > +static bool
> > +xfs_inode_matches_eofb(
> > + struct xfs_inode *ip,
> > + struct xfs_eofblocks *eofb)
> > +{
> > + int match;
> > +
> > + if (!eofb)
> > + return true;
> > +
> > + if (eofb->eof_flags & XFS_EOF_FLAGS_UNION)
> > + match = xfs_inode_match_id_union(ip, eofb);
> > + else
> > + match = xfs_inode_match_id(ip, eofb);
> > + if (match)
> > + return true;
> > +
> > + /* skip the inode if the file size is too small */
> > + return !(eofb->eof_flags & XFS_EOF_FLAGS_MINFILESIZE &&
> > + XFS_ISIZE(ip) < eofb->eof_min_file_size);
>
> This looks wrong - the size check should be applied if we did already
> find a match and not override it based on the current code, e.g.:
>
> if (eofb->eof_flags & XFS_EOF_FLAGS_UNION)
> match = xfs_inode_match_id_union(ip, eofb);
> else
> match = xfs_inode_match_id(ip, eofb);
>
> if (match) {
> /* skip the inode if the file size is too small */
> if ((eofb->eof_flags & XFS_EOF_FLAGS_MINFILESIZE) &&
> XFS_ISIZE(ip) < eofb->eof_min_file_size)
> return false;
> }
>
> return match;
Ah, I see what I did wrong here; the size check was another opportunity
for us to say no even if the inode matched. Ok, I'll go fix it.
--D
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 05/11] xfs: remove flags argument from xfs_inode_ag_walk
2020-05-20 6:38 ` Christoph Hellwig
@ 2020-05-20 18:36 ` Darrick J. Wong
2020-05-21 9:02 ` Christoph Hellwig
0 siblings, 1 reply; 28+ messages in thread
From: Darrick J. Wong @ 2020-05-20 18:36 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Wed, May 20, 2020 at 08:38:25AM +0200, Christoph Hellwig wrote:
> On Tue, May 19, 2020 at 06:45:49PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > The incore inode walk code passes a flags argument and a pointer from
> > the xfs_inode_ag_iterator caller all the way to the iteration function.
> > We can reduce the function complexity by passing flags through the
> > private pointer.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > fs/xfs/xfs_icache.c | 38 ++++++++++++++------------------------
> > fs/xfs/xfs_icache.h | 4 ++--
> > fs/xfs/xfs_qm_syscalls.c | 25 +++++++++++++++++--------
> > 3 files changed, 33 insertions(+), 34 deletions(-)
> >
> >
> > diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> > index e716b19879c6..87b98bfdf27d 100644
> > --- a/fs/xfs/xfs_icache.c
> > +++ b/fs/xfs/xfs_icache.c
> > @@ -790,9 +790,7 @@ STATIC int
> > xfs_inode_ag_walk(
> > struct xfs_mount *mp,
> > struct xfs_perag *pag,
> > - int (*execute)(struct xfs_inode *ip, int flags,
> > - void *args),
> > - int flags,
> > + int (*execute)(struct xfs_inode *ip, void *args),
> > void *args,
> > int tag,
> > int iter_flags)
> > @@ -868,7 +866,7 @@ xfs_inode_ag_walk(
> > if ((iter_flags & XFS_AGITER_INEW_WAIT) &&
> > xfs_iflags_test(batch[i], XFS_INEW))
> > xfs_inew_wait(batch[i]);
> > - error = execute(batch[i], flags, args);
> > + error = execute(batch[i], args);
> > xfs_irele(batch[i]);
> > if (error == -EAGAIN) {
> > skipped++;
> > @@ -972,9 +970,7 @@ int
> > xfs_inode_ag_iterator(
> > struct xfs_mount *mp,
> > int iter_flags,
> > - int (*execute)(struct xfs_inode *ip, int flags,
> > - void *args),
> > - int flags,
> > + int (*execute)(struct xfs_inode *ip, void *args),
> > void *args,
> > int tag)
> > {
> > @@ -986,7 +982,7 @@ xfs_inode_ag_iterator(
> > ag = 0;
> > while ((pag = xfs_ici_walk_get_perag(mp, ag, tag))) {
> > ag = pag->pag_agno + 1;
> > - error = xfs_inode_ag_walk(mp, pag, execute, flags, args, tag,
> > + error = xfs_inode_ag_walk(mp, pag, execute, args, tag,
> > iter_flags);
> > xfs_perag_put(pag);
> > if (error) {
> > @@ -1443,12 +1439,14 @@ xfs_inode_match_id_union(
> > STATIC int
> > xfs_inode_free_eofblocks(
> > struct xfs_inode *ip,
> > - int flags,
> > void *args)
> > {
> > - int ret = 0;
> > - struct xfs_eofblocks *eofb = args;
> > - int match;
> > + struct xfs_eofblocks *eofb = args;
> > + bool wait;
> > + int match;
> > + int ret = 0;
> > +
> > + wait = (eofb && (eofb->eof_flags & XFS_EOF_FLAGS_SYNC));
>
> No need for the outer braces.
Fixed.
> > @@ -1484,7 +1481,7 @@ xfs_inode_free_eofblocks(
> > * scanner moving and revisit the inode in a subsequent pass.
> > */
> > if (!xfs_ilock_nowait(ip, XFS_IOLOCK_EXCL)) {
> > - if (flags & SYNC_WAIT)
> > + if (wait)
> > ret = -EAGAIN;
> > return ret;
>
> Just me, but I'd prefer an explicit:
>
> if (wait)
> return -EAGAIN;
> return 0;
>
> here. Not really new in this patch, but if you touch this area anyway..
How about 'return wait ? -EAGAIN : 0;' ?
> > index a9460bdcca87..571ecb17b3bf 100644
> > --- a/fs/xfs/xfs_qm_syscalls.c
> > +++ b/fs/xfs/xfs_qm_syscalls.c
> > @@ -726,12 +726,17 @@ xfs_qm_scall_getquota_next(
> > return error;
> > }
> >
> > +struct xfs_dqrele {
> > + uint flags;
> > +};
>
> > + struct xfs_dqrele dqr = {
> > + .flags = flags,
> > + };
>
> Instead of a new structure we could just take the address of flags and
> pass that to simplify the code a bit.
Fixed.
--D
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 11/11] xfs: hide most of the incore inode walk interface
2020-05-20 6:46 ` Christoph Hellwig
@ 2020-05-20 18:54 ` Darrick J. Wong
2020-05-21 9:05 ` Christoph Hellwig
0 siblings, 1 reply; 28+ messages in thread
From: Darrick J. Wong @ 2020-05-20 18:54 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Wed, May 20, 2020 at 08:46:43AM +0200, Christoph Hellwig wrote:
> On Tue, May 19, 2020 at 06:46:27PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Hide the incore inode walk interface because callers outside of the
> > icache code don't need to know about iter_flags and radix tags and other
> > implementation details of the incore inode cache.
>
> I don't really see the point here. It isn't hiding much, and only from
> a single caller. I have to say I also prefer the old naming over _ici_
> and find the _all postfix not exactly descriptive.
This last patch is more of a prep patch for the patchsets that come
after it: cleaning up the block gc stuff and deferred inode
inactivation. It's getting kinda late so I didn't want to send 11 more
patches, but perhaps that would make it clearer where this is all
heading?
The quota dqrele_all code does not care about inode radix tree tags nor
does it need the ability to grab inodes /while/ they're in INEW state,
so there's no reason to pass those arguments around.
OTOH I guess I could have hid XFS_AGITER_INEW_WAIT in xfs_icache.c and
left the function names unchanged.
--D
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 05/11] xfs: remove flags argument from xfs_inode_ag_walk
2020-05-20 18:36 ` Darrick J. Wong
@ 2020-05-21 9:02 ` Christoph Hellwig
0 siblings, 0 replies; 28+ messages in thread
From: Christoph Hellwig @ 2020-05-21 9:02 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs
On Wed, May 20, 2020 at 11:36:27AM -0700, Darrick J. Wong wrote:
> > > - if (flags & SYNC_WAIT)
> > > + if (wait)
> > > ret = -EAGAIN;
> > > return ret;
> >
> > Just me, but I'd prefer an explicit:
> >
> > if (wait)
> > return -EAGAIN;
> > return 0;
> >
> > here. Not really new in this patch, but if you touch this area anyway..
>
> How about 'return wait ? -EAGAIN : 0;' ?
Yikes. Why does everyone hate the nice, explicit and readable if
statements?
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 11/11] xfs: hide most of the incore inode walk interface
2020-05-20 18:54 ` Darrick J. Wong
@ 2020-05-21 9:05 ` Christoph Hellwig
0 siblings, 0 replies; 28+ messages in thread
From: Christoph Hellwig @ 2020-05-21 9:05 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs
On Wed, May 20, 2020 at 11:54:22AM -0700, Darrick J. Wong wrote:
> On Wed, May 20, 2020 at 08:46:43AM +0200, Christoph Hellwig wrote:
> > On Tue, May 19, 2020 at 06:46:27PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > >
> > > Hide the incore inode walk interface because callers outside of the
> > > icache code don't need to know about iter_flags and radix tags and other
> > > implementation details of the incore inode cache.
> >
> > I don't really see the point here. It isn't hiding much, and only from
> > a single caller. I have to say I also prefer the old naming over _ici_
> > and find the _all postfix not exactly descriptive.
>
> This last patch is more of a prep patch for the patchsets that come
> after it: cleaning up the block gc stuff and deferred inode
> inactivation. It's getting kinda late so I didn't want to send 11 more
> patches, but perhaps that would make it clearer where this is all
> heading?
I'd say drop it from this series and resend it with that series if
you are going to send it out.
> The quota dqrele_all code does not care about inode radix tree tags nor
It only started to care about them because you merged the functions
and now exposed them to the dqrele code at the beginning of this series.
But with just a single user not caring and too aring I'm perfectly fine
with exposing the tags in the interface.
> does it need the ability to grab inodes /while/ they're in INEW state,
> so there's no reason to pass those arguments around.
>
> OTOH I guess I could have hid XFS_AGITER_INEW_WAIT in xfs_icache.c and
> left the function names unchanged.
Maybe just flip the default for the flag if that makes your series
easier?
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2020-05-21 9:05 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-20 1:45 [PATCH v3 00/11] xfs: refactor incore inode walking Darrick J. Wong
2020-05-20 1:45 ` [PATCH 01/11] xfs: move eofblocks conversion function to xfs_ioctl.c Darrick J. Wong
2020-05-20 6:30 ` Christoph Hellwig
2020-05-20 1:45 ` [PATCH 02/11] xfs: replace open-coded XFS_ICI_NO_TAG Darrick J. Wong
2020-05-20 6:30 ` Christoph Hellwig
2020-05-20 1:45 ` [PATCH 03/11] xfs: remove unused xfs_inode_ag_iterator function Darrick J. Wong
2020-05-20 6:30 ` Christoph Hellwig
2020-05-20 1:45 ` [PATCH 04/11] xfs: remove xfs_inode_ag_iterator_flags Darrick J. Wong
2020-05-20 6:35 ` Christoph Hellwig
2020-05-20 1:45 ` [PATCH 05/11] xfs: remove flags argument from xfs_inode_ag_walk Darrick J. Wong
2020-05-20 6:38 ` Christoph Hellwig
2020-05-20 18:36 ` Darrick J. Wong
2020-05-21 9:02 ` Christoph Hellwig
2020-05-20 1:45 ` [PATCH 06/11] xfs: remove __xfs_icache_free_eofblocks Darrick J. Wong
2020-05-20 6:38 ` Christoph Hellwig
2020-05-20 1:46 ` [PATCH 07/11] xfs: refactor eofb matching into a single helper Darrick J. Wong
2020-05-20 6:42 ` Christoph Hellwig
2020-05-20 18:35 ` Darrick J. Wong
2020-05-20 1:46 ` [PATCH 08/11] xfs: fix inode ag walk predicate function return values Darrick J. Wong
2020-05-20 6:42 ` Christoph Hellwig
2020-05-20 1:46 ` [PATCH 09/11] xfs: use bool for done in xfs_inode_ag_walk Darrick J. Wong
2020-05-20 6:43 ` Christoph Hellwig
2020-05-20 1:46 ` [PATCH 10/11] xfs: move xfs_inode_ag_iterator to be closer to the perag walking code Darrick J. Wong
2020-05-20 6:43 ` Christoph Hellwig
2020-05-20 1:46 ` [PATCH 11/11] xfs: hide most of the incore inode walk interface Darrick J. Wong
2020-05-20 6:46 ` Christoph Hellwig
2020-05-20 18:54 ` Darrick J. Wong
2020-05-21 9:05 ` Christoph Hellwig
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.