Messages in this thread | | | Date | Thu, 7 Jul 2011 15:54:26 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH v3 3/3] char drivers: ramoops debugfs entry |
| |
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?
| |