From patchwork Tue Jun 19 15:14:31 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Gruenbacher X-Patchwork-Id: 10474665 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 423AA603B5 for ; Tue, 19 Jun 2018 15:14:36 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3362D28B53 for ; Tue, 19 Jun 2018 15:14:36 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 25CF628D8B; Tue, 19 Jun 2018 15:14:36 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B5F0328B53 for ; Tue, 19 Jun 2018 15:14:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S937764AbeFSPOe (ORCPT ); Tue, 19 Jun 2018 11:14:34 -0400 Received: from mail-it0-f68.google.com ([209.85.214.68]:52520 "EHLO mail-it0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S937707AbeFSPOc (ORCPT ); Tue, 19 Jun 2018 11:14:32 -0400 Received: by mail-it0-f68.google.com with SMTP id m194-v6so877597itg.2 for ; Tue, 19 Jun 2018 08:14:32 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=fGciBL4YeypfjhY56y4Z5Qm7kSFl9+qpQRKPZh/exPI=; b=BwAhX6G84JTrwUmnkRo37IPnKzzzk2bcVj45rQKxuffYoip0h6M7neAu3ygp0ZrOzf xyuX9cmO2QbcPCysZF/NYcilSorzUe0PMpQ9R+N/uGRSYHg2dBerqHcaiCGwQtTVQTm4 fmSpv94lv84rRlEhPV4kX0v/baab/WYeI6gK1H5nu76lph7tX2MUXmMw2NPT36i3URWY m+CSaVUUWiKKD/9SbzD5S+MEov/Stbt5crwj9ogPhbuIpv1HbUg3lJe7CzYq+2t5/plr TSIBpTG3w+Hdjai+3ktzJG9Pe+eLvrgIt//LGanMau0/H/QLIwZ+xKPxvUzMZdyC4aZK CaYg== X-Gm-Message-State: APt69E05nxpSawoIAqciAW7NdeG0T9ucxoqDJAGEEQUeUIVZ1gLnKEZO 0Frbd/mpYXAY3bKM1aTrLGTz9ubaqVJoSM1q6yY1lg== X-Google-Smtp-Source: ADUXVKKZmykNKptJkE1adAye+ZfFFaaEAWd/WdV9h+sg7btpA5riK3ZLEAfQvdZCo/4iiASWzTuxp/jRdXb0+4FT8p0= X-Received: by 2002:a02:2341:: with SMTP id u62-v6mr14027963jau.62.1529421272266; Tue, 19 Jun 2018 08:14:32 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a4f:8407:0:0:0:0:0 with HTTP; Tue, 19 Jun 2018 08:14:31 -0700 (PDT) In-Reply-To: <20180619143550.GA15162@lst.de> References: <20180614120457.28285-1-hch@lst.de> <20180615080326.GB19525@lst.de> <5a1e303b-e96e-7faf-1bcd-36d63a237514@redhat.com> <20180619143550.GA15162@lst.de> From: Andreas Gruenbacher Date: Tue, 19 Jun 2018 17:14:31 +0200 Message-ID: Subject: Re: [Cluster-devel] iomap preparations for GFS2 v2 To: Christoph Hellwig Cc: Steven Whitehouse , cluster-devel , linux-xfs@vger.kernel.org, linux-fsdevel , Dan Williams , linux-ext4 Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 19 June 2018 at 16:35, Christoph Hellwig wrote: > On Tue, Jun 19, 2018 at 01:08:12PM +0200, Andreas Gruenbacher wrote: >> What I'm seeing in the readpage address space operation is pages which >> are not PageUptodate(), with a page-size buffer head that is >> buffer_uptodate(). The filesystem doesn't bother keeping the page >> flags in sync with the buffer head flags, nothing unusual. > > It is in fact highly unusual, as all the generic routines do set > the page uptodate once all buffers are uptodate. > >> When >> iomap_readpage is called on such a page, it will replace the current >> contents with what's on disk, losing the changes in memory. So we >> cannot just call iomap_readpages, we need to check the buffer head >> flags as well. Or, since the old code is still needed for page size != >> block size anyway, we can fall back to that for pages that have >> buffers for now. > > I'd like to understand where that buffer_head comes from, something > seems fishy here. Ok, here is one test case that triggered the problem for me. Starting from commit bd926eb58b13 on the iomap-readpage branch, https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/log/?h=iomap-readpage with this patch on top which causes iomap_readpage to be called even for pages with buffers: The following fsx operations, stored as junk.fsxops: write 0x11400 0x1800 0x6e6d4 * punch_hole 0xfa7a 0x2410 0x0 * mapread 0xd000 0x78ea 0x34200 * Can be replayed as: # mkfs.gfs2 -O -b 4096 -p lock_nolock $DEV # mount $DEV $MNT # ltp/fsx -N 10000 -o 32768 -l 500000 -r 4096 -t 512 -w 512 -Z -W --replay-ops junk.fsxops $MNT/junk (Most of the above fsx options could probably be removed ...) The hole in this example is unaligned, so punch_hole will zero the end of the first as well as the beginning of the last page of the hole. This will leave at least the last page of the hole as not PageUptodate(), with a buffer_uptodate() buffer head. The mapread will call into iomap_readpage. Because the page has buffers, the WARN_ON_ONCE(page_has_buffers(page)) in iomap_readpage will trigger. And iomap_readpage will reread the page from disk, overwriting the zeroes written by punch_hole. This will cause fsx to complain because it doesn't see the zeroes it expects. This could be a bug in __gfs2_punch_hole => gfs2_block_zero_range as well, but it's not clear to me how. Andreas --- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- a/fs/gfs2/aops.c +++ b/fs/gfs2/aops.c @@ -511,8 +511,7 @@ static int __gfs2_readpage(void *file, struct page *page) int error; - if (i_blocksize(page->mapping->host) == PAGE_SIZE && - !page_has_buffers(page)) { + if (i_blocksize(page->mapping->host) == PAGE_SIZE) { error = iomap_readpage(page, &gfs2_iomap_ops); } else if (gfs2_is_stuffed(ip)) { error = stuffed_readpage(ip, page);