On 3/22/19 12:21 AM, Andreas Gruenbacher wrote:
On Fri, 22 Mar 2019 at 00:01, Andreas Gruenbacher <agruenba@xxxxxxxxxx> wrote:
On Thu, 21 Mar 2019 at 22:43, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
The problem is calling balance_dirty_pages() inside the
->iomap_begin/->iomap_end calls and not that it is called by the
iomap infrastructure itself, right?
Is so, I'd prefer to see this in iomap_apply() after the call to
ops->iomap_end because iomap_file_buffered_write() can iterate and
call iomap_apply() multiple times. This would keep the balancing to
a per-iomap granularity, rather than a per-syscall granularity.
i.e. if we do write(2GB), we want more than one balancing call
during that syscall, so it would be up to the filesystem to a) limit
the size of write mappings to something smaller (e.g. 1024 pages)
so that there are still frequent balancing calls for large writes.
Hmm. The looping across multiple mappings isn't done in iomap_apply
but in iomap_file_buffered_write, so the balancing could go into
iomap_apply or iomap_file_buffered_write, but can't go further up the
stack. Given that, iomap_file_buffered_write seems the better place,
but this is still quite horrible.
Here's a more reasonable version of my first patch, with a cleaned up
and hopefully fixed gfs2 part.
In addition, this checks for IOMAP_F_UNBALANCED in iomap_dirty_actor,
the actor for iomap_file_dirty. We don't use iomap_file_dirty in gfs2,
but we should probably allowing to skip the dirty page balancing there
as well.
Thanks,
Andreas
---
fs/gfs2/bmap.c | 64 +++++++++++++++++++++++++++++++++----------
fs/iomap.c | 6 ++--
include/linux/iomap.h | 1 +
3 files changed, 55 insertions(+), 16 deletions(-)
Thanks, this fixes the reported deadlock. I haven't yet checked whether
it has any performance impact.
Tested-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
