On Fri, May 06, 2022 at 07:59:42PM +0530, Jagdish Gediya wrote:
> On Fri, May 06, 2022 at 04:02:28PM +0200, Greg KH wrote:
> > On Fri, May 06, 2022 at 07:03:09PM +0530, Jagdish Gediya wrote:
> > > Setting name as per the format is not only useful for kobjects.
> > > It can also be used to set name for other things for e.g. setting
> > > the name of the struct attribute when multiple same kind of attributes
> > > need to be created with some identifier in name, instead of managing
> > > memory for names at such places case by case, it would be good if
> > > something like current kobject_set_name_vargs() can be utilized.
> > >
> > > Refactor kobject_set_name_vargs(), Create a new generic function
> > > set_name_vargs() which can be used for kobjects as well as at
> > > other places.
> > >
> > > This patch doesn't introduce any functionality change.
> > >
> > > Signed-off-by: Jagdish Gediya <jvgediya@xxxxxxxxxxxxx>
> > > ---
> > > include/linux/string.h | 1 +
> > > lib/kobject.c | 30 +-----------------------------
> > > mm/util.c | 40 ++++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 42 insertions(+), 29 deletions(-)
> > >
> > > diff --git a/include/linux/string.h b/include/linux/string.h
> > > index b6572aeca2f5..f329962e5ae9 100644
> > > --- a/include/linux/string.h
> > > +++ b/include/linux/string.h
> > > @@ -9,6 +9,7 @@
> > > #include <linux/stdarg.h>
> > > #include <uapi/linux/string.h>
> > >
> > > +int set_name_vargs(const char **name, const char *fmt, va_list vargs);
> > > extern char *strndup_user(const char __user *, long);
> > > extern void *memdup_user(const void __user *, size_t);
> > > extern void *vmemdup_user(const void __user *, size_t);
> > > diff --git a/lib/kobject.c b/lib/kobject.c
> > > index 5f0e71ab292c..870d05971e3a 100644
> > > --- a/lib/kobject.c
> > > +++ b/lib/kobject.c
> > > @@ -249,35 +249,7 @@ static int kobject_add_internal(struct kobject *kobj)
> > > int kobject_set_name_vargs(struct kobject *kobj, const char *fmt,
> > > va_list vargs)
> > > {
> > > - const char *s;
> > > -
> > > - if (kobj->name && !fmt)
> > > - return 0;
> > > -
> > > - s = kvasprintf_const(GFP_KERNEL, fmt, vargs);
> > > - if (!s)
> > > - return -ENOMEM;
> > > -
> > > - /*
> > > - * ewww... some of these buggers have '/' in the name ... If
> > > - * that's the case, we need to make sure we have an actual
> > > - * allocated copy to modify, since kvasprintf_const may have
> > > - * returned something from .rodata.
> > > - */
> > > - if (strchr(s, '/')) {
> > > - char *t;
> > > -
> > > - t = kstrdup(s, GFP_KERNEL);
> > > - kfree_const(s);
> > > - if (!t)
> > > - return -ENOMEM;
> > > - strreplace(t, '/', '!');
> > > - s = t;
> > > - }
> > > - kfree_const(kobj->name);
> > > - kobj->name = s;
> > > -
> > > - return 0;
> > > + return set_name_vargs(&kobj->name, fmt, vargs);
> > > }
> > >
> > > /**
> > > diff --git a/mm/util.c b/mm/util.c
> > > index 54e5e761a9a9..808d29b17ea7 100644
> > > --- a/mm/util.c
> > > +++ b/mm/util.c
> > > @@ -112,6 +112,46 @@ char *kstrndup(const char *s, size_t max, gfp_t gfp)
> > > }
> > > EXPORT_SYMBOL(kstrndup);
> > >
> > > +/**
> > > + * set_name_vargs() - Set the name as per format
> > > + * @name: pointer to point to the name as per format
> > > + * @fmt: format string used to build the name
> > > + * @vargs: vargs to format the string.
> > > + */
> > > +int set_name_vargs(const char **name, const char *fmt, va_list vargs)
> >
> > Why is this a mm/ thing and not a lib/ thing?
>
> I think it can be moved to lib/, Will correct it in next version.
>
> > And who else will be needing to use this? Why the churn for no
> > actual users?
>
> I am working on a sepatare series for memory tiers where this kind
> of functionality is needed, Based on numa topology of the system,
> We can build the memory tiers nodemasks based on numa distances,
> such masks need to be viewed/stored from sysfs using interfaces
> /sys/*/memory_tier0, /sys/*/memory_tier1 etc. there can be upto
> MAX_TIERS of memory tiers in the system, so we can define struct
> device_attribute array statically but their names need to be set
> as per tier number where this function is useful.
>
> Also I think this requirement is generic although there are no
> current users, It would be useful to not just restrict it to
> kobjects.
We don't make changes unless they are needed. This can be part of a
patch series that uses the change, otherwise it's unneeded churn right
now.
thanks,
greg k-h
