From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C801BC433DB for ; Fri, 5 Feb 2021 22:23:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 865ED64EE4 for ; Fri, 5 Feb 2021 22:23:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232693AbhBEWWo (ORCPT ); Fri, 5 Feb 2021 17:22:44 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41312 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231915AbhBEO01 (ORCPT ); Fri, 5 Feb 2021 09:26:27 -0500 Received: from mail-pj1-x102b.google.com (mail-pj1-x102b.google.com [IPv6:2607:f8b0:4864:20::102b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 414A8C061356; Fri, 5 Feb 2021 08:04:47 -0800 (PST) Received: by mail-pj1-x102b.google.com with SMTP id cl8so3872834pjb.0; Fri, 05 Feb 2021 08:04:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:reply-to:from:date:message-id :subject:to:cc:content-transfer-encoding; bh=Rk6es5Odc/haVC290vOZ/DrPzl+e7E/xVq07/LjGq1c=; b=VxnW++F5gSNLGypVHizPxI7LX9NOuNgc00qjhJnHmIYEZ7QYs8bByXI4YqIGJtHcxC ShTIoWzG6sJ/tqsOJ7WlyyPzCgPeUyCqrhwsBPM3I3s1vBY2uxvbt9oLK+xx1wjKSmC6 QUMKjYtoJu+7MIfXV4tGc/lre/akRLkvOxKWhfCKqJd0MeRVdZFb3wV+7f0W4OFAsgpo +p9eC3ZfeOz7RJwrHkxOCiob1ns5iq8fBlsX7+kFtHorsdk83ELU20ckEqYklpilOD3g MBaZFEFUNRfENaraZ+FKGwab/v/rSrbBW+rjbcqCGSpiSvqnoWrWEDrGbQgWbsePVZEm 9EHg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:reply-to :from:date:message-id:subject:to:cc:content-transfer-encoding; bh=Rk6es5Odc/haVC290vOZ/DrPzl+e7E/xVq07/LjGq1c=; b=dg6O55eSWdLCHxCaZSrhq6XbiVkH2J1P+L2YfG/7mCK6cjgLPgffHJqB22+7LPiorm NOJuVULxdgtvhQsKV9GiCamv2TG4fzhp/TDqdF7bzKODh60UtOuM6Ls264IYWDJlFlWW U1cQ7ydtXBwI9MJN+i1kk8A0E9s/3DWZ9GIRsqxMhOFfqhxBv6RUlJBCBxOFgb7NkNcL 6y8UNxtw7A4nVuK4s1AmefRyGue0smeUtxhqgReAwHbL7I2UwHy0c4uokqZfo3eKMhlM 6YR2xiTzUAm2YcgPUSkUhIxEiqSZjA8yFsGHV+4a/KJcn9dk1jMkka8tgl5gyxLq5WyA OblA== X-Gm-Message-State: AOAM531NW+QKPslpQHG9pkWK8gtQ3awRIR8atr3LMz9MiuTU7VJLxgR6 2/G7j0e5GlhJJMNxrjlf+rI6QieSfBNd16EpFpgMQT1Zr0o= X-Google-Smtp-Source: ABdhPJyf/XQErWc2swFQn6YLU1TeO7IBzDbRcW86bxibi00GlO0fcY7Eog9OGyvTRv2PELRSc3xOtovYjdw7x6w6rYE= X-Received: by 2002:a0c:906c:: with SMTP id o99mr4539764qvo.28.1612534801508; Fri, 05 Feb 2021 06:20:01 -0800 (PST) MIME-Version: 1.0 References: <20210205092635.i6w3c7brawlv6pgs@naota-xeon> In-Reply-To: Reply-To: fdmanana@gmail.com From: Filipe Manana Date: Fri, 5 Feb 2021 14:19:50 +0000 Message-ID: Subject: Re: [PATCH v15 43/43] btrfs: zoned: deal with holes writing out tree-log pages To: Naohiro Aota Cc: linux-btrfs , David Sterba , hare@suse.com, linux-fsdevel , Johannes Thumshirn Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org On Fri, Feb 5, 2021 at 11:49 AM Filipe Manana wrote: > > On Fri, Feb 5, 2021 at 9:26 AM Naohiro Aota wrote: > > > > Since the zoned filesystem requires sequential write out of metadata, w= e > > cannot proceed with a hole in tree-log pages. When such a hole exists, > > btree_write_cache_pages() will return -EAGAIN. We cannot wait for the r= ange > > to be written, because it will cause a deadlock. So, let's bail out to = a > > full commit in this case. > > > > Cc: Filipe Manana > > Signed-off-by: Naohiro Aota > > --- > > fs/btrfs/tree-log.c | 19 ++++++++++++++++++- > > 1 file changed, 18 insertions(+), 1 deletion(-) > > > > This patch solves a regression introduced by fixing patch 40. I'm > > sorry for the confusing patch numbering. > > Hum, how does patch 40 can cause this? > And is it before the fixup or after? > > > > > diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c > > index 4e72794342c0..629e605cd62d 100644 > > --- a/fs/btrfs/tree-log.c > > +++ b/fs/btrfs/tree-log.c > > @@ -3120,6 +3120,14 @@ int btrfs_sync_log(struct btrfs_trans_handle *tr= ans, > > */ > > blk_start_plug(&plug); > > ret =3D btrfs_write_marked_extents(fs_info, &log->dirty_log_pag= es, mark); > > + /* > > + * There is a hole writing out the extents and cannot proceed i= t on > > + * zoned filesystem, which require sequential writing. We can > > require -> requires > > > + * ignore the error for now, since we don't wait for completion= for > > + * now. > > So why can we ignore the error for now? > Why not just bail out here and mark the log for full commit? (without > a transaction abort) > > > + */ > > + if (ret =3D=3D -EAGAIN) > > + ret =3D 0; Thinking again about this, it would be safer, and self-documenting to check here that we are in zoned mode: if (ret =3D=3D -EAGAIN && is_zoned) ret =3D 0; Because if we start to get -EAGAIN here one day, from non-zoned code, we risk not writing out some extent buffer and getting a corrupt log, which may be very hard to find. With that additional check in place, we'll end up aborting the transaction with -EAGAIN and notice the problem much sooner. > > if (ret) { > > blk_finish_plug(&plug); > > btrfs_abort_transaction(trans, ret); > > @@ -3229,7 +3237,16 @@ int btrfs_sync_log(struct btrfs_trans_handle *tr= ans, > > &log_root_tree->dirty_log_page= s, > > EXTENT_DIRTY | EXTENT_NEW); > > blk_finish_plug(&plug); > > - if (ret) { > > + /* > > + * There is a hole in the extents, and failed to sequential wri= te > > + * on zoned filesystem. We cannot wait for this write outs, sin= c it > > this -> these > > > + * cause a deadlock. Bail out to the full commit, instead. > > + */ > > + if (ret =3D=3D -EAGAIN) { I would add "&& is_zoned" here too. Thanks. > > + btrfs_wait_tree_log_extents(log, mark); > > + mutex_unlock(&log_root_tree->log_mutex); > > + goto out_wake_log_root; > > Must also call btrfs_set_log_full_commit(trans); > > Thanks. > > > + } else if (ret) { > > btrfs_set_log_full_commit(trans); > > btrfs_abort_transaction(trans, ret); > > mutex_unlock(&log_root_tree->log_mutex); > > -- > > 2.30.0 > > > > > -- > Filipe David Manana, > > =E2=80=9CWhether you think you can, or you think you can't =E2=80=94 you'= re right.=E2=80=9D -- Filipe David Manana, =E2=80=9CWhether you think you can, or you think you can't =E2=80=94 you're= right.=E2=80=9D