Received: by 2002:a05:6358:3188:b0:123:57c1:9b43 with SMTP id q8csp21428191rwd; Thu, 29 Jun 2023 16:17:16 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ5RBBoTvGxuqUsHNnMuiO2fKLmNE9u/ZQre6Ji8lJoiUK1m54DbXMYX8/6KsPllqrgl6x4s X-Received: by 2002:a05:6808:1895:b0:3a1:e796:d961 with SMTP id bi21-20020a056808189500b003a1e796d961mr1066403oib.2.1688080636273; Thu, 29 Jun 2023 16:17:16 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1688080636; cv=none; d=google.com; s=arc-20160816; b=WMmbwlfbp82Y6U4SNSZYoeIprx7DCBH/Sl+sjQdRulnQYtwk1x+1dU6qgdsXWWDoJg tlSN33CK9IYsR6heSX7AVqS9yU5fHruRTb89tHIyXaEXzY5+4jR3MUhYg2MvwzA5zaOO IZEHnIIaW3Kyp2x0t3b9ggedqqvoTuiLPRztrV4IpaFiHsef5bTg69O0ZuCicfZOC82y EdcHT/BVohuS53s29XUZ1iOiT7DiSUBh6udYilyiSjwv20nKgO5glh+TImCfbu863kyU H3Hl5dP5tmM/W9izCfbEyCb+Ae/60wgUSHjaaazQSt1oATalnuq0bKiqlF8WOfFQktb5 /uiw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:message-id:date:references :in-reply-to:subject:cc:to:dkim-signature:dkim-signature:from; bh=O6IsdFQkGO4fQ1fpvYXehWTkF4b4HmYIG/7dgnWgBZw=; fh=zT3wkfbbqyf90pfxpVbzShvkZykBi2NZGBPZ4HjhFXg=; b=EKYt7/RBeliK44VADLlP9tC/oHgHoC2Vnns4iLlLysrdd8w87inswbQ7KjWKxtGoCv k3uysozu5xmPVfThzesiNRWxI8v9cmqv9DXlq9dq0g4v6EQLNjaqVWUo0SBN3LMgKvc5 ZF1nd1Y9wddMKLZvkmzdOSUi7JKOju0SSu0F02Kk/jk4F4n2rzoJsW4GfEM8KZHVrRH5 pf//QSomrzZi0CFQQkvBNP04icAvkVPIuVdmym9CB1sUzc5qhA5blIcRPDGFtQKqwbbg em93PULzrKhnIkLBAQ2TYQWdecpLnVyfR2OmsB1NZQXCzFPsFAaqz1unzNB84APJkvzt R0mA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=CuHXJE4S; dkim=neutral (no key) header.i=@linutronix.de header.b=nKcM2l0f; 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=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id rm10-20020a17090b3eca00b0025bf6078049si14907571pjb.5.2023.06.29.16.17.01; Thu, 29 Jun 2023 16:17:16 -0700 (PDT) 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=@linutronix.de header.s=2020 header.b=CuHXJE4S; dkim=neutral (no key) header.i=@linutronix.de header.b=nKcM2l0f; 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=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230054AbjF2XD3 (ORCPT + 99 others); Thu, 29 Jun 2023 19:03:29 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55218 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229459AbjF2XD0 (ORCPT ); Thu, 29 Jun 2023 19:03:26 -0400 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 23EBA2D7F; Thu, 29 Jun 2023 16:03:25 -0700 (PDT) From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1688079803; h=from:from:reply-to:subject:subject: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=O6IsdFQkGO4fQ1fpvYXehWTkF4b4HmYIG/7dgnWgBZw=; b=CuHXJE4SvGDkEv0Ux9OA5qSwGbAV6ZDonazEYk+atu/9rvoih8fSuN3KPAJ4xteO5IIhQo Sxj3STAd2Pqs+ZsNWvOCx+GGMEhpAmQNaRGlH3/QiVZDRHzpn1y9rfvErz2y3cgve9PqX/ w3dqe0DgZ/Jksi9uLE69wgTU4Op/kBfcghiGJILFVBnf0ozrv6DxVVcrnpP3OJZ24gTnVh XtTmwdukYi4N4YE7XXrqQUxTO44TU/4/lttke9Vp9YyYhaZOMN5RsZFjpC9XCOXxjl9VeD ZmDuW6PX8TUsOfyFDmK4nh1VzbZuWFdpew9383c6IthwuJ9UKAX+2OhX/UvtdA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1688079803; h=from:from:reply-to:subject:subject: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=O6IsdFQkGO4fQ1fpvYXehWTkF4b4HmYIG/7dgnWgBZw=; b=nKcM2l0fZyg5ZZk1W+AEFfYVQfjD1ccR8J0/3No5lQ/xIYZCUvF6EOIfK5oaSKu+ofljdB dc5E/APS0QJtJSBA== To: lizhe.67@bytedance.com, tony.luck@intel.com, bp@alien8.de, mingo@redhat.com, dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com, rafael@kernel.org, viresh.kumar@linaro.org Cc: linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, lizefan.x@bytedance.com, yuanzhu@bytedance.com, lizhe.67@bytedance.com Subject: Re: [RFC] msr: judge the return val of function rdmsrl_on_cpu() by WARN_ON In-Reply-To: <20230629072754.39844-1-lizhe.67@bytedance.com> References: <20230629072754.39844-1-lizhe.67@bytedance.com> Date: Fri, 30 Jun 2023 01:03:22 +0200 Message-ID: <87r0ptzxad.ffs@tglx> MIME-Version: 1.0 Content-Type: text/plain 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,T_SCC_BODY_TEXT_LINE 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: X-Mailing-List: linux-kernel@vger.kernel.org Li! On Thu, Jun 29 2023 at 15:27, lizhe.67@bytedance.com wrote: > There are ten places call rdmsrl_on_cpu() in the current code without > judging the return value. This may introduce a potential bug. For example, > inj_bank_set() may return -EINVAL, show_base_frequency() may show an error > freq value, intel_pstate_hwp_set() may write an error value to the related > msr register and so on. But rdmsrl_on_cpu() do rarely returns an error, so > it seems that add a WARN_ON is enough for debugging. Can you please structure your changelogs as documented in: https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#changelog instead of providing a big lump of words? > There are ten places call rdmsrl_on_cpu() in the current code without > judging the return value. Return values are not judged. They are either ignored or checked/evaluated. > This may introduce a potential bug. Sure. Anything which does not check a return value from a function might be a bug, but you have to look at each instance whether its a bug or not. > For example, inj_bank_set() may return -EINVAL, show_base_frequency() > may show an error freq value, intel_pstate_hwp_set() may write an > error value to the related msr register and so on. But > rdmsrl_on_cpu() do rarely returns an error, so it seems that add a > WARN_ON is enough for debugging. This is hillarious at best. 1) It does not matter at all whether that function returns an error rarely or not. 2) Adding WARN_ON() without justification at each call site is not enough. Neither for debugging nor for real world usage. You have to come up with individual patches for each callsite to add the WARN_ON() and in each patch you have to explain why it is justified and why there is no other solution, e.g. taking an error exit path. Just slapping WARN_ON()'s into the code without any deeper analysis is worse than the current state of the code. If you have identified a real world problem at any of these call sites then adding a WARN_ON() does not solve it at all. I'm looking forward to your profound anlysis of each of these "problems". Thanks, tglx