[dpdk-dev,v4,07/11] eal: replace rte_panic instances in hugepage_info
Checks
Commit Message
replace panic calls with log and retrun value.
v4
static size calculation function changed to return success/fail code
in addition to filling the size result.
Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
---
lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 32 ++++++++++++++++---------
1 file changed, 21 insertions(+), 11 deletions(-)
Comments
On 19-Apr-18 7:01 AM, Arnon Warshavsky wrote:
> replace panic calls with log and retrun value.
typo: return
>
> v4
> static size calculation function changed to return success/fail code
> in addition to filling the size result.
>
> Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
> ---
Please do not add version notes to commit message. You might want to use
"git notes add" for this, or manually edit the patch and add notes after
"---" before sending.
On patch contents,
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
Thanks Anatoly. Will fix that in v5.
Is it preferred to keep all version notes in the cover letter alone?
On Thu, Apr 19, 2018 at 5:03 PM, Burakov, Anatoly <anatoly.burakov@intel.com
> wrote:
> On 19-Apr-18 7:01 AM, Arnon Warshavsky wrote:
>
>> replace panic calls with log and retrun value.
>>
>
> typo: return
>
>
>> v4
>> static size calculation function changed to return success/fail code
>> in addition to filling the size result.
>>
>> Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
>> ---
>>
>
> Please do not add version notes to commit message. You might want to use
> "git notes add" for this, or manually edit the patch and add notes after
> "---" before sending.
>
> On patch contents,
>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
>
> --
> Thanks,
> Anatoly
>
On 04/19/2018 07:01 AM, Arnon Warshavsky wrote:
> replace panic calls with log and retrun value.
>
> v4
> static size calculation function changed to return success/fail code
> in addition to filling the size result.
>
fyi - this patch doesn't apply on master branch without fuzz
> Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
> ---
> lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 32 ++++++++++++++++---------
> 1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
> index fb4b667..debae32 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
> @@ -145,8 +145,8 @@
> return num_pages;
> }
>
> -static uint64_t
> -get_default_hp_size(void)
> +static int
> +get_default_hp_size(uint64_t *result)
> {
> const char proc_meminfo[] = "/proc/meminfo";
> const char str_hugepagesz[] = "Hugepagesize:";
> @@ -155,8 +155,11 @@
> unsigned long long size = 0;
>
> FILE *fd = fopen(proc_meminfo, "r");
> - if (fd == NULL)
> - rte_panic("Cannot open %s\n", proc_meminfo);
> + if (fd == NULL) {
> + RTE_LOG(CRIT, EAL, "%s(): Cannot open %s\n",
> + __func__, proc_meminfo);
> + return -1;
> + }
> while(fgets(buffer, sizeof(buffer), fd)){
> if (strncmp(buffer, str_hugepagesz, hugepagesz_len) == 0){
> size = rte_str_to_size(&buffer[hugepagesz_len]);
> @@ -164,9 +167,13 @@
> }
> }
> fclose(fd);
> - if (size == 0)
> - rte_panic("Cannot get default hugepage size from %s\n", proc_meminfo);
> - return size;
> + if (size == 0) {
> + RTE_LOG(CRIT, EAL, "%s(): Cannot get default hugepage size from %s\n",
> + __func__, proc_meminfo);
> + return -1;
> + }
> + *result = size;
> + return 0;
> }
>
> static const char *
> @@ -191,11 +198,14 @@
> char *retval = NULL;
>
> FILE *fd = fopen(proc_mounts, "r");
> - if (fd == NULL)
> - rte_panic("Cannot open %s\n", proc_mounts);
> + if (fd == NULL) {
> + RTE_LOG(CRIT, EAL, "%s(): Cannot open %s\n",
> + __func__, proc_mounts);
> + return NULL;
> + }
>
> - if (default_size == 0)
> - default_size = get_default_hp_size();
> + if ((default_size == 0) && (get_default_hp_size(&default_size) != 0))
> + return NULL;
>
> while (fgets(buf, sizeof(buf), fd)){
> if (rte_strsplit(buf, sizeof(buf), splitstr, _FIELDNAME_MAX,
>
On 19-Apr-18 3:09 PM, Arnon Warshavsky wrote:
> Thanks Anatoly. Will fix that in v5.
> Is it preferred to keep all version notes in the cover letter alone?
>
Generally, cover letter should give general outline (i.e. "fixed 32 bit
compile"), while notes for individual patches should be more specific
about the changes between versions (but not too specific, i.e. don't do
"change variable X on line 100 to be Y").
So, whatever you think gets your point across best. Not all changes
deserve to be called out in the cover letter.
On 19-Apr-18 3:45 PM, Burakov, Anatoly wrote:
> On 19-Apr-18 3:09 PM, Arnon Warshavsky wrote:
>> Thanks Anatoly. Will fix that in v5.
>> Is it preferred to keep all version notes in the cover letter alone?
>>
>
> Generally, cover letter should give general outline (i.e. "fixed 32 bit
> compile"), while notes for individual patches should be more specific
> about the changes between versions (but not too specific, i.e. don't do
> "change variable X on line 100 to be Y").
>
> So, whatever you think gets your point across best. Not all changes
> deserve to be called out in the cover letter.
>
Just to be clear:
With my initial reply, i did not mean "patch notes should be in the
cover letter". What i meant was that you have put your version changes
"e.g. v4 - changed this to that" into the commit message.
What you should have done is put your patch notes after the commit
message, like this:
replace panic calls with log and retrun value.
Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
---
v4
static size calculation function changed to return success/fail code
in addition to filling the size result.
lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 32
++++++++++++++++---------
1 file changed, 21 insertions(+), 11 deletions(-)
Note that the v4 comments are after the "---" - this is where the commit
message ends as far as git concerned, so you can put your notes there.
Now clear.Thanks
On Thu, Apr 19, 2018 at 5:50 PM, Burakov, Anatoly <anatoly.burakov@intel.com
> wrote:
> On 19-Apr-18 3:45 PM, Burakov, Anatoly wrote:
>
>> On 19-Apr-18 3:09 PM, Arnon Warshavsky wrote:
>>
>>> Thanks Anatoly. Will fix that in v5.
>>> Is it preferred to keep all version notes in the cover letter alone?
>>>
>>>
>> Generally, cover letter should give general outline (i.e. "fixed 32 bit
>> compile"), while notes for individual patches should be more specific about
>> the changes between versions (but not too specific, i.e. don't do "change
>> variable X on line 100 to be Y").
>>
>> So, whatever you think gets your point across best. Not all changes
>> deserve to be called out in the cover letter.
>>
>> Just to be clear:
>
> With my initial reply, i did not mean "patch notes should be in the cover
> letter". What i meant was that you have put your version changes "e.g. v4 -
> changed this to that" into the commit message.
>
> What you should have done is put your patch notes after the commit
> message, like this:
>
> replace panic calls with log and retrun value.
>
> Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
> ---
>
> v4
> static size calculation function changed to return success/fail code
> in addition to filling the size result.
>
>
> lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 32
> ++++++++++++++++---------
> 1 file changed, 21 insertions(+), 11 deletions(-)
>
> Note that the v4 comments are after the "---" - this is where the commit
> message ends as far as git concerned, so you can put your notes there.
>
> --
> Thanks,
> Anatoly
>
Thanks Kevin
Will address that
On Thu, Apr 19, 2018 at 5:36 PM, Kevin Traynor <ktraynor@redhat.com> wrote:
> On 04/19/2018 07:01 AM, Arnon Warshavsky wrote:
> > replace panic calls with log and retrun value.
> >
> > v4
> > static size calculation function changed to return success/fail code
> > in addition to filling the size result.
> >
>
> fyi - this patch doesn't apply on master branch without fuzz
>
> > Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
> > ---
> > lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 32
> ++++++++++++++++---------
> > 1 file changed, 21 insertions(+), 11 deletions(-)
> >
> > diff --git a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
> b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
> > index fb4b667..debae32 100644
> > --- a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
> > +++ b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
> > @@ -145,8 +145,8 @@
> > return num_pages;
> > }
> >
> > -static uint64_t
> > -get_default_hp_size(void)
> > +static int
> > +get_default_hp_size(uint64_t *result)
> > {
> > const char proc_meminfo[] = "/proc/meminfo";
> > const char str_hugepagesz[] = "Hugepagesize:";
> > @@ -155,8 +155,11 @@
> > unsigned long long size = 0;
> >
> > FILE *fd = fopen(proc_meminfo, "r");
> > - if (fd == NULL)
> > - rte_panic("Cannot open %s\n", proc_meminfo);
> > + if (fd == NULL) {
> > + RTE_LOG(CRIT, EAL, "%s(): Cannot open %s\n",
> > + __func__, proc_meminfo);
> > + return -1;
> > + }
> > while(fgets(buffer, sizeof(buffer), fd)){
> > if (strncmp(buffer, str_hugepagesz, hugepagesz_len) == 0){
> > size = rte_str_to_size(&buffer[hugepagesz_len]);
> > @@ -164,9 +167,13 @@
> > }
> > }
> > fclose(fd);
> > - if (size == 0)
> > - rte_panic("Cannot get default hugepage size from %s\n",
> proc_meminfo);
> > - return size;
> > + if (size == 0) {
> > + RTE_LOG(CRIT, EAL, "%s(): Cannot get default hugepage size
> from %s\n",
> > + __func__, proc_meminfo);
> > + return -1;
> > + }
> > + *result = size;
> > + return 0;
> > }
> >
> > static const char *
> > @@ -191,11 +198,14 @@
> > char *retval = NULL;
> >
> > FILE *fd = fopen(proc_mounts, "r");
> > - if (fd == NULL)
> > - rte_panic("Cannot open %s\n", proc_mounts);
> > + if (fd == NULL) {
> > + RTE_LOG(CRIT, EAL, "%s(): Cannot open %s\n",
> > + __func__, proc_mounts);
> > + return NULL;
> > + }
> >
> > - if (default_size == 0)
> > - default_size = get_default_hp_size();
> > + if ((default_size == 0) && (get_default_hp_size(&default_size) !=
> 0))
> > + return NULL;
> >
> > while (fgets(buf, sizeof(buf), fd)){
> > if (rte_strsplit(buf, sizeof(buf), splitstr,
> _FIELDNAME_MAX,
> >
>
>
@@ -145,8 +145,8 @@
return num_pages;
}
-static uint64_t
-get_default_hp_size(void)
+static int
+get_default_hp_size(uint64_t *result)
{
const char proc_meminfo[] = "/proc/meminfo";
const char str_hugepagesz[] = "Hugepagesize:";
@@ -155,8 +155,11 @@
unsigned long long size = 0;
FILE *fd = fopen(proc_meminfo, "r");
- if (fd == NULL)
- rte_panic("Cannot open %s\n", proc_meminfo);
+ if (fd == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open %s\n",
+ __func__, proc_meminfo);
+ return -1;
+ }
while(fgets(buffer, sizeof(buffer), fd)){
if (strncmp(buffer, str_hugepagesz, hugepagesz_len) == 0){
size = rte_str_to_size(&buffer[hugepagesz_len]);
@@ -164,9 +167,13 @@
}
}
fclose(fd);
- if (size == 0)
- rte_panic("Cannot get default hugepage size from %s\n", proc_meminfo);
- return size;
+ if (size == 0) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot get default hugepage size from %s\n",
+ __func__, proc_meminfo);
+ return -1;
+ }
+ *result = size;
+ return 0;
}
static const char *
@@ -191,11 +198,14 @@
char *retval = NULL;
FILE *fd = fopen(proc_mounts, "r");
- if (fd == NULL)
- rte_panic("Cannot open %s\n", proc_mounts);
+ if (fd == NULL) {
+ RTE_LOG(CRIT, EAL, "%s(): Cannot open %s\n",
+ __func__, proc_mounts);
+ return NULL;
+ }
- if (default_size == 0)
- default_size = get_default_hp_size();
+ if ((default_size == 0) && (get_default_hp_size(&default_size) != 0))
+ return NULL;
while (fgets(buf, sizeof(buf), fd)){
if (rte_strsplit(buf, sizeof(buf), splitstr, _FIELDNAME_MAX,