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 61E08C25B77 for ; Wed, 22 May 2024 09:40:28 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E99246B009B; Wed, 22 May 2024 05:40:27 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E48B66B009C; Wed, 22 May 2024 05:40:27 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D11306B009E; Wed, 22 May 2024 05:40:27 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id B23526B009B for ; Wed, 22 May 2024 05:40:27 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 332DAC01E5 for ; Wed, 22 May 2024 09:40:27 +0000 (UTC) X-FDA: 82145536494.03.3DE27C5 Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by imf29.hostedemail.com (Postfix) with ESMTP id 53ED3120019 for ; Wed, 22 May 2024 09:40:24 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf29.hostedemail.com: domain of jonathan.cameron@huawei.com designates 185.176.79.56 as permitted sender) smtp.mailfrom=jonathan.cameron@huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1716370825; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=W2fLomzPUYamb3nB8HJDUhg0rPXLSZPRk7kUd6AQ8HU=; b=GSknC6gW+hD40LEcU2myOl3zGcQDNFKSnraPL1FNl7DdY6A56lKHPvIO6g8NJV0kJRfQ4w brWFtkJrEt8uyDeMPTGq22kvcI+5ZmeLdHilCYmjen+05B9cx+v3TlW8xqKcbRh0sn/4Qv oO4qq5+1vDycY0UuyVnKIIqPAArdAZ0= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf29.hostedemail.com: domain of jonathan.cameron@huawei.com designates 185.176.79.56 as permitted sender) smtp.mailfrom=jonathan.cameron@huawei.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1716370825; a=rsa-sha256; cv=none; b=i8XgCrsiqbaOesGLtV0+utuOp8efRQis8FZfiPl1vKwVc62i5BQP8deCBZcR8OFhjWAcSL b3bmTziAG8jufvXRTZq27QXpN+H7cJMlnPGmu44I7ksnNlbRFgq8lbRbFBM8STVUegd5K2 Wy1lAmgQr/Yfn+QxPhvqRTAdIBI+f80= Received: from mail.maildlp.com (unknown [172.18.186.216]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4VkmQw2rK4z6J9Sj; Wed, 22 May 2024 17:36:40 +0800 (CST) Received: from lhrpeml500005.china.huawei.com (unknown [7.191.163.240]) by mail.maildlp.com (Postfix) with ESMTPS id 44E6D140A70; Wed, 22 May 2024 17:40:19 +0800 (CST) Received: from localhost (10.202.227.76) by lhrpeml500005.china.huawei.com (7.191.163.240) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Wed, 22 May 2024 10:40:18 +0100 Date: Wed, 22 May 2024 10:40:17 +0100 From: Jonathan Cameron To: Borislav Petkov CC: Dan Williams , Shiju Jose , "linux-cxl@vger.kernel.org" , "linux-acpi@vger.kernel.org" , "linux-mm@kvack.org" , "dave@stgolabs.net" , "dave.jiang@intel.com" , "alison.schofield@intel.com" , "vishal.l.verma@intel.com" , "ira.weiny@intel.com" , "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 , "Zengtao (B)" , "kangkang.shen@futurewei.com" , wanghuiqiang , Linuxarm , "Greg Kroah-Hartman" , Jean Delvare , Guenter Roeck , Dmitry Torokhov Subject: Re: [RFC PATCH v8 01/10] ras: scrub: Add scrub subsystem Message-ID: <20240522104017.00003904@Huawei.com> In-Reply-To: <20240521080621.GBZkxV_ZWnbbrq-yV_@fat_crate.local> References: <20240509200306.GAZj0r-h5Tnc0ecIOz@fat_crate.local> <663d3e58a0f73_1c0a1929487@dwillia2-xfh.jf.intel.com.notmuch> <20240509215147.GBZj1Fc06Ieg8EQfnR@fat_crate.local> <663d55515a2d9_db82d2941e@dwillia2-xfh.jf.intel.com.notmuch> <20240510092511.GBZj3n9ye_BCSepFZy@fat_crate.local> <663e55c59d9d_3d7b429475@dwillia2-mobl3.amr.corp.intel.com.notmuch> <20240511101705.GAZj9FoVbThp7JUK16@fat_crate.local> <20240517121554.000031d4@Huawei.com> <20240517124418.00000b48@Huawei.com> <20240521080621.GBZkxV_ZWnbbrq-yV_@fat_crate.local> Organization: Huawei Technologies Research and Development (UK) Ltd. X-Mailer: Claws Mail 4.1.0 (GTK 3.24.33; x86_64-w64-mingw32) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.202.227.76] X-ClientProxiedBy: lhrpeml100005.china.huawei.com (7.191.160.25) To lhrpeml500005.china.huawei.com (7.191.163.240) X-Rspamd-Queue-Id: 53ED3120019 X-Stat-Signature: yp9g1wiwebpybriurwp5n7ra6nnqsbui X-Rspam-User: X-Rspamd-Server: rspam11 X-HE-Tag: 1716370824-794962 X-HE-Meta: U2FsdGVkX1+PdDYN7Nl6qbpMr9NCGuCKutvoinHTL8m+b3+atZmHPptjzXv5uCeZgQ9MTr+x18WD1e6U5VWDsbhCFTnJSk0C0jFgRllcGZTGEUfsRRl/88jWRAqQ+q8feLX0+u46A/h5O1EWxOp8iq4YPHDT/V2bTyUsAoW9Xjzf21x3bPWoMxxYuoFtBStNVBPOnSIEjqZ56GxM/YT1LhCx9CUD/WSfsv+8xHjxFkNStm4aUjCzSXoEZmwdTAZmlUN6bMie+vcI1K7qg67WkzZ0t+9RhWSkFIkNjwZyF+q3IMvrm3BKXETKcIqDh6/kI2Oa9IFvL/GNDhtMonn05lFzma+sWkWDkqP5BccFH+QmnwiGf9/WMXUGK58wUsR35wbebBMQ9YpGgSN8DBaODzM6KtUPcVxQ42v6TFTmyksc2jrQ1ZmZCKX8ZmpXzp/+OT5QdUeJ5wiFhBpE8ZQU7pZb+B+SU0u5NBgDLbBuoVoYG4qBeBAyuW1t8mfP9EtKKcQ9gsHid92eUnKYTc5U8Mqa5sR13R4nhbB1gAFTLbbtc8tFaDk4TF1fuYC9Rj5bun/wgbMC3BQ43ViMNw8dmOJlaA2Y0eDJqq9EF0lq82lzMGsK7EYDmtt5VX2/vQBPMyy6EWHtJsXBNR23TzX6yo1/Cx710FVjxDRWUaHXunjqjXGuXm0ymEMQAJar0OMOMtXQV3+JGJtUDfXquAPpnx5rDHWNKN9jnWohJ5OWoGFiybPzyNS3FxTz2Q3vP7NigvHzyhhBg269hhQBT+CTQ/CmfToxIKg7mIsiKk5fJiEVO3nEhqwm2XN9RNimxzAV7X/Hayi1BUZrCOdjpFMGeDPH3WU2t3qe+kHgACOtlhXb9mqCswQSUtkHNZJRIsCRwFznBgy6fqUaZLw6cQM/B3aFeOm+iDsZHzOkSvKhhXbXTWW/Kq/AnF6IE/S9RkaM/25v5vhGBrG5TQZdJ1b GxG+Xq89 OcJ8ZaHyWUoiStaHAvqZZCJGv4mu6htNbt/bpuYT7N3PXEb8LWbsqUSxWJLNn3ZDskCza4k/2gnRH1o7HYwc38MIQ4TMUrQrpJqMKZRin6wmrQVvcfWBttxFyGyekEKAofXv49drBu8qpX4trPEnxnqAHD0Er/hQyHVrvHRhHQvEYCVr2ZUKa6ho24loJClw1581mcj9sD9BVicVJ7f51mp5wUExeNGmr0wRTAmu4/l+K508ZYy37u6/OQ+c5gKQv3vBEmAOWhVEL8pqRqzwHTrYp4QzlSxxeOZMbMM+Wky2//8ZyWXBhY26kIiIkkGcZqAg3 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 Tue, 21 May 2024 10:06:21 +0200 Borislav Petkov wrote: > On Fri, May 17, 2024 at 12:44:18PM +0100, Jonathan Cameron wrote: > > Given we are talking about something new, maybe this is an opportunity > > to not perpetuate this? > > > > If we add scrub in here I'd prefer to just use the normal bus registration > > handling rather than creating a nest of additional nodes. So perhaps we > > could consider > > /sys/bus/edac/device/scrub0 (or whatever name makes sense, as per the > > earlier discussion of cxl_scrub0 or similar). > > Yes, my main worry is how this RAS functionality is going to be all > organized in the tree. Yes, EDAC legacy methods can die but the > user-visible part can't so we might as well use it to concentrate stuff > there. Understood. > > > Could consider moving the bus location of mc0 etc in future to there with > > symlinks to /sys/bus/edac/device/mc/* for backwards compatibility either > > via setting their parents or more explicit link creation. > > You can ignore the mc - that's the memory controller representation EDAC > does and that's also kind of semi-legacy considering how heterogeneous > devices are becoming. Nowadays, scrubbing functionality can be on > anything that has memory and that's not only a memory controller. > > So it would actually be the better thing to abstract that differently > and use .../edac/device/ for the different RAS functionalities. I.e., > have the "device" organize it all. I'm not sure I follow this. Definitely worth ensuring we are thinking the same thing wrt to layout before we go further, Do you mean keep it similar to the existing device/mc device/pci structure so /sys/bus/edac/devices/scrub/cxl_mem0_scrub etc? This would rely on symlinks to paper over the dev->parent not being the normal parent. Hence would be similar to /sys/bus/edac/devices/pci in edac_pci_create_sysfs() or equivalent in edac_device_create_sysfs(). Or is the ../edac/device bit about putting an extra device under edac/devices/? e.g. /sys/bus/edac/devices/cxl_memX/scrub /sys/bus/edac/devices/cxl_memX/other_ras_thing which would be fairly standard driver model stuff. This would sit alongside 'legacy' /sys/bus/edac/devices/mc/mcX /sys/bus/edac/devices/pci/pciX etc I'd prefer this second model as it's very standard and but grouping is per providing parent device, rather than functionality. However, it is rather different from the existing edac structure. Where I've used the symlink approach in the past, it has always been about keeping a legacy interface in place, not where I'd start with something new. Hence I think this is a question of how far we 'breakaway' from existing edac structure. > > > These scrub0 would have their dev->parent set to who ever actually > > registered them providing that reference cleanly and letting all the > > normal device model stuff work more simply. > > Ack. This suggests the second option above, but I wanted to confirm as Shiju and I read this differently. > > > If we did that with the scrub nodes, the only substantial change from > > a separate subsystem as seen in this patch set would be to register > > them on the edac bus rather than a separate class. > > > > As you pointed out, there is a simple scrub interface in the existing > > edac memory controller code. How would you suggest handling that? > > Have them all register an additional device on the bus (as a child > > of the mcX devices) perhaps? Seems an easy step forwards and should > > be no backwards compatibility concerns. > > Well, you guys want to control that scrubbing from userspace and those > old things probably do not fit that model? We could just not convert > them for now and add them later if really needed. I.e., leave sleeping > dogs lie. Ok. There is an existing is the minimal sysfs existing interface but I'm fine with ignoring it for now. > > > It absolutely doesn't as long as we can do it fairly cleanly within > > existing code. I wasn't sure that was possible, but you know edac > > a lot better than me and so I'll defer to you on that! > > Meh, I'm simply maintaining it because no one else wants to. :) *much sympathy!* As we ramp up more on this stuff, we'll try and help out where we can. > > > Several options for that, but fair question - bringing (at least some of) > > the RAS mess together will focus reviewer bandwidth etc better. > > Review is more than appreciated, as always. > > > I'm definitely keen on unifying things as I agree, this mixture of different > > RAS functionality is a ever worsening mess. > > Yap, it needs to be unified and reigned into something more > user-friendly and manageable. Hopefully we all agree on a unified solution being the target. Feels like we are converging. Now we are down to the details :) Thanks, Jonathan > > Thx. >