From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4AB0AA034F; Fri, 7 May 2021 11:49:56 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 33C7D40140; Fri, 7 May 2021 11:49:56 +0200 (CEST) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mails.dpdk.org (Postfix) with ESMTP id D3E9340140 for ; Fri, 7 May 2021 11:49:54 +0200 (CEST) IronPort-SDR: tuODaWTJ0QzKVgbI5UPfxuuH9ZWpzYl10mJY7SCNEfQKzRYbNR3nKTn0WXMVyQub8gPCvtQ/Ue zjbQW10Vf73A== X-IronPort-AV: E=McAfee;i="6200,9189,9976"; a="186152952" X-IronPort-AV: E=Sophos;i="5.82,280,1613462400"; d="scan'208";a="186152952" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 May 2021 02:49:45 -0700 IronPort-SDR: DQkHOmXTg4vUP/nj4HJ5z6rG8XviDR+A2+gBDHepxdgPgT1xDCNMGnbP6eC4Ef6giauehAgv5q pJ0QfnaIRSaQ== X-IronPort-AV: E=Sophos;i="5.82,280,1613462400"; d="scan'208";a="434839161" Received: from aburakov-mobl.ger.corp.intel.com (HELO [10.213.245.37]) ([10.213.245.37]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 May 2021 02:49:44 -0700 To: Richael Zhuang , "dev@dpdk.org" Cc: "stephen@networkplumber.org" , "reshma.pattan@intel.com" , "david.hunt@intel.com" , nd References: <2fae7084c99dc8223e8d3a0aa9c2e5ba18bf1e6b.1619104102.git.anatoly.burakov@intel.com> <83b1e89d14834251d4d7e72fcc19d82dfb52686d.1619175790.git.anatoly.burakov@intel.com> From: "Burakov, Anatoly" Message-ID: Date: Fri, 7 May 2021 10:49:40 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [21.08 PATCH v4 2/2] power: refactor pstate and acpi code X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 07-May-21 3:13 AM, Richael Zhuang wrote: >> @@ -262,18 +189,16 @@ static int >> power_init_for_setting_freq(struct acpi_power_info *pi) { >> FILE *f; >> - char fullpath[PATH_MAX]; >> char buf[BUFSIZ]; >> uint32_t i, freq; >> - char *s; >> + int ret; >> >> - snprintf(fullpath, sizeof(fullpath), POWER_SYSFILE_SETSPEED, >> - pi->lcore_id); >> - f = fopen(fullpath, "rw+"); >> - FOPEN_OR_ERR_RET(f, -1); >> + open_core_sysfs_file(POWER_SYSFILE_SETSPEED, pi->lcore_id, "r", >> + &f); > Hi Anatoly, > I tried to verify your patch on my arm platform, and I found several bugs. > > Here it should be "rw+", for it will write freq to POWER_SYSFILE_SETSPEED later. > > Best Regards, > Richael >> return 1; >> } >> + >> +int >> +open_core_sysfs_file(const char *template, unsigned int core, const char >> *mode, >> + FILE **f) >> +{ >> + char fullpath[PATH_MAX]; >> + FILE *tmpf; >> + >> + /* silenced -Wformat-nonliteral here */ >> + snprintf(fullpath, sizeof(fullpath), template, core); >> + tmpf = fopen(fullpath, mode); >> + if (tmpf == NULL) >> + return -1; >> + *f = tmpf; > > When the file that open_core_sysfs_file() opens doesn't exist, there's segmentation fault. > Moving *f=tmpf above runs OK. > >> + >> + return 0; >> +} >> + >> +int >> +read_core_sysfs_u32(FILE *f, uint32_t *val) { >> + char buf[BUFSIZ]; >> + uint32_t fval; >> + char *s; >> + >> + s = fgets(buf, sizeof(buf), f); >> + if (s == NULL) >> + return -1; >> + >> + /* strip off any terminating newlines */ >> + *strchrnul(buf, '\n') = '\0'; >> + >> + return 0; >> +} >> + >> +int >> +write_core_sysfs_s(FILE *f, const char *str) { >> + int ret; >> + >> + ret = fseek(f, 0, SEEK_SET); >> + if (ret != 0) >> + return -1; >> + >> + ret = fputs(str, f); >> + if (ret != 0) > > ret >=0 if success, EOF means failure. > Hi Richael, Thank you very much for testing! I'll address these issues in the next revision, and double-check everything else. -- Thanks, Anatoly