RE: [PATCH] eal/unix: lower log level for reading files

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Monday, 30 October 2023 11.35
> 
> I would add "sysfs" in the title.
> 
> 27/10/2023 11:13, David Marchand:
> > On Fri, Oct 27, 2023 at 11:00 AM Morten Brørup
> <mb@smartsharesystems.com> wrote:
> > >
> > > > From: David Marchand [mailto:david.marchand@redhat.com]
> > > > Sent: Friday, 27 October 2023 10.01
> > > >
> > > > The eal_parse_sysfs_value helper both returns an error code and
> logs an
> > > > error level message when something goes wrong.
> > > > On the other hand, internal users of this helper either ignore
> this
> > > > error code (like when trying to find out some numa information
> from the
> > > > Linux sysfs, or discovering some optional feature), or add their
> own
> > > > error
> > > > logging when reading the file actually matters.
> > > >
> > > > Lower this helper log messages to debug level as it provides no
> useful
> > > > information to final DPDK users.
> > >
> > > Such assumptions seem risky.
> > >
> > > Please add __attribute__ ((warn_unused_result)) to this function's
> header, to support the assumption.
> >
> > I can add this.
> 
> Would be __rte_warn_unused_result
> 
> I'm not sure it is required to mandate checking the result.
> I'm fine with or without it.

Me too.

> 
> I agree with this patch because not having a sysfs entry is never
> critical in itself.
> If the entry is required in a case, it should be handled by the caller
> with a more meaningful message.

I agree 100 %. And if all current callers do, then I consider it safe to lower the message level here. New callers are expected to follow the pattern of current callers.

> 
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> 
> 
> > > Alternatively, add a "bool may_not_exist" parameter to the function
> to choose the relevant log level.
> >
> > If an API update is to be considered, I would rather add some new
> > helpers with Windows support.
> 
> Please don't change the API for such detail.
> 

I don't feel strongly about my suggested changes, so however you handle it...

Acked-by: Morten Brørup <mb@smartsharesystems.com>