Re: [PATCH v2] pstore: pass allocated memory region back to caller

[Don Zickus <dzickus@redhat.com>]

On Wed, Nov 16, 2011 at 02:45:24PM -0800, Tony Luck wrote:
> On Wed, Nov 16, 2011 at 2:20 PM, Don Zickus <dzickus@redhat.com> wrote:
> > This is an interesting approach.  But are we leaving psinfo data exposed
> > when you have a reader and writer at the same time?
> 
> I look at this as the first step in separating the read & write paths.
> 
> I started out with the (good) idea that the back end should allocate & own
> the buffer for the write path ... this means that the buffer is ready to use
> when an oops/panic happens - which is obviously a bad time to need to
> allocate memory :-)

Of course. :-)  But don't we have mechanisms which can use pre-allocated
memory?  Like the lock-less link list or the ring buffer?

Or maybe something safer, perhaps pass the backend buffer size to the
dumper routine when it registers?  That way kmsg_dump can allocate memory
at registration time which is sync'd in size with the backend.  This
allows kmsg_dump to fill the buffer and pass it directly down to the
backend (through pstore_dumper), with minimal locks, without breaking it
up again and re-copying into yet another buffer.

Thoughts?

> 
> Then I reused the same buffer for read - in hindsight this was not such a
> good idea - it led to all the discussions we've had about how to guarantee
> that the dmesg data gets saved on panic - even in the cases where we
> can't get the locks (so proposals have been made to bust the locks).
> 
> So Kees' patch is the functional equivalent of busting the spinlock.

> 
> Next step would be to look at the back end drivers to see whether
> they can handle a simultaneous read & write in a graceful way.
> 
I was just wondering if we should put a 'const' on the psinfo data being
passed to the read/write routine, otherwise a broken backend could modify
psinfo and corrupt any concurrent access, no?

> I've queued this for linux-next. Probably missed the snapshot today,
> but I expect it should show up in next-20111118

Cheers,
Don