From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9AA7CC19F32 for ; Fri, 7 Mar 2025 19:53:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 874096B007B; Fri, 7 Mar 2025 14:53:25 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 7FC486B0082; Fri, 7 Mar 2025 14:53:25 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 676C16B0083; Fri, 7 Mar 2025 14:53:25 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 42F466B007B for ; Fri, 7 Mar 2025 14:53:25 -0500 (EST) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 371C3B82C6 for ; Fri, 7 Mar 2025 19:53:26 +0000 (UTC) X-FDA: 83195804412.13.F9D96EE Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.12]) by imf20.hostedemail.com (Postfix) with ESMTP id 708071C000D for ; Fri, 7 Mar 2025 19:53:23 +0000 (UTC) Authentication-Results: imf20.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=HZWE5OXj; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf20.hostedemail.com: domain of alison.schofield@intel.com designates 192.198.163.12 as permitted sender) smtp.mailfrom=alison.schofield@intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1741377204; a=rsa-sha256; cv=none; b=GVmD8kDq5/tciGNuQTkHjalmtLugtZyEyu/RQQG20FPEa2mMwYNSuSy4KNjJUoqQGLweIs uWMKg44WCPlXBYJ1fZ2IuDpmOuP8Wei5BZzqiOBXSquWsXOOHhjaGIS4iVTcH7gWDCfPGG CLsRic6inV4BXarxk1r/etvvn54jXRA= ARC-Authentication-Results: i=1; imf20.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=HZWE5OXj; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf20.hostedemail.com: domain of alison.schofield@intel.com designates 192.198.163.12 as permitted sender) smtp.mailfrom=alison.schofield@intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1741377204; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=32xszHlYnQ9CUWwkkgM65zRYlkL8Fp4aXYUpmI+yOmc=; b=zVT4OgmskT6FbUoNNCij5P6sp8aKbnzKHthXZ9GfdWdQvcgh3UrV5/4llW40reGbyLx+93 XIXVM78KAz4WZlVvnkMgTRIZAGiqgJhlQ+0qfJXpkGOh7mwsJqkZYgowvBZWHyQSufASps HnR7PSCY9XBVZHIO+NREIp+niZ4EC+c= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1741377204; x=1772913204; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=jW13cb5LfXx1elX2HCWSeQ4XXS/LTqhw13X2BO7S/yM=; b=HZWE5OXjNUVat9+mbNh33yJ+3/Lkle7DCewTtBD+rFXo4eH1S276HeoY Jv8Y+Eo36hd5WxdTqkoS62tH1Qt9dqxoU/EG+PudZEySMm8TKa5GFWguf B0Jz+mea1xbRO17p2QsnJmMFX789gY+/lw9fgaSURBE9GTtXCXHlGLTCf qeR4XjIvzBTH2O+1HnS90VgtDL+Vj0fs9fTuI9n64bCMLypJAA+SJ6Lib oswXooYFEcfwodb+hcyldfpLD+KqDbE+1FGyrdB9/11U4VYKdohYOzrJ9 1e5/F6xqeX0YHHC8R3UbgRyo1gGLcmRoqhDYXIrsQZVdd5BALhouZdk4l g==; X-CSE-ConnectionGUID: be54QPqjQhOjGMFK5u+xaw== X-CSE-MsgGUID: Nt1zkViGSZytvrtaovSG8g== X-IronPort-AV: E=McAfee;i="6700,10204,11366"; a="46362200" X-IronPort-AV: E=Sophos;i="6.14,230,1736841600"; d="scan'208";a="46362200" Received: from orviesa004.jf.intel.com ([10.64.159.144]) by fmvoesa106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Mar 2025 11:53:22 -0800 X-CSE-ConnectionGUID: 2GvcInyMSbeO20ibsiAMuA== X-CSE-MsgGUID: fLxSYKcnScOnnm+4wTrZrA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.14,230,1736841600"; d="scan'208";a="124430688" Received: from aschofie-mobl2.amr.corp.intel.com (HELO aschofie-mobl2.lan) ([10.125.110.159]) by orviesa004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Mar 2025 11:53:21 -0800 Date: Fri, 7 Mar 2025 11:53:18 -0800 From: Alison Schofield To: shiju.jose@huawei.com Cc: linux-cxl@vger.kernel.org, dan.j.williams@intel.com, dave@stgolabs.net, jonathan.cameron@huawei.com, dave.jiang@intel.com, vishal.l.verma@intel.com, ira.weiny@intel.com, david@redhat.com, Vilas.Sridharan@amd.com, linux-edac@vger.kernel.org, linux-acpi@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, bp@alien8.de, tony.luck@intel.com, rafael@kernel.org, lenb@kernel.org, mchehab@kernel.org, leo.duran@amd.com, Yazen.Ghannam@amd.com, rientjes@google.com, jiaqiyan@google.com, Jon.Grimm@amd.com, dave.hansen@linux.intel.com, naoya.horiguchi@nec.com, james.morse@arm.com, jthoughton@google.com, somasundaram.a@hpe.com, erdemaktas@google.com, pgonda@google.com, duenwen@google.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, roberto.sassu@huawei.com, kangkang.shen@futurewei.com, wanghuiqiang@huawei.com, linuxarm@huawei.com Subject: Re: [PATCH 2/8] cxl/memfeature: Add CXL memory device patrol scrub control feature Message-ID: References: <20250227223816.2036-1-shiju.jose@huawei.com> <20250227223816.2036-3-shiju.jose@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250227223816.2036-3-shiju.jose@huawei.com> X-Rspamd-Queue-Id: 708071C000D X-Rspamd-Server: rspam11 X-Stat-Signature: 6zqdn3g5x8w6cn65su4oxc45ntz8fhza X-Rspam-User: X-HE-Tag: 1741377203-345744 X-HE-Meta: U2FsdGVkX18sYAqb7dMPwUl+M/VOS5Y9lHshEHJykx8yui7DRdACicJgDPW9C6xF6wXY8bORUj96TpmMSyGJUYufErYknbUaLTcNt17BGSLgPFLg4bqBhXffzl9mPxmmFFqYOjSnHZ4b2FB9mhH/wNykR7FMyhFly4AUJEQtcc4nPL3vsxDY5bDa9pB5MLlTQ70oN2cgpFy8czdu3p+h7m17KKCHOrs9L4YDSZBWir1JL2zM4mM/el66aIOrYVhMJLyKqJLYBfmvIy55GgfiYqZbllLUmUi0Omenk5IXXNyVaKC2Zd+L/Ij0VMOLoqNmIVYvhyyaIVBIZssEZMdkoeIwhgu337GzsGMsnNdL8InxcON+hwO2mNyFbxlQ1asWK98kEM03r44R1/12GJsz0mz4sA4N0Pe//otmjBLy9QriU/r/WmI124zLZEfV1Lh8LiKsuUCU6KQrNoKR4TlCOQqZIgSXG4qgr13sQAtg7ip7b4OfqtzgcSNrM6kT+ju88CckF+LZ4dim9c3AE+m6NbUOqjLVh809CXVfVeOMczlgu1ZG4DBFBnyrzZUQY6OJxxmyP9YPZZ2jUMBLbHlwWBLuZ5TWk8kgKjYWtTKzkOYXco0q2v6FsHTbORp1i7s886onlQh/cs/ffXSda90AtMubiW1lBJDxnpRsFvD9Jp6656s3v8TnQ8SGqzAmFtezrsXG/43o4gE5wGrZep8HdXYnyrxsdlSfPFIQrEP0If5rCGdqW9p3AqmuoRUc57GDx4NNYUWWM/lOiDrkqoDsdKf3m739c9P4c62Q3psL8zBGJSUnFBapA2wqB7jSOtq25WTSC2de7YnYm3K9+yYQo5hxm//teaEK8XCecth/1644/nlHtBXUIwPv0CC6H9qWgRTo+fYoDagtwU1HbC2JwjKOzBYrI7dTs1L1chnm0SV14NCSrRrKDPvAfWHlvtfwtSkDfgqq3xRhcx2EEvk Z1/6VAEF 7I1ILjkrUeMv1S2LvN6SR8aAe6h5vBo3I3AtdYO3QhdkzPqlvrLDZFJJ8nBhMz8sVTBW/tFdX0kOOkTpwCZVOG7l1EBJYadrrGy2mVFOvNU+YIVizWhaIgCfZK9ZpnCBH9qFa5YdRIMv5sqRdCms9Vi/O1oM+0tWPF/z2BJ641ml57SlpYFYdIZ+49Y12pf8e7NyiQ3UVyqpZd0oWL6iy7x8lTHzwppdYH8OLrQGXgDOuikQ77zq5OEAEFUFG7btv0kpg X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Thu, Feb 27, 2025 at 10:38:09PM +0000, shiju.jose@huawei.com wrote: > From: Shiju Jose snip > > +static int cxl_ps_get_attrs(struct cxl_patrol_scrub_context *cxl_ps_ctx, > + struct cxl_memdev_ps_params *params) > +{ > + struct cxl_mailbox *cxl_mbox; > + struct cxl_memdev *cxlmd; > + u16 min_scrub_cycle = 0; > + int i, ret; > + > + if (cxl_ps_ctx->cxlr) { > + struct cxl_region *cxlr = cxl_ps_ctx->cxlr; > + struct cxl_region_params *p = &cxlr->params; > + > + ret = cxl_hold_region_and_dpa(); > + if (ret) > + return ret; > + for (i = p->interleave_ways - 1; i >= 0; i--) { > + struct cxl_endpoint_decoder *cxled = p->targets[i]; > + Hi Shiju, Although not functionally wrong, the above for loop caught my eye. p->nr_targets is a better loop initializer when walking p->targets[] (at this point nr_targets better equal interleave ways but lets use the count intended for the array being walked) Is there a reason to walk in reverse? This seems clearer: for (i = 0; i < p->nr_targets; i++) The same for loop header repeats below in: cxl_ps_set_attrs() cxl_region_scrub_init() If you change for one, change for all. snip > +static int cxl_memdev_scrub_init(struct cxl_memdev *cxlmd, > + struct edac_dev_feature *ras_feature, u8 scrub_inst) > +{ > + struct cxl_patrol_scrub_context *cxl_ps_ctx; > + struct cxl_feat_entry *feat_entry; > + > + feat_entry = cxl_get_feature_entry(cxlmd->cxlds, &CXL_FEAT_PATROL_SCRUB_UUID); > + if (IS_ERR(feat_entry)) > + return -EOPNOTSUPP; > + Along w patch 1 comment, perhaps just check for (!feat_entry) here and in a couple of other places below. snip >