Messages in this thread | | | Date | Fri, 17 Feb 2012 14:03:51 -0800 | From | Tejun Heo <> | Subject | Re: [PATCH 7/9] block: implement bio_associate_current() |
| |
Hello, Vivek.
On Fri, Feb 17, 2012 at 04:33:13PM -0500, Vivek Goyal wrote: > On Thu, Feb 16, 2012 at 02:37:56PM -0800, Tejun Heo wrote: > > [..] > > This patch implements bio_associate_current() which associates the > > specified bio with %current. The bio will record the associated ioc > > and blkcg at that point and block layer will use the recorded ones > > regardless of which task actually ends up issuing the bio. bio > > release puts the associated ioc and blkcg. > > How about storing blkcg information in io_context instead of bio. We will > have less copies of bio pointers and I think logically it makes sense.
I don't know. The problem with that approach is that we introduce a persistent state which needs to be kept in sync. cgroup is a task property and the current code just grabs the current cgroup of %current and uses it for that bio. It doesn't matter how the task changes its cgroup membership later - we're correct (in a sense) no matter what. If we add cgroup pointer to ioc, we need to keep that in sync with task changing cgroup memberships and need to introduce synchronization scheme for accessing ioc->blkcg, which is a much bigger headache.
I think it's better to take an explicit ref now. If the situation changes, it's an implementation detail only known to block layer proper anyway, so we should be able to change it without too much difficulty.
Thanks.
-- tejun
| |