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 28232C4345F for ; Thu, 18 Apr 2024 11:04:41 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 984976B009E; Thu, 18 Apr 2024 07:04:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 90C7A6B009F; Thu, 18 Apr 2024 07:04:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 75EEE6B00A0; Thu, 18 Apr 2024 07:04:40 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 5448E6B009E for ; Thu, 18 Apr 2024 07:04:40 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id EDFFDA15E3 for ; Thu, 18 Apr 2024 11:04:39 +0000 (UTC) X-FDA: 82022369478.29.1C1D9AB Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.223.130]) by imf15.hostedemail.com (Postfix) with ESMTP id 78CC2A0027 for ; Thu, 18 Apr 2024 11:04:37 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=iwuquuFV; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=5agX036W; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=XI4mK2VA; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=lRh68gjo; dmarc=none; spf=pass (imf15.hostedemail.com: domain of jack@suse.cz designates 195.135.223.130 as permitted sender) smtp.mailfrom=jack@suse.cz ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1713438278; 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=gv1S5z4U/Lpggcm746FJVB5sgVR9PpMNoPlpi/9ROxQ=; b=5VH8rmXbHVVWJ0okEsRcr25jF3w1in4sCp33gIPcOZG5vNT7nslMT0pBOirdG3H7SoV0Fg nt3DaQ38G9paN9UqYg/q0QmBGknIPA8Q380LR8eWFFNE7zC+1/aRvRIRqKRElXa2ofzqPG PRnbY07A75GaBVOwJFdxV8HUqU1sHoo= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=iwuquuFV; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=5agX036W; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=XI4mK2VA; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=lRh68gjo; dmarc=none; spf=pass (imf15.hostedemail.com: domain of jack@suse.cz designates 195.135.223.130 as permitted sender) smtp.mailfrom=jack@suse.cz ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1713438278; a=rsa-sha256; cv=none; b=A9snRIT/bGgv0Hz9K5zUsCdK01Z955IG+0zIK2ugcRuAbHshs+2ey5ITbyKFBWqGbXLq52 4d7S1i4GyWWdeX5fUhnRyVVUYNCM4O8WVWsVPCgWe29dXkoERZXU6/nANwjFD4TRWxLI2f Ikqy/uoN1BKoxIRg2JnAxiQ+KNzU3Ys= Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [IPv6:2a07:de40:b281:104:10:150:64:97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id B234C34DD5; Thu, 18 Apr 2024 11:04:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1713438275; 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=gv1S5z4U/Lpggcm746FJVB5sgVR9PpMNoPlpi/9ROxQ=; b=iwuquuFVSRj3wbZCLHn+iERrfqZ94jpQwcRoAjkTgPgCjZ9T5yHoslKOU7yjTp2Qn7GhQI 4bp/2Jm/BhOOW45u/3kTokz1zh3D9BRjs2v5BXX0U6hohw/BdwtSc4lvfUA5lsrufbci3b XSEcEqETcdua1ZbS1B7L450I6w+Rcnc= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1713438275; 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=gv1S5z4U/Lpggcm746FJVB5sgVR9PpMNoPlpi/9ROxQ=; b=5agX036W69nvNXNH3kJXYCpizMe4x2w1YlQxcUWUiKddCrUfqw1yXh3VJB7M6C0kkf0SzM 8aXAirlqboDm7SCg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1713438274; 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=gv1S5z4U/Lpggcm746FJVB5sgVR9PpMNoPlpi/9ROxQ=; b=XI4mK2VAsewf9Mau4YEWo81qOrnE2L6X7K447OkXOVwrenGAseIJdpA75NAw3F06xbZnV2 5oB1MfaA+dUoJIs2Ki0kWbes/57/3OEdsf29V7TVph11AOK07vbTL83ycbxzKMy/Mzv2di 5rBtIkL/Iq0VRQIDp11LMq2JREALHTw= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1713438274; 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=gv1S5z4U/Lpggcm746FJVB5sgVR9PpMNoPlpi/9ROxQ=; b=lRh68gjome/yMlDFGPIYA9g2odX3SgIGtGtf7dAvaJwfZUQxh0UVwOfZBK60hYYb/EcL07 PbLXKw4KbgMc1ECw== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id A60E91384C; Thu, 18 Apr 2024 11:04:34 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id LpV7KEL+IGYrWwAAD6G6ig (envelope-from ); Thu, 18 Apr 2024 11:04:34 +0000 Received: by quack3.suse.cz (Postfix, from userid 1000) id 4DEF6A0812; Thu, 18 Apr 2024 13:04:34 +0200 (CEST) Date: Thu, 18 Apr 2024 13:04:34 +0200 From: Jan Kara To: Zach O'Keefe Cc: Jan Kara , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Maxim Patlasov , stable@vger.kernel.org Subject: Re: [PATCH] mm/writeback: fix possible divide-by-zero in wb_dirty_limits(), again Message-ID: <20240418110434.g5bx5ntp2m4433eo@quack3> References: <20240118181954.1415197-1-zokeefe@google.com> <20240417111001.fa2eg5gp6t2wiwco@quack3> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Action: no action X-Rspam-User: X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: 78CC2A0027 X-Stat-Signature: nfbarryabbuck3qaruyiehach6tpsw6m X-HE-Tag: 1713438277-259198 X-HE-Meta: U2FsdGVkX19jeHH49HJxiOIK0SOTl32lKBk6rZi5EykaJCQ4ng9rBe/BBTaLBrpd/w/sxppCSKyuh7nD3w4x2zvKaW7HICmgtdm9zjATXhOMKrqWGWD1datahbag77pXBpNn2M4dymeb1fi53PeH4KQSHS1IUA3k+BZKtqxMPw1Uqj4kj17wg0+n/eej29QuYXYFmdIcEVY/wfz0JnIt5hCop/yp9HpnAwPxx+DjSz3nUHy2o+VCs6qibRS6VvdKu9wbDr3tvtqVIJOZ5HaRJi8Q2REjmS4bkOcDtXewcZJEPbtyvd/+8As0Op1s5sloQMsUz6enAfwNQcNARTh+cW2GSkHyI2cafk64csAYbh/QkF2FFv9C5WxAtLt6FR0oZCWEiRcZwMszfUaVPnkShuJFDj4mVay+jSBaANh3k10KwseXLwy5cnRMoGr8dy74Na6mbbuxoNhir7pCtKo+vuw0Kaj3MpUV1m1gydHelkbMtpFkTA1NLt6ScmNLLP3xaVOi0j+NXBXcXmRCk55RgbiUoZZjUqmpjeWFNMplWZRKO0L5W4ru/jH7i6gf1pT0YQQRJ4hN+wrGs2qVcVXVpd5FWo39kU6ZQyLWjzKkjxnJ2sAgmwnFfRvSJxCk+o5H/K8K7j/imNGEFMqIvD0vVzLkfRObGbveh9jHxeDBc9wC6HhbnlrF+D2rYINYKJ0F8RlvqmZk09Qgo6mtC9HC/aXFS/LZZPPb92ebmEB/iXVs82U5FzRKi5tvgpUdVRgfqG5gT5ElU2/Rlwu6HimXhXGOG/3jXksSVpawEvAdclC/HojrwpjzXfn7J7EgPqJQEgTytwUtJ6C6bHOoovoWxgiF18thQ5FmVR7suHagcpTMeUdtp3sr6b1J7IuAsSd/qkJ2jhkmnxusYGPJqOffSI4WvINyeRDyesj8fqSwj12NpjCD5GVymLruUn+wDo0s 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: List-Subscribe: List-Unsubscribe: On Wed 17-04-24 12:33:39, Zach O'Keefe wrote: > On Wed, Apr 17, 2024 at 4:10 AM Jan Kara wrote: > > > diff --git a/mm/page-writeback.c b/mm/page-writeback.c > > > index cd4e4ae77c40a..02147b61712bc 100644 > > > --- a/mm/page-writeback.c > > > +++ b/mm/page-writeback.c > > > @@ -1638,7 +1638,7 @@ static inline void wb_dirty_limits(struct dirty_throttle_control *dtc) > > > */ > > > dtc->wb_thresh = __wb_calc_thresh(dtc); > > > dtc->wb_bg_thresh = dtc->thresh ? > > > - div_u64((u64)dtc->wb_thresh * dtc->bg_thresh, dtc->thresh) : 0; > > > + div64_u64(dtc->wb_thresh * dtc->bg_thresh, dtc->thresh) : 0; ... > > Thirdly, if thresholds are larger than 1<<32 pages, then dirty balancing is > > going to blow up in many other spectacular ways - consider only the > > multiplication on this line - it will not necessarily fit into u64 anymore. > > The whole dirty limiting code is interspersed with assumptions that limits > > are actually within u32 and we do our calculations in unsigned longs to > > avoid worrying about overflows (with occasional typing to u64 to make it > > more interesting because people expected those entities to overflow 32 bits > > even on 32-bit archs). Which is lame I agree but so far people don't seem > > to be setting limits to 16TB or more. And I'm not really worried about > > security here since this is global-root-only tunable and that has much > > better ways to DoS the system. > > > > So overall I'm all for cleaning up this code but in a sensible way please. > > E.g. for these overflow issues at least do it one function at a time so > > that we can sensibly review it. > > > > Andrew, can you please revert this patch until we have a better fix? So far > > it does more harm than good... Thanks! > > Shall we just roll-forward with a suitable fix? I think all the > original code actually "needed" was to cast the ternary predicate, > like: > > ---8<--- > diff --git a/mm/page-writeback.c b/mm/page-writeback.c > index fba324e1a010..ca1bfc0c9bdd 100644 > --- a/mm/page-writeback.c > +++ b/mm/page-writeback.c > @@ -1637,8 +1637,8 @@ static inline void wb_dirty_limits(struct > dirty_throttle_control *dtc) > * at some rate <= (write_bw / 2) for bringing down wb_dirty. > */ > dtc->wb_thresh = __wb_calc_thresh(dtc); > - dtc->wb_bg_thresh = dtc->thresh ? > - div64_u64(dtc->wb_thresh * dtc->bg_thresh, dtc->thresh) : 0; > + dtc->wb_bg_thresh = (u32)dtc->thresh ? > + div_u64((u64)dtc->wb_thresh * dtc->bg_thresh, dtc->thresh) : 0; Well, this would fix the division by 0 but when you read the code you really start wondering what's going on :) And as I wrote above when thresholds pass UINT_MAX, the dirty limitting code breaks down anyway so I don't think the machine will be more usable after your fix. Would you be up for a challenge to modify mm/page-writeback.c so that such huge limits cannot be set instead? That would be actually a useful fix... Honza > > /* > * In order to avoid the stacked BDI deadlock we need > ---8<--- > > Thanks, and apologize for the inconvenience > > Zach > > > Honza > > -- > > Jan Kara > > SUSE Labs, CR > -- Jan Kara SUSE Labs, CR