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=-8.3 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 33F3BC5DF60 for ; Tue, 5 Nov 2019 17:06:57 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id DBA6B20656 for ; Tue, 5 Nov 2019 17:06:56 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="EG6j98nS" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DBA6B20656 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 6F12C6B0003; Tue, 5 Nov 2019 12:06:56 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 6A1826B0005; Tue, 5 Nov 2019 12:06:56 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 568EB6B0006; Tue, 5 Nov 2019 12:06:56 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 40C9A6B0003 for ; Tue, 5 Nov 2019 12:06:56 -0500 (EST) Received: from smtpin12.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with SMTP id E9A79180AD817 for ; Tue, 5 Nov 2019 17:06:55 +0000 (UTC) X-FDA: 76122853590.12.bears98_59935162a0e29 X-HE-Tag: bears98_59935162a0e29 X-Filterd-Recvd-Size: 7678 Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by imf04.hostedemail.com (Postfix) with ESMTP for ; Tue, 5 Nov 2019 17:06:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1572973614; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=x5gX8G0vnfueKdPpJcGj776JikjvLmPzMcNuP6rkJHg=; b=EG6j98nSZrzEXqrZGrPHVIHjnStHHu20+Bzqtb7U0jZsBfzZA7XifpbfJzhLez3PRGoUoI gMmGnw3YHgAE2H1FsyoNWOha4uY2aTvWUIXdD8xDFmW1tNBlb6uvLqEs1ThoutC8l1nbhf 15/AC6ncgyqaK5uAiU7FRqke13HMMPo= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-218-HPAS1klHPlioZDfIRul_cQ-1; Tue, 05 Nov 2019 12:06:52 -0500 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 658418017DD; Tue, 5 Nov 2019 17:06:51 +0000 (UTC) Received: from bfoster (dhcp-41-2.bos.redhat.com [10.18.41.2]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C92ED5D9C9; Tue, 5 Nov 2019 17:06:50 +0000 (UTC) Date: Tue, 5 Nov 2019 12:06:49 -0500 From: Brian Foster To: Dave Chinner Cc: linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 21/28] xfs: use AIL pushing for inode reclaim IO Message-ID: <20191105170649.GE28493@bfoster> References: <20191031234618.15403-1-david@fromorbit.com> <20191031234618.15403-22-david@fromorbit.com> MIME-Version: 1.0 In-Reply-To: <20191031234618.15403-22-david@fromorbit.com> User-Agent: Mutt/1.12.1 (2019-06-15) X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-MC-Unique: HPAS1klHPlioZDfIRul_cQ-1 X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, Nov 01, 2019 at 10:46:11AM +1100, Dave Chinner wrote: > From: Dave Chinner >=20 > Inode reclaim currently issues it's own inode IO when it comes > across dirty inodes. This is used to throttle direct reclaim down to > the rate at which we can reclaim dirty inodes. Failure to throttle > in this manner results in the OOM killer being trivial to trigger > even when there is lots of free memory available. >=20 > However, having direct reclaimers issue IO causes an amount of > IO thrashing to occur. We can have up to the number of AGs in the > filesystem concurrently issuing IO, plus the AIL pushing thread as > well. This means we can many competing sources of IO and they all > end up thrashing and competing for the request slots in the block > device. >=20 > Similar to dirty page throttling and the BDI flusher thread, we can > use the AIL pushing thread the sole place we issue inode writeback > from and everything else waits for it to make progress. To do this, > reclaim will skip over dirty inodes, but in doing so will record the > lowest LSN of all the dirty inodes it skips. It will then push the > AIL to this LSN and wait for it to complete that work. >=20 > In doing so, we block direct reclaim on the IO of at least one IO, > thereby providing some level of throttling for when we encounter > dirty inodes. However we gain the ability to scan and reclaim clean > inodes in a non-blocking fashion. >=20 > Hence direct reclaim will be throttled directly by the rate at which > dirty inodes are cleaned by AIL pushing, rather than by delays > caused by competing IO submissions. This allows us to reduce the > locking that limits direct reclaim concurrency to just protecting > the reclaim cursor state, hence greatly simplifying the inode > reclaim code as it now just skips dirty inodes. >=20 > Note: this patch by itself isn't completely able to throttle direct > reclaim sufficiently to prevent OOM killer madness. We can't do that > until we change the way we index reclaimable inodes in the next > patch and can feed back state to the mm core sanely. However, we > can't change the way we index reclaimable inodes until we have > IO-less non-blocking reclaim for both direct reclaim and kswapd > reclaim. Catch-22... >=20 > Signed-off-by: Dave Chinner > --- A couple random nits and a question... > fs/xfs/xfs_icache.c | 218 ++++++++++++++++++-------------------------- > 1 file changed, 89 insertions(+), 129 deletions(-) >=20 > diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c > index 7e175304e146..ff8ae32614a6 100644 > --- a/fs/xfs/xfs_icache.c > +++ b/fs/xfs/xfs_icache.c ... > @@ -1262,9 +1227,13 @@ xfs_reclaim_inodes_ag( > =09=09=09for (i =3D 0; i < nr_found; i++) { > =09=09=09=09struct xfs_inode *ip =3D batch[i]; > =20 > -=09=09=09=09if (done || xfs_reclaim_inode_grab(ip, flags)) > +=09=09=09=09if (done || > +=09=09=09=09 !xfs_reclaim_inode_grab(ip, flags, &lsn)) > =09=09=09=09=09batch[i] =3D NULL; Doesn't look like we can get here with done !=3D 0. > =20 > +=09=09=09=09if (lsn && XFS_LSN_CMP(lsn, lowest_lsn) < 0) > +=09=09=09=09=09lowest_lsn =3D lsn; > + This should probably have the same NULLCOMMITLSN treatment as the similar check below. > =09=09=09=09/* > =09=09=09=09 * Update the index for the next lookup. Catch > =09=09=09=09 * overflows into the next AG range which can > @@ -1289,41 +1258,34 @@ xfs_reclaim_inodes_ag( ... > =20 > -=09/* > -=09 * if we skipped any AG, and we still have scan count remaining, do > -=09 * another pass this time using blocking reclaim semantics (i.e > -=09 * waiting on the reclaim locks and ignoring the reclaim cursors). Th= is > -=09 * ensure that when we get more reclaimers than AGs we block rather > -=09 * than spin trying to execute reclaim. > -=09 */ > -=09if (skipped && (flags & SYNC_WAIT) && *nr_to_scan > 0) { > -=09=09trylock =3D 0; > -=09=09goto restart; > -=09} > -=09return last_error; > +=09if ((flags & SYNC_WAIT) && lowest_lsn !=3D NULLCOMMITLSN) > +=09=09xfs_ail_push_sync(mp->m_ail, lowest_lsn); > + > +=09return freed; Hm, this should have always been returning a free count instead of an error code right? If so, I'd normally suggest to fix this as an independent patch, but it's probably not worth splitting up at this point. The aforementioned nits seem harmless and the code is going away, so: Reviewed-by: Brian Foster > } > =20 > int > @@ -1331,9 +1293,7 @@ xfs_reclaim_inodes( > =09xfs_mount_t=09*mp, > =09int=09=09mode) > { > -=09int=09=09nr_to_scan =3D INT_MAX; > - > -=09return xfs_reclaim_inodes_ag(mp, mode, &nr_to_scan); > +=09return xfs_reclaim_inodes_ag(mp, mode, INT_MAX); > } > =20 > /* > @@ -1350,7 +1310,7 @@ xfs_reclaim_inodes_nr( > =09struct xfs_mount=09*mp, > =09int=09=09=09nr_to_scan) > { > -=09int=09=09=09sync_mode =3D SYNC_TRYLOCK; > +=09int=09=09=09sync_mode =3D 0; > =20 > =09/* > =09 * For kswapd, we kick background inode writeback. For direct > @@ -1362,7 +1322,7 @@ xfs_reclaim_inodes_nr( > =09else > =09=09sync_mode |=3D SYNC_WAIT; > =20 > -=09return xfs_reclaim_inodes_ag(mp, sync_mode, &nr_to_scan); > +=09return xfs_reclaim_inodes_ag(mp, sync_mode, nr_to_scan); > } > =20 > /* > --=20 > 2.24.0.rc0 >=20