lkml.org 
[lkml]   [2009]   [May]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] io-controller: Add io group reference handling for request
    On Fri, May 08, 2009 at 05:45:32PM +0800, Gui Jianfeng wrote:
    > Hi Vivek,
    >
    > This patch adds io group reference handling when allocating
    > and removing a request.
    >

    Hi Gui,

    Thanks for the patch. We were thinking that requests can take a reference
    on io queues and io queues can take a reference on io groups. That should
    make sure that io groups don't go away as long as active requests are
    present.

    But there seems to be a small window while allocating the new request
    where request gets allocated from a group first and then later it is
    mapped to that group and queue is created. IOW, in get_request_wait(),
    we allocate a request from a particular group and set rq->rl, then
    drop the queue lock and later call elv_set_request() which again maps
    the request to the group saves rq->iog and creates new queue. This window
    is troublesome because request can be mapped to a particular group at the
    time of allocation and during set_request() it can go to a different
    group as queue lock was dropped and group might have disappeared.

    In this case probably it might make sense that request also takes a
    reference on groups. At the same time it looks too much that request takes
    a reference on queue as well as group object. Ideas are welcome on how
    to handle it...

    Thanks
    Vivek

    > Signed-off-by: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
    > ---
    > elevator-fq.c | 15 ++++++++++++++-
    > elevator-fq.h | 5 +++++
    > elevator.c | 2 ++
    > 3 files changed, 21 insertions(+), 1 deletion(-)
    >
    > diff --git a/block/elevator-fq.c b/block/elevator-fq.c
    > index 9500619..e6d6712 100644
    > --- a/block/elevator-fq.c
    > +++ b/block/elevator-fq.c
    > @@ -1968,11 +1968,24 @@ void elv_fq_set_request_io_group(struct request_queue *q, struct request *rq,
    > spin_unlock_irqrestore(q->queue_lock, flags);
    > BUG_ON(!iog);
    >
    > - /* Store iog in rq. TODO: take care of referencing */
    > + elv_get_iog(iog);
    > rq->iog = iog;
    > }
    >
    > /*
    > + * This request has been serviced. Clean up iog info and drop the reference.
    > + */
    > +void elv_fq_unset_request_io_group(struct request *rq)
    > +{
    > + struct io_group *iog = rq->iog;
    > +
    > + if (iog) {
    > + rq->iog = NULL;
    > + elv_put_iog(iog);
    > + }
    > +}
    > +
    > +/*
    > * Find/Create the io queue the rq should go in. This is an optimization
    > * for the io schedulers (noop, deadline and AS) which maintain only single
    > * io queue per cgroup. In this case common layer can just maintain a
    > diff --git a/block/elevator-fq.h b/block/elevator-fq.h
    > index db3a347..96a28e9 100644
    > --- a/block/elevator-fq.h
    > +++ b/block/elevator-fq.h
    > @@ -512,6 +512,7 @@ static inline struct io_group *ioq_to_io_group(struct io_queue *ioq)
    > extern int io_group_allow_merge(struct request *rq, struct bio *bio);
    > extern void elv_fq_set_request_io_group(struct request_queue *q,
    > struct request *rq, struct bio *bio);
    > +extern void elv_fq_unset_request_io_group(struct request *rq);
    > static inline bfq_weight_t iog_weight(struct io_group *iog)
    > {
    > return iog->entity.weight;
    > @@ -571,6 +572,10 @@ static inline void elv_fq_set_request_io_group(struct request_queue *q,
    > {
    > }
    >
    > +static inline void elv_fq_unset_request_io_group(struct request *rq)
    > +{
    > +}
    > +
    > static inline bfq_weight_t iog_weight(struct io_group *iog)
    > {
    > /* Just root group is present and weight is immaterial. */
    > diff --git a/block/elevator.c b/block/elevator.c
    > index 44c9fad..d75eec7 100644
    > --- a/block/elevator.c
    > +++ b/block/elevator.c
    > @@ -992,6 +992,8 @@ void elv_put_request(struct request_queue *q, struct request *rq)
    > {
    > struct elevator_queue *e = q->elevator;
    >
    > + elv_fq_unset_request_io_group(rq);
    > +
    > /*
    > * Optimization for noop, deadline and AS which maintain only single
    > * ioq per io group
    >


    \
     
     \ /
      Last update: 2009-05-08 16:01    [W:0.028 / U:60.880 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site