From patchwork Thu Sep 21 01:39:43 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Chinner X-Patchwork-Id: 13393586 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3EEF9CD4938 for ; Thu, 21 Sep 2023 01:39:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229716AbjIUBkA (ORCPT ); Wed, 20 Sep 2023 21:40:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45190 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229473AbjIUBj7 (ORCPT ); Wed, 20 Sep 2023 21:39:59 -0400 Received: from mail-pl1-x631.google.com (mail-pl1-x631.google.com [IPv6:2607:f8b0:4864:20::631]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1E306CF for ; Wed, 20 Sep 2023 18:39:52 -0700 (PDT) Received: by mail-pl1-x631.google.com with SMTP id d9443c01a7336-1c44c0f9138so3307785ad.2 for ; Wed, 20 Sep 2023 18:39:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fromorbit-com.20230601.gappssmtp.com; s=20230601; t=1695260391; x=1695865191; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:from:to:cc:subject:date:message-id :reply-to; bh=VdEUFoKO7N+MBzfya+a8a8rW7BLRYTxESGZDQDBv8MQ=; b=3an1Xdhx0y4yYYG5Vruuk85TVC2MLk9l+nFeUELjwKrJEOWs4l8lMdkO6X/JG1vmuH V3y3o1/6eDU23hbTfy8S4yGV4Btd/CDhz+Lmz4j50q0uFjg2itAjyLCeHHofD1Vq/60w kNKA/duN9g3tvqIrPXGwAmeXYSr7fpRfzIG8d6s6xg2Am7mNG/U4rRRrgzmz7MoT3rTS sZQzraQ2rsOed92L2QmF7vlJz6typiJzf43fJLI+8oju44BFReyuyY8TYuis7TzM3oVj 5R+LC6Tz0NUYl2UPppeefABi1v/WNvXpkkTrZNqZFs8iKKXQlSkLBSeX+4LdjqHF9hOT RRYA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695260391; x=1695865191; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=VdEUFoKO7N+MBzfya+a8a8rW7BLRYTxESGZDQDBv8MQ=; b=Ic35BO7MRusu1yrQU6XhNccXVHmjRiulmdfIwnRrurgXTAeLZeDUWvAw4hJzKvosDm aTF3b7DpS5cj4REEsNSy6S+p/GvAvFhiwfRrmkfdS9FT0ZVfjYXEWfOGT0BRiNKBP+Fk 7R7njqrJE4Yyj4YKbdhQEJhv35rSoDCIDANyAxWnmQgKA9b0yKXPXItZIhDUsepm3sRG YET73kZ28GuhGMC80fdyv0xgrpAIAovDzk6zLr7TgN8uIh6Jnw+iXPau43WbJHn2xQzE 2GjRnTAamC0vTE+FLF05++LNcSRizah5UO1XO3KWflXK6MaTJiIUBTrFqExkwMMEMPey 2Gfw== X-Gm-Message-State: AOJu0YxdLynEhAtzeGKstoG7F/IzWVbnq1WyYFbyAFZ0SPkiro8rxZhV SfoquUXwgQaqGXYjtqEpfsd2V09Effv6iYDSwDA= X-Google-Smtp-Source: AGHT+IF92sj6J7mxscSGlFkZaXik6WJV13R7qm3zXjiEpCmpkGQvCE2+24AgYKfRlkWlg3hkKfkv5A== X-Received: by 2002:a17:903:24e:b0:1bf:728:745b with SMTP id j14-20020a170903024e00b001bf0728745bmr4518457plh.49.1695260391253; Wed, 20 Sep 2023 18:39:51 -0700 (PDT) Received: from dread.disaster.area (pa49-180-20-59.pa.nsw.optusnet.com.au. [49.180.20.59]) by smtp.gmail.com with ESMTPSA id m7-20020a170902db0700b001b9da42cd7dsm138307plx.279.2023.09.20.18.39.49 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Sep 2023 18:39:50 -0700 (PDT) Received: from [192.168.253.23] (helo=devoid.disaster.area) by dread.disaster.area with esmtp (Exim 4.96) (envelope-from ) id 1qj8fT-003Suz-1E for linux-xfs@vger.kernel.org; Thu, 21 Sep 2023 11:39:47 +1000 Received: from dave by devoid.disaster.area with local (Exim 4.97-RC0) (envelope-from ) id 1qj8fT-00000002MHI-0YLA for linux-xfs@vger.kernel.org; Thu, 21 Sep 2023 11:39:47 +1000 From: Dave Chinner To: linux-xfs@vger.kernel.org Subject: [PATCH 1/3] xfs: move log discard work to xfs_discard.c Date: Thu, 21 Sep 2023 11:39:43 +1000 Message-Id: <20230921013945.559634-2-david@fromorbit.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20230921013945.559634-1-david@fromorbit.com> References: <20230921013945.559634-1-david@fromorbit.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Dave Chinner Because we are going to use the same list-based discard submission interface for fstrim-based discards, too. Signed-off-by: Dave Chinner Reviewed-by: Darrick J. Wong --- fs/xfs/xfs_discard.c | 77 ++++++++++++++++++++++++++++++++- fs/xfs/xfs_discard.h | 6 ++- fs/xfs/xfs_extent_busy.h | 20 +++++++-- fs/xfs/xfs_log_cil.c | 93 ++++++---------------------------------- fs/xfs/xfs_log_priv.h | 5 ++- 5 files changed, 113 insertions(+), 88 deletions(-) diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c index afc4c78b9eed..3f45c7bb94f2 100644 --- a/fs/xfs/xfs_discard.c +++ b/fs/xfs/xfs_discard.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 /* - * Copyright (C) 2010 Red Hat, Inc. + * Copyright (C) 2010, 2023 Red Hat, Inc. * All Rights Reserved. */ #include "xfs.h" @@ -19,6 +19,81 @@ #include "xfs_log.h" #include "xfs_ag.h" +struct workqueue_struct *xfs_discard_wq; + +static void +xfs_discard_endio_work( + struct work_struct *work) +{ + struct xfs_busy_extents *extents = + container_of(work, struct xfs_busy_extents, endio_work); + + xfs_extent_busy_clear(extents->mount, &extents->extent_list, false); + kmem_free(extents->owner); +} + +/* + * Queue up the actual completion to a thread to avoid IRQ-safe locking for + * pagb_lock. + */ +static void +xfs_discard_endio( + struct bio *bio) +{ + struct xfs_busy_extents *extents = bio->bi_private; + + INIT_WORK(&extents->endio_work, xfs_discard_endio_work); + queue_work(xfs_discard_wq, &extents->endio_work); + bio_put(bio); +} + +/* + * Walk the discard list and issue discards on all the busy extents in the + * list. We plug and chain the bios so that we only need a single completion + * call to clear all the busy extents once the discards are complete. + */ +int +xfs_discard_extents( + struct xfs_mount *mp, + struct xfs_busy_extents *extents) +{ + struct xfs_extent_busy *busyp; + struct bio *bio = NULL; + struct blk_plug plug; + int error = 0; + + blk_start_plug(&plug); + list_for_each_entry(busyp, &extents->extent_list, list) { + trace_xfs_discard_extent(mp, busyp->agno, busyp->bno, + busyp->length); + + error = __blkdev_issue_discard(mp->m_ddev_targp->bt_bdev, + XFS_AGB_TO_DADDR(mp, busyp->agno, busyp->bno), + XFS_FSB_TO_BB(mp, busyp->length), + GFP_NOFS, &bio); + if (error && error != -EOPNOTSUPP) { + xfs_info(mp, + "discard failed for extent [0x%llx,%u], error %d", + (unsigned long long)busyp->bno, + busyp->length, + error); + break; + } + } + + if (bio) { + bio->bi_private = extents; + bio->bi_end_io = xfs_discard_endio; + submit_bio(bio); + } else { + xfs_discard_endio_work(&extents->endio_work); + } + blk_finish_plug(&plug); + + return error; +} + + STATIC int xfs_trim_extents( struct xfs_perag *pag, diff --git a/fs/xfs/xfs_discard.h b/fs/xfs/xfs_discard.h index de92d9cc958f..2b1a85223a56 100644 --- a/fs/xfs/xfs_discard.h +++ b/fs/xfs/xfs_discard.h @@ -3,8 +3,10 @@ #define XFS_DISCARD_H 1 struct fstrim_range; -struct list_head; +struct xfs_mount; +struct xfs_busy_extents; -extern int xfs_ioc_trim(struct xfs_mount *, struct fstrim_range __user *); +int xfs_discard_extents(struct xfs_mount *mp, struct xfs_busy_extents *busy); +int xfs_ioc_trim(struct xfs_mount *mp, struct fstrim_range __user *fstrim); #endif /* XFS_DISCARD_H */ diff --git a/fs/xfs/xfs_extent_busy.h b/fs/xfs/xfs_extent_busy.h index c37bf87e6781..71c28d031e3b 100644 --- a/fs/xfs/xfs_extent_busy.h +++ b/fs/xfs/xfs_extent_busy.h @@ -16,9 +16,6 @@ struct xfs_alloc_arg; /* * Busy block/extent entry. Indexed by a rbtree in perag to mark blocks that * have been freed but whose transactions aren't committed to disk yet. - * - * Note that we use the transaction ID to record the transaction, not the - * transaction structure itself. See xfs_extent_busy_insert() for details. */ struct xfs_extent_busy { struct rb_node rb_node; /* ag by-bno indexed search tree */ @@ -31,6 +28,23 @@ struct xfs_extent_busy { #define XFS_EXTENT_BUSY_SKIP_DISCARD 0x02 /* do not discard */ }; +/* + * List used to track groups of related busy extents all the way through + * to discard completion. + */ +struct xfs_busy_extents { + struct xfs_mount *mount; + struct list_head extent_list; + struct work_struct endio_work; + + /* + * Owner is the object containing the struct xfs_busy_extents to free + * once the busy extents have been processed. If only the + * xfs_busy_extents object needs freeing, then point this at itself. + */ + void *owner; +}; + void xfs_extent_busy_insert(struct xfs_trans *tp, struct xfs_perag *pag, xfs_agblock_t bno, xfs_extlen_t len, unsigned int flags); diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c index 3aec5589d717..c340987880c8 100644 --- a/fs/xfs/xfs_log_cil.c +++ b/fs/xfs/xfs_log_cil.c @@ -16,8 +16,7 @@ #include "xfs_log.h" #include "xfs_log_priv.h" #include "xfs_trace.h" - -struct workqueue_struct *xfs_discard_wq; +#include "xfs_discard.h" /* * Allocate a new ticket. Failing to get a new ticket makes it really hard to @@ -103,7 +102,7 @@ xlog_cil_ctx_alloc(void) ctx = kmem_zalloc(sizeof(*ctx), KM_NOFS); INIT_LIST_HEAD(&ctx->committing); - INIT_LIST_HEAD(&ctx->busy_extents); + INIT_LIST_HEAD(&ctx->busy_extents.extent_list); INIT_LIST_HEAD(&ctx->log_items); INIT_LIST_HEAD(&ctx->lv_chain); INIT_WORK(&ctx->push_work, xlog_cil_push_work); @@ -132,7 +131,7 @@ xlog_cil_push_pcp_aggregate( if (!list_empty(&cilpcp->busy_extents)) { list_splice_init(&cilpcp->busy_extents, - &ctx->busy_extents); + &ctx->busy_extents.extent_list); } if (!list_empty(&cilpcp->log_items)) list_splice_init(&cilpcp->log_items, &ctx->log_items); @@ -882,76 +881,6 @@ xlog_cil_free_logvec( } } -static void -xlog_discard_endio_work( - struct work_struct *work) -{ - struct xfs_cil_ctx *ctx = - container_of(work, struct xfs_cil_ctx, discard_endio_work); - struct xfs_mount *mp = ctx->cil->xc_log->l_mp; - - xfs_extent_busy_clear(mp, &ctx->busy_extents, false); - kmem_free(ctx); -} - -/* - * Queue up the actual completion to a thread to avoid IRQ-safe locking for - * pagb_lock. Note that we need a unbounded workqueue, otherwise we might - * get the execution delayed up to 30 seconds for weird reasons. - */ -static void -xlog_discard_endio( - struct bio *bio) -{ - struct xfs_cil_ctx *ctx = bio->bi_private; - - INIT_WORK(&ctx->discard_endio_work, xlog_discard_endio_work); - queue_work(xfs_discard_wq, &ctx->discard_endio_work); - bio_put(bio); -} - -static void -xlog_discard_busy_extents( - struct xfs_mount *mp, - struct xfs_cil_ctx *ctx) -{ - struct list_head *list = &ctx->busy_extents; - struct xfs_extent_busy *busyp; - struct bio *bio = NULL; - struct blk_plug plug; - int error = 0; - - ASSERT(xfs_has_discard(mp)); - - blk_start_plug(&plug); - list_for_each_entry(busyp, list, list) { - trace_xfs_discard_extent(mp, busyp->agno, busyp->bno, - busyp->length); - - error = __blkdev_issue_discard(mp->m_ddev_targp->bt_bdev, - XFS_AGB_TO_DADDR(mp, busyp->agno, busyp->bno), - XFS_FSB_TO_BB(mp, busyp->length), - GFP_NOFS, &bio); - if (error && error != -EOPNOTSUPP) { - xfs_info(mp, - "discard failed for extent [0x%llx,%u], error %d", - (unsigned long long)busyp->bno, - busyp->length, - error); - break; - } - } - - if (bio) { - bio->bi_private = ctx; - bio->bi_end_io = xlog_discard_endio; - submit_bio(bio); - } else { - xlog_discard_endio_work(&ctx->discard_endio_work); - } - blk_finish_plug(&plug); -} - /* * Mark all items committed and clear busy extents. We free the log vector * chains in a separate pass so that we unpin the log items as quickly as @@ -980,8 +909,8 @@ xlog_cil_committed( xlog_cil_ail_insert(ctx, abort); - xfs_extent_busy_sort(&ctx->busy_extents); - xfs_extent_busy_clear(mp, &ctx->busy_extents, + xfs_extent_busy_sort(&ctx->busy_extents.extent_list); + xfs_extent_busy_clear(mp, &ctx->busy_extents.extent_list, xfs_has_discard(mp) && !abort); spin_lock(&ctx->cil->xc_push_lock); @@ -990,10 +919,14 @@ xlog_cil_committed( xlog_cil_free_logvec(&ctx->lv_chain); - if (!list_empty(&ctx->busy_extents)) - xlog_discard_busy_extents(mp, ctx); - else - kmem_free(ctx); + if (!list_empty(&ctx->busy_extents.extent_list)) { + ctx->busy_extents.mount = mp; + ctx->busy_extents.owner = ctx; + xfs_discard_extents(mp, &ctx->busy_extents); + return; + } + + kmem_free(ctx); } void diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h index 9e276514cfb5..c3dfc0de87de 100644 --- a/fs/xfs/xfs_log_priv.h +++ b/fs/xfs/xfs_log_priv.h @@ -6,6 +6,8 @@ #ifndef __XFS_LOG_PRIV_H__ #define __XFS_LOG_PRIV_H__ +#include "xfs_extent_busy.h" /* for struct xfs_busy_extents */ + struct xfs_buf; struct xlog; struct xlog_ticket; @@ -223,12 +225,11 @@ struct xfs_cil_ctx { struct xlog_in_core *commit_iclog; struct xlog_ticket *ticket; /* chkpt ticket */ atomic_t space_used; /* aggregate size of regions */ - struct list_head busy_extents; /* busy extents in chkpt */ + struct xfs_busy_extents busy_extents; struct list_head log_items; /* log items in chkpt */ struct list_head lv_chain; /* logvecs being pushed */ struct list_head iclog_entry; struct list_head committing; /* ctx committing list */ - struct work_struct discard_endio_work; struct work_struct push_work; atomic_t order_id; From patchwork Thu Sep 21 01:39:44 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Chinner X-Patchwork-Id: 13393587 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 58EB9CD493C for ; Thu, 21 Sep 2023 01:39:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229473AbjIUBkA (ORCPT ); Wed, 20 Sep 2023 21:40:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45212 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229726AbjIUBj7 (ORCPT ); Wed, 20 Sep 2023 21:39:59 -0400 Received: from mail-pf1-x42d.google.com (mail-pf1-x42d.google.com [IPv6:2607:f8b0:4864:20::42d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EB07BAB for ; Wed, 20 Sep 2023 18:39:52 -0700 (PDT) Received: by mail-pf1-x42d.google.com with SMTP id d2e1a72fcca58-690bd8f89baso360734b3a.2 for ; Wed, 20 Sep 2023 18:39:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fromorbit-com.20230601.gappssmtp.com; s=20230601; t=1695260392; x=1695865192; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:from:to:cc:subject:date:message-id :reply-to; bh=o1sCydzNwgDhwiOxlmWaW+T13wv02WNUL5B9sZGWGi8=; b=o1Du2ZHkc6Q9kC2lQBvXNv8LLdJDWL2a2T1A2DAU+27ChKL9LKg5EUzA+8rPRzeb2n gfii1GPsLANdgeIYbdlpe2c8jbJm3t6quFh4CgfG0PoP0bSRcv93sskkU129X4wCD7/6 pPVNFM6+wVzE7DMir5UGR4UOtx92aQVvxX+0rTpwc6tRkqYwdf4iVvRCTVeMeCOGjgs9 1QJTTa3bE3KdR27K+1HG1AsiVG62N/KFgGaxtg1eRcZ4X+Zvzmioi+Ak50uE7/BOaJ6M fi7lu/JoDCqY/zHXT+ArRjIBJFJMUjK81vVn5ShR8rI8kYUFGYjM7G97z0gJPjJPbsuR SdNw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695260392; x=1695865192; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=o1sCydzNwgDhwiOxlmWaW+T13wv02WNUL5B9sZGWGi8=; b=bdjVf8Ng7Lsa6WY+6RG3FL4VYRSbNZs5aAQUyJ37k4JVhCbfSbrVUqn6V3aGzk3qzy 7ADjWqP8imqPB4nmbhDrZm0AEM3nfI97Z6dczAclmUsU1b1mtANATYB4qA5ZhKCJooqs a1oaHq34a/Yp2hglUqA+Yd9efxkqimmwyEVUH2vey1+dW460ZRKIwi6OQnJ4ISFnzQj6 c1eQpYDSWPFUwenHQE9CxVJAuWeI3uoZFb1aa5Coh22bagiP9YGODMECvl4lhuh2mobF 1TLG9o//4VSwyQ7FNyEgGudxlvzdk/q41YBj2CdMfEo1lTqfTSk2w4TEhuSz1zIixFyg r7pw== X-Gm-Message-State: AOJu0YxnC2BrQnUnvLzoc9Muqa/iW7g+FNnQ9ynWw4XgeDTeCIzbDqRo Z8ENOkXXP+1UAa6y26DVTg3nrZvINs2Hbv5RYzo= X-Google-Smtp-Source: AGHT+IHDJh52gpkZ5pMcsPgA+tYD5pvXBdyMTmopLPuvsDXbMQnSSBl0Xltjjn7acbe80cc/8kihUQ== X-Received: by 2002:a05:6a00:b44:b0:690:d413:ee13 with SMTP id p4-20020a056a000b4400b00690d413ee13mr4178471pfo.23.1695260392282; Wed, 20 Sep 2023 18:39:52 -0700 (PDT) Received: from dread.disaster.area (pa49-180-20-59.pa.nsw.optusnet.com.au. [49.180.20.59]) by smtp.gmail.com with ESMTPSA id a24-20020aa78658000000b00690c0cf97c9sm155355pfo.73.2023.09.20.18.39.50 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Sep 2023 18:39:51 -0700 (PDT) Received: from [192.168.253.23] (helo=devoid.disaster.area) by dread.disaster.area with esmtp (Exim 4.96) (envelope-from ) id 1qj8fT-003Sv2-1O for linux-xfs@vger.kernel.org; Thu, 21 Sep 2023 11:39:47 +1000 Received: from dave by devoid.disaster.area with local (Exim 4.97-RC0) (envelope-from ) id 1qj8fT-00000002MHM-0kc6 for linux-xfs@vger.kernel.org; Thu, 21 Sep 2023 11:39:47 +1000 From: Dave Chinner To: linux-xfs@vger.kernel.org Subject: [PATCH 2/3] xfs: reduce AGF hold times during fstrim operations Date: Thu, 21 Sep 2023 11:39:44 +1000 Message-Id: <20230921013945.559634-3-david@fromorbit.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20230921013945.559634-1-david@fromorbit.com> References: <20230921013945.559634-1-david@fromorbit.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Dave Chinner fstrim will hold the AGF lock for as long as it takes to walk and discard all the free space in the AG that meets the userspace trim criteria. For AGs with lots of free space extents (e.g. millions) or the underlying device is really slow at processing discard requests (e.g. Ceph RBD), this means the AGF hold time is often measured in minutes to hours, not a few milliseconds as we normal see with non-discard based operations. This can result in the entire filesystem hanging whilst the long-running fstrim is in progress. We can have transactions get stuck waiting for the AGF lock (data or metadata extent allocation and freeing), and then more transactions get stuck waiting on the locks those transactions hold. We can get to the point where fstrim blocks an extent allocation or free operation long enough that it ends up pinning the tail of the log and the log then runs out of space. At this point, every modification in the filesystem gets blocked. This includes read operations, if atime updates need to be made. To fix this problem, we need to be able to discard free space extents safely without holding the AGF lock. Fortunately, we already do this with online discard via busy extents. We can mark free space extents as "busy being discarded" under the AGF lock and then unlock the AGF, knowing that nobody will be able to allocate that free space extent until we remove it from the busy tree. Modify xfs_trim_extents to use the same asynchronous discard mechanism backed by busy extents as is used with online discard. This results in the AGF only needing to be held for short periods of time and it is never held while we issue discards. Hence if discard submission gets throttled because it is slow and/or there are lots of them, we aren't preventing other operations from being performed on AGF while we wait for discards to complete... Signed-off-by: Dave Chinner Reviewed-by: Darrick J. Wong --- fs/xfs/xfs_discard.c | 174 +++++++++++++++++++++++++++++++++++---- fs/xfs/xfs_extent_busy.c | 34 ++++++-- fs/xfs/xfs_extent_busy.h | 4 + 3 files changed, 188 insertions(+), 24 deletions(-) diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c index 3f45c7bb94f2..f16b254b5eaa 100644 --- a/fs/xfs/xfs_discard.c +++ b/fs/xfs/xfs_discard.c @@ -19,6 +19,56 @@ #include "xfs_log.h" #include "xfs_ag.h" +/* + * Notes on an efficient, low latency fstrim algorithm + * + * We need to walk the filesystem free space and issue discards on the free + * space that meet the search criteria (size and location). We cannot issue + * discards on extents that might be in use, or are so recently in use they are + * still marked as busy. To serialise against extent state changes whilst we are + * gathering extents to trim, we must hold the AGF lock to lock out other + * allocations and extent free operations that might change extent state. + * + * However, we cannot just hold the AGF for the entire AG free space walk whilst + * we issue discards on each free space that is found. Storage devices can have + * extremely slow discard implementations (e.g. ceph RBD) and so walking a + * couple of million free extents and issuing synchronous discards on each + * extent can take a *long* time. Whilst we are doing this walk, nothing else + * can access the AGF, and we can stall transactions and hence the log whilst + * modifications wait for the AGF lock to be released. This can lead hung tasks + * kicking the hung task timer and rebooting the system. This is bad. + * + * Hence we need to take a leaf from the bulkstat playbook. It takes the AGI + * lock, gathers a range of inode cluster buffers that are allocated, drops the + * AGI lock and then reads all the inode cluster buffers and processes them. It + * loops doing this, using a cursor to keep track of where it is up to in the AG + * for each iteration to restart the INOBT lookup from. + * + * We can't do this exactly with free space - once we drop the AGF lock, the + * state of the free extent is out of our control and we cannot run a discard + * safely on it in this situation. Unless, of course, we've marked the free + * extent as busy and undergoing a discard operation whilst we held the AGF + * locked. + * + * This is exactly how online discard works - free extents are marked busy when + * they are freed, and once the extent free has been committed to the journal, + * the busy extent record is marked as "undergoing discard" and the discard is + * then issued on the free extent. Once the discard completes, the busy extent + * record is removed and the extent is able to be allocated again. + * + * In the context of fstrim, if we find a free extent we need to discard, we + * don't have to discard it immediately. All we need to do it record that free + * extent as being busy and under discard, and all the allocation routines will + * now avoid trying to allocate it. Hence if we mark the extent as busy under + * the AGF lock, we can safely discard it without holding the AGF lock because + * nothing will attempt to allocate that free space until the discard completes. + * + * This also allows us to issue discards asynchronously like we do with online + * discard, and so for fast devices fstrim will run much faster as we can have + * multiple discard operations in flight at once, as well as pipeline the free + * extent search so that it overlaps in flight discard IO. + */ + struct workqueue_struct *xfs_discard_wq; static void @@ -94,21 +144,22 @@ xfs_discard_extents( } -STATIC int -xfs_trim_extents( +static int +xfs_trim_gather_extents( struct xfs_perag *pag, xfs_daddr_t start, xfs_daddr_t end, xfs_daddr_t minlen, + struct xfs_alloc_rec_incore *tcur, + struct xfs_busy_extents *extents, uint64_t *blocks_trimmed) { struct xfs_mount *mp = pag->pag_mount; - struct block_device *bdev = mp->m_ddev_targp->bt_bdev; struct xfs_btree_cur *cur; struct xfs_buf *agbp; - struct xfs_agf *agf; int error; int i; + int batch = 100; /* * Force out the log. This means any transactions that might have freed @@ -120,20 +171,28 @@ xfs_trim_extents( error = xfs_alloc_read_agf(pag, NULL, 0, &agbp); if (error) return error; - agf = agbp->b_addr; cur = xfs_allocbt_init_cursor(mp, NULL, agbp, pag, XFS_BTNUM_CNT); /* - * Look up the longest btree in the AGF and start with it. + * Look up the extent length requested in the AGF and start with it. */ - error = xfs_alloc_lookup_ge(cur, 0, be32_to_cpu(agf->agf_longest), &i); + if (tcur->ar_startblock == NULLAGBLOCK) + error = xfs_alloc_lookup_ge(cur, 0, tcur->ar_blockcount, &i); + else + error = xfs_alloc_lookup_le(cur, tcur->ar_startblock, + tcur->ar_blockcount, &i); if (error) goto out_del_cursor; + if (i == 0) { + /* nothing of that length left in the AG, we are done */ + tcur->ar_blockcount = 0; + goto out_del_cursor; + } /* * Loop until we are done with all extents that are large - * enough to be worth discarding. + * enough to be worth discarding or we hit batch limits. */ while (i) { xfs_agblock_t fbno; @@ -148,7 +207,16 @@ xfs_trim_extents( error = -EFSCORRUPTED; break; } - ASSERT(flen <= be32_to_cpu(agf->agf_longest)); + + if (--batch <= 0) { + /* + * Update the cursor to point at this extent so we + * restart the next batch from this extent. + */ + tcur->ar_startblock = fbno; + tcur->ar_blockcount = flen; + break; + } /* * use daddr format for all range/len calculations as that is @@ -163,6 +231,7 @@ xfs_trim_extents( */ if (dlen < minlen) { trace_xfs_discard_toosmall(mp, pag->pag_agno, fbno, flen); + tcur->ar_blockcount = 0; break; } @@ -185,29 +254,98 @@ xfs_trim_extents( goto next_extent; } - trace_xfs_discard_extent(mp, pag->pag_agno, fbno, flen); - error = blkdev_issue_discard(bdev, dbno, dlen, GFP_NOFS); - if (error) - break; + xfs_extent_busy_insert_discard(pag, fbno, flen, + &extents->extent_list); *blocks_trimmed += flen; - next_extent: error = xfs_btree_decrement(cur, 0, &i); if (error) break; - if (fatal_signal_pending(current)) { - error = -ERESTARTSYS; - break; - } + /* + * If there's no more records in the tree, we are done. Set the + * cursor block count to 0 to indicate to the caller that there + * is no more extents to search. + */ + if (i == 0) + tcur->ar_blockcount = 0; } + /* + * If there was an error, release all the gathered busy extents because + * we aren't going to issue a discard on them any more. + */ + if (error) + xfs_extent_busy_clear(mp, &extents->extent_list, false); out_del_cursor: xfs_btree_del_cursor(cur, error); xfs_buf_relse(agbp); return error; } +/* + * Iterate the free list gathering extents and discarding them. We need a cursor + * for the repeated iteration of gather/discard loop, so use the longest extent + * we found in the last batch as the key to start the next. + */ +static int +xfs_trim_extents( + struct xfs_perag *pag, + xfs_daddr_t start, + xfs_daddr_t end, + xfs_daddr_t minlen, + uint64_t *blocks_trimmed) +{ + struct xfs_alloc_rec_incore tcur = { + .ar_blockcount = pag->pagf_longest, + .ar_startblock = NULLAGBLOCK, + }; + int error = 0; + + do { + struct xfs_busy_extents *extents; + + extents = kzalloc(sizeof(*extents), GFP_KERNEL); + if (!extents) { + error = -ENOMEM; + break; + } + + extents->mount = pag->pag_mount; + extents->owner = extents; + INIT_LIST_HEAD(&extents->extent_list); + + error = xfs_trim_gather_extents(pag, start, end, minlen, + &tcur, extents, blocks_trimmed); + if (error) { + kfree(extents); + break; + } + + /* + * We hand the extent list to the discard function here so the + * discarded extents can be removed from the busy extent list. + * This allows the discards to run asynchronously with gathering + * the next round of extents to discard. + * + * However, we must ensure that we do not reference the extent + * list after this function call, as it may have been freed by + * the time control returns to us. + */ + error = xfs_discard_extents(pag->pag_mount, extents); + if (error) + break; + + if (fatal_signal_pending(current)) { + error = -ERESTARTSYS; + break; + } + } while (tcur.ar_blockcount != 0); + + return error; + +} + /* * trim a range of the filesystem. * diff --git a/fs/xfs/xfs_extent_busy.c b/fs/xfs/xfs_extent_busy.c index 7c2fdc71e42d..746814815b1d 100644 --- a/fs/xfs/xfs_extent_busy.c +++ b/fs/xfs/xfs_extent_busy.c @@ -19,13 +19,13 @@ #include "xfs_log.h" #include "xfs_ag.h" -void -xfs_extent_busy_insert( - struct xfs_trans *tp, +static void +xfs_extent_busy_insert_list( struct xfs_perag *pag, xfs_agblock_t bno, xfs_extlen_t len, - unsigned int flags) + unsigned int flags, + struct list_head *busy_list) { struct xfs_extent_busy *new; struct xfs_extent_busy *busyp; @@ -40,7 +40,7 @@ xfs_extent_busy_insert( new->flags = flags; /* trace before insert to be able to see failed inserts */ - trace_xfs_extent_busy(tp->t_mountp, pag->pag_agno, bno, len); + trace_xfs_extent_busy(pag->pag_mount, pag->pag_agno, bno, len); spin_lock(&pag->pagb_lock); rbp = &pag->pagb_tree.rb_node; @@ -62,10 +62,32 @@ xfs_extent_busy_insert( rb_link_node(&new->rb_node, parent, rbp); rb_insert_color(&new->rb_node, &pag->pagb_tree); - list_add(&new->list, &tp->t_busy); + list_add(&new->list, busy_list); spin_unlock(&pag->pagb_lock); } +void +xfs_extent_busy_insert( + struct xfs_trans *tp, + struct xfs_perag *pag, + xfs_agblock_t bno, + xfs_extlen_t len, + unsigned int flags) +{ + xfs_extent_busy_insert_list(pag, bno, len, flags, &tp->t_busy); +} + +void +xfs_extent_busy_insert_discard( + struct xfs_perag *pag, + xfs_agblock_t bno, + xfs_extlen_t len, + struct list_head *busy_list) +{ + xfs_extent_busy_insert_list(pag, bno, len, XFS_EXTENT_BUSY_DISCARDED, + busy_list); +} + /* * Search for a busy extent within the range of the extent we are about to * allocate. You need to be holding the busy extent tree lock when calling diff --git a/fs/xfs/xfs_extent_busy.h b/fs/xfs/xfs_extent_busy.h index 71c28d031e3b..0639aab336f3 100644 --- a/fs/xfs/xfs_extent_busy.h +++ b/fs/xfs/xfs_extent_busy.h @@ -49,6 +49,10 @@ void xfs_extent_busy_insert(struct xfs_trans *tp, struct xfs_perag *pag, xfs_agblock_t bno, xfs_extlen_t len, unsigned int flags); +void +xfs_extent_busy_insert_discard(struct xfs_perag *pag, xfs_agblock_t bno, + xfs_extlen_t len, struct list_head *busy_list); + void xfs_extent_busy_clear(struct xfs_mount *mp, struct list_head *list, bool do_discard); From patchwork Thu Sep 21 01:39:45 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Chinner X-Patchwork-Id: 13393584 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2A0E4CD4938 for ; Thu, 21 Sep 2023 01:39:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229700AbjIUBj5 (ORCPT ); Wed, 20 Sep 2023 21:39:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45180 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229473AbjIUBj4 (ORCPT ); Wed, 20 Sep 2023 21:39:56 -0400 Received: from mail-pl1-x62b.google.com (mail-pl1-x62b.google.com [IPv6:2607:f8b0:4864:20::62b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 323D2AB for ; Wed, 20 Sep 2023 18:39:51 -0700 (PDT) Received: by mail-pl1-x62b.google.com with SMTP id d9443c01a7336-1c0c6d4d650so3569115ad.0 for ; Wed, 20 Sep 2023 18:39:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fromorbit-com.20230601.gappssmtp.com; s=20230601; t=1695260390; x=1695865190; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:from:to:cc:subject:date:message-id :reply-to; bh=MLvhZIcM0SpJ7wSPoi1+k37DPAhj8JFuXNxcyOg8Bl8=; b=Pux4oQWpUKGb9EBWgxzQB9tTvg+vsQa76w8EUGRqKGZYwP09SM4D+opwfA8AdDKrPs C02UR+Cwlem7TZ8SeqCMbXgVLNN/2kcHiBMUJBPoSoaeWm7CRONbefy4iwmgoQYSFtSb oYiAXHLIa7WfwoC+i64eE1tuMR6cRWnd1GwlsDQUxsbho3uhkwbeL2W6Iwx2XJKGF+HF tIAaxuj2dzV+H0ClGIhVLqJ5O2H5P2K+GfYUYT47bE1nsIgk6LCreUabwHaKDXxUGzRi qS3fqCSSff2nHCGuHClSww+uNPRUvn7fFxYf1OywtUWRaWBlkie1BjeWdIVAD06CtTL2 dFrw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695260390; x=1695865190; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=MLvhZIcM0SpJ7wSPoi1+k37DPAhj8JFuXNxcyOg8Bl8=; b=rGO1oCuZ14DA2EmUtjUufh8j5tHKrsWuBBvOFspLcYXIqUbuBxoRsMRQUMkdmxH+y2 K431qKk+ngf7hIPPq69YddG2wd0jdrQ27zAR7hpQlkEv/+kjzAydF2YRhWVWtTCiYotE lpHdTR60b6v3/D/+jUxGyjbOMyhG7IxWw/8cc0+tU3pdBixe2AyGFOrayKv1S43Ehozd KiUQhoa2GYZoZfbw8wb0v2MH3RcdFINiogtT+89DaP6DEjv4Do8V1kn2D49zdlUybLkK PwOpmF4cCchDC/+igZAvDBvhcNqsdYdi1hkCr107xoROoW6iQXW+6dGV/ZpxIeiCL3gL Lnqw== X-Gm-Message-State: AOJu0YyB38WgCRBfcIv4HgYf0YdezgexeGyovdh18WYpd6FbVSUcOPL3 WW51A3iszoEbgjhzSRJ1Fp5ezrjqLUwLSB6b7hE= X-Google-Smtp-Source: AGHT+IEyIEmr2paM4FY8CWZBJ52WVvDxd4B8hGlItu/j/jXz4y6u+KTYomiz1e4VImWf1Y2UnnRksA== X-Received: by 2002:a17:902:e886:b0:1c2:1068:1f4f with SMTP id w6-20020a170902e88600b001c210681f4fmr3976835plg.17.1695260390599; Wed, 20 Sep 2023 18:39:50 -0700 (PDT) Received: from dread.disaster.area (pa49-180-20-59.pa.nsw.optusnet.com.au. [49.180.20.59]) by smtp.gmail.com with ESMTPSA id jj18-20020a170903049200b001c5c0d6fc76sm143442plb.233.2023.09.20.18.39.49 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Sep 2023 18:39:49 -0700 (PDT) Received: from [192.168.253.23] (helo=devoid.disaster.area) by dread.disaster.area with esmtp (Exim 4.96) (envelope-from ) id 1qj8fT-003Sv6-1Y for linux-xfs@vger.kernel.org; Thu, 21 Sep 2023 11:39:47 +1000 Received: from dave by devoid.disaster.area with local (Exim 4.97-RC0) (envelope-from ) id 1qj8fT-00000002MHQ-0xJO for linux-xfs@vger.kernel.org; Thu, 21 Sep 2023 11:39:47 +1000 From: Dave Chinner To: linux-xfs@vger.kernel.org Subject: [PATCH 3/3] xfs: abort fstrim if kernel is suspending Date: Thu, 21 Sep 2023 11:39:45 +1000 Message-Id: <20230921013945.559634-4-david@fromorbit.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20230921013945.559634-1-david@fromorbit.com> References: <20230921013945.559634-1-david@fromorbit.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org From: Dave Chinner A recent ext4 patch posting from Jan Kara reminded me of a discussion a year ago about fstrim in progress preventing kernels from suspending. The fix is simple, we should do the same for XFS. This removes the -ERESTARTSYS error return from this code, replacing it with either the last error seen or the number of blocks successfully trimmed up to the point where we detected the stop condition. References: https://bugzilla.kernel.org/show_bug.cgi?id=216322 Signed-off-by: Dave Chinner Reviewed-by: Darrick J. Wong --- fs/xfs/xfs_discard.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c index f16b254b5eaa..d5787991bb5b 100644 --- a/fs/xfs/xfs_discard.c +++ b/fs/xfs/xfs_discard.c @@ -283,6 +283,12 @@ xfs_trim_gather_extents( return error; } +static bool +xfs_trim_should_stop(void) +{ + return fatal_signal_pending(current) || freezing(current); +} + /* * Iterate the free list gathering extents and discarding them. We need a cursor * for the repeated iteration of gather/discard loop, so use the longest extent @@ -336,10 +342,9 @@ xfs_trim_extents( if (error) break; - if (fatal_signal_pending(current)) { - error = -ERESTARTSYS; + if (xfs_trim_should_stop()) break; - } + } while (tcur.ar_blockcount != 0); return error; @@ -408,12 +413,12 @@ xfs_ioc_trim( for_each_perag_range(mp, agno, xfs_daddr_to_agno(mp, end), pag) { error = xfs_trim_extents(pag, start, end, minlen, &blocks_trimmed); - if (error) { + if (error) last_error = error; - if (error == -ERESTARTSYS) { - xfs_perag_rele(pag); - break; - } + + if (xfs_trim_should_stop()) { + xfs_perag_rele(pag); + break; } }