Received-SPF: pass (google.com: domain of linux-kernel+bounces-175176-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Thu, 9 May 2024 17:26:46 -0700
From: Dan Williams <dan.j.williams@intel.com>
To: <shiju.jose@huawei.com>, <linux-cxl@vger.kernel.org>,
	<linux-acpi@vger.kernel.org>, <linux-mm@kvack.org>,
	<dan.j.williams@intel.com>, <dave@stgolabs.net>,
	<jonathan.cameron@huawei.com>, <dave.jiang@intel.com>,
	<alison.schofield@intel.com>, <vishal.l.verma@intel.com>,
	<ira.weiny@intel.com>
CC: <linux-edac@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<david@redhat.com>, <Vilas.Sridharan@amd.com>, <leo.duran@amd.com>,
	<Yazen.Ghannam@amd.com>, <rientjes@google.com>, <jiaqiyan@google.com>,
	<tony.luck@intel.com>, <Jon.Grimm@amd.com>, <dave.hansen@linux.intel.com>,
	<rafael@kernel.org>, <lenb@kernel.org>, <naoya.horiguchi@nec.com>,
	<james.morse@arm.com>, <jthoughton@google.com>, <somasundaram.a@hpe.com>,
	<erdemaktas@google.com>, <pgonda@google.com>, <duenwen@google.com>,
	<mike.malvestuto@intel.com>, <gthelen@google.com>,
	<wschwartz@amperecomputing.com>, <dferguson@amperecomputing.com>,
	<wbs@os.amperecomputing.com>, <nifan.cxl@gmail.com>, <tanxiaofei@huawei.com>,
	<prime.zeng@hisilicon.com>, <kangkang.shen@futurewei.com>,
	<wanghuiqiang@huawei.com>, <linuxarm@huawei.com>, <shiju.jose@huawei.com>
Subject: Re: [RFC PATCH v8 05/10] cxl/memscrub: Add CXL device patrol scrub
 control feature
Message-ID: <663d69c61db8c_3d7b4294e0@dwillia2-mobl3.amr.corp.intel.com.notmuch>
References: <20240419164720.1765-1-shiju.jose@huawei.com>
 <20240419164720.1765-6-shiju.jose@huawei.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240419164720.1765-6-shiju.jose@huawei.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?nRKtxp9cIUh9ttLQus6XAqc9HEgyHBRx83D6Bb7yc7vTJBXJ8qbxXKs7Fw17?=
 =?us-ascii?Q?Y1CYtGYyyeTTt4Vx4Y0sSqSJUxrGwaJsleGwHGFpPrI6svvVxmhmNTta6rL4?=
 =?us-ascii?Q?cvOonhZR2Vf57zEIizMI7biEmcEhvA/UgTPhB1rhvAbw8xH9ARjxqKdBZVCU?=
 =?us-ascii?Q?fJTiQTOqN9SxMof6/9CIyvOS0GzECUk2ESOZkHr22AIT3GvKArmP3HknFMAd?=
 =?us-ascii?Q?8BJnUrTFnwqfq0CVRjGZhmu91ZWlngNnBjbovKKtLvb5B1/86vqPK/qKHy2I?=
 =?us-ascii?Q?+3vlxxOg4FD4S5JLqXTfkoKsLBfdo2ihmQKTspCCktwFByBYhvQQnEL8jhiM?=
 =?us-ascii?Q?HYmrAD2nPWB7bT0Ft3nXDe52muEhdkheJEeR0GPL9fkdXpszyTPW8MzaIqbc?=
 =?us-ascii?Q?J9Z73hnwMe9miMuD9XMdNqNBf/6CfGFaCKkwZYcqTHqY7Jo2JphifxlUviyl?=
 =?us-ascii?Q?eXj6LYTsppdL2pWghfZ2ZWwN5gCvOmtf/s7onqY39/U3RippWuD4/p4+m+bl?=
 =?us-ascii?Q?r92D0+YMjt9XshGvEO0ba+7gxaT2kjhHS25bM29G7M42chzTYvczttzaVK+4?=
 =?us-ascii?Q?forL9Wsk1+PnMf7t9jzBGZyc6W/AtI55GelfVn4fr1EDwacvGHV62utZeTBF?=
 =?us-ascii?Q?yh61XDVmk6ZWUn034lSjmUVf7OgYD0hz9yfQ70U+3fOZZ4VZzoMHqgdnSTWa?=
 =?us-ascii?Q?PoXMJ4oxf6tYglSnauXYGLizb/sGN1sNj2l4ybqZ4SCvCJL8SsI7lWU6yDXA?=
 =?us-ascii?Q?bzw/P4jkbwRhLesUFEPdC1qkk+/FJ0QotkUvv5RBCYmCREgd9bOaP0O1uhIS?=
 =?us-ascii?Q?6g4s8X1VdA3DJujkzwkxwD3UzhMiVHLqr++KQRzZBMp7pfrAlSgkU1jABdBt?=
 =?us-ascii?Q?qSk7y8Hh753VIVzlwRcF3Ho0uNT8i2Ne3UwjaA46FiRD4cecO+GcRgxqYIiU?=
 =?us-ascii?Q?KSir3YchTbCTGZhKdgRmgu9zphz25uig0QS1wsO8pytVXzXd7A0q7J9LbkPU?=
 =?us-ascii?Q?PQcC8/YQygeN476a6PYmbejgmBziSmuMws7i8/LbsBCI2BwktWxJYGWTNjSu?=
 =?us-ascii?Q?g0gDDuWMz8RhgYlfrE1xWdMKsjGVkUdIWgUVdxcqCMnY7JiBrjG5F9wU0Gyw?=
 =?us-ascii?Q?875qWxSXLrF5dJY8uwxFzuRPySyRldRBh2XVFxfpRINZJaGIRYQ8zRq/A/NL?=
 =?us-ascii?Q?R68n62G23Dwgrr52sB0RMBa0AThj7VOIVGAM35rS/AhL9xiJOPkHSpd13CG+?=
 =?us-ascii?Q?8kXetiVYvQJgOGw0uBPtsHOS1fnAAKa78oS/mDNIVQiieKSh/djarMaYDRjN?=
 =?us-ascii?Q?sDb/7FwZMtmcKJrrWFUcS8rZpjXJXFVutsCG+Eu9+AXyENr7rhbT5PjZaEmk?=
 =?us-ascii?Q?SCA6fcdL0mPLqM7gIASiPP84wMST6p0XX67yck7cp6pU7yi1PuPezQhvE4bl?=
 =?us-ascii?Q?xnZnFEasrxFIO+SEX4tsWwEtv4+8fAO13SybvJ3PrxWBBbBdz6CMkap+oczl?=
 =?us-ascii?Q?7lkQD+yIAf9P2ryP1rwqiGsp3BLDQ4q53z16GfzkIz/XdKNtdu9zh9KGujtY?=
 =?us-ascii?Q?aHUotKAvYcNFRfFsL6wf+hPhhi4IXxxVRWHV3QEvQL1tBnwliV3MwnElW7BZ?=
 =?us-ascii?Q?KQ=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: d8d06172-a345-4e3c-fcde-08dc7087e29b
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB8107.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 May 2024 00:26:50.5864
 (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: NbTBOJmrBm2lUuN6fCk3Cwh7Yjw23Mb4nrMKhVAQ5UG7zJWEygAA7hyXtlucly2O7/vPSXo56PGZv1wt0d6m/mkhT8gt55CJ2MWbeHknr2s=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB8490
X-OriginatorOrg: intel.com

shiju.jose@ wrote:
> From: Shiju Jose <shiju.jose@huawei.com>
> 
> CXL spec 3.1 section 8.2.9.9.11.1 describes the device patrol scrub control
> feature. The device patrol scrub proactively locates and makes corrections
> to errors in regular cycle.
> 
> Allow specifying the number of hours within which the patrol scrub must be
> completed, subject to minimum and maximum limits reported by the device.
> Also allow disabling scrub allowing trade-off error rates against
> performance.
> 
> Register with scrub subsystem to provide scrub control attributes to the
> user.
> 
> Co-developed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
[..]
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 0c79d9ce877c..399e43463626 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -117,6 +117,12 @@ static int cxl_mem_probe(struct device *dev)
>  	if (!cxlds->media_ready)
>  		return -EBUSY;
>  
> +	rc = cxl_mem_patrol_scrub_init(cxlmd);
> +	if (rc) {
> +		dev_dbg(&cxlmd->dev, "CXL patrol scrub init failed\n");
> +		return rc;
> +	}

2 concerns:

* Why should cxl_mem_probe() fail just because this optional
  scrub interface did not register?

* Why is this not located in cxl_region_probe()? If the ras2 scrub is an
  HPA-based scrub I think CXL should do the work to interface with the scrub
  interface at the same level. This also provides another in-kernel user
  for all the DPA-to-HPA translation infrastructure that the CXL driver
  contains. Pretty much the only reason the CXL driver needs to exist at
  all is address translation, so at a minimum it seems a waste to inflict
  more need to understand DPAs on userspace.