On Thu 03-02-22 14:03:58, Waiman Long wrote: > 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. Cutting a name short sounds like a source of confusion and there doesn't seem to be any good reason for that. -- Michal Hocko SUSE Labs
