Re: early_printk accessing __log_buf

[Mike Frysinger]

On 7/23/07, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Mon, 23 Jul 2007 14:19:12 -0400
> Robin Getz <rgetz@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> > On Sun 22 Jul 2007 19:50, Mike Frysinger pondered:
> > >
> > > i think the attached two functions account for what Robin and Andrew
> > > were thinking ...
> >
> > A note about why/when grab_lock would be set to zero (pre-kernel init, or
> > OOPs) might be nice.
>
> Definitely.
>
> The pre-kernel init shouldn't be an issue: logbuf_lock is initialised at
> compile time.
>
> At oops time we could possibly use oops_in_progress to work out whether to
> avoid taking the lock.  That's not terribly nice, but nor is it nice for
> callers to know about printk internals.

maybe, but for early debug users (the reason we wanted this
originally), it wouldnt be an oops in progress ... but i guess we can
just as easily set oops_in_progress to 1 in our code before calling
this function to keep from having to worry over locks from being
doubly grabbed.

> > Or - remove it - and tell people they should be using do_syslog interface in
> > a "normal" kernel environment (where log_end or log_start can be updated).
>
> You can't (or al least, shouldn't) use do_syslog() from within kernel code?
>
> > Either would be fine with me. Andrew?
>
> The interface is pretty bad, IMO:
>
>  /*
> + * Read the log buffer into the supplied buffer.  The len option
> + * tells whether to copy from the beginning (> 0), the end (< 0), or
> + * just query the number of existing chars.   The number of bytes
> + * actually copied is returned.
> + */
> +int log_buf_read(int len, char *buf, int grab_lock)
>
> The magical interpretation of len isn't very nice.  This code would be
> simpler if it did not attempt to read more than one char at a time.  It
> will be plenty fast enough.
>
> If the caller knows the length of the buffer, why is there any need for the
> index-from-the-end function?

fair enough ... my php side likes to overload the meaning of length :)

> I think a sufficient implementation is
>
>         int log_buf_len(void);
>
> and
>
>         int log_buf_read(int index);
>
> which will return the character at `index', or -1 if `index' is out of
> bounds.

the idea originally was to have a buffer based function and an index
based function due to the idea of copying chunks of the log at a time,
but i kind of forgot about that once was all said and done.  now that
i look at it, unless the buffer based function dives into the log
internals to figure out how to split the copy based on the log
wraparound, there's no need for it.

log_buf_len is already the variable used to track the size, so the
function is called 'log_buf_get_len()' ...
-mike

Attachment: linux-log_buf_read.patch
Description: Binary data