Andrea Righi wrote:
> On Fri, May 15, 2009 at 01:15:24PM +0800, Gui Jianfeng wrote:
>> Vivek Goyal wrote:
>> ...
>>> }
>>> @@ -1462,20 +1462,27 @@ struct io_cgroup *get_iocg_from_bio(stru
>>> /*
>>> * Find the io group bio belongs to.
>>> * If "create" is set, io group is created if it is not already present.
>>> + * If "curr" is set, io group is information is searched for current
>>> + * task and not with the help of bio.
>>> + *
>>> + * FIXME: Can we assume that if bio is NULL then lookup group for current
>>> + * task and not create extra function parameter ?
>>> *
>>> - * Note: There is a narrow window of race where a group is being freed
>>> - * by cgroup deletion path and some rq has slipped through in this group.
>>> - * Fix it.
>>> */
>>> -struct io_group *io_get_io_group_bio(struct request_queue *q, struct bio *bio,
>>> - int create)
>>> +struct io_group *io_get_io_group(struct request_queue *q, struct bio *bio,
>>> + int create, int curr)
>> Hi Vivek,
>>
>> IIUC we can get rid of curr, and just determine iog from bio. If bio is not NULL,
>> get iog from bio, otherwise get it from current task.
>
> Consider also that get_cgroup_from_bio() is much more slow than
> task_cgroup() and need to lock/unlock_page_cgroup() in
> get_blkio_cgroup_id(), while task_cgroup() is rcu protected.
>
> BTW another optimization could be to use the blkio-cgroup functionality
> only for dirty pages and cut out some blkio_set_owner(). For all the
> other cases IO always occurs in the same context of the current task,
> and you can use task_cgroup().
>
> However, this is true only for page cache pages, for IO generated by
> anonymous pages (swap) you still need the page tracking functionality
> both for reads and writes.
Hi Andrea,
Thanks for pointing this out. Yes, i think we can determine io group in
terms of bio->bi_rw. If bio is a READ bio, just taking io group by
task_cgroup(). If it's a WRITE bio, getting it from blkio_cgroup.
>
> -Andrea
>
>>> {
>>> struct cgroup *cgroup;
>>> struct io_group *iog;
>>> struct elv_fq_data *efqd = &q->elevator->efqd;
>>>
>>> rcu_read_lock();
>>> - cgroup = get_cgroup_from_bio(bio);
>>> +
>>> + if (curr)
>>> + cgroup = task_cgroup(current, io_subsys_id);
>>> + else
>>> + cgroup = get_cgroup_from_bio(bio);
>>> +
>>> if (!cgroup) {
>>> if (create)
>>> iog = efqd->root_group;
>>> @@ -1500,7 +1507,7 @@ out:
>>> rcu_read_unlock();
>>> return iog;
>>> }
>> --
>> Regards
>> Gui Jianfeng
>>
>
>
>
--
Regards
Gui Jianfeng
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
