Re: [PATCH] ftrace: Documentation

Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
On Thu, 10 Jul 2008, Elias Oltmanns wrote:


Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
[...]
+ times the number of possible CPUS. The buffers
+ are saved as individual pages, and the actual entries
+ will always be rounded up to entries per page.

Not sure I understand the last sentence, but may be it's just me not
being familiar with the terminology.

I should rewrite it then. I allocate the buffer by pages (a block of
memory that is used in kernel allocation. Usually 4K). Since the entries
are less than a page, if there is extra padding on the last page after
all the requested entries have been allocated, I use the rest of the page
to add entries that can still fit.

I see, thanks.

[...]
+ ftrace - function tracer that uses mcount to trace all functions.
+ It is possible to filter out which functions that are

are to be

Both ways sound OK to me. But then again, I would trust a non-native
speaker more. Since they were actually taught the language ;-)

Most likely, both ways *are* OK; I was mainly concerned about the `that'
which irritates me.



+ traced when dynamic ftrace is configured in.
[...]
+ stacktrace - This is one of the options that changes the trace itself.

change

Hmm, now this would be a good English question. "change" is for plural and
"changes" is for singular. Now is "This is one of options" plural or
singular. I'm thinking singular, because it is "one of", but I'm not an
English major.

Well, I couldn't name any particular rule to make my point but I'll
dissect that sentence in order to explain my thinking:

1. This is one of the options.

Well, we knew that anyway, so surely, something is missing.

2. This is an option that changes the trace itself.

Quite conclusive.

3. This is one of the options that change the trace itself.

The `that' clause refers to options, thus implying that there are more
then just this one that change the trace itself. I'm not sure whether
the fact that there is no comma before the `that' implies that the rest
of the sentence refers to the subject, though.

4. This is one of the options that changes the trace itself.

Personally, I think of this as the sum of 1. (no valuable information)
and 2. (a perfectly valid and meaningful statement), i.e., why not stick
to 2. In constrast, the third variant really does provide additional
information because the relative clause refers to options (plural).

Well, I really am clutching at straws here, I suppose. Actually, I dare
not argue with with you *and* Randy about this.

[...]
+The irqsoff tracer tracks the time interrupts are disabled and when

when

Hmm, I don't like either "when"s. How about:

The irqsoff tracer tracks the time interrupts are disabled to the time
they are re-enabled.

??

Fine with me.



+they are re-enabled. When a new maximum latency is hit, it saves off
+the trace so that it may be retrieved at a later time. Every time a
+new maximum in reached, the old saved trace is discarded and the new
+trace is saved.
[...]
+Note the above had ftrace_enabled not set. If we set the ftrace_enabled

(comma)

I have a lot of these. I just commented on someones writing in saying that
you can use a "then" or a "comma" but don't leave both out. Seems I've
been doing that a lot here.

I doubt you really can omit the comma either way since the conditional
precedes the main clause.

[...]
+Where as the setting of the NEED_RESCHED bit happens on the
+task's stack. But because we are in a hard interrupt, the test
+is with the interrupts stack which has that to be false. We don't

^^^^
Superfluous that? Don't understand that sentence.

No really, I did proof read it...

God that was an awful explanation. OK, how about something like this:

Some task data is stored at the top of the tasks stack (need_resched and
preempt_count). The setting of the NEED_RESCHED sets the bit on the
task's real stack. The test for NEED_RESCHED looks at the current stack.
Since the current stack is the hard interrupt stack (as the kernel is
configured to use a separate stack for interrupts), the trace shows that
the need_resched bit has not yet been set.

Righht, thanks.

[...]
+Two files that contain to the enabling and disabling of recorded
+functions are:

Can this be expressed somewhat differently?

Did I write that??
How about:

Two files are used, one for enabling and one for disabling the tracing
of recorded functions.

I'm sure you'll tell me I missed a comma in there somewhere ;-)

Sorry to disappoint you, I could think of one. ;-)

[...]
+ftraced
+-------
+
+As mentioned above, when dynamic ftrace is configured in, a kernel
+thread wakes up once a second and checks to see if there are mcount
+calls that need to be converted into nops. If there is not, then

are

That sounds off. I see your point, but I'm not sure this applies for
plural. I can be wrong though.

Well, I could be persuaded to believe that this is idiomatic.



+it simply goes back to sleep. But if there is, it will call

are

Same here.

Of course.



+kstop_machine to convert the calls to nops.
[...]
+Any write to the ftraced_enabled file will cause the kstop_machine
+to run if there are recorded calls to mcount. This means that a

Incomplete sentence.

hmm, how so? Although I am missing a comma. I could also write it like
"If there are recorded calls to mcount, any write to the ftraced_enabled
file will cause kstop_machine to run".

Oh dear, I didn't exactly excel myself there, did I. Your original
sentence was quite alright and there was no comma missing either because
the conditional follows the main clause. Personally, though, I find the
new version easier to understand.



+user can manually perform the updates when they want to by simply

(s)he wants

This is where I hate the English language, and will not be including this
update. Sorry, I hate the whole she/he thing. I simply rebel and use
"they"!

Yes, I appreciate that. Actually, I was wondering at the time whether I
should suggest saying `users' insdead of `a user'.



+echoing a '0' into the ftraced_enabled file.

Regards,

Elias
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages