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 A97ACCDD0CB for ; Tue, 22 Oct 2024 19:05:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EAA476B007B; Tue, 22 Oct 2024 15:05:57 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E7F606B0082; Tue, 22 Oct 2024 15:05:57 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D47E96B0083; Tue, 22 Oct 2024 15:05:57 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id BA0916B007B for ; Tue, 22 Oct 2024 15:05:57 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 270AEA040B for ; Tue, 22 Oct 2024 19:05:27 +0000 (UTC) X-FDA: 82702167240.19.9B84D62 Received: from mail.alien8.de (mail.alien8.de [65.109.113.108]) by imf16.hostedemail.com (Postfix) with ESMTP id 9EBE7180010 for ; Tue, 22 Oct 2024 19:05:38 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=alien8.de header.s=alien8 header.b=QCuIUCEj; dmarc=pass (policy=none) header.from=alien8.de; spf=pass (imf16.hostedemail.com: domain of bp@alien8.de designates 65.109.113.108 as permitted sender) smtp.mailfrom=bp@alien8.de ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1729623918; a=rsa-sha256; cv=none; b=Gxv3YU8fRrjVWjx8b8xjgfz49g6glAzWu9mHZzHQHqyKUM5A4UBll4geobjCLbpPtbQxMr tWal9NsNwMSJl6M4trIomIZaisEX2WWp8tm1XixhDD3LJiJeUV2dM7deo7lXw3+Ro+OBcC rjf0bk+LB8PyE0vBQv0WQkwzDhWT5GQ= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=alien8.de header.s=alien8 header.b=QCuIUCEj; dmarc=pass (policy=none) header.from=alien8.de; spf=pass (imf16.hostedemail.com: domain of bp@alien8.de designates 65.109.113.108 as permitted sender) smtp.mailfrom=bp@alien8.de ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1729623918; 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=ppGZ1xT0ZqqGN7Q64cHQWTKY4xHYzn5soCR639b+QDo=; b=qhElJ3qLN0xtVQZFxDfH+ZFiQLqtiHHM3YVAXhC2NAfn2FYHeIYCwJc6hn69OeM3aXMEbY +Sb8VtJiMojtMtEN/8c+Im/GKR5CpeD7ik9mZJmmY2A3eWMQcKRkg7UrP5w0eZqIMMsQX9 TTRGYram/tuKBpRiC9brlWhL10XBlhg= Received: from localhost (localhost.localdomain [127.0.0.1]) by mail.alien8.de (SuperMail on ZX Spectrum 128k) with ESMTP id B99F240E0198; Tue, 22 Oct 2024 19:05:49 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at mail.alien8.de Received: from mail.alien8.de ([127.0.0.1]) by localhost (mail.alien8.de [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id q3aQ9N_t_KX4; Tue, 22 Oct 2024 19:05:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alien8.de; s=alien8; t=1729623944; bh=ppGZ1xT0ZqqGN7Q64cHQWTKY4xHYzn5soCR639b+QDo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=QCuIUCEjsFCP18Md2JNqMqIn1FwuwkZ2vZnBwxxUmRu0qMG2llbF1LFHGb1ZpD46G vFcAVNROerVPlayap1OFa/NFoym1ZlY5A0idJcdPJk9XwpuedUSimvD2v4FzEVjU2C vF40FFj6uOHNBfH04Vn6nAfHNURxDBwssmQ9T+EYn498FqZpIrXoMMSsGH9t5X5WgH +i50AsUiLvbjF5vnDfaA/zDFtSkVp0rxR5wi2Peg8BfO4e05snsr4FzG1idjIHBuYx VueK1zwtMdEVkS3scEeGe3EnUa2ri4iQjyGrw2HifTOOkh5lUmr8HxFAJhR5LW2hTb o/6MnHpPXXGcNo4KwuZQVUK958nSPa0WKU6BB6DvTCEel7TJ+hcfWtzrzkD6oLN/OA IdR1y/BguKgE40mGAssz3hq1PrAk8bKPPSd7f44TetrU4yKX3K+oeKQA05VsjqHZo9 3tnheR6/o515Bu7XeH4kS2YPnqCuZmKrxyfKWkLjTfnn82P+Cncm3GtT6lj40yuTRy 0ZVLrjzGSM+LvUUFuYH/ooTeYOE9pf/zXqe5On30mJistYc+EC7ZDEluVHdvRkdoYr +2Lu8jmdp+go0EE3FBx6MP1E/DDbq7Bf6UfFPW3TXe1hkDIKDCM12gbXNEveoqV1xa oWAJYSJeKVcKHM9Y32+NwBN0= Received: from zn.tnic (p5de8e8eb.dip0.t-ipconnect.de [93.232.232.235]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature ECDSA (P-256) server-digest SHA256) (No client certificate requested) by mail.alien8.de (SuperMail on ZX Spectrum 128k) with ESMTPSA id 29F7340E015F; Tue, 22 Oct 2024 19:05:01 +0000 (UTC) Date: Tue, 22 Oct 2024 21:04:54 +0200 From: Borislav Petkov To: shiju.jose@huawei.com Cc: linux-edac@vger.kernel.org, linux-cxl@vger.kernel.org, linux-acpi@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, tony.luck@intel.com, rafael@kernel.org, lenb@kernel.org, mchehab@kernel.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, david@redhat.com, Vilas.Sridharan@amd.com, 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 v13 02/18] EDAC: Add scrub control feature Message-ID: <20241022190454.GIZxf3VkmLVR-JLeUc@fat_crate.local> References: <20241009124120.1124-1-shiju.jose@huawei.com> <20241009124120.1124-3-shiju.jose@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20241009124120.1124-3-shiju.jose@huawei.com> X-Rspam-User: X-Stat-Signature: dpc7rhxgyzkzztq9ebttript1wjq7agn X-Rspamd-Queue-Id: 9EBE7180010 X-Rspamd-Server: rspam02 X-HE-Tag: 1729623938-327434 X-HE-Meta: U2FsdGVkX18Dv4YeZMlOQYfG/qRw5hK6iOMoEvyc3dgT58cmSfLgkufkxv8k1omrBmTrLrYvQS+9reqIOcaUwEWfYpN81KMcHouv4qyclVeGAtZKpm6pO3G3MWbs7vGYYpqMijGIGT8u7ewoTnvlNAmxiFMhMX4h8gcZLeqCjgM/xMJfqVFU8zsp5hs8lmjES+ZPWpqosEI91xnVx+Oyr3MGjQXtI4vpx3qeYQ7ejDhuriPm9COYUUwYYT0tQZ7ia2nL0F/GQlfhWFMSUPCcpe3ktAjKhsDbHul6nEYySckUzeh2DX9TMzwLKgyx3QBurxbU2LR+tc0Si2Liiv7AA5rEMTr712YHw7kAfpNDh3MDuwJaI9m4M5n3CGe9uoda95QMJkzyZSWjdxPaoJQ1Bk8yS4jDA2b92dytzu36VQcfNS424AfSGB/K+JwRdUSC0rRuttjqjf5K+e6GdqVlzA2+J2Pbf2F/4WaC3tNPMpYfWTr8Ue1LKpp90kt9V2mg5HrQ3p9ojraK/VKEUpSxBXF03gHORY8Fiu93XebENB0kPWHt1k5zdXNbr2r748AX4+FoJzlQAcUHAtNf+fEB8UyVsdueVK9Gx0FHTL8fhNO4U5q6i1oajbNQD/4cBBhFOr++p3dkEshYXDqt2w5SMBctq5A56Uhqc39tpX3gEFg2TuwznrNzGdt8AplyKsdG/MAe9xHOvBVtGsunHOLdPAuv9fhHKGLVyCgSzMH0cUjKhwMlU72j7QLT8RiJjY/SZVUPzKYkma4A6LCkovc0W5xelPkRwVbNgJANLPD2GF8m6t3XSjOn+sMptB/BfKaxaNBHMGU5BcjDG6SaCgK0FZ6Fq4i1aWNoPxY7C9nuCU5ywfpwvZzIYQXA73cyJC5C9MQou3C5/gUzLlNcmJ+esRtcXsLWO5EATzUBqWt76wlN6WETvVbw32CmDvbUOc4/Z2J/0SxAH76QiSIJFpe CaQfKl1t l4fDG4IC79sHXkSM4CiW9D5DwhNr3qe5RVhgnAs5+ZHocsUqAy2Z2sekmwV1fhWWDQ93QhzKoUKcv/vT0jrWRIOGUecBh5AZ3fiyQJTrVDdKROaF/3ImUtEld5v73OGJooOkQt/zDXZGmRQa3EpyozadiVcU5RCogjk7K7nAASPEYIvnkmax5kFnRkT9TjfzSwm8Ha16gK/na2YdAlEnRdXLNZ6ZQZ/QCPOqMUPRzLAblnttaYx3DQ8xijJ6a79Sb0Wb2qZXfXzzFrHL6urwlzsQ/ys9c+KrPUtoe61L3OnNxW5fQ3RCa/GPFlvv/WqhPlGhu 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 Wed, Oct 09, 2024 at 01:41:03PM +0100, shiju.jose@huawei.com wrote: > diff --git a/Documentation/ABI/testing/sysfs-edac-scrub b/Documentation/ABI/testing/sysfs-edac-scrub > new file mode 100644 > index 000000000000..b4f8f0bba17b > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-edac-scrub > @@ -0,0 +1,69 @@ > +What: /sys/bus/edac/devices//scrubX > +Date: Oct 2024 > +KernelVersion: 6.12 > +Contact: linux-edac@vger.kernel.org > +Description: > + The sysfs EDAC bus devices //scrubX subdirectory > + belongs to an instance of memory scrub control feature, > + where directory corresponds to a device/memory > + region registered with the EDAC device driver for the > + scrub control feature. > + The sysfs scrub attr nodes would be present only if the > + client driver has implemented the corresponding attr > + callback function and passed in ops to the EDAC RAS feature > + driver during registration. > + > +What: /sys/bus/edac/devices//scrubX/addr_range_base > +Date: Oct 2024 > +KernelVersion: 6.12 > +Contact: linux-edac@vger.kernel.org > +Description: > + (RW) The base of the address range of the memory region > + to be scrubbed (on-demand scrubbing). Why does this sound more complicated than it is? Why isn't this simply "addr" and the next one "size"? On-demand scrubbing should scrub at address "addr" and "size" bytes. Can't get any simpler than that. > + > +What: /sys/bus/edac/devices//scrubX/addr_range_size > +Date: Oct 2024 > +KernelVersion: 6.12 > +Contact: linux-edac@vger.kernel.org > +Description: > + (RW) The size of the address range of the memory region > + to be scrubbed (on-demand scrubbing). > + > +What: /sys/bus/edac/devices//scrubX/enable_background > +Date: Oct 2024 > +KernelVersion: 6.12 > +Contact: linux-edac@vger.kernel.org > +Description: > + (RW) Start/Stop background(patrol) scrubbing if supported. > + > +What: /sys/bus/edac/devices//scrubX/enable_on_demand > +Date: Oct 2024 > +KernelVersion: 6.12 > +Contact: linux-edac@vger.kernel.org > +Description: > + (RW) Start/Stop on-demand scrubbing the memory region > + if supported. Why do you need a separate "enable" flag? Why can't it be: "writing into "addr" starts the on-demand scrubbing"? > + > +What: /sys/bus/edac/devices//scrubX/min_cycle_duration > +Date: Oct 2024 > +KernelVersion: 6.12 > +Contact: linux-edac@vger.kernel.org > +Description: > + (RO) Supported minimum scrub cycle duration in seconds > + by the memory scrubber. > + > +What: /sys/bus/edac/devices//scrubX/max_cycle_duration > +Date: Oct 2024 > +KernelVersion: 6.12 > +Contact: linux-edac@vger.kernel.org > +Description: > + (RO) Supported maximum scrub cycle duration in seconds > + by the memory scrubber. > + > +What: /sys/bus/edac/devices//scrubX/current_cycle_duration > +Date: Oct 2024 > +KernelVersion: 6.12 > +Contact: linux-edac@vger.kernel.org > +Description: > + (RW) The current scrub cycle duration in seconds and must be > + within the supported range by the memory scrubber. What are those three good for and why are they exposed? > diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile > index 4edfb83ffbee..a96a74de8b9e 100644 > --- a/drivers/edac/Makefile > +++ b/drivers/edac/Makefile > @@ -10,6 +10,7 @@ obj-$(CONFIG_EDAC) := edac_core.o > > edac_core-y := edac_mc.o edac_device.o edac_mc_sysfs.o > edac_core-y += edac_module.o edac_device_sysfs.o wq.o > +edac_core-y += scrub.o > > edac_core-$(CONFIG_EDAC_DEBUG) += debugfs.o > > diff --git a/drivers/edac/edac_device.c b/drivers/edac/edac_device.c > index 0b8aa8150239..0c9da55d18bc 100644 > --- a/drivers/edac/edac_device.c > +++ b/drivers/edac/edac_device.c > @@ -576,6 +576,7 @@ static void edac_dev_release(struct device *dev) > { > struct edac_dev_feat_ctx *ctx = container_of(dev, struct edac_dev_feat_ctx, dev); > > + kfree(ctx->scrub); > kfree(ctx->dev.groups); > kfree(ctx); > } > @@ -610,7 +611,9 @@ int edac_dev_register(struct device *parent, char *name, > const struct edac_dev_feature *ras_features) > { > const struct attribute_group **ras_attr_groups; > + struct edac_dev_data *dev_data; > struct edac_dev_feat_ctx *ctx; > + int scrub_cnt = 0, scrub_inst = 0; > int attr_gcnt = 0; > int ret, feat; The EDAC tree preferred ordering of variable declarations at the beginning of a function is reverse fir tree order:: struct long_struct_name *descriptive_name; unsigned long foo, bar; unsigned int tmp; int ret; The above is faster to parse than the reverse ordering:: int ret; unsigned int tmp; unsigned long foo, bar; struct long_struct_name *descriptive_name; And even more so than random ordering:: unsigned long foo, bar; int ret; struct long_struct_name *descriptive_name; unsigned int tmp; > > @@ -620,7 +623,10 @@ int edac_dev_register(struct device *parent, char *name, > /* Double parse to make space for attributes */ > for (feat = 0; feat < num_features; feat++) { > switch (ras_features[feat].ft_type) { > - /* Add feature specific code */ > + case RAS_FEAT_SCRUB: > + attr_gcnt++; > + scrub_cnt++; > + break; > default: > return -EINVAL; > } > @@ -639,13 +645,36 @@ int edac_dev_register(struct device *parent, char *name, > goto ctx_free; > } > > + if (scrub_cnt) { > + ctx->scrub = kcalloc(scrub_cnt, sizeof(*ctx->scrub), GFP_KERNEL); > + if (!ctx->scrub) { > + ret = -ENOMEM; > + goto groups_free; > + } > + } > + > attr_gcnt = 0; > for (feat = 0; feat < num_features; feat++, ras_features++) { > switch (ras_features->ft_type) { > - /* Add feature specific code */ > + case RAS_FEAT_SCRUB: > + if (!ras_features->scrub_ops) > + continue; Continue? I think fail. What is a scrub feature good for if it doesn't have ops? > + if (scrub_inst != ras_features->instance) > + goto data_mem_free; > + dev_data = &ctx->scrub[scrub_inst]; > + dev_data->instance = scrub_inst; > + dev_data->scrub_ops = ras_features->scrub_ops; > + dev_data->private = ras_features->ctx; > + ret = edac_scrub_get_desc(parent, &ras_attr_groups[attr_gcnt], > + ras_features->instance); > + if (ret) > + goto data_mem_free; > + scrub_inst++; > + attr_gcnt++; > + break; > default: > ret = -EINVAL; > - goto groups_free; > + goto data_mem_free; > } > } > ... -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette