From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark Syms Date: Mon, 18 Mar 2019 15:10:19 +0000 Subject: [Cluster-devel] [PATCH] gfs2: Prevent writeback in gfs2_file_write_iter In-Reply-To: References: Message-ID: List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit -----Original Message----- From: Andreas Gruenbacher Sent: 17 March 2019 20:06 To: Mark Syms Cc: cluster-devel at redhat.com; Sergey Dyasli ; Igor Druzhinin ; Edvin Torok Subject: Re: [Cluster-devel] [PATCH] gfs2: Prevent writeback in gfs2_file_write_iter On Sun, 17 Mar 2019 at 00:59, Mark Syms 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.