Re: early_printk accessing __log_buf

From: Andrew Morton
Date: Mon Jul 23 2007 - 16:16:57 EST


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.

> 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?

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.

-
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/