Re: [PATCH v4] mm: cma: support sysfs

[Greg Kroah-Hartman]

On Fri, Mar 19, 2021 at 06:38:00PM +0300, Dmitry Osipenko wrote:
> 19.03.2021 17:27, Greg Kroah-Hartman пишет:
> > On Fri, Mar 19, 2021 at 05:19:47PM +0300, Dmitry Osipenko wrote:
> >> 19.03.2021 16:51, Dmitry Osipenko пишет:
> >>> 19.03.2021 16:47, Greg Kroah-Hartman пишет:
> >>>> On Fri, Mar 19, 2021 at 04:45:21PM +0300, Dmitry Osipenko wrote:
> >>>>> 19.03.2021 16:42, Greg Kroah-Hartman пишет:
> >>>>>> On Fri, Mar 19, 2021 at 04:39:41PM +0300, Dmitry Osipenko wrote:
> >>>>>>> 19.03.2021 15:44, Dmitry Osipenko пишет:
> >>>>>>> ...
> >>>>>>>>>  #include <linux/debugfs.h>
> >>>>>>>>> +#include <linux/kobject.h>
> >>>>>>>>> +
> >>>>>>>>> +struct cma_stat {
> >>>>>>>>> +	spinlock_t lock;
> >>>>>>>>> +	/* the number of CMA page successful allocations */
> >>>>>>>>> +	unsigned long nr_pages_succeeded;
> >>>>>>>>> +	/* the number of CMA page allocation failures */
> >>>>>>>>> +	unsigned long nr_pages_failed;
> >>>>>>>>> +	struct kobject kobj;
> >>>>>>>>> +};
> >>>>>>>>>  
> >>>>>>>>>  struct cma {
> >>>>>>>>>  	unsigned long   base_pfn;
> >>>>>>>>> @@ -16,6 +26,9 @@ struct cma {
> >>>>>>>>>  	struct debugfs_u32_array dfs_bitmap;
> >>>>>>>>>  #endif
> >>>>>>>>>  	char name[CMA_MAX_NAME];
> >>>>>>>>> +#ifdef CONFIG_CMA_SYSFS
> >>>>>>>>> +	struct cma_stat	*stat;
> >>>>>>>>> +#endif
> >>>>>>>
> >>>>>>> What is the point of allocating stat dynamically?
> >>>>>>
> >>>>>> Because static kobjects make me cry.
> >>>>>>
> >>>>>
> >>>>> I meant that it's already a part of struct cma, it looks like the stat
> >>>>> could be embedded into struct cma and then kobj could be initialized
> >>>>> separately.
> >>>>
> >>>> But that structure is statically allocated, so it can not be.  This has
> >>>> been discussed in the past threads for when this was reviewed if you are
> >>>> curious :)
> >>>
> >>> Indeed, I missed that cma_areas[] is static, thank you.
> >>>
> >>
> >> And in this case should be better to make only the kobj allocated
> >> dynamically instead of the whole cma_stat.
> > 
> > Why does it matter?
> > 
> 
> Then initialization order won't be a problem.

I don't understand the problem here, what is wrong with the patch as-is?

Also, watch out, if you only make the kobject dynamic, how are you going
to get backwards from the kobject to the values you want to send to
userspace in the show functions?

thanks,

greg k-h