Message ID | 20230306163425.8324-8-jgross@suse.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:5915:0:0:0:0:0 with SMTP id v21csp1949828wrd; Mon, 6 Mar 2023 08:53:24 -0800 (PST) X-Google-Smtp-Source: AK7set9sjBKgSTO6SSl6WVW3UUA0YlaJzcGl0vCdyUW7DGLrcvb5xJTtJxRbWscBh3E5sBOPF8Cy X-Received: by 2002:a17:903:22c5:b0:19d:1686:989 with SMTP id y5-20020a17090322c500b0019d16860989mr14583070plg.59.1678121604042; Mon, 06 Mar 2023 08:53:24 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1678121604; cv=none; d=google.com; s=arc-20160816; b=TdRzbjdNdLvzDIkWTGFORrLd744ZQ8+Zp8dHxJ4ZqW9AagTA+KYimCr6pQwr2xFDLL pr5ZIcgw4hq8nnT+0ooG44FsjzrEvEff2pUiYSEGWdFIFaWHHu2NgUI3iM/tAh0wZ5V2 LIiw93MRMzLTh6MWk/2OKaeBHbnSMldnmw0FudYIjw4O4RJx/Bs/UKRJhpjAOHCOWFp8 I18MPJvO1x0C+LtafoI0wa8go9Z0amm1YRsvkJoCoBOfzh8GNzbGd2LVdzG+eeI7hV1i 5HgYXjIxO0lwAlCmgD3MMKoNoC8r1FRsCnw3/qJMoGo3sLqDvA/I1HXOf7pwBeaHZvkp l4vw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=YeH0hO+srtzbwE8WX1MqylWAbFnZLLMVUNqDdf20xnc=; b=zsYzvGtdHTLZd66rxW4e4VpNPN+BaxiI9nyAncsVpF3u1dl25kO1l1rkeBxEHl3Jta YTjyonjptiq+UrcofxJV4PCesLN9qZpwaQn50r7W/zU6xAAFFKgeTyQFLnjOK1/ooMmH yVAJMZb/LBh5jx0mNCCnsWXG2dDlQOg7l8oCBQJIO5t/9WTfsZZI3TZOObKHxgkJvTQq bytG80HmK2QdhVnC2FpM+a1tv3CVHJtxm7VVq4B9h+8rrrOr9ckVDdzXxWmw6dddVZ+8 R1aXBiiUlntryAvI4Nf3EwOVK2jwmPoxEBgmnjF+4TTfXAe4rTiyWnz+EnlWjRNzhD6A PcEA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@suse.com header.s=susede1 header.b=uMStH4Od; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=suse.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id z18-20020a1709028f9200b0019caa0b9701si9241302plo.170.2023.03.06.08.53.11; Mon, 06 Mar 2023 08:53:24 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@suse.com header.s=susede1 header.b=uMStH4Od; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=suse.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230154AbjCFQgs (ORCPT <rfc822;toshivichauhan@gmail.com> + 99 others); Mon, 6 Mar 2023 11:36:48 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48054 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229973AbjCFQfy (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 6 Mar 2023 11:35:54 -0500 Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7FB203E607 for <linux-kernel@vger.kernel.org>; Mon, 6 Mar 2023 08:35:20 -0800 (PST) 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-out1.suse.de (Postfix) with ESMTPS id 6E91F21E61; Mon, 6 Mar 2023 16:35:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1678120507; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=YeH0hO+srtzbwE8WX1MqylWAbFnZLLMVUNqDdf20xnc=; b=uMStH4OdzkDbYwZMmUuDbr+2yIPidh6L/M/CHXQI8mLkK3ohNycm7fb71xF7NZgzu06/lr XdeX6fnkZ5mQoaAOAVedcDzlLkngNvTY2X9XFSu1x/Qq7aeaAt7dqHqeYo/5+QfdQuXUvG 8Z5tU5HOGhxcPB3RKtCItgph/nWOzGA= 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 334DB13A66; Mon, 6 Mar 2023 16:35:07 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id +SczCzsWBmR3UwAAMHmgww (envelope-from <jgross@suse.com>); Mon, 06 Mar 2023 16:35:07 +0000 From: Juergen Gross <jgross@suse.com> To: linux-kernel@vger.kernel.org, x86@kernel.org Cc: Juergen Gross <jgross@suse.com>, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, Dave Hansen <dave.hansen@linux.intel.com>, "H. Peter Anvin" <hpa@zytor.com> Subject: [PATCH v4 07/12] x86/mtrr: allocate mtrr_value array dynamically Date: Mon, 6 Mar 2023 17:34:20 +0100 Message-Id: <20230306163425.8324-8-jgross@suse.com> X-Mailer: git-send-email 2.35.3 In-Reply-To: <20230306163425.8324-1-jgross@suse.com> References: <20230306163425.8324-1-jgross@suse.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE, SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1759638038656601353?= X-GMAIL-MSGID: =?utf-8?q?1759638038656601353?= |
Series |
x86/mtrr: fix handling with PAT but without MTRR
|
|
Commit Message
Juergen Gross
March 6, 2023, 4:34 p.m. UTC
The mtrr_value[] array is a static variable, which is used only in a
few configurations. Consuming 6kB is ridiculous for this case,
especially as the array doesn't need to be that large and it can easily
be allocated dynamically.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
arch/x86/kernel/cpu/mtrr/mtrr.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Comments
On Mon, 2023-03-06 at 17:34 +0100, Juergen Gross wrote: > The mtrr_value[] array is a static variable, which is used only in a > few configurations. Consuming 6kB is ridiculous for this case, > especially as the array doesn't need to be that large and it can easily > be allocated dynamically. > > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > arch/x86/kernel/cpu/mtrr/mtrr.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.c b/arch/x86/kernel/cpu/mtrr/mtrr.c > index 0c83990501f5..50cd2287b6e1 100644 > --- a/arch/x86/kernel/cpu/mtrr/mtrr.c > +++ b/arch/x86/kernel/cpu/mtrr/mtrr.c > @@ -581,7 +581,7 @@ struct mtrr_value { > unsigned long lsize; > }; > > -static struct mtrr_value mtrr_value[MTRR_MAX_VAR_RANGES]; > +static struct mtrr_value *mtrr_value; > > static int mtrr_save(void) > { > @@ -750,6 +750,7 @@ static int __init mtrr_init_finialize(void) > * TBD: is there any system with such CPU which supports > * suspend/resume? If no, we should remove the code. > */ > + mtrr_value = kcalloc(num_var_ranges, sizeof(*mtrr_value), GFP_KERNEL); Theoretically dynamic allocation can fail, although it should not happen as this happens during kernel boot and the size is small. Maybe a WARN()? > register_syscore_ops(&mtrr_syscore_ops); > > return 0;
On 20.03.23 13:25, Huang, Kai wrote: > On Mon, 2023-03-06 at 17:34 +0100, Juergen Gross wrote: >> The mtrr_value[] array is a static variable, which is used only in a >> few configurations. Consuming 6kB is ridiculous for this case, >> especially as the array doesn't need to be that large and it can easily >> be allocated dynamically. >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> arch/x86/kernel/cpu/mtrr/mtrr.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.c b/arch/x86/kernel/cpu/mtrr/mtrr.c >> index 0c83990501f5..50cd2287b6e1 100644 >> --- a/arch/x86/kernel/cpu/mtrr/mtrr.c >> +++ b/arch/x86/kernel/cpu/mtrr/mtrr.c >> @@ -581,7 +581,7 @@ struct mtrr_value { >> unsigned long lsize; >> }; >> >> -static struct mtrr_value mtrr_value[MTRR_MAX_VAR_RANGES]; >> +static struct mtrr_value *mtrr_value; >> >> static int mtrr_save(void) >> { >> @@ -750,6 +750,7 @@ static int __init mtrr_init_finialize(void) >> * TBD: is there any system with such CPU which supports >> * suspend/resume? If no, we should remove the code. >> */ >> + mtrr_value = kcalloc(num_var_ranges, sizeof(*mtrr_value), GFP_KERNEL); > > Theoretically dynamic allocation can fail, although it should not happen as this > happens during kernel boot and the size is small. Maybe a WARN()? Fine with me. Juergen
On 3/20/23 06:49, Juergen Gross wrote: >>> >>> @@ -750,6 +750,7 @@ static int __init mtrr_init_finialize(void) >>> * TBD: is there any system with such CPU which supports >>> * suspend/resume? If no, we should remove the code. >>> */ >>> + mtrr_value = kcalloc(num_var_ranges, sizeof(*mtrr_value), >>> GFP_KERNEL); >> >> Theoretically dynamic allocation can fail, although it should not >> happen as this >> happens during kernel boot and the size is small. Maybe a WARN()? > > Fine with me. What *actually* happens if the system is running out of memory and this is the _first_ failure? Does a WARN_ON() here help someone debug what is going on?
On 20.03.23 16:31, Dave Hansen wrote: > On 3/20/23 06:49, Juergen Gross wrote: >>>> >>>> @@ -750,6 +750,7 @@ static int __init mtrr_init_finialize(void) >>>> * TBD: is there any system with such CPU which supports >>>> * suspend/resume? If no, we should remove the code. >>>> */ >>>> + mtrr_value = kcalloc(num_var_ranges, sizeof(*mtrr_value), >>>> GFP_KERNEL); >>> >>> Theoretically dynamic allocation can fail, although it should not >>> happen as this >>> happens during kernel boot and the size is small. Maybe a WARN()? >> >> Fine with me. > > What *actually* happens if the system is running out of memory and this > is the _first_ failure? Does a WARN_ON() here help someone debug what > is going on? Good question. Especially as we don't set __GFP_NOWARN here. Juergen
On Mon, Mar 06, 2023 at 05:34:20PM +0100, Juergen Gross wrote: > The mtrr_value[] array is a static variable, which is used only in a > few configurations. Consuming 6kB is ridiculous for this case, Ah, that struct mtrr_value is of size 24 due to that first member mtrr_type getting padded even if it is a u8. > especially as the array doesn't need to be that large and it can easily > be allocated dynamically. > > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > arch/x86/kernel/cpu/mtrr/mtrr.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.c b/arch/x86/kernel/cpu/mtrr/mtrr.c > index 0c83990501f5..50cd2287b6e1 100644 > --- a/arch/x86/kernel/cpu/mtrr/mtrr.c > +++ b/arch/x86/kernel/cpu/mtrr/mtrr.c > @@ -581,7 +581,7 @@ struct mtrr_value { > unsigned long lsize; > }; > > -static struct mtrr_value mtrr_value[MTRR_MAX_VAR_RANGES]; > +static struct mtrr_value *mtrr_value; > > static int mtrr_save(void) > { > @@ -750,6 +750,7 @@ static int __init mtrr_init_finialize(void) > * TBD: is there any system with such CPU which supports > * suspend/resume? If no, we should remove the code. > */ > + mtrr_value = kcalloc(num_var_ranges, sizeof(*mtrr_value), GFP_KERNEL); Pls put that over the comment. Also, you need to handle kcalloc() returning an error.
On 27.03.23 00:05, Borislav Petkov wrote: > On Mon, Mar 06, 2023 at 05:34:20PM +0100, Juergen Gross wrote: >> The mtrr_value[] array is a static variable, which is used only in a >> few configurations. Consuming 6kB is ridiculous for this case, > > Ah, that struct mtrr_value is of size 24 due to that first member > mtrr_type getting padded even if it is a u8. > >> especially as the array doesn't need to be that large and it can easily >> be allocated dynamically. >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> arch/x86/kernel/cpu/mtrr/mtrr.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.c b/arch/x86/kernel/cpu/mtrr/mtrr.c >> index 0c83990501f5..50cd2287b6e1 100644 >> --- a/arch/x86/kernel/cpu/mtrr/mtrr.c >> +++ b/arch/x86/kernel/cpu/mtrr/mtrr.c >> @@ -581,7 +581,7 @@ struct mtrr_value { >> unsigned long lsize; >> }; >> >> -static struct mtrr_value mtrr_value[MTRR_MAX_VAR_RANGES]; >> +static struct mtrr_value *mtrr_value; >> >> static int mtrr_save(void) >> { >> @@ -750,6 +750,7 @@ static int __init mtrr_init_finialize(void) >> * TBD: is there any system with such CPU which supports >> * suspend/resume? If no, we should remove the code. >> */ >> + mtrr_value = kcalloc(num_var_ranges, sizeof(*mtrr_value), GFP_KERNEL); > > Pls put that over the comment. > > Also, you need to handle kcalloc() returning an error. Okay. Juergen
diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.c b/arch/x86/kernel/cpu/mtrr/mtrr.c index 0c83990501f5..50cd2287b6e1 100644 --- a/arch/x86/kernel/cpu/mtrr/mtrr.c +++ b/arch/x86/kernel/cpu/mtrr/mtrr.c @@ -581,7 +581,7 @@ struct mtrr_value { unsigned long lsize; }; -static struct mtrr_value mtrr_value[MTRR_MAX_VAR_RANGES]; +static struct mtrr_value *mtrr_value; static int mtrr_save(void) { @@ -750,6 +750,7 @@ static int __init mtrr_init_finialize(void) * TBD: is there any system with such CPU which supports * suspend/resume? If no, we should remove the code. */ + mtrr_value = kcalloc(num_var_ranges, sizeof(*mtrr_value), GFP_KERNEL); register_syscore_ops(&mtrr_syscore_ops); return 0;