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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9D700C7618E for ; Fri, 21 Apr 2023 17:22:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2957C6B0072; Fri, 21 Apr 2023 13:22:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 245B56B0074; Fri, 21 Apr 2023 13:22:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 10E956B0075; Fri, 21 Apr 2023 13:22:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 020DD6B0072 for ; Fri, 21 Apr 2023 13:22:18 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id D2050A04AF for ; Fri, 21 Apr 2023 17:22:17 +0000 (UTC) X-FDA: 80706066714.05.9B9D183 Received: from mail-ej1-f49.google.com (mail-ej1-f49.google.com [209.85.218.49]) by imf25.hostedemail.com (Postfix) with ESMTP id ED43EA0031 for ; Fri, 21 Apr 2023 17:22:15 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=M7kgruF3; spf=pass (imf25.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.49 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1682097736; h=from:from:sender: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:dkim-signature; bh=BfR9lcqvLbO+sXRYYQ4Al5uQ4a/w4jqcaiHNX0XG2d8=; b=7gP2GvZhOKIzdpy2okoBJAua1wz2LjdFs2jYTgUiymIEJTwi/eKZVqRW6xshPYsxGIkWGv mCRxUT6aMFh1KcJjBxDktAlzt/PzWEbL5n5YeURitVTyiLzV9xRdHJMnBZwJ4WSw3yhBih +TzXTxvuvLFiZ2eLKpIyAd3tIlqkrCw= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=M7kgruF3; spf=pass (imf25.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.49 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1682097736; a=rsa-sha256; cv=none; b=6VzPfzaozEKeMNEilP6Tam65GShV5hP1y3bBiHnB2a+TL7hipfsZZ3iLUaqiaVLCSRQ5hp zHqoYg7pNHkary1H+xGj3d96/5ok2OtftcMBYeADMLcdG5bEIARLYBhUmgpTaUujqz8D4h pOZlh3OIymRyAiZaTgpn0oRu1Bkd00g= Received: by mail-ej1-f49.google.com with SMTP id a640c23a62f3a-94a342f7c4cso323977666b.0 for ; Fri, 21 Apr 2023 10:22:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1682097734; x=1684689734; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=BfR9lcqvLbO+sXRYYQ4Al5uQ4a/w4jqcaiHNX0XG2d8=; b=M7kgruF3UfRfDWNCLtS06HOrbY/My1F5qsoj3AdCxeDtL9AnMCKstRKn/+mzyTSJXD nUELrf0ki/bVD0Bf6VaQ/BPcAyOipMfOJft9lnwibEAhqePhPxl+6N2xFt8K2aLmxT4d eI7byFtpOOMmoO8AzyO787vo6Mv+NG+LT4/rW6D9JlgChyif1F0ltmnqqBIp/0Lsuu0m IO2ZCJHgpTWXkJgsbgBAHGD8t9exscbi9IJH23YwGaJndjTrVI5DhTELy21qO5sIXB3J AiAoU6sruhfjdjNyk8Fg+wuorOokt2S7FMxMvRhCaYZ/12NWLYAdQflE/0HQdBDR8ffd FQfw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682097734; x=1684689734; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=BfR9lcqvLbO+sXRYYQ4Al5uQ4a/w4jqcaiHNX0XG2d8=; b=PWgkwlD9dVvMiBjTSOP1K9OLRyKYQjTI345rT63Sqvfe69Trpzte4x2vBaBiraB9+F wp7wJGGPKVKU+6aeIX9Q2wM+OJZ8NA8Kn+d0+b7ei9w5zHASXRkmtS36KgIA1lSLhI8Z TcYja9Vv6/XqE0Symg9Vrjo4xMui2RMZ9hfdErf/ZF5cODBCf14fRfbFWH06m0qrlT6d GesIW4VKeehtn/ussySgc0ab32YoWAdrDPLTO4UF5yGsU6hvOvwI/M+oTa9R82uXiYKW 3lXCP4ekkffVQDPBOi03oTIzdxxbkTgonswpr0OnQmoT3J7ImwjU7Dq7cJLqxTb3CKNh C8Mw== X-Gm-Message-State: AAQBX9eWiNcyelib0EjiI0TjnDFuo2VTraza6Gt+ivZgXVGLa1iQCdHQ OYPMLHbEiW2ui83l9QEBgHIVUUNFN0FNWswjbKvLLg== X-Google-Smtp-Source: AKy350axrab997pt4/P+KsZ/mjGlBLpHSvPcnzMc+GCcSQviGW1yDnMwiUwpJtmJyvjReOIzY0QnplK4idGv54zBasw= X-Received: by 2002:aa7:d385:0:b0:4fb:7ccf:3b33 with SMTP id x5-20020aa7d385000000b004fb7ccf3b33mr5514114edq.31.1682097734305; Fri, 21 Apr 2023 10:22:14 -0700 (PDT) MIME-Version: 1.0 References: <20230403220337.443510-1-yosryahmed@google.com> <20230403220337.443510-2-yosryahmed@google.com> <20230421085341.b2zvzeuc745bs6sa@quack3> In-Reply-To: <20230421085341.b2zvzeuc745bs6sa@quack3> From: Yosry Ahmed Date: Fri, 21 Apr 2023 10:21:36 -0700 Message-ID: Subject: Re: [PATCH mm-unstable RFC 1/5] writeback: move wb_over_bg_thresh() call outside lock section To: Jan Kara Cc: Alexander Viro , Christian Brauner , Johannes Weiner , Michal Hocko , Roman Gushchin , Shakeel Butt , Muchun Song , Andrew Morton , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: ED43EA0031 X-Stat-Signature: m7armdh48bpm8iapzupww5zb3sfjd7fu X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1682097735-801384 X-HE-Meta: U2FsdGVkX193BWbKPzR41bkd4X9qZlXYOExqy2N2/bGPi3SeSS3H2Cr9j3/8TYUpIAFVEU/EA5AQBsAf05KrfSqXLEXYYRAclNsbsO4aRH3MLDZbLMM4LMBkXiQur1O0Eqk33KXDAKYH0tuajKF3+0k0VP5kqRTSq4hc84b2/Ac5BNHPjuQ5/WOyD5hxB8xh0ziJagDeRDV4ZOx857uFUA7GS2PXIvno/YxE5rr9wmnNW2VpKqfecerz437mtImBfyopkalTk/jBORg1zHHglBlIX8F3mrLjZg2/ZSbqMKvnD8+J8h6pGayPwjLLsQCQVr8Hv0OYLuFkl5CFOI5gmM9tDA9UURkAr5qCJlUdnaIAWzPlu8NLRp5jRxFlO2NF2bQ9z7F1wxQUcyawrg8nLNjQVvFkwY2ozHppncpyAp9kXnC0E39oxmBGAWYU9/oMh6HRgC0A+zUOVjd/uaNt28O3J4HJ6hCna2LI0+o6asFHoQwAD7EH5+yNDduoQVVuUKSuqtFOmQLvw5cayfOMcvMmTGL1g3Y+2LswaGKk3q1uomYDs4EtvV7jYlIptGlgBCXyhTW9eW1y4XhbqDRU3Z1qVh/jA9HV8QduB/E+g2sn7S1Qj5qfQZteSpg4beYM89jnos8QsgP8iE/SP63qZEpDnbMxotcre7QBGNyn/NumrLBdf6bqnomfJcDB/7AFwQTgBu/JOWYe88ocF0HnW4TYK8pztqGa13T3bZawYWQfPj+Xoe4ymbh9fHj6gvzKSo5Z77j61vh3Zj/60tFhUIha1dZpW4vQ/6EBsmPwARjxpzQOE9nA5/jKXkZoUra+ajP013cez9tzcHezs7msuS7wIrAgEPyq1YV6GKThDAC9V+xZ6WuPzSsY9X5XvKkOx4D8tz/rVYHQN5EK+rAe7oKxv42c9DKTyID6gLiEbLD+MlzN+bmZFNUwFEiwl8s0JuJSj7BTdroK43zsHve QHapasXf EQ6exW19EKzCs4j46o1GzT88GOSiF0rCukKWWFxFxbO3vloSf+EFW/h8Shuw5+TMmm7s5bgb2EoIOo/ILikc1P1cCxrGNl70wGXFgqoEIe5OSZo8CB3fsEAxNvLFlcIqW5PxD67i4FE85RJ0wYxLpHLGM04Ap9EFJn7Yw2eC/t0BZYbWyb5tazNuvtE3tlW7r281KXhDSPwsulnZ01k5uNlNnWzMn57mIsn5B/KMiQukLiwifepEBv9QQxlWk77J9emofJXxng+SCeMrc2tQmQBC9vvizZagBR3btgVZ9FYTF6PmCMGvrFOXwk8NAx+MDQbCs2aow9QQTZRbx1vFp7FZCYedwmVEkaxyuMTQDYNjtCpo= 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, Apr 21, 2023 at 1:53=E2=80=AFAM Jan Kara wrote: > > On Mon 03-04-23 22:03:33, Yosry Ahmed wrote: > > wb_over_bg_thresh() calls mem_cgroup_wb_stats() which invokes an rstat > > flush, which can be expensive on large systems. Currently, > > wb_writeback() calls wb_over_bg_thresh() within a lock section, so we > > have to make the rstat flush atomically. On systems with a lot of > > cpus/cgroups, this can cause us to disable irqs for a long time, > > potentially causing problems. > > > > Move the call to wb_over_bg_thresh() outside the lock section in > > preparation to make the rstat flush in mem_cgroup_wb_stats() non-atomic= . > > The list_empty(&wb->work_list) should be okay outside the lock section > > of wb->list_lock as it is protected by a separate lock (wb->work_lock), > > and wb_over_bg_thresh() doesn't seem like it is modifying any of the b_= * > > lists the wb->list_lock is protecting. Also, the loop seems to be > > already releasing and reacquring the lock, so this refactoring looks > > safe. > > > > Signed-off-by: Yosry Ahmed > > The patch looks good to me. Nice find. Feel free to add: > > Reviewed-by: Jan Kara Thanks for taking a look! > > Honza > > > --- > > fs/fs-writeback.c | 16 +++++++++++----- > > 1 file changed, 11 insertions(+), 5 deletions(-) > > > > diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c > > index 195dc23e0d831..012357bc8daa3 100644 > > --- a/fs/fs-writeback.c > > +++ b/fs/fs-writeback.c > > @@ -2021,7 +2021,6 @@ static long wb_writeback(struct bdi_writeback *wb= , > > struct blk_plug plug; > > > > blk_start_plug(&plug); > > - spin_lock(&wb->list_lock); > > for (;;) { > > /* > > * Stop writeback when nr_pages has been consumed > > @@ -2046,6 +2045,9 @@ static long wb_writeback(struct bdi_writeback *wb= , > > if (work->for_background && !wb_over_bg_thresh(wb)) > > break; > > > > + > > + spin_lock(&wb->list_lock); > > + > > /* > > * Kupdate and background works are special and we want t= o > > * include all inodes that need writing. Livelock avoidan= ce is > > @@ -2075,13 +2077,19 @@ static long wb_writeback(struct bdi_writeback *= wb, > > * mean the overall work is done. So we keep looping as l= ong > > * as made some progress on cleaning pages or inodes. > > */ > > - if (progress) > > + if (progress) { > > + spin_unlock(&wb->list_lock); > > continue; > > + } > > + > > /* > > * No more inodes for IO, bail > > */ > > - if (list_empty(&wb->b_more_io)) > > + if (list_empty(&wb->b_more_io)) { > > + spin_unlock(&wb->list_lock); > > break; > > + } > > + > > /* > > * Nothing written. Wait for some inode to > > * become available for writeback. Otherwise > > @@ -2093,9 +2101,7 @@ static long wb_writeback(struct bdi_writeback *wb= , > > spin_unlock(&wb->list_lock); > > /* This function drops i_lock... */ > > inode_sleep_on_writeback(inode); > > - spin_lock(&wb->list_lock); > > } > > - spin_unlock(&wb->list_lock); > > blk_finish_plug(&plug); > > > > return nr_pages - work->nr_pages; > > -- > > 2.40.0.348.gf938b09366-goog > > > -- > Jan Kara > SUSE Labs, CR