DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
To: Richael Zhuang <Richael.Zhuang@arm.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "stephen@networkplumber.org" <stephen@networkplumber.org>,
	"reshma.pattan@intel.com" <reshma.pattan@intel.com>,
	"david.hunt@intel.com" <david.hunt@intel.com>, nd <nd@arm.com>
Subject: Re: [dpdk-dev] [21.08 PATCH v4 2/2] power: refactor pstate and acpi code
Date: Fri, 7 May 2021 10:49:40 +0100	[thread overview]
Message-ID: <d8f176a5-1e6a-aa61-406f-6221659da0d1@intel.com> (raw)
In-Reply-To: <AM8PR08MB5796E0FB8FB2D995966913B292579@AM8PR08MB5796.eurprd08.prod.outlook.com>

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

<snip>

>>   	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;
>> +

<snip>

>> +	/* 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

      reply	other threads:[~2021-05-07  9:49 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-01 15:05 [dpdk-dev] [PATCH 21.08] power: refactor pstate sysfs handling Anatoly Burakov
2021-04-02  9:27 ` [dpdk-dev] [21.08 v2] " Anatoly Burakov
2021-04-02 17:45 ` [dpdk-dev] [PATCH 21.08] " Stephen Hemminger
2021-04-06 10:06   ` Burakov, Anatoly
2021-04-02 17:46 ` Stephen Hemminger
2021-04-06 10:05   ` Burakov, Anatoly
2021-04-22 15:08     ` [dpdk-dev] [21.08 PATCH v3 1/1] " Anatoly Burakov
2021-04-23 11:03       ` [dpdk-dev] [21.08 PATCH v4 1/2] power: don't use rte prefix in internal code Anatoly Burakov
2021-05-31 10:23         ` David Hunt
2021-06-22 12:43         ` [dpdk-dev] [PATCH v2 " David Hunt
2021-06-22 12:43           ` [dpdk-dev] [PATCH v2 2/2] power: refactor pstate and acpi code David Hunt
2021-06-22 13:00             ` David Hunt
2021-06-22 12:59           ` [dpdk-dev] [PATCH v2 1/2] power: don't use rte prefix in internal code David Hunt
2021-06-22 12:58         ` [dpdk-dev] [PATCH v5 " David Hunt
2021-06-22 12:58           ` [dpdk-dev] [PATCH v5 2/2] power: refactor pstate and acpi code David Hunt
2021-06-22 13:27             ` David Hunt
2021-06-23  8:54               ` Richael Zhuang
2021-06-23  9:00                 ` David Hunt
2021-06-23 12:03           ` [dpdk-dev] [PATCH v6 1/2] power: don't use rte prefix in internal code David Hunt
2021-06-23 12:03             ` [dpdk-dev] [PATCH v6 2/2] power: refactor pstate and acpi code David Hunt
2021-06-23 12:27               ` Burakov, Anatoly
2021-06-30  5:32               ` Richael Zhuang
2021-07-08 12:49               ` David Marchand
2021-07-08 13:33                 ` David Hunt
2021-07-08 15:38             ` [dpdk-dev] [PATCH v7 1/2] power: don't use rte prefix in internal code David Hunt
2021-07-08 15:38               ` [dpdk-dev] [PATCH v7 2/2] power: refactor pstate and acpi code David Hunt
2021-07-08 20:41               ` [dpdk-dev] [PATCH v7 1/2] power: don't use rte prefix in internal code David Marchand
2021-04-23 11:03       ` [dpdk-dev] [21.08 PATCH v4 2/2] power: refactor pstate and acpi code Anatoly Burakov
2021-05-07  2:13         ` Richael Zhuang
2021-05-07  9:49           ` Burakov, Anatoly [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d8f176a5-1e6a-aa61-406f-6221659da0d1@intel.com \
    --to=anatoly.burakov@intel.com \
    --cc=Richael.Zhuang@arm.com \
    --cc=david.hunt@intel.com \
    --cc=dev@dpdk.org \
    --cc=nd@arm.com \
    --cc=reshma.pattan@intel.com \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).