Re: [PATCH v4 3/4] mm/page_owner: Print memcg information

[Waiman Long <longman@redhat.com>]

On 2/3/22 07:46, Michal Hocko wrote:
> On Wed 02-02-22 15:30:35, Waiman Long wrote:
> [...]
>> +#ifdef CONFIG_MEMCG
>> +	unsigned long memcg_data;
>> +	struct mem_cgroup *memcg;
>> +	bool online;
>> +	char name[80];
>> +
>> +	rcu_read_lock();
>> +	memcg_data = READ_ONCE(page->memcg_data);
>> +	if (!memcg_data)
>> +		goto out_unlock;
>> +
>> +	if (memcg_data & MEMCG_DATA_OBJCGS)
>> +		ret += scnprintf(kbuf + ret, count - ret,
>> +				"Slab cache page\n");
>> +
>> +	memcg = page_memcg_check(page);
>> +	if (!memcg)
>> +		goto out_unlock;
>> +
>> +	online = (memcg->css.flags & CSS_ONLINE);
>> +	cgroup_name(memcg->css.cgroup, name, sizeof(name));
> Is there any specific reason to use another buffer allocated on the
> stack? Also 80B seems too short to cover NAME_MAX.
>
> Nothing else jumped at me.

I suppose we can print directly into kbuf with cgroup_name(), but using 
a separate buffer is easier to read and understand. 79 characters should 
be enough for most cgroup names. Some auto-generated names with some 
kind of embedded uuids may be longer than that, but the random sequence 
of hex digits that may be missing do not convey much information for 
identification purpose. We can always increase the buffer length later 
if it turns out to be an issue.

Cheers,
Longman