2009-04-24 05:50:57

by Nikanth Karthikesan

[permalink] [raw]
Subject: [PATCH linux-2.6-block/for-linus] Fix discard requests accounting in the diskstats

When 2 discard requests are merged, the stats gets updated, but we do not
update statistics normally when a discard request is issued or completed. For
example the in_flight counter would be decremented when 2 discard requests are
merged, but it was not at all incremented when they were issued, and in_flight
counter will not be decremented, when they are completed as well.

This patch fixes this by adding discard requests to the statistics.

Signed-off-by: Nikanth Karthikesan <[email protected]>

---

diff --git a/block/blk-core.c b/block/blk-core.c
index 2998fe3..0175b9f 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -68,7 +68,7 @@ static void drive_stat_acct(struct request *rq, int new_io)
int rw = rq_data_dir(rq);
int cpu;

- if (!blk_fs_request(rq) || !blk_do_io_stat(rq))
+ if (!rq_accountable(rq) || !blk_do_io_stat(rq))
return;

cpu = part_stat_lock();
@@ -1700,7 +1700,7 @@ static void blk_account_io_done(struct request *req)
* IO on queueing nor completion. Accounting the containing
* request is enough.
*/
- if (blk_fs_request(req) && req != &req->q->bar_rq) {
+ if (rq_accountable(req) && req != &req->q->bar_rq) {
unsigned long duration = jiffies - req->start_time;
const int rw = rq_data_dir(req);
struct hd_struct *part;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 2755d5c..a54d3ba 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -670,7 +670,9 @@ static inline void blk_clear_queue_full(struct request_queue *q, int sync)
(REQ_NOMERGE | REQ_STARTED | REQ_HARDBARRIER | REQ_SOFTBARRIER)
#define rq_mergeable(rq) \
(!((rq)->cmd_flags & RQ_NOMERGE_FLAGS) && \
- (blk_discard_rq(rq) || blk_fs_request((rq))))
+ (blk_discard_rq(rq) || blk_fs_request(rq)))
+#define rq_accountable(rq) \
+ (blk_discard_rq(rq) || blk_fs_request(rq))

/*
* q->prep_rq_fn return values


2009-04-24 06:15:30

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH linux-2.6-block/for-linus] Fix discard requests accounting in the diskstats

On Fri, Apr 24 2009, Nikanth Karthikesan wrote:
> When 2 discard requests are merged, the stats gets updated, but we do not
> update statistics normally when a discard request is issued or completed. For
> example the in_flight counter would be decremented when 2 discard requests are
> merged, but it was not at all incremented when they were issued, and in_flight
> counter will not be decremented, when they are completed as well.
>
> This patch fixes this by adding discard requests to the statistics.
>
> Signed-off-by: Nikanth Karthikesan <[email protected]>

OK, so that's not quite what I had in mind. If we first do this as a
preparatory patch:

http://git.kernel.dk/?p=linux-2.6-block.git;a=commit;h=42fd769da4edadad83a779867ff1910ec35c6de7

then the discard fix becomes this one-liner:

http://git.kernel.dk/?p=linux-2.6-block.git;a=commit;h=999ae79ae7f853bb9e60e95753ebf8d1628e632a

--
Jens Axboe

2009-04-24 07:41:04

by Nikanth Karthikesan

[permalink] [raw]
Subject: Re: [PATCH linux-2.6-block/for-linus] Fix discard requests accounting in the diskstats

On Friday 24 April 2009 11:45:18 Jens Axboe wrote:
> On Fri, Apr 24 2009, Nikanth Karthikesan wrote:
> > When 2 discard requests are merged, the stats gets updated, but we do not
> > update statistics normally when a discard request is issued or completed.
> > For example the in_flight counter would be decremented when 2 discard
> > requests are merged, but it was not at all incremented when they were
> > issued, and in_flight counter will not be decremented, when they are
> > completed as well.
> >
> > This patch fixes this by adding discard requests to the statistics.
> >
> > Signed-off-by: Nikanth Karthikesan <[email protected]>
>
> OK, so that's not quite what I had in mind. If we first do this as a
> preparatory patch:
>
> http://git.kernel.dk/?p=linux-2.6-block.git;a=commit;h=42fd769da4edadad83a7
>79867ff1910ec35c6de7
>

Oh! I thought this cleanup along with the
block-turn-some-buggy-macros-into-c-functions.patch from Andrew could be done
on top of my original fix. But didn't, as you said, you would do it. Anyway,
Thanks.

> then the discard fix becomes this one-liner:
>
> http://git.kernel.dk/?p=linux-2.6-block.git;a=commit;h=999ae79ae7f853bb9e60
>e95753ebf8d1628e632a

Thanks
Nikanth

2009-04-24 07:43:47

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH linux-2.6-block/for-linus] Fix discard requests accounting in the diskstats

On Fri, Apr 24 2009, Nikanth Karthikesan wrote:
> On Friday 24 April 2009 11:45:18 Jens Axboe wrote:
> > On Fri, Apr 24 2009, Nikanth Karthikesan wrote:
> > > When 2 discard requests are merged, the stats gets updated, but we do not
> > > update statistics normally when a discard request is issued or completed.
> > > For example the in_flight counter would be decremented when 2 discard
> > > requests are merged, but it was not at all incremented when they were
> > > issued, and in_flight counter will not be decremented, when they are
> > > completed as well.
> > >
> > > This patch fixes this by adding discard requests to the statistics.
> > >
> > > Signed-off-by: Nikanth Karthikesan <[email protected]>
> >
> > OK, so that's not quite what I had in mind. If we first do this as a
> > preparatory patch:
> >
> > http://git.kernel.dk/?p=linux-2.6-block.git;a=commit;h=42fd769da4edadad83a7
> >79867ff1910ec35c6de7
> >
>
> Oh! I thought this cleanup along with the
> block-turn-some-buggy-macros-into-c-functions.patch from Andrew could be done
> on top of my original fix. But didn't, as you said, you would do it. Anyway,
> Thanks.

Refactor and then fix is the cleanest I think. Anyway, queued up for
2.6.31!

--
Jens Axboe