lkml.org 
[lkml]   [2009]   [Apr]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 09/14] block: clean up request completion API
    Date
    Request completion has gone through several changes and became a bit
    messy over the time. Clean it up.

    1. end_that_request_data() is a thin wrapper around
    end_that_request_data_first() which checks whether bio is NULL
    before doing anything and handles bidi completion.
    blk_update_request() is a thin wrapper around
    end_that_request_data() which clears nr_sectors on the last
    iteration but doesn't use the bidi completion.

    Clean it up by moving the initial bio NULL check and nr_sectors
    clearing on the last iteration into end_that_request_data() and
    renaming it to blk_update_request(), which makes blk_end_io() the
    only user of end_that_request_data(). Collapse
    end_that_request_data() into blk_end_io().

    2. There are four visible completion variants - blk_end_request(),
    __blk_end_request(), blk_end_bidi_request() and end_request().
    blk_end_request() and blk_end_bidi_request() uses blk_end_request()
    as the backend but __blk_end_request() and end_request() use
    separate implementation in __blk_end_request() due to different
    locking rules.

    Make blk_end_io() handle both cases thus all four public completion
    functions are thin wrappers around blk_end_io(). Rename
    blk_end_io() to __blk_end_io() and export it and inline all public
    completion functions.

    3. As the whole request issue/completion usages are about to be
    modified and audited, it's a good chance to convert completion
    functions return bool which better indicates the intended meaning
    of return values.

    4. The function name end_that_request_last() is from the days when it
    was a public interface and slighly confusing. Give it a proper
    internal name - finish_request().

    5. Add description explaning that blk_end_bidi_request() can be safely
    used for uni requests as suggested by Boaz Harrosh.

    The only visible behavior change is from #1. nr_sectors counts are
    cleared after the final iteration no matter which function is used to
    complete the request. I couldn't find any place where the code
    assumes those nr_sectors counters contain the values for the last
    segment and this change is good as it makes the API much more
    consistent as the end result is now same whether a request is
    completed using [__]blk_end_request() alone or in combination with
    blk_update_request().

    [ Impact: cleanup, rq->*nr_sectors always updated after req completion ]

    Signed-off-by: Tejun Heo <tj@kernel.org>
    Reviewed-by: Boaz Harrosh <bharrosh@panasas.com>
    ---
    block/blk-core.c | 215 ++++++++++++------------------------------------
    include/linux/blkdev.h | 117 +++++++++++++++++++++++---
    2 files changed, 157 insertions(+), 175 deletions(-)

    diff --git a/block/blk-core.c b/block/blk-core.c
    index 0f402c2..274888b 100644
    --- a/block/blk-core.c
    +++ b/block/blk-core.c
    @@ -1806,25 +1806,35 @@ void elv_dequeue_request(struct request_queue *q, struct request *rq)
    }

    /**
    - * __end_that_request_first - end I/O on a request
    - * @req: the request being processed
    + * blk_update_request - Special helper function for request stacking drivers
    + * @rq: the request being processed
    * @error: %0 for success, < %0 for error
    - * @nr_bytes: number of bytes to complete
    + * @nr_bytes: number of bytes to complete @rq
    *
    * Description:
    - * Ends I/O on a number of bytes attached to @req, and sets it up
    - * for the next range of segments (if any) in the cluster.
    + * Ends I/O on a number of bytes attached to @rq, but doesn't complete
    + * the request structure even if @rq doesn't have leftover.
    + * If @rq has leftover, sets it up for the next range of segments.
    + *
    + * This special helper function is only for request stacking drivers
    + * (e.g. request-based dm) so that they can handle partial completion.
    + * Actual device drivers should use blk_end_request instead.
    + *
    + * Passing the result of blk_rq_bytes() as @nr_bytes guarantees
    + * %false return from this function.
    *
    * Return:
    - * %0 - we are done with this request, call end_that_request_last()
    - * %1 - still buffers pending for this request
    + * %false - this request doesn't have any more data
    + * %true - this request has more data
    **/
    -static int __end_that_request_first(struct request *req, int error,
    - int nr_bytes)
    +bool blk_update_request(struct request *req, int error, unsigned int nr_bytes)
    {
    int total_bytes, bio_nbytes, next_idx = 0;
    struct bio *bio;

    + if (!req->bio)
    + return false;
    +
    trace_block_rq_complete(req->q, req);

    /*
    @@ -1901,8 +1911,16 @@ static int __end_that_request_first(struct request *req, int error,
    /*
    * completely done
    */
    - if (!req->bio)
    - return 0;
    + if (!req->bio) {
    + /*
    + * Reset counters so that the request stacking driver
    + * can find how many bytes remain in the request
    + * later.
    + */
    + req->nr_sectors = req->hard_nr_sectors = 0;
    + req->current_nr_sectors = req->hard_cur_sectors = 0;
    + return false;
    + }

    /*
    * if the request wasn't completed, update state
    @@ -1916,29 +1934,14 @@ static int __end_that_request_first(struct request *req, int error,

    blk_recalc_rq_sectors(req, total_bytes >> 9);
    blk_recalc_rq_segments(req);
    - return 1;
    -}
    -
    -static int end_that_request_data(struct request *rq, int error,
    - unsigned int nr_bytes, unsigned int bidi_bytes)
    -{
    - if (rq->bio) {
    - if (__end_that_request_first(rq, error, nr_bytes))
    - return 1;
    -
    - /* Bidi request must be completed as a whole */
    - if (blk_bidi_rq(rq) &&
    - __end_that_request_first(rq->next_rq, error, bidi_bytes))
    - return 1;
    - }
    -
    - return 0;
    + return true;
    }
    +EXPORT_SYMBOL_GPL(blk_update_request);

    /*
    * queue lock must be held
    */
    -static void end_that_request_last(struct request *req, int error)
    +static void finish_request(struct request *req, int error)
    {
    if (blk_rq_tagged(req))
    blk_queue_end_tag(req->q, req);
    @@ -1964,161 +1967,47 @@ static void end_that_request_last(struct request *req, int error)
    }

    /**
    - * blk_end_io - Generic end_io function to complete a request.
    + * __blk_end_io - Generic end_io function to complete a request.
    * @rq: the request being processed
    * @error: %0 for success, < %0 for error
    * @nr_bytes: number of bytes to complete @rq
    * @bidi_bytes: number of bytes to complete @rq->next_rq
    + * @locked: whether rq->q->queue_lock is held on entry
    *
    * Description:
    * Ends I/O on a number of bytes attached to @rq and @rq->next_rq.
    * If @rq has leftover, sets it up for the next range of segments.
    *
    * Return:
    - * %0 - we are done with this request
    - * %1 - this request is not freed yet, it still has pending buffers.
    + * %false - we are done with this request
    + * %true - this request is not freed yet, it still has pending buffers.
    **/
    -static int blk_end_io(struct request *rq, int error, unsigned int nr_bytes,
    - unsigned int bidi_bytes)
    +bool __blk_end_io(struct request *rq, int error, unsigned int nr_bytes,
    + unsigned int bidi_bytes, bool locked)
    {
    struct request_queue *q = rq->q;
    unsigned long flags = 0UL;

    - if (end_that_request_data(rq, error, nr_bytes, bidi_bytes))
    - return 1;
    -
    - add_disk_randomness(rq->rq_disk);
    -
    - spin_lock_irqsave(q->queue_lock, flags);
    - end_that_request_last(rq, error);
    - spin_unlock_irqrestore(q->queue_lock, flags);
    -
    - return 0;
    -}
    + if (blk_update_request(rq, error, nr_bytes))
    + return true;

    -/**
    - * blk_end_request - Helper function for drivers to complete the request.
    - * @rq: the request being processed
    - * @error: %0 for success, < %0 for error
    - * @nr_bytes: number of bytes to complete
    - *
    - * Description:
    - * Ends I/O on a number of bytes attached to @rq.
    - * If @rq has leftover, sets it up for the next range of segments.
    - *
    - * Return:
    - * %0 - we are done with this request
    - * %1 - still buffers pending for this request
    - **/
    -int blk_end_request(struct request *rq, int error, unsigned int nr_bytes)
    -{
    - return blk_end_io(rq, error, nr_bytes, 0);
    -}
    -EXPORT_SYMBOL_GPL(blk_end_request);
    -
    -/**
    - * __blk_end_request - Helper function for drivers to complete the request.
    - * @rq: the request being processed
    - * @error: %0 for success, < %0 for error
    - * @nr_bytes: number of bytes to complete
    - *
    - * Description:
    - * Must be called with queue lock held unlike blk_end_request().
    - *
    - * Return:
    - * %0 - we are done with this request
    - * %1 - still buffers pending for this request
    - **/
    -int __blk_end_request(struct request *rq, int error, unsigned int nr_bytes)
    -{
    - if (rq->bio && __end_that_request_first(rq, error, nr_bytes))
    - return 1;
    + /* Bidi request must be completed as a whole */
    + if (unlikely(blk_bidi_rq(rq)) &&
    + blk_update_request(rq->next_rq, error, bidi_bytes))
    + return true;

    add_disk_randomness(rq->rq_disk);

    - end_that_request_last(rq, error);
    -
    - return 0;
    -}
    -EXPORT_SYMBOL_GPL(__blk_end_request);
    -
    -/**
    - * blk_end_bidi_request - Helper function for drivers to complete bidi request.
    - * @rq: the bidi request being processed
    - * @error: %0 for success, < %0 for error
    - * @nr_bytes: number of bytes to complete @rq
    - * @bidi_bytes: number of bytes to complete @rq->next_rq
    - *
    - * Description:
    - * Ends I/O on a number of bytes attached to @rq and @rq->next_rq.
    - *
    - * Return:
    - * %0 - we are done with this request
    - * %1 - still buffers pending for this request
    - **/
    -int blk_end_bidi_request(struct request *rq, int error, unsigned int nr_bytes,
    - unsigned int bidi_bytes)
    -{
    - return blk_end_io(rq, error, nr_bytes, bidi_bytes);
    -}
    -EXPORT_SYMBOL_GPL(blk_end_bidi_request);
    -
    -/**
    - * end_request - end I/O on the current segment of the request
    - * @req: the request being processed
    - * @uptodate: error value or %0/%1 uptodate flag
    - *
    - * Description:
    - * Ends I/O on the current segment of a request. If that is the only
    - * remaining segment, the request is also completed and freed.
    - *
    - * This is a remnant of how older block drivers handled I/O completions.
    - * Modern drivers typically end I/O on the full request in one go, unless
    - * they have a residual value to account for. For that case this function
    - * isn't really useful, unless the residual just happens to be the
    - * full current segment. In other words, don't use this function in new
    - * code. Use blk_end_request() or __blk_end_request() to end a request.
    - **/
    -void end_request(struct request *req, int uptodate)
    -{
    - int error = 0;
    -
    - if (uptodate <= 0)
    - error = uptodate ? uptodate : -EIO;
    -
    - __blk_end_request(req, error, req->hard_cur_sectors << 9);
    -}
    -EXPORT_SYMBOL(end_request);
    + if (!locked) {
    + spin_lock_irqsave(q->queue_lock, flags);
    + finish_request(rq, error);
    + spin_unlock_irqrestore(q->queue_lock, flags);
    + } else
    + finish_request(rq, error);

    -/**
    - * blk_update_request - Special helper function for request stacking drivers
    - * @rq: the request being processed
    - * @error: %0 for success, < %0 for error
    - * @nr_bytes: number of bytes to complete @rq
    - *
    - * Description:
    - * Ends I/O on a number of bytes attached to @rq, but doesn't complete
    - * the request structure even if @rq doesn't have leftover.
    - * If @rq has leftover, sets it up for the next range of segments.
    - *
    - * This special helper function is only for request stacking drivers
    - * (e.g. request-based dm) so that they can handle partial completion.
    - * Actual device drivers should use blk_end_request instead.
    - */
    -void blk_update_request(struct request *rq, int error, unsigned int nr_bytes)
    -{
    - if (!end_that_request_data(rq, error, nr_bytes, 0)) {
    - /*
    - * These members are not updated in end_that_request_data()
    - * when all bios are completed.
    - * Update them so that the request stacking driver can find
    - * how many bytes remain in the request later.
    - */
    - rq->nr_sectors = rq->hard_nr_sectors = 0;
    - rq->current_nr_sectors = rq->hard_cur_sectors = 0;
    - }
    + return false;
    }
    -EXPORT_SYMBOL_GPL(blk_update_request);
    +EXPORT_SYMBOL_GPL(__blk_end_io);

    void blk_rq_bio_prep(struct request_queue *q, struct request *rq,
    struct bio *bio)
    diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
    index e24f56d..0d94ad6 100644
    --- a/include/linux/blkdev.h
    +++ b/include/linux/blkdev.h
    @@ -837,27 +837,120 @@ extern unsigned int blk_rq_bytes(struct request *rq);
    extern unsigned int blk_rq_cur_bytes(struct request *rq);

    /*
    - * blk_end_request() and friends.
    - * __blk_end_request() and end_request() must be called with
    - * the request queue spinlock acquired.
    + * Request completion related functions.
    + *
    + * blk_update_request() completes given number of bytes and updates
    + * the request without completing it.
    + *
    + * blk_end_request() and friends. __blk_end_request() and
    + * end_request() must be called with the request queue spinlock
    + * acquired.
    *
    * Several drivers define their own end_request and call
    * blk_end_request() for parts of the original function.
    * This prevents code duplication in drivers.
    */
    -extern int blk_end_request(struct request *rq, int error,
    - unsigned int nr_bytes);
    -extern int __blk_end_request(struct request *rq, int error,
    - unsigned int nr_bytes);
    -extern int blk_end_bidi_request(struct request *rq, int error,
    - unsigned int nr_bytes, unsigned int bidi_bytes);
    -extern void end_request(struct request *, int);
    +extern bool blk_update_request(struct request *rq, int error,
    + unsigned int nr_bytes);
    +
    +/* internal function, subject to change, don't ever use directly */
    +extern bool __blk_end_io(struct request *rq, int error,
    + unsigned int nr_bytes, unsigned int bidi_bytes,
    + bool locked);
    +
    +/**
    + * blk_end_request - Helper function for drivers to complete the request.
    + * @rq: the request being processed
    + * @error: %0 for success, < %0 for error
    + * @nr_bytes: number of bytes to complete
    + *
    + * Description:
    + * Ends I/O on a number of bytes attached to @rq.
    + * If @rq has leftover, sets it up for the next range of segments.
    + *
    + * Return:
    + * %false - we are done with this request
    + * %true - still buffers pending for this request
    + **/
    +static inline bool blk_end_request(struct request *rq, int error,
    + unsigned int nr_bytes)
    +{
    + return __blk_end_io(rq, error, nr_bytes, 0, false);
    +}
    +
    +/**
    + * __blk_end_request - Helper function for drivers to complete the request.
    + * @rq: the request being processed
    + * @error: %0 for success, < %0 for error
    + * @nr_bytes: number of bytes to complete
    + *
    + * Description:
    + * Must be called with queue lock held unlike blk_end_request().
    + *
    + * Return:
    + * %false - we are done with this request
    + * %true - still buffers pending for this request
    + **/
    +static inline bool __blk_end_request(struct request *rq, int error,
    + unsigned int nr_bytes)
    +{
    + return __blk_end_io(rq, error, nr_bytes, 0, true);
    +}
    +
    +/**
    + * blk_end_bidi_request - Helper function for drivers to complete bidi request.
    + * @rq: the bidi request being processed
    + * @error: %0 for success, < %0 for error
    + * @nr_bytes: number of bytes to complete @rq
    + * @bidi_bytes: number of bytes to complete @rq->next_rq
    + *
    + * Description:
    + * Ends I/O on a number of bytes attached to @rq and @rq->next_rq.
    + * Drivers that supports bidi can safely call this member for any
    + * type of request, bidi or uni. In the later case @bidi_bytes is
    + * just ignored.
    + *
    + * Return:
    + * %false - we are done with this request
    + * %true - still buffers pending for this request
    + **/
    +static inline bool blk_end_bidi_request(struct request *rq, int error,
    + unsigned int nr_bytes,
    + unsigned int bidi_bytes)
    +{
    + return __blk_end_io(rq, error, nr_bytes, bidi_bytes, false);
    +}
    +
    +/**
    + * end_request - end I/O on the current segment of the request
    + * @rq: the request being processed
    + * @uptodate: error value or %0/%1 uptodate flag
    + *
    + * Description:
    + * Ends I/O on the current segment of a request. If that is the only
    + * remaining segment, the request is also completed and freed.
    + *
    + * This is a remnant of how older block drivers handled I/O completions.
    + * Modern drivers typically end I/O on the full request in one go, unless
    + * they have a residual value to account for. For that case this function
    + * isn't really useful, unless the residual just happens to be the
    + * full current segment. In other words, don't use this function in new
    + * code. Use blk_end_request() or __blk_end_request() to end a request.
    + **/
    +static inline void end_request(struct request *rq, int uptodate)
    +{
    + int error = 0;
    +
    + if (uptodate <= 0)
    + error = uptodate ? uptodate : -EIO;
    +
    + __blk_end_io(rq, error, rq->hard_cur_sectors << 9, 0, true);
    +}
    +
    extern void blk_complete_request(struct request *);
    extern void __blk_complete_request(struct request *);
    extern void blk_abort_request(struct request *);
    extern void blk_abort_queue(struct request_queue *);
    -extern void blk_update_request(struct request *rq, int error,
    - unsigned int nr_bytes);

    /*
    * Access functions for manipulating queue properties
    --
    1.6.0.2


    \
     
     \ /
      Last update: 2009-04-21 18:45    [W:0.048 / U:0.780 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site