Re: [PATCH v9 1/4] ftrace: Implement fs notification for tracing_max_latency

From: Steven Rostedt
Date: Tue Oct 15 2019 - 21:17:48 EST


On Tue, 15 Oct 2019 20:41:20 -0400
Joel Fernandes <joel@xxxxxxxxxxxxxxxxx> wrote:

> > +static const struct file_operations tracing_max_lat_fops;
> > +
> > +#if (defined(CONFIG_TRACER_MAX_TRACE) || defined(CONFIG_HWLAT_TRACER)) && \
> > + defined(CONFIG_FSNOTIFY)
>
> Something bothers me. If you dropped support for HWLAT_TRACER as you
> mentioned in the cover letter, then why does this #if look for the CONFIG
> option?
>
> (and similar comment on the rest of the patch..)

Also, if you have a complex if statement like this, it is better to
create a new define to set it in a header file:

#if (defined(CONFIG_TRACER_MAX_TRACE) || defined(CONFIG_HWLAT_TRACER)) && \
defined(CONFIG_FSNOTIFY)
# define HAVE_TRACER_FSNOTIFY
#endif

And then just use that:

#ifdef HAVE_TRACER_FSNOTIFY
...
#endif

It keeps things a bit more manageable.

-- Steve