Messages in this thread | | | Date | Thu, 24 Oct 2019 10:44:33 -0700 | From | Tejun Heo <> | Subject | Re: [PATCH 1/2] cgroup: Add generation number with cgroup id |
| |
Hello,
First of all, thanks a lot for working on this.
On Wed, Oct 16, 2019 at 09:50:18PM +0900, Namhyung Kim wrote: > Current cgroup id is 32-bit and might be recycled while system is > running. To support unique id, add generation number (gen) to catch > recycling and make 64 bit number. This number will be used as kernfs > id and inode number (and file handle). > > Also introduced cgroup_idr struct to keep the idr and generation > together. The related functions are little bit modified as well and I > made some change to cgroup_idr_alloc() to use cyclic allocator. > > Later 64 bit system can have a simpler implementation with a single 64 > bit sequence number and a RB tree. But it'll need to grab a spinlock > during lookup. I'm not entirely sure it's ok, so I left it as is.
Any chance I can persuade you into making this conversion? idr is exactly the wrong data structure to use for cyclic allocations. We've been doing it mostly for historical reasons but I really hope we can move away from it. These lookups aren't in super hot paths and doing locked lookups should be fine.
> /* > * A cgroup_root represents the root of a cgroup hierarchy, and may be > * associated with a kernfs_root to form an active hierarchy. This is > @@ -521,7 +529,7 @@ struct cgroup_root { > unsigned int flags; > > /* IDs for cgroups in this hierarchy */ > - struct idr cgroup_idr; > + struct cgroup_idr cgroup_idr;
Given that there's cgroup->self css, can we get rid of the above? Also, can we make css->id a 64bit value too?
Thanks.
-- tejun
| |