All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] gfs2 iomap dealock, IOMAP_F_UNBALANCED
Date: Thu, 28 Mar 2019 17:51:04 +0100	[thread overview]
Message-ID: <20190328165104.GA21552@lst.de> (raw)
In-Reply-To: <20190321131304.21618-1-agruenba@redhat.com>

On Thu, Mar 21, 2019 at 02:13:04PM +0100, Andreas Gruenbacher wrote:
> Hi Christoph,
> 
> we need your help fixing a gfs2 deadlock involving iomap.  What's going
> on is the following:
> 
> * During iomap_file_buffered_write, gfs2_iomap_begin grabs the log flush
>   lock and keeps it until gfs2_iomap_end.  It currently always does that
>   even though there is no point other than for journaled data writes.
> 
> * iomap_file_buffered_write then calls balance_dirty_pages_ratelimited.
>   If that ends up calling gfs2_write_inode, gfs2 will try to grab the
>   log flush lock again and deadlock.

What is the exactly call chain?  balance_dirty_pages_ratelimited these
days doesn't start I/O, but just wakes up the flusher threads.  Or
do we have a issue where it is blocking on those threads?

Also why do you need to flush the log for background writeback in
->write_inode?

balance_dirty_pages_ratelimited is per definition not a data integrity
writeback, so there shouldn't be a good reason to flush the log
(which I assume the log flush log is for).  If we look gfs2_write_inode,
this seems to be the code:

	bool flush_all = (wbc->sync_mode == WB_SYNC_ALL || gfs2_is_jdata(ip));

        if (flush_all)
		gfs2_log_flush(GFS2_SB(inode), ip->i_gl,
			       GFS2_LOG_HEAD_FLUSH_NORMAL |
			       GFS2_LFC_WRITE_INODE);

But what is the requirement to do this in writeback context?  Can't
we move it out into another context instead?



WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@lst.de>
To: Andreas Gruenbacher <agruenba@redhat.com>
Cc: "Christoph Hellwig" <hch@lst.de>,
	cluster-devel@redhat.com, "Dave Chinner" <david@fromorbit.com>,
	"Ross Lagerwall" <ross.lagerwall@citrix.com>,
	"Mark Syms" <Mark.Syms@citrix.com>,
	"Edwin Török" <edvin.torok@citrix.com>,
	linux-fsdevel@vger.kernel.org
Subject: Re: gfs2 iomap dealock, IOMAP_F_UNBALANCED
Date: Thu, 28 Mar 2019 17:51:04 +0100	[thread overview]
Message-ID: <20190328165104.GA21552@lst.de> (raw)
In-Reply-To: <20190321131304.21618-1-agruenba@redhat.com>

On Thu, Mar 21, 2019 at 02:13:04PM +0100, Andreas Gruenbacher wrote:
> Hi Christoph,
> 
> we need your help fixing a gfs2 deadlock involving iomap.  What's going
> on is the following:
> 
> * During iomap_file_buffered_write, gfs2_iomap_begin grabs the log flush
>   lock and keeps it until gfs2_iomap_end.  It currently always does that
>   even though there is no point other than for journaled data writes.
> 
> * iomap_file_buffered_write then calls balance_dirty_pages_ratelimited.
>   If that ends up calling gfs2_write_inode, gfs2 will try to grab the
>   log flush lock again and deadlock.

What is the exactly call chain?  balance_dirty_pages_ratelimited these
days doesn't start I/O, but just wakes up the flusher threads.  Or
do we have a issue where it is blocking on those threads?

Also why do you need to flush the log for background writeback in
->write_inode?

balance_dirty_pages_ratelimited is per definition not a data integrity
writeback, so there shouldn't be a good reason to flush the log
(which I assume the log flush log is for).  If we look gfs2_write_inode,
this seems to be the code:

	bool flush_all = (wbc->sync_mode == WB_SYNC_ALL || gfs2_is_jdata(ip));

        if (flush_all)
		gfs2_log_flush(GFS2_SB(inode), ip->i_gl,
			       GFS2_LOG_HEAD_FLUSH_NORMAL |
			       GFS2_LFC_WRITE_INODE);

But what is the requirement to do this in writeback context?  Can't
we move it out into another context instead?

  parent reply	other threads:[~2019-03-28 16:51 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-21 13:13 [Cluster-devel] gfs2 iomap dealock, IOMAP_F_UNBALANCED Andreas Gruenbacher
2019-03-21 13:13 ` Andreas Gruenbacher
2019-03-21 21:43 ` [Cluster-devel] " Dave Chinner
2019-03-21 21:43   ` Dave Chinner
2019-03-21 23:01   ` [Cluster-devel] " Andreas Gruenbacher
2019-03-21 23:01     ` Andreas Gruenbacher
2019-03-22  0:21     ` [Cluster-devel] " Andreas Gruenbacher
2019-03-22  0:21       ` Andreas Gruenbacher
2019-03-27 16:49       ` [Cluster-devel] " Ross Lagerwall
2019-03-27 16:49         ` Ross Lagerwall
2019-03-28 16:51 ` Christoph Hellwig [this message]
2019-03-28 16:51   ` Christoph Hellwig
2019-03-29 22:13   ` [Cluster-devel] " Andreas Gruenbacher
2019-03-29 22:13     ` Andreas Gruenbacher
2019-04-07  7:32     ` [Cluster-devel] " Christoph Hellwig
2019-04-07  7:32       ` Christoph Hellwig
2019-04-08  8:53       ` [Cluster-devel] " Andreas Gruenbacher
2019-04-08  8:53         ` Andreas Gruenbacher
2019-04-08 13:44         ` [Cluster-devel] " Jan Kara
2019-04-08 13:44           ` Jan Kara
2019-04-09 12:15           ` [Cluster-devel] " Christoph Hellwig
2019-04-09 12:15             ` Christoph Hellwig
2019-04-09 12:27             ` [Cluster-devel] " Andreas Gruenbacher
2019-04-09 12:27               ` Andreas Gruenbacher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190328165104.GA21552@lst.de \
    --to=hch@lst.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.