Re: [PATCH V3 5/9] tracing/trace: Add a generic function to read/write u64 values from tracefs

From: Steven Rostedt
Date: Fri Jun 04 2021 - 12:18:08 EST


On Fri, 4 Jun 2021 18:05:06 +0200
Daniel Bristot de Oliveira <bristot@xxxxxxxxxx> wrote:

>
> The reason for this patch is that hwlat, osnoise, and timerlat have "u64 config"
> options that are read/write via tracefs "files." In the previous version, I had
> multiple functions doing basically the same thing:
>
> A write function that:
> read a u64 from user-space
> get a lock,
> check for min/max acceptable values
> save the value
> release the lock.
>
> and a read function that:
> write the config value to the "read" buffer.
>
> And so, I tried to come up with a way to avoid code duplication.
>
> question: are only the names that are bad? (I agree that they are bad) or do you
> think that the overall idea is bad? :-)
>
> Suggestions?

I don't think the overall idea is bad, if it is what I think you are
doing. I just don't believe you articulated what you are doing.

It has nothing to do with 64 bit reads and writes, but instead has to
do with reading and writing values that depend on each other for what
is acceptable.

Perhaps have it called trace_min_max_write() and trace_min_max_read(),
and document what it is used for.

-- Steve