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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CB632C433F5 for ; Fri, 22 Oct 2021 08:39:32 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 527CD6112F for ; Fri, 22 Oct 2021 08:39:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 527CD6112F Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=techsingularity.net Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id B9A5A900003; Fri, 22 Oct 2021 04:39:31 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B2314900002; Fri, 22 Oct 2021 04:39:31 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9EAAC900003; Fri, 22 Oct 2021 04:39:31 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0197.hostedemail.com [216.40.44.197]) by kanga.kvack.org (Postfix) with ESMTP id 911E4900002 for ; Fri, 22 Oct 2021 04:39:31 -0400 (EDT) Received: from smtpin35.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 4AA2430B4F for ; Fri, 22 Oct 2021 08:39:31 +0000 (UTC) X-FDA: 78723424542.35.B0B87B4 Received: from outbound-smtp55.blacknight.com (outbound-smtp55.blacknight.com [46.22.136.239]) by imf27.hostedemail.com (Postfix) with ESMTP id 5234E7000091 for ; Fri, 22 Oct 2021 08:39:29 +0000 (UTC) Received: from mail.blacknight.com (pemlinmail03.blacknight.ie [81.17.254.16]) by outbound-smtp55.blacknight.com (Postfix) with ESMTPS id 5E803FB31B for ; Fri, 22 Oct 2021 09:39:29 +0100 (IST) Received: (qmail 28203 invoked from network); 22 Oct 2021 08:39:29 -0000 Received: from unknown (HELO techsingularity.net) (mgorman@techsingularity.net@[84.203.17.29]) by 81.17.254.9 with ESMTPSA (AES256-SHA encrypted, authenticated); 22 Oct 2021 08:39:29 -0000 Date: Fri, 22 Oct 2021 09:39:27 +0100 From: Mel Gorman To: NeilBrown Cc: Andrew Morton , Theodore Ts'o , Andreas Dilger , "Darrick J . Wong" , Matthew Wilcox , Michal Hocko , Dave Chinner , Rik van Riel , Vlastimil Babka , Johannes Weiner , Jonathan Corbet , Linux-MM , Linux-fsdevel , LKML Subject: Re: [PATCH v4 0/8] Remove dependency on congestion_wait in mm/ Message-ID: <20211022083927.GI3959@techsingularity.net> References: <20211019090108.25501-1-mgorman@techsingularity.net> <163486531001.17149.13533181049212473096@noble.neil.brown.name> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <163486531001.17149.13533181049212473096@noble.neil.brown.name> User-Agent: Mutt/1.10.1 (2018-07-13) X-Stat-Signature: zrjj9erxcrrhturobgsjmzjj6siespda Authentication-Results: imf27.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf27.hostedemail.com: domain of mgorman@techsingularity.net designates 46.22.136.239 as permitted sender) smtp.mailfrom=mgorman@techsingularity.net X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 5234E7000091 X-HE-Tag: 1634891969-672764 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, Oct 22, 2021 at 12:15:10PM +1100, NeilBrown wrote: > On Tue, 19 Oct 2021, Mel Gorman wrote: > > Changelog since v3 > > o Count writeback completions for NR_THROTTLED_WRITTEN only > > o Use IRQ-safe inc_node_page_state > > o Remove redundant throttling > > > > This series is also available at > > > > git://git.kernel.org/pub/scm/linux/kernel/git/mel/linux.git mm-reclaimcongest-v4r2 > > > > This series that removes all calls to congestion_wait > > in mm/ and deletes wait_iff_congested. > > Thanks for this. > I don't have sufficient expertise for a positive review, but it seems to > make sense with one exception which I have commented on separately. > A test battering NFS would still be nice! > In general, I still don't like the use of wake_up_all(), though it won't > cause incorrect behaviour. > Removing wake_up_all would be tricky. Ideally it would be prioritised but more importantly, some sort of guarantee should exist that enough wakeup events trigger to wake tasks before the timeout. That would need careful thinking about each reclaim reason. For example, if N tasks throttle on NOPROGRESS, there is no guarantee that N tasks are currently in reclaim that would wake each sleeping task as progress is made. It's similar for writeback, are enough pages under writeback to trigger each wakeup? A more subtle issue is if each reason should be strict if waking tasks one at a time. For example, a task sleeping on writeback might make progress for other reasons such as the working set changing during reclaim or a large task exiting. Of course the same concerns exist for the series as it stands but the worst case scenarios are mitigated by wake_up_all. > I would prefer the first patch would: > - define NR_VMSCAN_THROTTLE > - make reclaim_wait an array > - spelled nr_reclaim_throttled as nr_writeback_throttled > > rather than leaving those changes for the second patch. I think that > would make review easier. > I can do this. Normally I try structure series from least-to-most controversial so that it can be cut at any point and still make sense so the array was defined in the second patch because that's when it is required. However, I already had defined the enum in patch 1 for the tracepoint so I might as well make it an array too. -- Mel Gorman SUSE Labs