On Mon, Apr 20, 2009 at 09:15:24PM -0700, Paul E. McKenney wrote:
> > > How does the above lock relate to the iot->lock called out in the comment
> > > headers in the earlier functions? Hmmm... Come to think of it, I don't
> > > see an acquisition of iot->lock anywhere.
> > >
> > > So, what is the story here?
> >
> > As said before, only the comment in struct iothrottle is correct, we use
> > cgroup_lock() to protect iot->list, so there's no need to introduce
> > another lock inside struct iothrottle.
> >
> > And the other comments about iot->lock must be fixed.
>
> Sounds good!
>
> So this code is compiled into the kernel only when cgroups are defined,
> correct? Otherwise, cgroup_lock() seems to be an empty function.
Right, from init/Kconfig:
config CGROUP_IO_THROTTLE
bool "Enable cgroup I/O throttling"
depends on CGROUPS && RESOURCE_COUNTERS && EXPERIMENTAL
...
> > > > +int iothrottle_set_page_owner(struct page *page, struct mm_struct *mm)
> > > > +{
> > > > + struct iothrottle *iot;
> > > > + unsigned short id = 0;
> > > > +
> > > > + if (iothrottle_disabled())
> > > > + return 0;
> > > > + if (!mm)
> > > > + goto out;
> > > > + rcu_read_lock();
> > > > + iot = task_to_iothrottle(rcu_dereference(mm->owner));
> > >
> > > Given that task_to_iothrottle() calls task_subsys_state(), which contains
> > > an rcu_dereference(), why is the rcu_dereference() above required?
> > > (There might well be a good reason, just cannot see it right offhand.)
> >
> > The first rcu_dereference() is required to safely get a task_struct from
> > mm_struct. The second rcu_dereference() inside task_to_iothrottle() is
> > required to safely get the struct iothrottle from task_struct.
>
> Why not put the rcu_dereference() down inside task_to_iothrottle()?
>
mmmh... it is needed only when task_struct is taken from mm->owner,
task_to_iothrottle(current) for example works fine without the
rcu_dereference(current).
-Andrea
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
