Received-SPF: pass (google.com: domain of linux-kernel+bounces-169291-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Sun, 5 May 2024 21:06:02 -0700
From: Ira Weiny <ira.weiny@intel.com>
To: Dan Williams <dan.j.williams@intel.com>, Ira Weiny <ira.weiny@intel.com>,
	Alison Schofield <alison.schofield@intel.com>
CC: Dave Jiang <dave.jiang@intel.com>, Fan Ni <fan.ni@samsung.com>, "Jonathan
 Cameron" <Jonathan.Cameron@huawei.com>, Navneet Singh
	<navneet.singh@intel.com>, Dan Williams <dan.j.williams@intel.com>, Davidlohr
 Bueso <dave@stgolabs.net>, Vishal Verma <vishal.l.verma@intel.com>,
	<linux-btrfs@vger.kernel.org>, <linux-cxl@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 05/26] cxl/core: Simplify cxl_dpa_set_mode()
Message-ID: <6638572a2afd_258421294e0@iweiny-mobl.notmuch>
References: <20240324-dcd-type2-upstream-v1-0-b7b00d623625@intel.com>
 <20240324-dcd-type2-upstream-v1-5-b7b00d623625@intel.com>
 <ZhSPHvoTHl28GXt1@aschofie-mobl2>
 <6635366717079_e1f582941d@iweiny-mobl.notmuch>
 <66358d2dab5f9_1aefb294f1@dwillia2-mobl3.amr.corp.intel.com.notmuch>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <66358d2dab5f9_1aefb294f1@dwillia2-mobl3.amr.corp.intel.com.notmuch>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?OqyWkye/BJ5Cx50R8vUvYC6UkTxhnplxyrfRBAx7m9lN/KHTn/kmn5+N7rMO?=
 =?us-ascii?Q?zVi6z6rozC3jalx8uxUXDR5//0BjAhZTp/yLRDDts35iz5ZI3uK1P8RavkBz?=
 =?us-ascii?Q?HEgRHmizQ79eyk9E4P/oSXnUIEOCHaPcdbavEIkMvI0gafwvBGsDHw87lpBq?=
 =?us-ascii?Q?Zr7VLmT0CKwUctx3nTa+h2MFoHCMNAqtWsWNjliVbjYR0YQ9rre0DiNRknQC?=
 =?us-ascii?Q?96pKdwVFxXXXVZ+eFOTxD4MlBNDFEQJPLIRL1UbckAblC9164JsxE8g0qyZo?=
 =?us-ascii?Q?9djGSWST6W0eP5oxtRTfeFytuS+UG5FzWyJYKaZ+RERIrTLtWJKgFyOHNRHm?=
 =?us-ascii?Q?y7wqtagfYBfd8gIq0T3fRb8Df7/6BZTg6L4UWJel8BnDmiIDAHJjyDN/9M66?=
 =?us-ascii?Q?iGGiP5OFOEYf6XdFV708nwFjrJduUy+wI5OX5n2pyGvduonGxucPrhOqybZl?=
 =?us-ascii?Q?XON3lVymzVPiORsDsAeJKTv18iwRBTadjC5rIyzWSHSnoINAGwwaa6fGFzVp?=
 =?us-ascii?Q?DfVaXXEl2TUKtd7zhkEWcntU0lkmqgufmtjMkBN/RSEfDwxhqn9hIjZ9ps1D?=
 =?us-ascii?Q?X/9JlDQ5l/9RD8NlWS8lhODoUsXYWnjjzA/V4qzHmXGQMahAzKTVc2YM22ev?=
 =?us-ascii?Q?Gfx9cmG18mdeEGXL7XJsli6y1fNumeuSH6jhJ34CRrVuRerSbHmmcl1PK4NK?=
 =?us-ascii?Q?RDJxnu/g6vrLe9GqTQ5ILAL4/9kt1EezO+PcbsxC1ir+ZDhS/Il+SCMWhW0l?=
 =?us-ascii?Q?8v0Qn+DytXVwpzfrKZZkH8qJgv0qo+IJ8pdBjV0YcnNfPZCccRfPaZHUf++D?=
 =?us-ascii?Q?r35T3bIOHDQ59j1NdlU9RAvsePE+l2xEsA3NDcKfSNEgCfMaFcZbk8InSstd?=
 =?us-ascii?Q?9NrEJ5LoZMCQjp4Pm9M+LBbd1w8z+62e0U2usE5cgZmDVe8AnNfqaN2/Hrxd?=
 =?us-ascii?Q?Xwv75TEEhbgTC8ZuIgN8g5EM+ymxoIIEJwJPGZAPaj3GbgWB6MEb8V66bmrW?=
 =?us-ascii?Q?cbm0a2hrIuxnftD3RP3zUOQjRWVNFPP5iASeBFZztxK6/ln065suSXFIh/P3?=
 =?us-ascii?Q?XuT6qRZNMhmPRjsOL4jd/Xg/F2Bdxh6XT/rFsfQIffqry91IyMfE6ByOfcPh?=
 =?us-ascii?Q?/SsT1z/asW1DTUFLNKJr2AqVpc1siKUtaOpTR/S7scGYZHu8rzwt47buCmxC?=
 =?us-ascii?Q?wKCkaxmDzfYoTIZcG+uNKULiXAO664uoakSQFA7pYLLPNSOrRae/mCNr97mZ?=
 =?us-ascii?Q?ZuCOkvL8SoA83N76C7Oycrahvsx2EjR7Gpnvit74LwsaU3egveriptnbthqE?=
 =?us-ascii?Q?m6s+YQJVaAqSLze1oNfhn7HKcRrQUqS99W5AoWMrBoZdGffuNH/tfFQGUI1x?=
 =?us-ascii?Q?xawl/z8sMxj52Jny03LS8ECwq2Uu/oMWBUzNrTxo9Lp/ZXeoMnNYCzpNc1PV?=
 =?us-ascii?Q?MTwjUGbxYlQJUKPOvgL0y3MIWb82sffwIm+wO77iE6Of4OPNA72I8RUQgcW0?=
 =?us-ascii?Q?TaFh3B4WRIGhoLmiG4nodWrwLn/XWBncLz49lPd0rOY69CEqtarsENE8dYPc?=
 =?us-ascii?Q?ED2wsVwGH4otTlJAT2OYpi6h5TQJL44wTqgywGDA?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 46845077-e973-4adb-3b50-08dc6d81d9c8
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 May 2024 04:06:05.3921
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: DW7RiQGCX7tiqEXXROHq8ntY5SSmUs7QycMd4mrwG/+mfYTckNJsRxWkrPMdlCcNazWZ+AIWX27nPZkx5dNczA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR11MB7867
X-OriginatorOrg: intel.com

Dan Williams wrote:
> Ira Weiny wrote:
> > Alison Schofield wrote:
> > > On Sun, Mar 24, 2024 at 04:18:08PM -0700, Ira Weiny wrote:
> > > > cxl_dpa_set_mode() checks the mode for validity two times, once outside
> > > > of the DPA RW semaphore and again within.
> > > 
> > > Not true.
> > 
> > Sorry for not being clear.  It does check the mode 2x but not for
> > validity.  I'll clarify.
> > 
> > > It only checks mode once before the lock. It checks for
> > > capacity after the lock. If it didn't check mode before the lock,
> > > then unsupported modes would fall through.
> > 
> > But we can check the mode 1 time and either check the size or fail.
> > 
> > > 
> > > > The function is not in a critical path.
> > > 
> > > Implying what here?  OK to check twice (even though it wasn't)
> > > or OK to expand scope of locking.
> > 
> > Implying that checking the mode outside the lock is not required.
> 
> The @mode check outside the lock is there to taking the lock when not
> necessary because the passed in mode is already bogus.

Sorry I meant to say 'is required'.

> 
> The lock is about making sure the write of cxled->mode relative to the
> state of the dpa partitions is an atomic check-and-set.
> 
> So this makes the function unconditionally take the lock when it might
> be bogus to do so. The value of reorganizing this is questionable.

Why would it be bogus?  I don't see that.  Regardless I dropped the patch as it
is not worth spending more time on.  There are bigger issues to resolve with
this series.

Ira