All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anton Vorontsov <anton.vorontsov@linaro.org>
To: "Luck, Tony" <tony.luck@intel.com>
Cc: "Liu, Chuansheng" <chuansheng.liu@intel.com>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"keescook@chromium.org" <keescook@chromium.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Colin Cross <ccross@android.com>
Subject: Re: [PATCH] pstore: avoid recursive spinlocks in the oops_in_progress case
Date: Thu, 20 Sep 2012 17:37:06 -0700	[thread overview]
Message-ID: <20120921003706.GB14399@lizard> (raw)
In-Reply-To: <3908561D78D1C84285E8C5FCA982C28F19D43892@ORSMSX108.amr.corp.intel.com>

On Thu, Sep 20, 2012 at 11:48:32PM +0000, Luck, Tony wrote:
> > True, but the lock is used to protect pstore->buf, I doubt that
> > any backend will actually want to grab it, no?
> 
> The lock is doing double duty to protect the buffer, and the back-end driver.
> 
> But even if we split it into two (one for the buffer, taken by pstore, and one
> internal to the backend to protect interaction with the f/w). Ifwe ignore the
> fact that we can't get the lock that protects  the buffer means it is very likely
> that we corrupt the previous record that was being written by clobbering the
> buffer with the data for this new record.
> 
> I'd prefer to maximize the chances that the earlier record gets written.

Sure, I applied the original patch.

Btw, do you expect that backends protect themselves from concurrent
->write calls, or pstore guarantees to protect backends?

Because the latter is not always possible, for example in tracing: we
won't able to grab locks at all (but not all backends can do tracing
anyway -- they must do things atomically).

Plus, sometimes having the global lock is not "efficient", backends
know better: they might have separate locks per message type.

And my plan was to get rid of the fact that backends touch pstore->buf
directly. Backends would always receive anonymous 'buf' pointer (we
already have write_buf callback that does exactly this), and thus it
would be backends' worry to protect against concurrency. In this
scheme, pstore's console code won't need to grab locks at all: we'll
just pass console string to the backend directly.

And backends, if they can't do writes atomically, will grab their
own locks.

Thanks,
Anton.

  reply	other threads:[~2012-09-21  0:39 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-17 17:43 [PATCH] pstore: avoid recursive spinlocks in the oops_in_progress case Chuansheng Liu
2012-09-20 22:57 ` Anton Vorontsov
2012-09-20 23:09   ` Luck, Tony
2012-09-20 23:25     ` Anton Vorontsov
2012-09-20 23:48       ` Luck, Tony
2012-09-21  0:37         ` Anton Vorontsov [this message]
2012-09-24 15:02           ` Luck, Tony
2012-09-26 23:35             ` Anton Vorontsov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120921003706.GB14399@lizard \
    --to=anton.vorontsov@linaro.org \
    --cc=ccross@android.com \
    --cc=chuansheng.liu@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tony.luck@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.