Re: [Cluster-devel] [GFS2 PATCH 06/12] gfs2: Create transaction for inodes with i_nlink != 0

[Steven Whitehouse]

Hi,

On 27/08/2020 07:00, Andreas Gruenbacher wrote:
On Fri, Aug 21, 2020 at 7:33 PM Bob Peterson <rpete...@redhat.com> wrote:
Before this patch, function gfs2_evict_inode would check if i_nlink
was non-zero, and if so, go to label out. The problem is, the evicted
file may still have outstanding pages that need invalidating, but
the call to truncate_inode_pages_final at label out doesn't start a
transaction. It needs a transaction in order to write revokes for any
pages it has to invalidate.
This is only true for jdata inodes though, right? If so, I'd rather
just create transactions in the jdata case.

Yes, and also if the inode is being deallocated, then we might be able 
to skip that step. We'll no doubt have to retain it in case this is just 
an unlink and there are still openers somewhere,

Steve.


This patch removes the early check for i_nlink in gfs2_evict_inode.
Not much further down in the code, there's another check for i_nlink
that skips to out_truncate. That one is proper because the calls
to truncate_inode_pages after out_truncate use a proper transaction,
so the page invalidates and subsequent revokes may be done properly.

Signed-off-by: Bob Peterson <rpete...@redhat.com>
---
  fs/gfs2/super.c | 21 +++++++++++++--------
  1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 80ac446f0110..1f3dee740431 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -1344,7 +1344,7 @@ static void gfs2_evict_inode(struct inode *inode)
                 return;
         }

-       if (inode->i_nlink || sb_rdonly(sb))
+       if (sb_rdonly(sb))
                 goto out;
         if (test_bit(GIF_ALLOC_FAILED, &ip->i_flags)) {
@@ -1370,15 +1370,19 @@ static void gfs2_evict_inode(struct inode *inode)
         }

         if (gfs2_inode_already_deleted(ip->i_gl, ip->i_no_formal_ino))
-               goto out_truncate;
+               goto out_flush;
         error = gfs2_check_blk_type(sdp, ip->i_no_addr, GFS2_BLKST_UNLINKED);
-       if (error)
-               goto out_truncate;
+       if (error) {
+               error = 0;
+               goto out_flush;
+       }

         if (test_bit(GIF_INVALID, &ip->i_flags)) {
                 error = gfs2_inode_refresh(ip);
-               if (error)
-                       goto out_truncate;
+               if (error) {
+                       error = 0;
+                       goto out_flush;
+               }
         }

         /*
@@ -1392,7 +1396,7 @@ static void gfs2_evict_inode(struct inode *inode)
             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;
+                       goto out_flush;
                 }
         }

@@ -1424,7 +1428,7 @@ static void gfs2_evict_inode(struct inode *inode)
         gfs2_inode_remember_delete(ip->i_gl, ip->i_no_formal_ino);
         goto out_unlock;

-out_truncate:
+out_flush:
         gfs2_log_flush(sdp, ip->i_gl, GFS2_LOG_HEAD_FLUSH_NORMAL |
                        GFS2_LFC_EVICT_INODE);
         metamapping = gfs2_glock2aspace(ip->i_gl);
@@ -1435,6 +1439,7 @@ static void gfs2_evict_inode(struct inode *inode)
         write_inode_now(inode, 1);
         gfs2_ail_flush(ip->i_gl, 0);

+out_truncate:
         nr_revokes = inode->i_mapping->nrpages + metamapping->nrpages;
         if (!nr_revokes)
                 goto out_unlock;
--
2.26.2

Thanks,
Andreas