From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Luka Perkov <luka.perkov@sartura.hr>,
Robert Marko <robert.marko@sartura.hr>,
Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
<linux-kernel@vger.kernel.org>,
Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
Rafael J Wysocki <rafael@kernel.org>,
Miquel Raynal <miquel.raynal@bootlin.com>
Subject: [PATCH v3 2/4] sysfs: Skip empty folders creation
Date: Mon, 5 Jun 2023 15:34:20 +0200 [thread overview]
Message-ID: <20230605133422.45840-3-miquel.raynal@bootlin.com> (raw)
In-Reply-To: <20230605133422.45840-1-miquel.raynal@bootlin.com>
Most sysfs attributes are statically defined, the goal with this design
being to be able to move all the filesystem description into read-only
memory. Anyway, it may be relevant in some cases to populate attributes
at run time. This leads to situation where an attribute may or may not be
present depending on conditions which are not known at compile
time, up to the point where no attribute at all gets added in a folder
which then becomes "sometimes" empty. Problem is, providing an attribute
group with a name and without .[bin_]attrs members will be loudly
refused by the core, leading in most cases to a device registration
failure.
The simple way to support such situation right now is to dynamically
allocate an empty attribute array, which is:
* a (small) waste of space
* a waste of time
* disturbing, to say the least, as an empty sysfs folder will be created
anyway.
Another (even worse) possibility would be to dynamically overwrite a
member of the attribute_group list, hopefully the last, which is also
supposed to remain in the read-only section.
In order to avoid these hackish situations, while still giving a little
bit of flexibility, we might just check the validity of the .[bin_]attrs
list and, if empty, just skip the attribute group creation instead of
failing. This way, developers will not be tempted to workaround the
core with useless allocations or strange writes on supposedly read-only
structures.
The content of the WARN() message is kept but turned into a debug
message in order to help developers understanding why their sysfs
folders might now silently fail to be created.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
Hello Greg,
This is the solution I opted for to avoid core splats when creating
empty sysfs directories. The nice side effect is that the directories do
not even get created which seem the right approach in this case and
avoids to deal with any kind of cleanup either (or maybe I overlooked
something?). Let me know what you think of it. This is a try of course,
perhaps we need something way more robust, but at a first look it seemed
perfectly consistent.
Thanks,
Miquèl
fs/sysfs/group.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
index 990309132c93..138676463336 100644
--- a/fs/sysfs/group.c
+++ b/fs/sysfs/group.c
@@ -118,11 +118,13 @@ static int internal_create_group(struct kobject *kobj, int update,
/* Updates may happen before the object has been instantiated */
if (unlikely(update && !kobj->sd))
return -EINVAL;
+
if (!grp->attrs && !grp->bin_attrs) {
- WARN(1, "sysfs: (bin_)attrs not set by subsystem for group: %s/%s\n",
- kobj->name, grp->name ?: "");
- return -EINVAL;
+ pr_debug("sysfs: (bin_)attrs not set by subsystem for group: %s/%s, skipping\n",
+ kobj->name, grp->name ?: "");
+ return 0;
}
+
kobject_get_ownership(kobj, &uid, &gid);
if (grp->name) {
if (update) {
--
2.34.1
next prev parent reply other threads:[~2023-06-05 13:34 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-05 13:34 [PATCH v3 0/4] NVMEM cells in sysfs Miquel Raynal
2023-06-05 13:34 ` [PATCH v3 1/4] sysfs: Improve readability by following the kernel coding style Miquel Raynal
2023-06-05 13:34 ` Miquel Raynal [this message]
2023-06-05 13:34 ` [PATCH v3 3/4] ABI: sysfs-nvmem-cells: Expose cells through sysfs Miquel Raynal
2023-06-05 13:34 ` [PATCH v3 4/4] nvmem: core: " Miquel Raynal
2023-06-06 10:31 ` Srinivas Kandagatla
2023-06-09 16:05 ` Miquel Raynal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230605133422.45840-3-miquel.raynal@bootlin.com \
--to=miquel.raynal@bootlin.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luka.perkov@sartura.hr \
--cc=rafael@kernel.org \
--cc=robert.marko@sartura.hr \
--cc=srinivas.kandagatla@linaro.org \
--cc=thomas.petazzoni@bootlin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®