Received-SPF: pass (google.com: domain of linux-kernel+bounces-168484-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Fri, 3 May 2024 18:19:41 -0700
From: Dan Williams <dan.j.williams@intel.com>
To: 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: <66358d2dab5f9_1aefb294f1@dwillia2-mobl3.amr.corp.intel.com.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>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <6635366717079_e1f582941d@iweiny-mobl.notmuch>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?KVhBVdNBfhgilj+kwHuJpqqobQLJV84ZFej3uWu5W44sVnctDffRr0p/lpzP?=
 =?us-ascii?Q?YetEV8W0MBnEeNe3Er4Kqlt/kGe5R0x0dJoak3IUjRnsWSM5b7tpL7SE29XC?=
 =?us-ascii?Q?+e1JIJguEXwVzwVL+MHRjQ5j19UO27lgBATUYPO2Te1KbJTE87uDqileXybT?=
 =?us-ascii?Q?Mh3NG+BCw+Nups0I2WPELX7uDnjzvSo8FyRl7wO4XzH1eQQTjWcOPagyCvn+?=
 =?us-ascii?Q?jYoFtiGQZyvF37nRMGUEthQQCFPG4tNiJzA6pq1XZndTGf0l57sM8Z7SpLjT?=
 =?us-ascii?Q?QjHvvGUakZiJ50ywm/LiBb4VGT4qiN1Eg9faQ/ovNrR0m2yqQt9w19swCbPJ?=
 =?us-ascii?Q?21XfD6n7q+HPWkkLg8joh2zXKl9BgQdoBs8Zhy6FCBydsyaZCMz2Vc8WVryr?=
 =?us-ascii?Q?MFeag4gygj+6+PrY6nBDRRg1xbByKwLtKBB26t1l6m10qAmzjf2L4qccICXw?=
 =?us-ascii?Q?XqWrf9iGN9ojsRYMSsdLWhEvfrxPcTNvn1T3Y3XwMryAN8Oat4rcguh4Jd9n?=
 =?us-ascii?Q?7JH9Tn1SEnIJ0UTzMxYveG5QdSck1GsO6rUKVrlmlu5h3T+R+sJocdXyWWVa?=
 =?us-ascii?Q?9TJJsUXbVSIaSPu3thGH4ccNEC/wAjVRZ4a6p5FsjMiR6/c9vHeR7UghT2oW?=
 =?us-ascii?Q?M+kVXxdxAGqTM9ajnAlcSibpCAyy+CUSdUxCWKcW6sWkkk/OpW+PYlg1UEMy?=
 =?us-ascii?Q?gSHuiR/QCfZ6JVsc/YHNbqIiR28ekGpaYlfQuj6sCPnjID88L15HwdcomCh4?=
 =?us-ascii?Q?ggdD6fSIysxE2bHjzVvigr2zMsSFZXnRfrB0k5ILegNaumt6JI0vj3qb19NC?=
 =?us-ascii?Q?xPVejiq3DkTK11qRPUjux1wvLyUz72LzDCRUi5I1oWf/gKB+/ajSqzmrtx4h?=
 =?us-ascii?Q?Fj55zvvxx5L5VAhcVmMW/CSq5BuMYVCQ0kCeyKff6DjEcc6zwwev5Vg9uAiJ?=
 =?us-ascii?Q?bUKld3WGZmbohdPQmS9ySrexx5bbQo48Y5t4lsLJfACTwFmDvyCu3g/0CbMY?=
 =?us-ascii?Q?yO5ZseFrVpleOsndIsyyQTJuw48oDS07M5eg2hxC2QzBh+38SsAWRA6Oq/L0?=
 =?us-ascii?Q?9k37REH8au6AGQ5or+rF4ODChlfwBDuA6g7tvO+ksEGPUP2gwhe4OMxbbThy?=
 =?us-ascii?Q?JZ+NCKQb8JnrDkUeh7J5dPPrQX2m7yYpQt1Rb4F7AWPICJhGlrO+eveo3HZ6?=
 =?us-ascii?Q?0yWjhtFGCK/es8SZNQJlz4fMJxR/9uJSsuEfbxAZxnpLO8Gk2o/WDwLvPxjh?=
 =?us-ascii?Q?qfLPExp7He4Mq7CFzpXEybGiiTYPcIo78gsKpyr97lQowGEK4SUaRnHfeMsF?=
 =?us-ascii?Q?LgJRkgyMfmUzOSY92B7w2XtUE09kl6sY+MnKgH3I2CxXR/S7vDB8i+kTbfBZ?=
 =?us-ascii?Q?P0l5btre+C/ovR+QEJtgao8lW5LAcBAeY83n+rx/MMxpWU+j/JNnVDdTruaB?=
 =?us-ascii?Q?eCnjVa7CpNaPgRyxN706tX+tfiAEtEGcvzqVer1vcScxTtqebITxbUifWY+q?=
 =?us-ascii?Q?oOMc5V0ONOVmpaegUQ+nm81HRX8DfegGBkamtTzXS25Jxp7R+AasWPok+QUT?=
 =?us-ascii?Q?rrRkh6SBZvS48ZnZ9/SxpcaCYTNwTxSW80BYJ3VAxS+jGwQvsh57LdN5MtP9?=
 =?us-ascii?Q?jg=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: c470013b-50a8-42a9-c347-08dc6bd8479e
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB8107.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 May 2024 01:19:43.9791
 (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: 6sIm/kM4erUYpanMbCRJsCjuxhxU1f3hSwFvXUI2jXO/qa9MQndZdxTxOkkYCGe6bJO9Y9zppgOPcISwi+ufRjXwMQ324i7dO92bG1Bnoio=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB7418
X-OriginatorOrg: intel.com

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.

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.