* [Cluster-devel] [GFS2 PATCH 0/6] Refactor gfs2_evict_inode (V2)
@ 2020-09-16 15:00 Bob Peterson
2020-09-16 15:00 ` [Cluster-devel] [GFS2 PATCH 1/6] gfs2: switch variable error to ret in gfs2_evict_inode Bob Peterson
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: Bob Peterson @ 2020-09-16 15:00 UTC (permalink / raw
To: cluster-devel.redhat.com
Function gfs2_evict_inode is very large and messy. This patch set is an
attempt to simplify the function and make it more understandable. This
will make it easier to maintain.
This is the second version of this patch set, and it implements the suggestions
Andreas made.
It also now adds a patch to fix a bug in which unmount causes a kernel panic
when system inode glocks still have pages in memory.
Bob Peterson (6):
gfs2: switch variable error to ret in gfs2_evict_inode
gfs2: factor evict_deleted_inode out of gfs2_evict_inode
gfs2: further simplify gfs2_evict_inode with new func
evict_should_delete
gfs2: factor out evict code related to dinodes we are not deleting
gfs2: simplify the logic in gfs2_evict_inode
gfs2: special evict process for system inodes
fs/gfs2/ops_fstype.c | 15 ++-
fs/gfs2/super.c | 244 ++++++++++++++++++++++++++++---------------
2 files changed, 170 insertions(+), 89 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Cluster-devel] [GFS2 PATCH 1/6] gfs2: switch variable error to ret in gfs2_evict_inode
2020-09-16 15:00 [Cluster-devel] [GFS2 PATCH 0/6] Refactor gfs2_evict_inode (V2) Bob Peterson
@ 2020-09-16 15:00 ` Bob Peterson
2020-09-16 15:00 ` [Cluster-devel] [GFS2 PATCH 2/6] gfs2: factor evict_deleted_inode out of gfs2_evict_inode Bob Peterson
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Bob Peterson @ 2020-09-16 15:00 UTC (permalink / raw
To: cluster-devel.redhat.com
Function gfs2_evict_inode is too big and unreadable. This patch is
just a baby step toward improving that. This first step just renames
variable error to ret. This will help make future patches more readable.
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
fs/gfs2/super.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 19add2da1013..ab08b9a1102c 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -1338,7 +1338,7 @@ static void gfs2_evict_inode(struct inode *inode)
struct gfs2_inode *ip = GFS2_I(inode);
struct gfs2_holder gh;
struct address_space *metamapping;
- int error;
+ int ret;
if (test_bit(GIF_FREE_VFS_INODE, &ip->i_flags)) {
clear_inode(inode);
@@ -1362,8 +1362,8 @@ static void gfs2_evict_inode(struct inode *inode)
goto out;
/* Must not read inode block until block type has been verified */
- error = gfs2_glock_nq_init(ip->i_gl, LM_ST_EXCLUSIVE, GL_SKIP, &gh);
- if (unlikely(error)) {
+ ret = gfs2_glock_nq_init(ip->i_gl, LM_ST_EXCLUSIVE, GL_SKIP, &gh);
+ if (unlikely(ret)) {
glock_clear_object(ip->i_iopen_gh.gh_gl, ip);
ip->i_iopen_gh.gh_flags |= GL_NOCACHE;
gfs2_glock_dq_uninit(&ip->i_iopen_gh);
@@ -1372,13 +1372,13 @@ static void gfs2_evict_inode(struct inode *inode)
if (gfs2_inode_already_deleted(ip->i_gl, ip->i_no_formal_ino))
goto out_truncate;
- error = gfs2_check_blk_type(sdp, ip->i_no_addr, GFS2_BLKST_UNLINKED);
- if (error)
+ ret = gfs2_check_blk_type(sdp, ip->i_no_addr, GFS2_BLKST_UNLINKED);
+ if (ret)
goto out_truncate;
if (test_bit(GIF_INVALID, &ip->i_flags)) {
- error = gfs2_inode_refresh(ip);
- if (error)
+ ret = gfs2_inode_refresh(ip);
+ if (ret)
goto out_truncate;
}
@@ -1399,20 +1399,20 @@ static void gfs2_evict_inode(struct inode *inode)
if (S_ISDIR(inode->i_mode) &&
(ip->i_diskflags & GFS2_DIF_EXHASH)) {
- error = gfs2_dir_exhash_dealloc(ip);
- if (error)
+ ret = gfs2_dir_exhash_dealloc(ip);
+ if (ret)
goto out_unlock;
}
if (ip->i_eattr) {
- error = gfs2_ea_dealloc(ip);
- if (error)
+ ret = gfs2_ea_dealloc(ip);
+ if (ret)
goto out_unlock;
}
if (!gfs2_is_stuffed(ip)) {
- error = gfs2_file_dealloc(ip);
- if (error)
+ ret = gfs2_file_dealloc(ip);
+ if (ret)
goto out_unlock;
}
@@ -1421,7 +1421,7 @@ static void gfs2_evict_inode(struct inode *inode)
location and try to set gl_object again. We clear gl_object here so
that subsequent inode creates don't see an old gl_object. */
glock_clear_object(ip->i_gl, ip);
- error = gfs2_dinode_dealloc(ip);
+ ret = gfs2_dinode_dealloc(ip);
gfs2_inode_remember_delete(ip->i_gl, ip->i_no_formal_ino);
goto out_unlock;
@@ -1436,8 +1436,8 @@ static void gfs2_evict_inode(struct inode *inode)
write_inode_now(inode, 1);
gfs2_ail_flush(ip->i_gl, 0);
- error = gfs2_trans_begin(sdp, 0, sdp->sd_jdesc->jd_blocks);
- if (error)
+ ret = gfs2_trans_begin(sdp, 0, sdp->sd_jdesc->jd_blocks);
+ if (ret)
goto out_unlock;
/* Needs to be done before glock release & also in a transaction */
truncate_inode_pages(&inode->i_data, 0);
@@ -1452,8 +1452,8 @@ static void gfs2_evict_inode(struct inode *inode)
glock_clear_object(ip->i_gl, ip);
gfs2_glock_dq_uninit(&gh);
}
- if (error && error != GLR_TRYFAILED && error != -EROFS)
- fs_warn(sdp, "gfs2_evict_inode: %d\n", error);
+ if (ret && ret != GLR_TRYFAILED && ret != -EROFS)
+ fs_warn(sdp, "gfs2_evict_inode: %d\n", ret);
out:
truncate_inode_pages_final(&inode->i_data);
if (ip->i_qadata)
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Cluster-devel] [GFS2 PATCH 2/6] gfs2: factor evict_deleted_inode out of gfs2_evict_inode
2020-09-16 15:00 [Cluster-devel] [GFS2 PATCH 0/6] Refactor gfs2_evict_inode (V2) Bob Peterson
2020-09-16 15:00 ` [Cluster-devel] [GFS2 PATCH 1/6] gfs2: switch variable error to ret in gfs2_evict_inode Bob Peterson
@ 2020-09-16 15:00 ` Bob Peterson
2020-09-16 15:00 ` [Cluster-devel] [GFS2 PATCH 3/6] gfs2: further simplify gfs2_evict_inode with new func evict_should_delete Bob Peterson
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Bob Peterson @ 2020-09-16 15:00 UTC (permalink / raw
To: cluster-devel.redhat.com
Function gfs2_evict_inode is way too big, complex and unreadable. This is a
baby step toward breaking it apart to be more readable. It factors out
the portion that deletes the online bits for a dinode that is unlinked and
needs to be deleted. A future patch will factor out more. (If I factor
out too much, the patch itself becomes unreadable).
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
fs/gfs2/super.c | 67 +++++++++++++++++++++++++++++--------------------
1 file changed, 40 insertions(+), 27 deletions(-)
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index ab08b9a1102c..1e00a72a158d 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -1310,6 +1310,45 @@ static bool gfs2_upgrade_iopen_glock(struct inode *inode)
return true;
}
+/**
+ * evict_deleted_inode - delete the pieces of an unlinked evicted inode
+ * @inode: The inode to evict
+ */
+static int evict_deleted_inode(struct inode *inode)
+{
+ struct gfs2_inode *ip = GFS2_I(inode);
+ int ret;
+
+ if (S_ISDIR(inode->i_mode) &&
+ (ip->i_diskflags & GFS2_DIF_EXHASH)) {
+ ret = gfs2_dir_exhash_dealloc(ip);
+ if (ret)
+ goto out;
+ }
+
+ if (ip->i_eattr) {
+ ret = gfs2_ea_dealloc(ip);
+ if (ret)
+ goto out;
+ }
+
+ if (!gfs2_is_stuffed(ip)) {
+ ret = gfs2_file_dealloc(ip);
+ if (ret)
+ goto out;
+ }
+
+ /* We're about to clear the bitmap for the dinode, but as soon as we
+ do, gfs2_create_inode can create another inode at the same block
+ location and try to set gl_object again. We clear gl_object here so
+ that subsequent inode creates don't see an old gl_object. */
+ glock_clear_object(ip->i_gl, ip);
+ ret = gfs2_dinode_dealloc(ip);
+ gfs2_inode_remember_delete(ip->i_gl, ip->i_no_formal_ino);
+out:
+ return ret;
+}
+
/**
* gfs2_evict_inode - Remove an inode from cache
* @inode: The inode to evict
@@ -1396,33 +1435,7 @@ static void gfs2_evict_inode(struct inode *inode)
goto out_truncate;
}
}
-
- if (S_ISDIR(inode->i_mode) &&
- (ip->i_diskflags & GFS2_DIF_EXHASH)) {
- ret = gfs2_dir_exhash_dealloc(ip);
- if (ret)
- goto out_unlock;
- }
-
- if (ip->i_eattr) {
- ret = gfs2_ea_dealloc(ip);
- if (ret)
- goto out_unlock;
- }
-
- if (!gfs2_is_stuffed(ip)) {
- ret = gfs2_file_dealloc(ip);
- if (ret)
- goto out_unlock;
- }
-
- /* We're about to clear the bitmap for the dinode, but as soon as we
- do, gfs2_create_inode can create another inode at the same block
- location and try to set gl_object again. We clear gl_object here so
- that subsequent inode creates don't see an old gl_object. */
- glock_clear_object(ip->i_gl, ip);
- ret = gfs2_dinode_dealloc(ip);
- gfs2_inode_remember_delete(ip->i_gl, ip->i_no_formal_ino);
+ ret = evict_deleted_inode(inode);
goto out_unlock;
out_truncate:
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Cluster-devel] [GFS2 PATCH 3/6] gfs2: further simplify gfs2_evict_inode with new func evict_should_delete
2020-09-16 15:00 [Cluster-devel] [GFS2 PATCH 0/6] Refactor gfs2_evict_inode (V2) Bob Peterson
2020-09-16 15:00 ` [Cluster-devel] [GFS2 PATCH 1/6] gfs2: switch variable error to ret in gfs2_evict_inode Bob Peterson
2020-09-16 15:00 ` [Cluster-devel] [GFS2 PATCH 2/6] gfs2: factor evict_deleted_inode out of gfs2_evict_inode Bob Peterson
@ 2020-09-16 15:00 ` Bob Peterson
2020-09-16 15:00 ` [Cluster-devel] [GFS2 PATCH 4/6] gfs2: factor out evict code related to dinodes we are not deleting Bob Peterson
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Bob Peterson @ 2020-09-16 15:00 UTC (permalink / raw
To: cluster-devel.redhat.com
This patch further simplifies function gfs2_evict_inode() by adding a new
function evict_should_delete. The function may also lock the inode glock.
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
fs/gfs2/super.c | 112 +++++++++++++++++++++++++++++++-----------------
1 file changed, 72 insertions(+), 40 deletions(-)
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 1e00a72a158d..1e2ff066f8d4 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -44,6 +44,12 @@
#include "xattr.h"
#include "lops.h"
+enum dinode_demise {
+ SHOULD_DELETE_DINODE,
+ SHOULD_NOT_DELETE_DINODE,
+ SHOULD_DEFER_EVICTION,
+};
+
/**
* gfs2_jindex_free - Clear all the journal index information
* @sdp: The GFS2 superblock
@@ -1350,91 +1356,117 @@ static int evict_deleted_inode(struct inode *inode)
}
/**
- * gfs2_evict_inode - Remove an inode from cache
+ * evict_should_delete - determine whether the inode is eligible for deletion
* @inode: The inode to evict
*
- * There are three cases to consider:
- * 1. i_nlink == 0, we are final opener (and must deallocate)
- * 2. i_nlink == 0, we are not the final opener (and cannot deallocate)
- * 3. i_nlink > 0
- *
- * If the fs is read only, then we have to treat all cases as per #3
- * since we are unable to do any deallocation. The inode will be
- * deallocated by the next read/write node to attempt an allocation
- * in the same resource group
+ * This function determines whether the evicted inode is eligible to be deleted
+ * and locks the inode glock.
*
- * We have to (at the moment) hold the inodes main lock to cover
- * the gap between unlocking the shared lock on the iopen lock and
- * taking the exclusive lock. I'd rather do a shared -> exclusive
- * conversion on the iopen lock, but we can change that later. This
- * is safe, just less efficient.
+ * Returns: the fate of the dinode
*/
-
-static void gfs2_evict_inode(struct inode *inode)
+static enum dinode_demise evict_should_delete(struct inode *inode,
+ struct gfs2_holder *gh)
{
+ struct gfs2_inode *ip = GFS2_I(inode);
struct super_block *sb = inode->i_sb;
struct gfs2_sbd *sdp = sb->s_fs_info;
- struct gfs2_inode *ip = GFS2_I(inode);
- struct gfs2_holder gh;
- struct address_space *metamapping;
int ret;
- if (test_bit(GIF_FREE_VFS_INODE, &ip->i_flags)) {
- clear_inode(inode);
- return;
- }
-
- if (inode->i_nlink || sb_rdonly(sb))
- goto out;
-
if (test_bit(GIF_ALLOC_FAILED, &ip->i_flags)) {
BUG_ON(!gfs2_glock_is_locked_by_me(ip->i_gl));
- gfs2_holder_mark_uninitialized(&gh);
- goto out_delete;
+ goto should_delete;
}
if (test_bit(GIF_DEFERRED_DELETE, &ip->i_flags))
- goto out;
+ return SHOULD_DEFER_EVICTION;
/* Deletes should never happen under memory pressure anymore. */
if (WARN_ON_ONCE(current->flags & PF_MEMALLOC))
- goto out;
+ return SHOULD_DEFER_EVICTION;
/* Must not read inode block until block type has been verified */
- ret = gfs2_glock_nq_init(ip->i_gl, LM_ST_EXCLUSIVE, GL_SKIP, &gh);
+ ret = gfs2_glock_nq_init(ip->i_gl, LM_ST_EXCLUSIVE, GL_SKIP, gh);
if (unlikely(ret)) {
glock_clear_object(ip->i_iopen_gh.gh_gl, ip);
ip->i_iopen_gh.gh_flags |= GL_NOCACHE;
gfs2_glock_dq_uninit(&ip->i_iopen_gh);
- goto out;
+ return SHOULD_DEFER_EVICTION;
}
if (gfs2_inode_already_deleted(ip->i_gl, ip->i_no_formal_ino))
- goto out_truncate;
+ return SHOULD_NOT_DELETE_DINODE;
ret = gfs2_check_blk_type(sdp, ip->i_no_addr, GFS2_BLKST_UNLINKED);
if (ret)
- goto out_truncate;
+ return SHOULD_NOT_DELETE_DINODE;
if (test_bit(GIF_INVALID, &ip->i_flags)) {
ret = gfs2_inode_refresh(ip);
if (ret)
- goto out_truncate;
+ return SHOULD_NOT_DELETE_DINODE;
}
/*
* The inode may have been recreated in the meantime.
*/
if (inode->i_nlink)
- goto out_truncate;
+ return SHOULD_NOT_DELETE_DINODE;
-out_delete:
+should_delete:
if (gfs2_holder_initialized(&ip->i_iopen_gh) &&
test_bit(HIF_HOLDER, &ip->i_iopen_gh.gh_iflags)) {
if (!gfs2_upgrade_iopen_glock(inode)) {
gfs2_holder_uninit(&ip->i_iopen_gh);
- goto out_truncate;
+ return SHOULD_NOT_DELETE_DINODE;
}
}
+ return SHOULD_DELETE_DINODE;
+}
+
+/**
+ * gfs2_evict_inode - Remove an inode from cache
+ * @inode: The inode to evict
+ *
+ * There are three cases to consider:
+ * 1. i_nlink == 0, we are final opener (and must deallocate)
+ * 2. i_nlink == 0, we are not the final opener (and cannot deallocate)
+ * 3. i_nlink > 0
+ *
+ * If the fs is read only, then we have to treat all cases as per #3
+ * since we are unable to do any deallocation. The inode will be
+ * deallocated by the next read/write node to attempt an allocation
+ * in the same resource group
+ *
+ * We have to (at the moment) hold the inodes main lock to cover
+ * the gap between unlocking the shared lock on the iopen lock and
+ * taking the exclusive lock. I'd rather do a shared -> exclusive
+ * conversion on the iopen lock, but we can change that later. This
+ * is safe, just less efficient.
+ */
+
+static void gfs2_evict_inode(struct inode *inode)
+{
+ struct super_block *sb = inode->i_sb;
+ struct gfs2_sbd *sdp = sb->s_fs_info;
+ struct gfs2_inode *ip = GFS2_I(inode);
+ struct gfs2_holder gh;
+ struct address_space *metamapping;
+ int ret;
+
+ if (test_bit(GIF_FREE_VFS_INODE, &ip->i_flags)) {
+ clear_inode(inode);
+ return;
+ }
+
+ if (inode->i_nlink || sb_rdonly(sb))
+ goto out;
+
+ gfs2_holder_mark_uninitialized(&gh);
+ ret = evict_should_delete(inode, &gh);
+ if (ret == SHOULD_DEFER_EVICTION)
+ goto out;
+ if (ret == SHOULD_NOT_DELETE_DINODE)
+ goto out_truncate;
+
ret = evict_deleted_inode(inode);
goto out_unlock;
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Cluster-devel] [GFS2 PATCH 4/6] gfs2: factor out evict code related to dinodes we are not deleting
2020-09-16 15:00 [Cluster-devel] [GFS2 PATCH 0/6] Refactor gfs2_evict_inode (V2) Bob Peterson
` (2 preceding siblings ...)
2020-09-16 15:00 ` [Cluster-devel] [GFS2 PATCH 3/6] gfs2: further simplify gfs2_evict_inode with new func evict_should_delete Bob Peterson
@ 2020-09-16 15:00 ` Bob Peterson
2020-09-16 15:00 ` [Cluster-devel] [GFS2 PATCH 5/6] gfs2: simplify the logic in gfs2_evict_inode Bob Peterson
2020-09-16 15:00 ` [Cluster-devel] [GFS2 PATCH 6/6] gfs2: special evict process for system inodes Bob Peterson
5 siblings, 0 replies; 7+ messages in thread
From: Bob Peterson @ 2020-09-16 15:00 UTC (permalink / raw
To: cluster-devel.redhat.com
Now that we've factored out the delete-dinode case to simplify gfs2_evict_inode
we take it a step further and factor out the other case: where we don't
delete the inode.
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
fs/gfs2/super.c | 52 ++++++++++++++++++++++++++++++++-----------------
1 file changed, 34 insertions(+), 18 deletions(-)
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 1e2ff066f8d4..9fc4135a35c0 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -1422,6 +1422,39 @@ static enum dinode_demise evict_should_delete(struct inode *inode,
return SHOULD_DELETE_DINODE;
}
+/**
+ * evict_saved_inode - evict an inode whose dinode has not been deleted
+ * @inode: The inode to evict
+ */
+static int evict_saved_inode(struct inode *inode)
+{
+ struct super_block *sb = inode->i_sb;
+ struct gfs2_sbd *sdp = sb->s_fs_info;
+ struct gfs2_inode *ip = GFS2_I(inode);
+ struct address_space *metamapping;
+ int ret;
+
+ gfs2_log_flush(sdp, ip->i_gl, GFS2_LOG_HEAD_FLUSH_NORMAL |
+ GFS2_LFC_EVICT_INODE);
+ metamapping = gfs2_glock2aspace(ip->i_gl);
+ if (test_bit(GLF_DIRTY, &ip->i_gl->gl_flags)) {
+ filemap_fdatawrite(metamapping);
+ filemap_fdatawait(metamapping);
+ }
+ write_inode_now(inode, 1);
+ gfs2_ail_flush(ip->i_gl, 0);
+
+ ret = gfs2_trans_begin(sdp, 0, sdp->sd_jdesc->jd_blocks);
+ if (ret)
+ return ret;
+
+ /* Needs to be done before glock release & also in a transaction */
+ truncate_inode_pages(&inode->i_data, 0);
+ truncate_inode_pages(metamapping, 0);
+ gfs2_trans_end(sdp);
+ return 0;
+}
+
/**
* gfs2_evict_inode - Remove an inode from cache
* @inode: The inode to evict
@@ -1449,7 +1482,6 @@ static void gfs2_evict_inode(struct inode *inode)
struct gfs2_sbd *sdp = sb->s_fs_info;
struct gfs2_inode *ip = GFS2_I(inode);
struct gfs2_holder gh;
- struct address_space *metamapping;
int ret;
if (test_bit(GIF_FREE_VFS_INODE, &ip->i_flags)) {
@@ -1471,23 +1503,7 @@ static void gfs2_evict_inode(struct inode *inode)
goto out_unlock;
out_truncate:
- gfs2_log_flush(sdp, ip->i_gl, GFS2_LOG_HEAD_FLUSH_NORMAL |
- GFS2_LFC_EVICT_INODE);
- metamapping = gfs2_glock2aspace(ip->i_gl);
- if (test_bit(GLF_DIRTY, &ip->i_gl->gl_flags)) {
- filemap_fdatawrite(metamapping);
- filemap_fdatawait(metamapping);
- }
- write_inode_now(inode, 1);
- gfs2_ail_flush(ip->i_gl, 0);
-
- ret = gfs2_trans_begin(sdp, 0, sdp->sd_jdesc->jd_blocks);
- if (ret)
- goto out_unlock;
- /* Needs to be done before glock release & also in a transaction */
- truncate_inode_pages(&inode->i_data, 0);
- truncate_inode_pages(metamapping, 0);
- gfs2_trans_end(sdp);
+ ret = evict_saved_inode(inode);
out_unlock:
if (gfs2_rs_active(&ip->i_res))
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Cluster-devel] [GFS2 PATCH 5/6] gfs2: simplify the logic in gfs2_evict_inode
2020-09-16 15:00 [Cluster-devel] [GFS2 PATCH 0/6] Refactor gfs2_evict_inode (V2) Bob Peterson
` (3 preceding siblings ...)
2020-09-16 15:00 ` [Cluster-devel] [GFS2 PATCH 4/6] gfs2: factor out evict code related to dinodes we are not deleting Bob Peterson
@ 2020-09-16 15:00 ` Bob Peterson
2020-09-16 15:00 ` [Cluster-devel] [GFS2 PATCH 6/6] gfs2: special evict process for system inodes Bob Peterson
5 siblings, 0 replies; 7+ messages in thread
From: Bob Peterson @ 2020-09-16 15:00 UTC (permalink / raw
To: cluster-devel.redhat.com
Now that we've factored out the deleted and undeleted dinode cases
in gfs2_evict_inode, we can greatly simplify the logic. Now the function
is easy to read and understand.
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
fs/gfs2/super.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 9fc4135a35c0..e07399110cd0 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -1496,16 +1496,11 @@ static void gfs2_evict_inode(struct inode *inode)
ret = evict_should_delete(inode, &gh);
if (ret == SHOULD_DEFER_EVICTION)
goto out;
- if (ret == SHOULD_NOT_DELETE_DINODE)
- goto out_truncate;
-
- ret = evict_deleted_inode(inode);
- goto out_unlock;
-
-out_truncate:
- ret = evict_saved_inode(inode);
+ if (ret == SHOULD_DELETE_DINODE)
+ ret = evict_deleted_inode(inode);
+ else
+ ret = evict_saved_inode(inode);
-out_unlock:
if (gfs2_rs_active(&ip->i_res))
gfs2_rs_deltree(&ip->i_res);
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Cluster-devel] [GFS2 PATCH 6/6] gfs2: special evict process for system inodes
2020-09-16 15:00 [Cluster-devel] [GFS2 PATCH 0/6] Refactor gfs2_evict_inode (V2) Bob Peterson
` (4 preceding siblings ...)
2020-09-16 15:00 ` [Cluster-devel] [GFS2 PATCH 5/6] gfs2: simplify the logic in gfs2_evict_inode Bob Peterson
@ 2020-09-16 15:00 ` Bob Peterson
5 siblings, 0 replies; 7+ messages in thread
From: Bob Peterson @ 2020-09-16 15:00 UTC (permalink / raw
To: cluster-devel.redhat.com
This is tricky. Ordinary user inodes are iput from function evict_inodes
before gfs2_put_super is called, which means the file system is still RW.
If not deleted, the evict truncates both the normal address space and inode
glock address space, but first it needs to start a transaction which is
needed in order to add revokes for any changes.
truncate_inode_pages
truncate_inode_pages_range
gfs2_invalidatepage
gfs2_discard
gfs2_remove_from_journal
gfs2_trans_add_revoke <------- Needs a transaction
Deleted inodes go through a different code path which should end with a
log flush to add the necessary revokes.
However, system inodes (like sd_quota_inode) cannot be iput until after
the call to gfs2_make_fs_ro. This is because inodes (like statfs) may be
accessed, even while the file system is mounted ro. In the process of
reading them in, we use the glock address space to fetch the metadata,
which gives the address space some pages (nrpages) that later need
truncating.
However, at that point it is impossible to start a new transaction because
of the read-only status. So we need to truncate_inode_pages for the inode's
glock's address space. System inodes that have been changed should already
be committed to the journal and revoked when this happens, so we only need
to truncate the glock's address space. The iput's evict will take care of
truncating the inode's address space, but we need to truncate the glock's
address space first, because the evict may free the glock.
This patch adds two functions: evict_system_inode and dput_system_dentry.
Func evict_system_inode calls truncate_inode_pages for system files.
Func dput_system_dentry calls truncate_inode_pages for system directories.
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
fs/gfs2/ops_fstype.c | 15 +++++++++++++--
fs/gfs2/super.c | 28 +++++++++++++++++++++-------
2 files changed, 34 insertions(+), 9 deletions(-)
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 6d18d2c91add..3d543ab54682 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -1585,6 +1585,17 @@ static int gfs2_meta_init_fs_context(struct fs_context *fc)
return 0;
}
+static void dput_system_dentry(struct dentry *dentry)
+{
+ struct inode *inode = dentry->d_inode;
+ struct gfs2_inode *ip = GFS2_I(inode);
+ struct gfs2_glock *gl = ip->i_gl;
+
+ if (gl)
+ truncate_inode_pages(gfs2_glock2aspace(gl), 0);
+ dput(dentry);
+}
+
static void gfs2_kill_sb(struct super_block *sb)
{
struct gfs2_sbd *sdp = sb->s_fs_info;
@@ -1595,8 +1606,8 @@ static void gfs2_kill_sb(struct super_block *sb)
}
gfs2_log_flush(sdp, NULL, GFS2_LOG_HEAD_FLUSH_SYNC | GFS2_LFC_KILL_SB);
- dput(sdp->sd_root_dir);
- dput(sdp->sd_master_dir);
+ dput_system_dentry(sdp->sd_root_dir);
+ dput_system_dentry(sdp->sd_master_dir);
sdp->sd_root_dir = NULL;
sdp->sd_master_dir = NULL;
shrink_dcache_sb(sb);
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index e07399110cd0..96c6582c8883 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -50,6 +50,20 @@ enum dinode_demise {
SHOULD_DEFER_EVICTION,
};
+/**
+ * evict_system_inode - evict a system inode truncating its glock address space
+ * @inode: the inode to be evicted
+ */
+static void evict_system_inode(struct inode *inode)
+{
+ struct gfs2_inode *ip = GFS2_I(inode);
+ struct gfs2_glock *gl = ip->i_gl;
+
+ if (gl)
+ truncate_inode_pages(gfs2_glock2aspace(gl), 0);
+ iput(inode);
+}
+
/**
* gfs2_jindex_free - Clear all the journal index information
* @sdp: The GFS2 superblock
@@ -72,7 +86,7 @@ void gfs2_jindex_free(struct gfs2_sbd *sdp)
jd = list_first_entry(&list, struct gfs2_jdesc, jd_list);
gfs2_free_journal_extents(jd);
list_del(&jd->jd_list);
- iput(jd->jd_inode);
+ evict_system_inode(jd->jd_inode);
jd->jd_inode = NULL;
kfree(jd);
}
@@ -714,10 +728,10 @@ static void gfs2_put_super(struct super_block *sb)
/* Release stuff */
- iput(sdp->sd_jindex);
- iput(sdp->sd_statfs_inode);
- iput(sdp->sd_rindex);
- iput(sdp->sd_quota_inode);
+ evict_system_inode(sdp->sd_jindex);
+ evict_system_inode(sdp->sd_statfs_inode);
+ evict_system_inode(sdp->sd_rindex);
+ evict_system_inode(sdp->sd_quota_inode);
gfs2_glock_put(sdp->sd_rename_gl);
gfs2_glock_put(sdp->sd_freeze_gl);
@@ -729,8 +743,8 @@ static void gfs2_put_super(struct super_block *sb)
gfs2_glock_dq_uninit(&sdp->sd_jinode_gh);
gfs2_glock_dq_uninit(&sdp->sd_sc_gh);
gfs2_glock_dq_uninit(&sdp->sd_qc_gh);
- iput(sdp->sd_sc_inode);
- iput(sdp->sd_qc_inode);
+ evict_system_inode(sdp->sd_sc_inode);
+ evict_system_inode(sdp->sd_qc_inode);
}
gfs2_glock_dq_uninit(&sdp->sd_live_gh);
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-09-16 15:00 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-16 15:00 [Cluster-devel] [GFS2 PATCH 0/6] Refactor gfs2_evict_inode (V2) Bob Peterson
2020-09-16 15:00 ` [Cluster-devel] [GFS2 PATCH 1/6] gfs2: switch variable error to ret in gfs2_evict_inode Bob Peterson
2020-09-16 15:00 ` [Cluster-devel] [GFS2 PATCH 2/6] gfs2: factor evict_deleted_inode out of gfs2_evict_inode Bob Peterson
2020-09-16 15:00 ` [Cluster-devel] [GFS2 PATCH 3/6] gfs2: further simplify gfs2_evict_inode with new func evict_should_delete Bob Peterson
2020-09-16 15:00 ` [Cluster-devel] [GFS2 PATCH 4/6] gfs2: factor out evict code related to dinodes we are not deleting Bob Peterson
2020-09-16 15:00 ` [Cluster-devel] [GFS2 PATCH 5/6] gfs2: simplify the logic in gfs2_evict_inode Bob Peterson
2020-09-16 15:00 ` [Cluster-devel] [GFS2 PATCH 6/6] gfs2: special evict process for system inodes Bob Peterson
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.