[Cluster-devel] [PATCH] gfs2: Prevent writeback in gfs2_file_write_iter

[Mark Syms]

-----Original Message-----
From: Andreas Gruenbacher <agruenba at redhat.com> 
Sent: 17 March 2019 20:06
To: Mark Syms <Mark.Syms at citrix.com>
Cc: cluster-devel at redhat.com; Sergey Dyasli <sergey.dyasli at citrix.com>; Igor Druzhinin <igor.druzhinin at citrix.com>; Edvin Torok <edvin.torok at citrix.com>
Subject: Re: [Cluster-devel] [PATCH] gfs2: Prevent writeback in gfs2_file_write_iter

On Sun, 17 Mar 2019 at 00:59, Mark Syms <Mark.Syms at citrix.com> wrote:
>
> Sadly, this doesn't help and seems to make the situation worse. Our automated tests were previously seeing about 5% failure rate and with this patch its 20%.

So the PF_MEMALLOC flag doesn't get set on that code path. In that case, we should probably check wbc->sync_mode instead. How about the following?

diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c index 540535c..36de2f7 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -757,7 +757,7 @@ static int gfs2_write_inode(struct inode *inode, struct writeback_control *wbc)
     bool flush_all = (wbc->sync_mode == WB_SYNC_ALL || gfs2_is_jdata(ip));

     /* (see ext4_dirty_inode) */
-    if (current->flags & PF_MEMALLOC)
+    if (wbc->sync_mode != WB_SYNC_ALL)
         return 0;

     if (flush_all)

It may help to wrap that condition in WARN_ON_ONCE for debugging.

[Mark Syms] That works better. We were wondering whether it might be a bit too aggressive though in that it skips writing the inode entirely unless we have WB_SYNC_ALL whereas the patch that Ross Lagerwall posted originally would use a trylock in the case where we don't have WB_SYNC_ALL and then fail out. Whether or not this should just silently return 0 or -EAGAIN as Ross' patch does is a matter of style I guess. What are your thoughts on this?

Thanks,

Mark.