From mboxrd@z Thu Jan 1 00:00:00 1970 From: Benjamin Herrenschmidt 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 Message-ID: <1305783373.7481.44.camel@pasglop> References: <20110504115324.GE17855@lsi.com> <1305616571.6008.23.camel@mulgrave.site> <20110518041551.GL15227@parisc-linux.org> <1305692584.2580.3.camel@mulgrave.site> <1305702010.2781.33.camel@pasglop> <4565AEA676113A449269C2F3A549520F80B66280@cosmail03.lsi.com> <4565AEA676113A449269C2F3A549520F80BE7F37@cosmail03.lsi.com> <1305780360.2576.20.camel@mulgrave.site> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: Received: from gate.crashing.org ([63.228.1.57]:33422 "EHLO gate.crashing.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754004Ab1ESFk7 (ORCPT ); Thu, 19 May 2011 01:40:59 -0400 In-Reply-To: <1305780360.2576.20.camel@mulgrave.site> Sender: linux-arch-owner@vger.kernel.org List-ID: To: James Bottomley Cc: Hitoshi Mitake , "Moore, Eric" , Milton Miller , Sam Ravnborg , Ingo Molnar , Ingo Molnar , "Desai, Kashyap" , "Prakash, Sathya" , Matthew Wilcox , linux scsi dev , "paulus@samba.org" , linux powerpc dev , linux pci , linux kernel , linux-arch , Roland Dreier 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. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id A085E1007D8 for ; Thu, 19 May 2011 15:40:50 +1000 (EST) Subject: Re: [PATCH 1/3] mpt2sas: remove the use of writeq, since writeq is not atomic From: Benjamin Herrenschmidt To: James Bottomley In-Reply-To: <1305780360.2576.20.camel@mulgrave.site> References: <20110504115324.GE17855@lsi.com> <1305616571.6008.23.camel@mulgrave.site> <20110518041551.GL15227@parisc-linux.org> <1305692584.2580.3.camel@mulgrave.site> <1305702010.2781.33.camel@pasglop> <4565AEA676113A449269C2F3A549520F80B66280@cosmail03.lsi.com> <4565AEA676113A449269C2F3A549520F80BE7F37@cosmail03.lsi.com> <1305780360.2576.20.camel@mulgrave.site> Content-Type: text/plain; charset="UTF-8" Date: Thu, 19 May 2011 15:36:13 +1000 Message-ID: <1305783373.7481.44.camel@pasglop> Mime-Version: 1.0 Cc: linux-arch , "Prakash, Sathya" , Roland Dreier , "Desai, Kashyap" , Hitoshi Mitake , Matthew Wilcox , "Moore, Eric" , linux pci , linux powerpc dev , Milton Miller , linux kernel , Ingo Molnar , "paulus@samba.org" , linux scsi dev , Ingo Molnar , Sam Ravnborg List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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.