Re: [PATCH -next v3 1/3] lib/string_choices: Add str_true_false()/str_false_true() helper

[Andrew Morton <akpm@linux-foundation.org>]

On Wed, 28 Aug 2024 11:49:51 +0800 Hongbo Li <lihongbo22@huawei.com> wrote:

> > Anything which is calling these functions is not performance-sensitive,
> > so optimizing for space is preferred.  An out-of-line function which
> > returns a const char * will achieve this?
> I think this helper can achieve this. Because it is tiny enough, the 
> compiler will handle this like #define macro (do the replacement) 
> without allocating extra functional stack. On the contrary, if it is 
> implemented as a non-inline function, it will cause extra functional 
> stack when it was called every time. And it also should be implemented 
> in a source file (.c file), not in header file(.h file).

No, my concern is that if, for example, str_high_low() gets used in 100
.c files then we get 100 copies of the strings "high" and "low" in
vmlinux.  Making str_high_low() uninlined would fix this.

However a quick experiment tells me that the compiler and linker are
indeed able to perform this cross-object-file optimization:

--- a/fs/open.c~a
+++ a/fs/open.c
@@ -34,6 +34,8 @@
 #include <linux/mnt_idmapping.h>
 #include <linux/filelock.h>
 
+#include <linux/string_choices.h>
+
 #include "internal.h"
 
 int do_truncate(struct mnt_idmap *idmap, struct dentry *dentry,
@@ -42,6 +44,8 @@ int do_truncate(struct mnt_idmap *idmap,
 	int ret;
 	struct iattr newattrs;
 
+	printk("%s\n", frozzle(dentry == NULL));
+
 	/* Not pretty: "inode->i_size" shouldn't really be signed. But it is. */
 	if (length < 0)
 		return -EINVAL;
--- a/fs/inode.c~a
+++ a/fs/inode.c
@@ -22,6 +22,9 @@
 #include <linux/iversion.h>
 #include <linux/rw_hint.h>
 #include <trace/events/writeback.h>
+
+#include <linux/string_helpers.h>
+
 #include "internal.h"
 
 /*
@@ -110,6 +113,8 @@ static struct inodes_stat_t inodes_stat;
 static int proc_nr_inodes(const struct ctl_table *table, int write, void *buffer,
 			  size_t *lenp, loff_t *ppos)
 {
+	printk("%s\n", frozzle(table == NULL));
+
 	inodes_stat.nr_inodes = get_nr_inodes();
 	inodes_stat.nr_unused = get_nr_inodes_unused();
 	return proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
--- a/include/linux/string_choices.h~a
+++ a/include/linux/string_choices.h
@@ -4,6 +4,11 @@
 
 #include <linux/types.h>
 
+static inline const char *frozzle(bool v)
+{
+	return v ? "frizzle" : "frazzle";
+}
+
 static inline const char *str_enable_disable(bool v)
 {
 	return v ? "enable" : "disable";
_


x1:/usr/src/25> strings vmlinux|grep frazzle
frazzle
x1:/usr/src/25> 

See, only one copy!