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=-5.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=no 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 56369C433EF for ; Tue, 21 Sep 2021 21:41:13 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id DDD9B61165 for ; Tue, 21 Sep 2021 21:41:12 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org DDD9B61165 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 534C7900002; Tue, 21 Sep 2021 17:41:12 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4E3A16B0071; Tue, 21 Sep 2021 17:41:12 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3F95B900002; Tue, 21 Sep 2021 17:41:12 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0081.hostedemail.com [216.40.44.81]) by kanga.kvack.org (Postfix) with ESMTP id 313786B006C for ; Tue, 21 Sep 2021 17:41:12 -0400 (EDT) Received: from smtpin07.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id E6EA5181C1958 for ; Tue, 21 Sep 2021 21:41:11 +0000 (UTC) X-FDA: 78612901542.07.42986CA Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by imf29.hostedemail.com (Postfix) with ESMTP id 9BF7B900013E for ; Tue, 21 Sep 2021 21:41:11 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 60AD11FF24; Tue, 21 Sep 2021 21:41:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1632260470; h=from:from:reply-to: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=I0GqJYRTLtj5brv4Bac0gTMm/xYvL7+Yx0c3tiayK/k=; b=vMIuuQA6sKKov3GBq6bp3HF6ImlswNZ8QL88oU7KwnEnDh4vSRf7bjMaAGoAn42aeQnLUc WdeeswDTbEseAAr6RJdEbqnwfwIz/8Rr1b6J9rU7mzMznBgjowrwApazbpIswb9gwT5yDP ElAFdF9oLF7WKJV3L/sl0qg5DMPeAWk= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1632260470; h=from:from:reply-to: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=I0GqJYRTLtj5brv4Bac0gTMm/xYvL7+Yx0c3tiayK/k=; b=v45T6WfDNNIpJAkeEid0sdThJriTirP9PcSXknSMnBWz+DqFKWZxQgYk730mh93tmRf6FX V3/uWkZDGihpibDw== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 2C7A413BF7; Tue, 21 Sep 2021 21:41:05 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id 5PsjN3FRSmELIwAAMHmgww (envelope-from ); Tue, 21 Sep 2021 21:41:05 +0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 From: "NeilBrown" To: "Mel Gorman" Cc: "Linux-MM" , "Theodore Ts'o" , "Andreas Dilger" , "Darrick J . Wong" , "Matthew Wilcox" , "Michal Hocko" , "Dave Chinner" , "Rik van Riel" , "Vlastimil Babka" , "Johannes Weiner" , "Jonathan Corbet" , "Linux-fsdevel" , "LKML" Subject: Re: [PATCH 1/5] mm/vmscan: Throttle reclaim until some writeback completes if congested In-reply-to: <20210921105831.GO3959@techsingularity.net> References: <20210920085436.20939-1-mgorman@techsingularity.net>, <20210920085436.20939-2-mgorman@techsingularity.net>, <163218319798.3992.1165186037496786892@noble.neil.brown.name>, <20210921105831.GO3959@techsingularity.net> Date: Wed, 22 Sep 2021 07:40:59 +1000 Message-id: <163226045956.21861.7998898955979000139@noble.neil.brown.name> X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 9BF7B900013E X-Stat-Signature: ro4rknd7a1s6ur6jd4dpj8po996b9gre Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=suse.de header.s=susede2_rsa header.b=vMIuuQA6; dkim=pass header.d=suse.de header.s=susede2_ed25519 header.b=v45T6WfD; spf=pass (imf29.hostedemail.com: domain of neilb@suse.de designates 195.135.220.29 as permitted sender) smtp.mailfrom=neilb@suse.de; dmarc=pass (policy=none) header.from=suse.de X-HE-Tag: 1632260471-102787 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 Tue, 21 Sep 2021, Mel Gorman wrote: > On Tue, Sep 21, 2021 at 10:13:17AM +1000, NeilBrown wrote: > > On Mon, 20 Sep 2021, Mel Gorman wrote: > > > -long wait_iff_congested(int sync, long timeout) > > > -{ > > > - long ret; > > > - unsigned long start =3D jiffies; > > > - DEFINE_WAIT(wait); > > > - wait_queue_head_t *wqh =3D &congestion_wqh[sync]; > > > - > > > - /* > > > - * If there is no congestion, yield if necessary instead > > > - * of sleeping on the congestion queue > > > - */ > > > - if (atomic_read(&nr_wb_congested[sync]) =3D=3D 0) { > > > - cond_resched(); > > > - > > > - /* In case we scheduled, work out time remaining */ > > > - ret =3D timeout - (jiffies - start); > > > - if (ret < 0) > > > - ret =3D 0; > > > - > > > - goto out; > > > - } > > > - > > > - /* Sleep until uncongested or a write happens */ > > > - prepare_to_wait(wqh, &wait, TASK_UNINTERRUPTIBLE); > >=20 > > Uninterruptible wait. > >=20 > > .... > > > +static void > > > +reclaim_throttle(pg_data_t *pgdat, enum vmscan_throttle_state reason, > > > + long timeout) > > > +{ > > > + wait_queue_head_t *wqh =3D &pgdat->reclaim_wait; > > > + unsigned long start =3D jiffies; > > > + long ret; > > > + DEFINE_WAIT(wait); > > > + > > > + atomic_inc(&pgdat->nr_reclaim_throttled); > > > + WRITE_ONCE(pgdat->nr_reclaim_start, > > > + node_page_state(pgdat, NR_THROTTLED_WRITTEN)); > > > + > > > + prepare_to_wait(wqh, &wait, TASK_INTERRUPTIBLE); > >=20 > > Interruptible wait. > >=20 > > Why the change? I think these waits really need to be TASK_UNINTERRUPTIB= LE. > >=20 >=20 > Because from mm/ context, I saw no reason why the task *should* be > uninterruptible. It's waiting on other tasks to complete IO and it is not > protecting device state, filesystem state or anything else. If it gets > a signal, it's safe to wake up, particularly if that signal is KILL and > the context is a direct reclaimer. I disagree. An Interruptible sleep only makes sense if the "was interrupted" status can propagate up to user-space (or to some in-kernel handler that will clear the signal). In particular, if reclaim_throttle() is called in a loop (which it is), and if that loop doesn't check for signal_pending (which it doesn't), then the next time around the loop after receiving a signal, it won't sleep at all. That would be bad. In general, if you don't return an error, then you probably shouldn't sleep Interruptible. I notice that tasks sleep on kswapd_wait as TASK_INTERRUPTIBLE, but they don't have any signal handling. I suspect this isn't actually a defect because I suspect that is it not even possible to SIGKILL kswapd. But the code seems misleading. I guess I should write a patch. Unless reclaim knows to abort completely on a signal (__GFP_KILLABLE ???) this must be an UNINTERRUPTIBLE wait. Thanks, NeilBrown >=20 > The original TASK_UNINTERRUPTIBLE is almost certainly a copy&paste from > congestion_wait which may be called because a filesystem operation must > complete before it can return to userspace so a signal waking it up is > pointless. >=20 > --=20 > Mel Gorman > SUSE Labs >=20 >=20