linux-kernel - Re: [PATCH 1/3] mpt2sas: remove the use of writeq, since writeq is not atomic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1305780360.2576.20.camel@mulgrave.site>
Date:	Thu, 19 May 2011 08:46:00 +0400
From:	James Bottomley <James.Bottomley@...senPartnership.com>
To:	Hitoshi Mitake <h.mitake@...il.com>
Cc:	"Moore, Eric" <Eric.Moore@....com>,
	Milton Miller <miltonm@....com>,
	Sam Ravnborg <sam@...nborg.org>, Ingo Molnar <mingo@...e.hu>,
	Ingo Molnar <mingo@...hat.com>,
	"Desai, Kashyap" <Kashyap.Desai@....com>,
	"Prakash, Sathya" <Sathya.Prakash@....com>,
	Matthew Wilcox <matthew@....cx>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	linux scsi dev <linux-scsi@...r.kernel.org>,
	"paulus@...ba.org" <paulus@...ba.org>,
	linux powerpc dev <linuxppc-dev@...ts.ozlabs.org>,
	linux pci <linux-pci@...r.kernel.org>,
	linux kernel <linux-kernel@...r.kernel.org>,
	linux-arch <linux-arch@...r.kernel.org>,
	Roland Dreier <roland@...nel.org>
Subject: Re: [PATCH 1/3] mpt2sas: remove the use of writeq, since writeq is
 not atomic

On Thu, 2011-05-19 at 13:08 +0900, Hitoshi Mitake wrote:
> On Thu, May 19, 2011 at 04:11, Moore, Eric <Eric.Moore@....com> wrote:
> > On Wednesday, May 18, 2011 12:31 PM Milton Miller wrote:
> >> Ingo I would propose the following commits added in 2.6.29 be reverted.
> >> I think the current concensus is drivers must know if the writeq is
> >> not atomic so they can provide their own locking or other workaround.
> >>
> >
> >
> > Exactly.
> >
> 
> The original motivation of preparing common readq/writeq is that
> letting each driver
> have their own readq/writeq is bad for maintenance of source code.
> 
> But if you really dislike them, there might be two solutions:
> 
> 1. changing the name of readq/writeq to readq_nonatomic/writeq_nonatomic

This is fine, but not really very useful

> 2. adding new C file to somewhere and defining spinlock for them.
>     With spin_lock_irqsave() and spin_unlock_irqrestore() on the spinlock,
>     readq/writeq can be atomic.

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).

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.  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).

James

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/