From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@vger.kernel.org>
Received: from mail-io0-f171.google.com ([209.85.223.171]:41287 "EHLO
        mail-io0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S967009AbeEXSFE (ORCPT
        <rfc822;linux-fsdevel@vger.kernel.org>);
        Thu, 24 May 2018 14:05:04 -0400
Received: by mail-io0-f171.google.com with SMTP id z5-v6so886979iob.8
        for <linux-fsdevel@vger.kernel.org>; Thu, 24 May 2018 11:05:04 -0700 (PDT)
MIME-Version: 1.0
From: Andreas Gruenbacher <agruenba@redhat.com>
Date: Thu, 24 May 2018 20:05:03 +0200
Message-ID: <CAHc6FU5a5E9K_AizwWkoPjb4cCWzqKspjc9YDVOzpCxcsDnKeA@mail.gmail.com>
Subject: gfs2 iomap: BUG_ON(buffer_unmapped) in submit_bh_wbc
To: Christoph Hellwig <hch@lst.de>
Cc: "K.V, Aneesh" <aneesh.kumar@linux.vnet.ibm.com>,
        cluster-devel <cluster-devel@redhat.com>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

Hi Christoph,

I'm running into the following problem with the GFS2 iomap patches:

In order to get iomap_to_bh to set the New flag of bhs for newly
allocated blocks in __block_write_begin_int, I set iomap->type to
IOMAP_UNWRITTEN when allocating blocks. This has the side effect of
setting the Unwritten bh flag as well. GFS2 doesn't have the concept
of unwritten extents, so it doesn't use the Unwritten flag itself.
There's also nothing in gfs2 that would clear that flag.

When I write to a file on that filesystem with something like:

  xfs_io -t -f -c "pwrite -S 0x5a 2048 2048" -c "fsync" /mnt/scratch/foo

I run into a BUG_ON(buffer_unwritten(bh)) in __mpage_writepage ->
__block_write_full_page -> submit_bh_wbc. That BUG_ON was added in
commit 8fb0e34248 by Aneesh. So clearly the VFS expects the unwritten
flags to be cleared by the filesystem somewhere.

I can think of two ways to fix this:

(1) make iomap_to_bh set the buffer_new flag for IOMAP_MAPPED mappings
with the IOMAP_F_NEW flag set as well and stop abusing the Unwritten
bh flag,

(2) clear the Unwritten flags in the filesystem.


Approach (1) is quite trivial:

--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1908,7 +1908,7 @@ iomap_to_bh(struct inode *inode, sector_t,
struct buffer_head *bh,
                set_buffer_unwritten(bh);
                /* FALLTHRU */
        case IOMAP_MAPPED:
-               if (offset >= i_size_read(inode))
+               if ((iomap->flags & IOMAP_F_NEW) || offset >=
i_size_read(inode))
                        set_buffer_new(bh);
                bh->b_blocknr = (iomap->addr + offset - iomap->offset) >>
                                inode->i_blkbits;


If you think (2) is preferable, then where and when should the
filesystem clear the Unwritten bh flags?


Thanks,
Andreas