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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210604121802.192caa07@oasis.local.home>
Date:   Fri, 4 Jun 2021 12:18:02 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Daniel Bristot de Oliveira <bristot@...hat.com>
Cc:     linux-kernel@...r.kernel.org, Phil Auld <pauld@...hat.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Kate Carcia <kcarcia@...hat.com>,
        Jonathan Corbet <corbet@....net>,
        Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Alexandre Chartre <alexandre.chartre@...cle.com>,
        Clark Willaims <williams@...hat.com>,
        John Kacur <jkacur@...hat.com>,
        Juri Lelli <juri.lelli@...hat.com>, linux-doc@...r.kernel.org
Subject: Re: [PATCH V3 5/9] tracing/trace: Add a generic function to
 read/write u64 values from tracefs

On Fri, 4 Jun 2021 18:05:06 +0200
Daniel Bristot de Oliveira <bristot@...hat.com> 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