Re: [PATCH] trace bio queueing trial only when it occurs

From: Minchan Kim
Date: Tue Sep 08 2009 - 00:12:27 EST


On Tue, Sep 8, 2009 at 11:02 AM, Li Zefan<lizf@xxxxxxxxxxxxxx> 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@xxxxxxxxx>
>>> ---
>>>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/