Re: [PATCH v3 3/3] char drivers: ramoops debugfs entry

[Andrew Morton <akpm@linux-foundation.org>]

On Thu, 7 Jul 2011 15:34:26 -0700
Sergiu Iordache <sergiu@chromium.org> wrote:

> On Thu, Jul 7, 2011 at 1:01 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> > On Wed, __6 Jul 2011 16:29:50 -0700
> > Sergiu Iordache <sergiu@chromium.org> wrote:
> >
> >> While ramoops writes to ram, accessing the dump requires using /dev/mem
> >> and knowing the memory location (or a similar solution). This patch
> >> provides a debugfs interface through which the respective memory
> >> area can be easily accessed.
> >>
> >> The entry added is /sys/kernel/debug/ramoops/next
> >>
> >> The entry returns a dump of size record_size each time, skipping invalid
> >> dumps. When it reaches the end of the memory area reserved for dumps it
> >> returns an empty record and resets the current record count.
> >
> > The patch increases the size of ramoops.o text&data from 1725 bytes to
> > 2282 even when CONFIG_DEBUG_FS=n. __That's quite a lot of bloat!
> >
> > Furthermore, I think debugfs is the wrong place to put this. __debugfs
> > is, umm, for debugging. __It's a place in which to put transitory junk
> > which may or may not be there and where interfaces may change without
> > notice and whose presence userspace should not assume. __But in this
> > patch you're proposing a permanent and formal new interface into the
> > ramoops driver. __It should go into a permanent well-known place where
> > it will be maintained with the formality and care of all the other
> > parts of the kernel API.
> How about not using the debugfs entry and adding a char driver? There
> are 2 possibilities here as well: using read operations to return the
> data like the current patch does or using ioctl to return the data(and
> maybe return other info as well such as the records size and the
> memory size). Any suggestions regarding a valid approach?

Well.  I haven't seen a description of what the interfaces *does* (and
I'm too obstinate to work that out from the patch) so it's hard to
advise.

ioctls are unpopular.

A char driver always seems weird and fake to me - there's no underlying
device.  /dev/zero considered harmful!

Perhaps switch it to sysfs?