linux-kernel.vger.kernel.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).