* [PATCH] trace bio queueing trial only when it occurs
@ 2009-09-08 0:34 Minchan Kim
2009-09-08 0:55 ` Wu Fengguang
0 siblings, 1 reply; 8+ messages in thread
From: Minchan Kim @ 2009-09-08 0:34 UTC (permalink / raw)
To: Andrew Morton, Wu Fengguang; +Cc: lkml, Jens Axboe
It got lost in LKML mail storm.
I resend this.
Wu, Could you review this patch, please?
== CUT_HERE ==
If BIO is discarded or cross over end of device,
BIO queueing trial doesn't occur.
Let's trace it only when it happens.
Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
---
block/blk-core.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 5b59592..1a0cfd5 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1463,8 +1463,6 @@ static inline void __generic_make_request(struct bio *bio)
if (old_sector != -1)
trace_block_remap(q, bio, old_dev, old_sector);
- trace_block_bio_queue(q, bio);
-
old_sector = bio->bi_sector;
old_dev = bio->bi_bdev->bd_dev;
@@ -1477,6 +1475,8 @@ static inline void __generic_make_request(struct bio *bio)
goto end_io;
}
+ trace_block_bio_queue(q, bio);
+
ret = q->make_request_fn(q, bio);
} while (ret);
--
1.6.4
--
Kind regards,
Minchan Kim
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] trace bio queueing trial only when it occurs
2009-09-08 0:34 [PATCH] trace bio queueing trial only when it occurs Minchan Kim
@ 2009-09-08 0:55 ` Wu Fengguang
2009-09-08 2:02 ` Li Zefan
2009-09-08 10:50 ` Jens Axboe
0 siblings, 2 replies; 8+ messages in thread
From: Wu Fengguang @ 2009-09-08 0:55 UTC (permalink / raw)
To: Minchan Kim; +Cc: Andrew Morton, lkml, Jens Axboe, lizf@cn.fujitsu.com
Minchan,
I tend to agree with the change, but somehow confused by blkparse(1):
Q -- queued This notes *intent* to queue i/o at the given location. No real requests exists yet.
Li Zefan has been working on blktrace and he can tell more.
Thanks,
Fengguang
On Tue, Sep 08, 2009 at 08:34:16AM +0800, Minchan Kim wrote:
>
> It got lost in LKML mail storm.
> I resend this.
> Wu, Could you review this patch, please?
>
> == CUT_HERE ==
>
> If BIO is discarded or cross over end of device,
> BIO queueing trial doesn't occur.
>
> Let's trace it only when it happens.
>
> Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
> ---
> block/blk-core.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 5b59592..1a0cfd5 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -1463,8 +1463,6 @@ static inline void __generic_make_request(struct bio *bio)
> if (old_sector != -1)
> trace_block_remap(q, bio, old_dev, old_sector);
>
> - trace_block_bio_queue(q, bio);
> -
> old_sector = bio->bi_sector;
> old_dev = bio->bi_bdev->bd_dev;
>
> @@ -1477,6 +1475,8 @@ static inline void __generic_make_request(struct bio *bio)
> goto end_io;
> }
>
> + trace_block_bio_queue(q, bio);
> +
> ret = q->make_request_fn(q, bio);
> } while (ret);
>
> --
> 1.6.4
>
>
>
> --
> Kind regards,
> Minchan Kim
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] trace bio queueing trial only when it occurs
2009-09-08 0:55 ` Wu Fengguang
@ 2009-09-08 2:02 ` Li Zefan
2009-09-08 4:10 ` Minchan Kim
2009-09-08 10:50 ` Jens Axboe
1 sibling, 1 reply; 8+ messages in thread
From: Li Zefan @ 2009-09-08 2:02 UTC (permalink / raw)
To: Wu Fengguang; +Cc: Minchan Kim, Andrew Morton, lkml, Jens Axboe
Wu Fengguang wrote:
> Minchan,
>
> I tend to agree with the change, but somehow confused by blkparse(1):
>
> Q -- queued This notes *intent* to queue i/o at the given location. No real requests exists yet.
>
> Li Zefan has been working on blktrace and he can tell more.
>
I work on ftrace-plugin blktrace and blk TRACE_EVENT, but don't know
much about the old blktrace history. ;)
I think the manpage says it's called before ->make_request_fn(), so
"No real requests exist yet".
Actually the trace was called just before make_request at first:
2056a782f8e7e65fd4bfd027506b4ce1c5e9ccd4
And then 2 patches added some checks between them:
5ddfe9691c91a244e8d1be597b6428fcefd58103
51fd77bd9f512ab6cc9df0733ba1caaab89eb957
It seems to me it makes sense to have this patch.
> Thanks,
> Fengguang
>
> On Tue, Sep 08, 2009 at 08:34:16AM +0800, Minchan Kim wrote:
>> It got lost in LKML mail storm.
>> I resend this.
>> Wu, Could you review this patch, please?
>>
>> == CUT_HERE ==
>>
>> If BIO is discarded or cross over end of device,
>> BIO queueing trial doesn't occur.
>>
>> Let's trace it only when it happens.
>>
>> Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
>> ---
>> block/blk-core.c | 4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/block/blk-core.c b/block/blk-core.c
>> index 5b59592..1a0cfd5 100644
>> --- a/block/blk-core.c
>> +++ b/block/blk-core.c
>> @@ -1463,8 +1463,6 @@ static inline void __generic_make_request(struct bio *bio)
>> if (old_sector != -1)
>> trace_block_remap(q, bio, old_dev, old_sector);
>>
>> - trace_block_bio_queue(q, bio);
>> -
>> old_sector = bio->bi_sector;
>> old_dev = bio->bi_bdev->bd_dev;
>>
>> @@ -1477,6 +1475,8 @@ static inline void __generic_make_request(struct bio *bio)
>> goto end_io;
>> }
>>
>> + trace_block_bio_queue(q, bio);
>> +
>> ret = q->make_request_fn(q, bio);
>> } while (ret);
>>
>> --
>> 1.6.4
>>
>>
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] trace bio queueing trial only when it occurs
2009-09-08 2:02 ` Li Zefan
@ 2009-09-08 4:10 ` Minchan Kim
2009-09-08 4:13 ` Wu Fengguang
0 siblings, 1 reply; 8+ messages in thread
From: Minchan Kim @ 2009-09-08 4:10 UTC (permalink / raw)
To: Li Zefan; +Cc: Wu Fengguang, Andrew Morton, lkml, Jens Axboe
On Tue, Sep 8, 2009 at 11:02 AM, Li Zefan<lizf@cn.fujitsu.com> wrote:
> Wu Fengguang wrote:
>> Minchan,
>>
>> I tend to agree with the change, but somehow confused by blkparse(1):
>>
>> Q -- queued This notes *intent* to queue i/o at the given location. No real requests exists yet.
>>
>> Li Zefan has been working on blktrace and he can tell more.
>>
>
> I work on ftrace-plugin blktrace and blk TRACE_EVENT, but don't know
> much about the old blktrace history. ;)
>
> I think the manpage says it's called before ->make_request_fn(), so
> "No real requests exist yet".
>
> Actually the trace was called just before make_request at first:
> 2056a782f8e7e65fd4bfd027506b4ce1c5e9ccd4
>
> And then 2 patches added some checks between them:
> 5ddfe9691c91a244e8d1be597b6428fcefd58103
> 51fd77bd9f512ab6cc9df0733ba1caaab89eb957
>
> It seems to me it makes sense to have this patch.
Thanks for good information.
Yes. It seems above 2 patches seem to break the rule.
Could I add your ACK?
>> Thanks,
>> Fengguang
>>
>> On Tue, Sep 08, 2009 at 08:34:16AM +0800, Minchan Kim wrote:
>>> It got lost in LKML mail storm.
>>> I resend this.
>>> Wu, Could you review this patch, please?
>>>
>>> == CUT_HERE ==
>>>
>>> If BIO is discarded or cross over end of device,
>>> BIO queueing trial doesn't occur.
>>>
>>> Let's trace it only when it happens.
>>>
>>> Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
>>> ---
>>> block/blk-core.c | 4 ++--
>>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/block/blk-core.c b/block/blk-core.c
>>> index 5b59592..1a0cfd5 100644
>>> --- a/block/blk-core.c
>>> +++ b/block/blk-core.c
>>> @@ -1463,8 +1463,6 @@ static inline void __generic_make_request(struct bio *bio)
>>> if (old_sector != -1)
>>> trace_block_remap(q, bio, old_dev, old_sector);
>>>
>>> - trace_block_bio_queue(q, bio);
>>> -
>>> old_sector = bio->bi_sector;
>>> old_dev = bio->bi_bdev->bd_dev;
>>>
>>> @@ -1477,6 +1475,8 @@ static inline void __generic_make_request(struct bio *bio)
>>> goto end_io;
>>> }
>>>
>>> + trace_block_bio_queue(q, bio);
>>> +
>>> ret = q->make_request_fn(q, bio);
>>> } while (ret);
>>>
>>> --
>>> 1.6.4
>>>
>>>
>>>
>
--
Kind regards,
Minchan Kim
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] trace bio queueing trial only when it occurs
2009-09-08 4:10 ` Minchan Kim
@ 2009-09-08 4:13 ` Wu Fengguang
2009-09-08 4:24 ` Minchan Kim
0 siblings, 1 reply; 8+ messages in thread
From: Wu Fengguang @ 2009-09-08 4:13 UTC (permalink / raw)
To: Minchan Kim; +Cc: Li Zefan, Andrew Morton, lkml, Jens Axboe
On Tue, Sep 08, 2009 at 12:10:55PM +0800, Minchan Kim wrote:
> On Tue, Sep 8, 2009 at 11:02 AM, Li Zefan<lizf@cn.fujitsu.com> wrote:
> > Wu Fengguang wrote:
> >> Minchan,
> >>
> >> I tend to agree with the change, but somehow confused by blkparse(1):
> >>
> >> Q -- queued This notes *intent* to queue i/o at the given location. No real requests exists yet.
> >>
> >> Li Zefan has been working on blktrace and he can tell more.
> >>
> >
> > I work on ftrace-plugin blktrace and blk TRACE_EVENT, but don't know
> > much about the old blktrace history. ;)
> >
> > I think the manpage says it's called before ->make_request_fn(), so
> > "No real requests exist yet".
> >
> > Actually the trace was called just before make_request at first:
> > 2056a782f8e7e65fd4bfd027506b4ce1c5e9ccd4
> >
> > And then 2 patches added some checks between them:
> > 5ddfe9691c91a244e8d1be597b6428fcefd58103
> > 51fd77bd9f512ab6cc9df0733ba1caaab89eb957
> >
> > It seems to me it makes sense to have this patch.
>
> Thanks for good information.
> Yes. It seems above 2 patches seem to break the rule.
> Could I add your ACK?
Sure, thanks!
Acked-by: Wu Fengguang <fengguang.wu@intel.com>
> >> Thanks,
> >> Fengguang
> >>
> >> On Tue, Sep 08, 2009 at 08:34:16AM +0800, Minchan Kim wrote:
> >>> It got lost in LKML mail storm.
> >>> I resend this.
> >>> Wu, Could you review this patch, please?
> >>>
> >>> == CUT_HERE ==
> >>>
> >>> If BIO is discarded or cross over end of device,
> >>> BIO queueing trial doesn't occur.
> >>>
> >>> Let's trace it only when it happens.
> >>>
> >>> Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
> >>> ---
> >>> block/blk-core.c | 4 ++--
> >>> 1 files changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/block/blk-core.c b/block/blk-core.c
> >>> index 5b59592..1a0cfd5 100644
> >>> --- a/block/blk-core.c
> >>> +++ b/block/blk-core.c
> >>> @@ -1463,8 +1463,6 @@ static inline void __generic_make_request(struct bio *bio)
> >>> if (old_sector != -1)
> >>> trace_block_remap(q, bio, old_dev, old_sector);
> >>>
> >>> - trace_block_bio_queue(q, bio);
> >>> -
> >>> old_sector = bio->bi_sector;
> >>> old_dev = bio->bi_bdev->bd_dev;
> >>>
> >>> @@ -1477,6 +1475,8 @@ static inline void __generic_make_request(struct bio *bio)
> >>> goto end_io;
> >>> }
> >>>
> >>> + trace_block_bio_queue(q, bio);
> >>> +
> >>> ret = q->make_request_fn(q, bio);
> >>> } while (ret);
> >>>
> >>> --
> >>> 1.6.4
> >>>
> >>>
> >>>
> >
>
>
>
> --
> Kind regards,
> Minchan Kim
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] trace bio queueing trial only when it occurs
2009-09-08 4:13 ` Wu Fengguang
@ 2009-09-08 4:24 ` Minchan Kim
0 siblings, 0 replies; 8+ messages in thread
From: Minchan Kim @ 2009-09-08 4:24 UTC (permalink / raw)
To: Wu Fengguang; +Cc: Li Zefan, Andrew Morton, lkml, Jens Axboe
On Tue, Sep 8, 2009 at 1:13 PM, Wu Fengguang<fengguang.wu@intel.com> wrote:
> On Tue, Sep 08, 2009 at 12:10:55PM +0800, Minchan Kim wrote:
>> On Tue, Sep 8, 2009 at 11:02 AM, Li Zefan<lizf@cn.fujitsu.com> wrote:
>> > Wu Fengguang wrote:
>> >> Minchan,
>> >>
>> >> I tend to agree with the change, but somehow confused by blkparse(1):
>> >>
>> >> Q -- queued This notes *intent* to queue i/o at the given location. No real requests exists yet.
>> >>
>> >> Li Zefan has been working on blktrace and he can tell more.
>> >>
>> >
>> > I work on ftrace-plugin blktrace and blk TRACE_EVENT, but don't know
>> > much about the old blktrace history. ;)
>> >
>> > I think the manpage says it's called before ->make_request_fn(), so
>> > "No real requests exist yet".
>> >
>> > Actually the trace was called just before make_request at first:
>> > 2056a782f8e7e65fd4bfd027506b4ce1c5e9ccd4
>> >
>> > And then 2 patches added some checks between them:
>> > 5ddfe9691c91a244e8d1be597b6428fcefd58103
>> > 51fd77bd9f512ab6cc9df0733ba1caaab89eb957
>> >
>> > It seems to me it makes sense to have this patch.
>>
>> Thanks for good information.
>> Yes. It seems above 2 patches seem to break the rule.
>> Could I add your ACK?
>
> Sure, thanks!
>
> Acked-by: Wu Fengguang <fengguang.wu@intel.com>
Thanks for careful review, Wu.
--
Kind regards,
Minchan Kim
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] trace bio queueing trial only when it occurs
2009-09-08 0:55 ` Wu Fengguang
2009-09-08 2:02 ` Li Zefan
@ 2009-09-08 10:50 ` Jens Axboe
1 sibling, 0 replies; 8+ messages in thread
From: Jens Axboe @ 2009-09-08 10:50 UTC (permalink / raw)
To: Wu Fengguang; +Cc: Minchan Kim, Andrew Morton, lkml, lizf@cn.fujitsu.com
On Tue, Sep 08 2009, Wu Fengguang wrote:
> Minchan,
>
> I tend to agree with the change, but somehow confused by blkparse(1):
>
> Q -- queued This notes *intent* to queue i/o at the given location. No real requests exists yet.
>
> Li Zefan has been working on blktrace and he can tell more.
The documentation details the original implementation. I'm guessing the
need to move this trace hook is due to some user space tracking that
gets confused, because it cannot match the queue intent with a merge or
an insert.
I have no problems moving the trace call. If we had a matching bio_endio
trace, then I'd say moving it would lose information. But right now it
doesn't.
--
Jens Axboe
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] trace bio queueing trial only when it occurs
@ 2009-09-07 2:15 Minchan Kim
0 siblings, 0 replies; 8+ messages in thread
From: Minchan Kim @ 2009-09-07 2:15 UTC (permalink / raw)
To: Andrew Morton, Jens Axboe; +Cc: Wu Fengguang, lkml
If BIO is discarded or cross over end of device,
BIO queueing trial doesn't occur.
Let's trace it only when it happens.
Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
---
block/blk-core.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 5b59592..1a0cfd5 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1463,8 +1463,6 @@ static inline void __generic_make_request(struct bio *bio)
if (old_sector != -1)
trace_block_remap(q, bio, old_dev, old_sector);
- trace_block_bio_queue(q, bio);
-
old_sector = bio->bi_sector;
old_dev = bio->bi_bdev->bd_dev;
@@ -1477,6 +1475,8 @@ static inline void __generic_make_request(struct bio *bio)
goto end_io;
}
+ trace_block_bio_queue(q, bio);
+
ret = q->make_request_fn(q, bio);
} while (ret);
--
1.6.4
--
Kind regards,
Minchan Kim
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-09-08 10:50 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-09-08 0:34 [PATCH] trace bio queueing trial only when it occurs Minchan Kim
2009-09-08 0:55 ` Wu Fengguang
2009-09-08 2:02 ` Li Zefan
2009-09-08 4:10 ` Minchan Kim
2009-09-08 4:13 ` Wu Fengguang
2009-09-08 4:24 ` Minchan Kim
2009-09-08 10:50 ` Jens Axboe
-- strict thread matches above, loose matches on Subject: below --
2009-09-07 2:15 Minchan 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).