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=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 7754CC433EF for ; Wed, 15 Sep 2021 11:56:41 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 17E6C61214 for ; Wed, 15 Sep 2021 11:56:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 17E6C61214 Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 9BC6B900002; Wed, 15 Sep 2021 07:56:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 96AC66B0072; Wed, 15 Sep 2021 07:56:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8322B900002; Wed, 15 Sep 2021 07:56:40 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0094.hostedemail.com [216.40.44.94]) by kanga.kvack.org (Postfix) with ESMTP id 6E3586B0071 for ; Wed, 15 Sep 2021 07:56:40 -0400 (EDT) Received: from smtpin11.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 3579A3A8DF for ; Wed, 15 Sep 2021 11:56:40 +0000 (UTC) X-FDA: 78589655760.11.CF47EF2 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by imf13.hostedemail.com (Postfix) with ESMTP id DFF8D10305ED for ; Wed, 15 Sep 2021 11:56:39 +0000 (UTC) Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id C25912219B; Wed, 15 Sep 2021 11:56:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1631706998; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=ljaWHC1T8Skn0Rf1cGPURwlxvxHk7GU37PQK7AHo5Ak=; b=iVg5vqyA8HnewlU0YSSvcXOSiOTRI257ABgLY0+39SFVopH5uL8Kus61aAmyvhnfAnO2f9 kDJyyAOBJhNHYpLeyvousEo9JTKZFJp6PQtHk8yDj7Ni4c9lHSz7iL2lqJSm6eQ5shG8Y0 whOq9QkH7iAj1o7t9a7tF+X1Fz+UlGU= Received: from suse.cz (unknown [10.100.201.86]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 85764A3B8F; Wed, 15 Sep 2021 11:56:38 +0000 (UTC) Date: Wed, 15 Sep 2021 13:56:38 +0200 From: Michal Hocko To: NeilBrown Cc: Andrew Morton , Theodore Ts'o , Andreas Dilger , "Darrick J. Wong" , Matthew Wilcox , Mel Gorman , linux-xfs@vger.kernel.org, linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/6] MM: annotate congestion_wait() and wait_iff_congested() as ineffective. Message-ID: References: <163157808321.13293.486682642188075090.stgit@noble.brown> <163157838437.13293.15392955714346973750.stgit@noble.brown> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <163157838437.13293.15392955714346973750.stgit@noble.brown> Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=iVg5vqyA; spf=pass (imf13.hostedemail.com: domain of mhocko@suse.com designates 195.135.220.28 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com X-Stat-Signature: ci3dj9qegt4ihog5iz4gqfwqejaxp9y5 X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: DFF8D10305ED X-HE-Tag: 1631706999-78723 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 14-09-21 10:13:04, Neil Brown wrote: > Only 4 subsystems call set_bdi_congested() or clear_bdi_congested(): > block/pktcdvd, fs/ceph fs/fuse fs/nfs > > It may make sense to use congestion_wait() or wait_iff_congested() > within these subsystems, but they have no value outside of these. > > Add documentation comments to these functions to discourage further use. This is an unfortunate state. The MM layer still relies on the API. While adding a documentation to clarify the current status can stop more usage I am wondering what is a real alternative. My experience tells me that a lack of real alternative will lead to new creative ways of doing things instead. > Signed-off-by: NeilBrown > --- > include/linux/backing-dev.h | 7 +++++++ > mm/backing-dev.c | 9 +++++++++ > 2 files changed, 16 insertions(+) > > diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h > index ac7f231b8825..cc9513840351 100644 > --- a/include/linux/backing-dev.h > +++ b/include/linux/backing-dev.h > @@ -153,6 +153,13 @@ static inline int wb_congested(struct bdi_writeback *wb, int cong_bits) > return wb->congested & cong_bits; > } > > +/* NOTE congestion_wait() and wait_iff_congested() are > + * largely useless except as documentation. > + * congestion_wait() will (almost) always wait for the given timeout. > + * wait_iff_congested() will (almost) never wait, but will call > + * cond_resched(). > + * Were possible an alternative waiting strategy should be found. > + */ > long congestion_wait(int sync, long timeout); > long wait_iff_congested(int sync, long timeout); > > diff --git a/mm/backing-dev.c b/mm/backing-dev.c > index 4a9d4e27d0d9..53472ab38796 100644 > --- a/mm/backing-dev.c > +++ b/mm/backing-dev.c > @@ -1023,6 +1023,11 @@ EXPORT_SYMBOL(set_bdi_congested); > * Waits for up to @timeout jiffies for a backing_dev (any backing_dev) to exit > * write congestion. If no backing_devs are congested then just wait for the > * next write to be completed. > + * > + * NOTE: in the current implementation, hardly any backing_devs are ever > + * marked as congested, and write-completion is rarely reported (see calls > + * to clear_bdi_congested). So this should not be assumed to ever wake before > + * the timeout. > */ > long congestion_wait(int sync, long timeout) > { > @@ -1054,6 +1059,10 @@ EXPORT_SYMBOL(congestion_wait); > * The return value is 0 if the sleep is for the full timeout. Otherwise, > * it is the number of jiffies that were still remaining when the function > * returned. return_value == timeout implies the function did not sleep. > + * > + * NOTE: in the current implementation, hardly any backing_devs are ever > + * marked as congested, and write-completion is rarely reported (see calls > + * to clear_bdi_congested). So this should not be assumed to sleep at all. > */ > long wait_iff_congested(int sync, long timeout) > { > -- Michal Hocko SUSE Labs