Re: [RFC][PATCH 1/3] trace_seq: Move the trace_seq code to lib/

From: Johannes Berg <johannes@sipsolutions.net>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Ingo Molnar <mingo@kernel.org>, Jiri Kosina <jkosina@suse.cz>,
	Michal Hocko <mhocko@suse.cz>, Jan Kara <jack@suse.cz>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Dave Anderson <anderson@redhat.com>,
	Petr Mladek <pmladek@suse.cz>
Subject: Re: [RFC][PATCH 1/3] trace_seq: Move the trace_seq code to lib/
Date: Tue, 24 Jun 2014 10:19:21 +0200	[thread overview]
Message-ID: <1403597961.4418.39.camel@jlt4.sipsolutions.net> (raw)
In-Reply-To: <20140623140413.3cebe156@gandalf.local.home>

On Mon, 2014-06-23 at 14:04 -0400, Steven Rostedt wrote:

> Ah! I see what you are saying. You are saying that once you are full
> you need to flush. Honestly, the trace_puts() should check the return.
> Perhaps it wants to try again with a shorter string?

Perhaps, but perhaps not? I don't remember exactly the issue that I saw
without the full flag, and it may very well be a purely theoretical
thing too. I don't think I mind either way.

> The full flag means that once we hit full, we are done, no more can be
> written till the buffer is flushed.

Right. Arguably, it was already full or overflowed, but as it now stands
we make sure the data is consistent with the input (or rejected if the
input didn't fit.)

> Is that what we would want from this utility? Or do we want to allow
> the user to try again with a smaller string?

That's a good question - right now of course the first attempt would
make it full, and then any further attempt would fail.

> I was thinking of reversing the error code and have the trace_seq_*()
> return 0 as success and the remaining buffer size on error.

That would make some sense if we wanted to make it post-determined (you
fail, and then you can use the remaining buffer). Maybe it would make
sense to pre-determine that, e.g. by having a function that can reserve
a range, say

 void trace_seq_put_buf(..., int min_len, int max_len,
                        int *out_avail, void *out_data_ptr)

(of course that prototype needs to be adjusted appropriately)

If less than min_len is available, we could still have the "full"
semantics.

Either way, I don't really know or even care. I'm only using this
indirectly through the tracing, where I don't think it makes a lot of
sense without the full semantics since afaik the buffers need to be in
the right order in the page(s), and where you likely don't want to cut a
string short since you couldn't continue it on the next page, but you're
clearly the expert on this and I'm probably wrong anyway :)

johannes