* [PATCH 1/3] block: add missing block_bio_complete() tracepoint
@ 2012-01-17 1:32 Namhyung Kim
2012-01-17 1:32 ` [PATCH 2/3] block: prevent duplicated bio completion report Namhyung Kim
2012-01-17 1:32 ` [PATCH 3/3] block: don't export block_bio_complete tracepoint Namhyung Kim
0 siblings, 2 replies; 6+ messages in thread
From: Namhyung Kim @ 2012-01-17 1:32 UTC (permalink / raw)
To: Jens Axboe; +Cc: Namhyung Kim, linux-kernel, Tejun Heo, Steven Rostedt
The block_bio_complete() TP has been missed so long,
so that bio-based drivers haven't been able to trace
its IO behavior. Add it.
In some rare cases, such as loop_switch, @bio->bi_bdev
can be NULL. Thus convert it to TRACE_EVENT_CONDITION()
as Steven suggested.
>From now on, request-based drivers will also get
BLK_TA_COMPLETEs for all bio's in requests. This needs
to be handled in userland properly.
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
fs/bio.c | 2 ++
include/trace/events/block.h | 4 +++-
2 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/fs/bio.c b/fs/bio.c
index b1fe82cf88cf..14c03eaf384e 100644
--- a/fs/bio.c
+++ b/fs/bio.c
@@ -1447,6 +1447,8 @@ void bio_endio(struct bio *bio, int error)
else if (!test_bit(BIO_UPTODATE, &bio->bi_flags))
error = -EIO;
+ trace_block_bio_complete(bdev_get_queue(bio->bi_bdev), bio, error);
+
if (bio->bi_end_io)
bio->bi_end_io(bio, error);
}
diff --git a/include/trace/events/block.h b/include/trace/events/block.h
index 05c5e61f0a7c..96955f4828b3 100644
--- a/include/trace/events/block.h
+++ b/include/trace/events/block.h
@@ -213,12 +213,14 @@ TRACE_EVENT(block_bio_bounce,
* This tracepoint indicates there is no further work to do on this
* block IO operation @bio.
*/
-TRACE_EVENT(block_bio_complete,
+TRACE_EVENT_CONDITION(block_bio_complete,
TP_PROTO(struct request_queue *q, struct bio *bio, int error),
TP_ARGS(q, bio, error),
+ TP_CONDITION(bio->bi_bdev != NULL),
+
TP_STRUCT__entry(
__field( dev_t, dev )
__field( sector_t, sector )
--
1.7.9.rc1.dirty
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] block: prevent duplicated bio completion report
2012-01-17 1:32 [PATCH 1/3] block: add missing block_bio_complete() tracepoint Namhyung Kim
@ 2012-01-17 1:32 ` Namhyung Kim
2012-01-17 17:45 ` Tejun Heo
2012-01-17 1:32 ` [PATCH 3/3] block: don't export block_bio_complete tracepoint Namhyung Kim
1 sibling, 1 reply; 6+ messages in thread
From: Namhyung Kim @ 2012-01-17 1:32 UTC (permalink / raw)
To: Jens Axboe; +Cc: Namhyung Kim, linux-kernel, Tejun Heo
Since previous patch make block_bio_complete TP working,
it will generate duplicated BLK_TA_COMPLETEs for bounced
bios. Fix it.
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Cc: Tejun Heo <tj@kernel.org>
---
include/linux/blk_types.h | 3 +++
include/trace/events/block.h | 3 ++-
2 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 4053cbd4490e..45cd0074a1c8 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -97,6 +97,9 @@ struct bio {
#define BIO_MAPPED_INTEGRITY 11/* integrity metadata has been remapped */
#define bio_flagged(bio, flag) ((bio)->bi_flags & (1 << (flag)))
+/* masked bio's won't report its completion via tracepoint */
+#define BIO_COMPLETE_MASK (1 << BIO_BOUNCED)
+
/*
* top 4 bits of bio flags indicate the pool this bio came from
*/
diff --git a/include/trace/events/block.h b/include/trace/events/block.h
index 96955f4828b3..72888542e186 100644
--- a/include/trace/events/block.h
+++ b/include/trace/events/block.h
@@ -219,7 +219,8 @@ TRACE_EVENT_CONDITION(block_bio_complete,
TP_ARGS(q, bio, error),
- TP_CONDITION(bio->bi_bdev != NULL),
+ TP_CONDITION(bio->bi_bdev != NULL &&
+ !(bio->bi_flags & BIO_COMPLETE_MASK)),
TP_STRUCT__entry(
__field( dev_t, dev )
--
1.7.9.rc1.dirty
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] block: prevent duplicated bio completion report
2012-01-17 1:32 ` [PATCH 2/3] block: prevent duplicated bio completion report Namhyung Kim
@ 2012-01-17 17:45 ` Tejun Heo
2012-01-18 1:20 ` Namhyung Kim
0 siblings, 1 reply; 6+ messages in thread
From: Tejun Heo @ 2012-01-17 17:45 UTC (permalink / raw)
To: Namhyung Kim; +Cc: Jens Axboe, Namhyung Kim, linux-kernel
Hello,
On Tue, Jan 17, 2012 at 10:32:07AM +0900, Namhyung Kim wrote:
> Since previous patch make block_bio_complete TP working,
> it will generate duplicated BLK_TA_COMPLETEs for bounced
> bios. Fix it.
>
> Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
> Cc: Tejun Heo <tj@kernel.org>
This and the third patch should probably be merged to the first patch.
As it currently stands, it introduces window where spurious events are
generated.
> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
> index 4053cbd4490e..45cd0074a1c8 100644
> --- a/include/linux/blk_types.h
> +++ b/include/linux/blk_types.h
> @@ -97,6 +97,9 @@ struct bio {
> #define BIO_MAPPED_INTEGRITY 11/* integrity metadata has been remapped */
> #define bio_flagged(bio, flag) ((bio)->bi_flags & (1 << (flag)))
>
> +/* masked bio's won't report its completion via tracepoint */
> +#define BIO_COMPLETE_MASK (1 << BIO_BOUNCED)
And, who's setting this flag?
> /*
> * top 4 bits of bio flags indicate the pool this bio came from
> */
> diff --git a/include/trace/events/block.h b/include/trace/events/block.h
> index 96955f4828b3..72888542e186 100644
> --- a/include/trace/events/block.h
> +++ b/include/trace/events/block.h
> @@ -219,7 +219,8 @@ TRACE_EVENT_CONDITION(block_bio_complete,
>
> TP_ARGS(q, bio, error),
>
> - TP_CONDITION(bio->bi_bdev != NULL),
> + TP_CONDITION(bio->bi_bdev != NULL &&
> + !(bio->bi_flags & BIO_COMPLETE_MASK)),
Bounced bio's are separate bio's too and I don't think masking its
completion from the TP itself is a good idea. As I wrote before, why
not do this from blktrace code?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] block: prevent duplicated bio completion report
2012-01-17 17:45 ` Tejun Heo
@ 2012-01-18 1:20 ` Namhyung Kim
2012-01-19 1:14 ` Namhyung Kim
0 siblings, 1 reply; 6+ messages in thread
From: Namhyung Kim @ 2012-01-18 1:20 UTC (permalink / raw)
To: linux-kernel; +Cc: Namhyung Kim, Jens Axboe, linux-kernel
Hi,
2012-01-18 2:45 AM, Tejun Heo wrote:
> Hello,
>
> On Tue, Jan 17, 2012 at 10:32:07AM +0900, Namhyung Kim wrote:
>> Since previous patch make block_bio_complete TP working,
>> it will generate duplicated BLK_TA_COMPLETEs for bounced
>> bios. Fix it.
>>
>> Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
>> Cc: Tejun Heo <tj@kernel.org>
>
> This and the third patch should probably be merged to the first patch.
> As it currently stands, it introduces window where spurious events are
> generated.
OK, will do.
>
>> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
>> index 4053cbd4490e..45cd0074a1c8 100644
>> --- a/include/linux/blk_types.h
>> +++ b/include/linux/blk_types.h
>> @@ -97,6 +97,9 @@ struct bio {
>> #define BIO_MAPPED_INTEGRITY 11/* integrity metadata has been remapped */
>> #define bio_flagged(bio, flag) ((bio)->bi_flags& (1<< (flag)))
>>
>> +/* masked bio's won't report its completion via tracepoint */
>> +#define BIO_COMPLETE_MASK (1<< BIO_BOUNCED)
>
> And, who's setting this flag?
__blk_queue_bounce() does.
>
>> /*
>> * top 4 bits of bio flags indicate the pool this bio came from
>> */
>> diff --git a/include/trace/events/block.h b/include/trace/events/block.h
>> index 96955f4828b3..72888542e186 100644
>> --- a/include/trace/events/block.h
>> +++ b/include/trace/events/block.h
>> @@ -219,7 +219,8 @@ TRACE_EVENT_CONDITION(block_bio_complete,
>>
>> TP_ARGS(q, bio, error),
>>
>> - TP_CONDITION(bio->bi_bdev != NULL),
>> + TP_CONDITION(bio->bi_bdev != NULL&&
>> + !(bio->bi_flags& BIO_COMPLETE_MASK)),
>
> Bounced bio's are separate bio's too and I don't think masking its
> completion from the TP itself is a good idea. As I wrote before, why
> not do this from blktrace code?
Because blktrace cannot know about the bi_flags, as I said before. :)
And although the bounced bio's are separate ones, they aren't queued
separately. They just get replaced on the way.
Besides, I think accounting wait_time of them will result in an invalid
value unless it's handled somehow in block_bio_bounce and/or
block_rq_issue TP.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] block: prevent duplicated bio completion report
2012-01-18 1:20 ` Namhyung Kim
@ 2012-01-19 1:14 ` Namhyung Kim
0 siblings, 0 replies; 6+ messages in thread
From: Namhyung Kim @ 2012-01-19 1:14 UTC (permalink / raw)
Cc: linux-kernel, Namhyung Kim, Jens Axboe
Hello,
2012-01-18 10:20 AM, Namhyung Kim wrote:
> Hi,
>
> 2012-01-18 2:45 AM, Tejun Heo wrote:
>>
>>> /*
>>> * top 4 bits of bio flags indicate the pool this bio came from
>>> */
>>> diff --git a/include/trace/events/block.h b/include/trace/events/block.h
>>> index 96955f4828b3..72888542e186 100644
>>> --- a/include/trace/events/block.h
>>> +++ b/include/trace/events/block.h
>>> @@ -219,7 +219,8 @@ TRACE_EVENT_CONDITION(block_bio_complete,
>>>
>>> TP_ARGS(q, bio, error),
>>>
>>> - TP_CONDITION(bio->bi_bdev != NULL),
>>> + TP_CONDITION(bio->bi_bdev != NULL &&
>>> + !(bio->bi_flags & BIO_COMPLETE_MASK)),
>>
>> Bounced bio's are separate bio's too and I don't think masking its
>> completion from the TP itself is a good idea. As I wrote before, why
>> not do this from blktrace code?
>
> Because blktrace cannot know about the bi_flags, as I said before. :)
> And although the bounced bio's are separate ones, they aren't queued
> separately. They just get replaced on the way.
Oh, now I guess you meant this:
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index 16fc34a0806f..2ef57fb2566e 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -792,6 +792,9 @@ static void blk_add_trace_bio_complete(void *ignore,
struct request_queue *q, struct bio *bio,
int error)
{
+ if (bio->bi_flags & BIO_COMPLETE_MASK)
+ return;
+
blk_add_trace_bio(q, bio, BLK_TA_COMPLETE, error);
}
Anyway do you still think masking on TP is not a good idea?
Thanks,
Namhyung
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] block: don't export block_bio_complete tracepoint
2012-01-17 1:32 [PATCH 1/3] block: add missing block_bio_complete() tracepoint Namhyung Kim
2012-01-17 1:32 ` [PATCH 2/3] block: prevent duplicated bio completion report Namhyung Kim
@ 2012-01-17 1:32 ` Namhyung Kim
1 sibling, 0 replies; 6+ messages in thread
From: Namhyung Kim @ 2012-01-17 1:32 UTC (permalink / raw)
To: Jens Axboe; +Cc: Namhyung Kim, linux-kernel, dm-devel
Now bio_endio() contains the tracepoint in it, so we don't
need to have it twice in DM. Plus, as the only external user
of the tracepoint was the DM, we can unexport the symbol.
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Cc: dm-devel@redhat.com
---
block/blk-core.c | 1 -
drivers/md/dm.c | 1 -
2 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 1b4fd93af2c0..399c128f516c 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -37,7 +37,6 @@
EXPORT_TRACEPOINT_SYMBOL_GPL(block_bio_remap);
EXPORT_TRACEPOINT_SYMBOL_GPL(block_rq_remap);
-EXPORT_TRACEPOINT_SYMBOL_GPL(block_bio_complete);
DEFINE_IDA(blk_queue_ida);
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 4720f68f817e..01185fa0eb74 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -648,7 +648,6 @@ static void dec_pending(struct dm_io *io, int error)
queue_io(md, bio);
} else {
/* done with normal IO or empty flush */
- trace_block_bio_complete(md->queue, bio, io_error);
bio_endio(bio, io_error);
}
}
--
1.7.9.rc1.dirty
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-01-19 1:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-17 1:32 [PATCH 1/3] block: add missing block_bio_complete() tracepoint Namhyung Kim
2012-01-17 1:32 ` [PATCH 2/3] block: prevent duplicated bio completion report Namhyung Kim
2012-01-17 17:45 ` Tejun Heo
2012-01-18 1:20 ` Namhyung Kim
2012-01-19 1:14 ` Namhyung Kim
2012-01-17 1:32 ` [PATCH 3/3] block: don't export block_bio_complete tracepoint Namhyung Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).