linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: linux-arch <linux-arch@vger.kernel.org>,
	"Prakash, Sathya" <Sathya.Prakash@lsi.com>,
	Roland Dreier <roland@kernel.org>,
	"Desai, Kashyap" <Kashyap.Desai@lsi.com>,
	Hitoshi Mitake <h.mitake@gmail.com>,
	Matthew Wilcox <matthew@wil.cx>,
	"Moore, Eric" <Eric.Moore@lsi.com>,
	linux pci <linux-pci@vger.kernel.org>,
	linux powerpc dev <linuxppc-dev@lists.ozlabs.org>,
	Milton Miller <miltonm@bga.com>,
	linux kernel <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@redhat.com>,
	"paulus@samba.org" <paulus@samba.org>,
	linux scsi dev <linux-scsi@vger.kernel.org>,
	Ingo Molnar <mingo@elte.hu>, Sam Ravnborg <sam@ravnborg.org>
Subject: Re: [PATCH 1/3] mpt2sas: remove the use of writeq, since writeq is not atomic
Date: Thu, 19 May 2011 15:36:13 +1000	[thread overview]
Message-ID: <1305783373.7481.44.camel@pasglop> (raw)
In-Reply-To: <1305780360.2576.20.camel@mulgrave.site>

On Thu, 2011-05-19 at 08:46 +0400, James Bottomley wrote:
> This can't really be done generically.  There are several considerations
> to do with hardware requirements.  I can see some hw requiring a
> specific write order (I think this applies more to read order, though).

Right. Or there can be a need for a completely different access pattern
to do 32-bit, or maybe write only one half because both might have a
side effect etc etc etc ...

Also a global lock would be suboptimal vs. a per device lock burried in
the driver.

> The specific mpt2sas problem is that if we write a 64 bit register non
> atomically, we can't allow any interleaving writes for any other region
> on the chip, otherwise the HW will take the write as complete in the 64
> bit register and latch the wrong value.  The only way to achieve that
> given the semantics of writeq is a global static spinlock.
> 
> > How do you think about them? If you cannot agree with the above two
> > solutions, I'll agree with reverting them.
> 
> Having x86 roll its own never made any sense, so I think they need
> reverting anyway. 

Agreed.

>  This is a driver/platform bus problem not an
> architecture problem.  The assumption we can make is that the platform
> CPU can write atomically at its chip width.  We *may* be able to make
> the assumption that the bus controller can translate an atomic chip
> width transaction to a single atomic bus transaction; I think that
> assumption holds true for at least PCI and on the parisc legacy busses,
> so if we can agree on semantics, this should be a global define
> somewhere.  If there are problems with the bus assumption, we'll likely
> need some type of opt-in (or just not bother).

And we want a well defined #ifdef drivers test to know whether there's a
writeq/readq (just #define writeq/readq itself is fine even if it's an
inline function, we do that elsewhere) so they can have a fallback
scenario.

This is important as these can be used in very performance critical code
path.

Cheers,
Ben.

  reply	other threads:[~2011-05-19  5:40 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20110504115324.GE17855@lsi.com>
     [not found] ` <1305616571.6008.23.camel@mulgrave.site>
     [not found]   ` <B2FD678A64EAAD45B089B123FDFC3ED70157F7BCE5@inbmail01.lsi.com>
2011-05-18  4:15     ` [PATCH 1/3] mpt2sas: remove the use of writeq, since writeq is not atomic Matthew Wilcox
2011-05-18  4:23       ` James Bottomley
2011-05-18  7:00         ` Benjamin Herrenschmidt
2011-05-18  8:23           ` Milton Miller
2011-05-18 15:35             ` Moore, Eric
2011-05-18 18:31               ` Milton Miller
2011-05-18 19:11                 ` Moore, Eric
2011-05-19  4:08                   ` Hitoshi Mitake
2011-05-19  4:46                     ` James Bottomley
2011-05-19  5:36                       ` Benjamin Herrenschmidt [this message]
2011-05-19  8:35                       ` [PATCH 1/3] mpt2sas: remove the use of writeq, since writeq isnot atomic David Laight
2011-05-19  4:16                 ` [PATCH 1/3] mpt2sas: remove the use of writeq, since writeq is not atomic Roland Dreier
2011-05-19  5:34                   ` Benjamin Herrenschmidt
2011-05-19 18:15                     ` Ingo Molnar
2011-05-18 21:30               ` Benjamin Herrenschmidt
2011-05-18 22:05                 ` Moore, Eric
2011-05-18  8:04         ` [PATCH 1/3] mpt2sas: remove the use of writeq, since writeq isnot atomic David Laight
2011-05-18  5:45       ` [PATCH 1/3] mpt2sas: remove the use of writeq, since writeq is not atomic Benjamin Herrenschmidt

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=1305783373.7481.44.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=Eric.Moore@lsi.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=Kashyap.Desai@lsi.com \
    --cc=Sathya.Prakash@lsi.com \
    --cc=h.mitake@gmail.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=matthew@wil.cx \
    --cc=miltonm@bga.com \
    --cc=mingo@elte.hu \
    --cc=mingo@redhat.com \
    --cc=paulus@samba.org \
    --cc=roland@kernel.org \
    --cc=sam@ravnborg.org \
    /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).