Re: [PATCH 08/11] ramoops: Move to fs/pstore/ram.c

[Shuah Khan <shuahkhan@gmail.com>]

On Wed, 2012-05-16 at 00:30 -0700, Anton Vorontsov wrote:
> Hi Shuah,
> 
> On Tue, May 15, 2012 at 09:12:59AM -0600, Shuah Khan wrote:
> > On Fri, 2012-05-11 at 17:18 -0700, Anton Vorontsov wrote:
> > > Since ramoops was converted to pstore, it has nothing to do with character
> > > devices nowadays. Instead, today it is just a RAM backend for pstore.
> > > 
> > > The patch just moves things around. There are a few changes were needed
> > > because of the move:
> > > 
> > > 1. Kconfig and Makefiles fixups, of course.
> > > 
> > > 2. In pstore/ram.c we have to play a bit with MODULE_PARAM_PREFIX, this
> > >    is needed to keep user experience the same as with ramoops driver
> > >    (i.e. so that ramoops.foo kernel command line arguments would still
> > >    work).
> > 
> > Anton,
> > 
> > Could you please enhance Kconfig as well as ram.c with information with
> > the new functionality it supports.
> 
> Sure, will do.
> 
> > Also ram.c in my opinion doesn't
> > really reflect the feature it currently supports and its future plans.
> > ramoops doesn't either. ramdesg or ramkmsg probably are better suited.
> 
> No, I actually think we shouldn't mention neither dmesg nor kmsg in
> the name of the module. We might support MCE messages, tracing
> messages and so on, and this all will be handled by ram.c.

Good point.
> 
> So, ram.c is a generic backend for pstore.
> 
> > Also leaving the ABI that ramoops specific might lead confusion in the
> > long run. It might make sense to update the ABI to reflect its new
> > features, if it doesn't impact existing ramoops users.
> 
> We can do this, I can prepare a separate patch to change the ABI, but
> so far I tend to not break any ABIs. We can always do it later -- it is
> easy. :-D

Yes it can be done later.
> 
> > Would you be interested in adding a doc file for usage describing how
> > users can configure the driver - the details I would like to see are how
> > to pick a ram address especially when mem_address and mem_size are
> > passed in as module parameters.
> 
> We actually have Documentation/ramoops.txt already, but I'll add
> a documentation for the new ecc option.
> 
> Thanks!

Thanks for doing this. One thing that would be helpful for users is some
kind of guidance/tips on how to pick ram range for module parameter
passing, which is missing from the current ramoops.txt

Thanks,
-- Shuah