Commit 35fe6d7632 breaks blktrace API

Commit 35fe6d7632 breaks blktrace API

[Jan Kara]

Hello,

I was investigating a performance issue with BFQ IO scheduler and I was
pondering why I'm not seeing informational messages from BFQ. After quite
some debugging I have found out that commit 35fe6d763229 "block: use
standard blktrace API to output cgroup info for debug notes" broke standard
blktrace API - namely the informational messages logged by bfq_log_bfqq()
are no longer displayed by blkparse(8) tool. This is because these messages
have now __BLK_TA_CGROUP bit set and that breaks flags checking in
blkparse(8). It isn't that hard to fix blkparse once you know what the
problem is but I've wasted couple hours on this...

Also apparently nobody tested the patch with blkparse(8) since 4.14
days? Admittedly this requires CONFIG_BFQ_GROUP_IOSCHED and having cgroups
set up for the cgroup info to get emitted which probably is not that common
with non-production machines.

Anyway, what to do now? Update blkparse(8) and hope nobody else is using
the blktrace API (likely I'd say)? Revert the change?

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: Commit 35fe6d7632 breaks blktrace API

[Jens Axboe]

On 4/30/20 5:47 AM, Jan Kara wrote:
> Hello,
> 
> I was investigating a performance issue with BFQ IO scheduler and I was
> pondering why I'm not seeing informational messages from BFQ. After quite
> some debugging I have found out that commit 35fe6d763229 "block: use
> standard blktrace API to output cgroup info for debug notes" broke standard
> blktrace API - namely the informational messages logged by bfq_log_bfqq()
> are no longer displayed by blkparse(8) tool. This is because these messages
> have now __BLK_TA_CGROUP bit set and that breaks flags checking in
> blkparse(8). It isn't that hard to fix blkparse once you know what the
> problem is but I've wasted couple hours on this...
> 
> Also apparently nobody tested the patch with blkparse(8) since 4.14
> days? Admittedly this requires CONFIG_BFQ_GROUP_IOSCHED and having cgroups
> set up for the cgroup info to get emitted which probably is not that common
> with non-production machines.
> 
> Anyway, what to do now? Update blkparse(8) and hope nobody else is using
> the blktrace API (likely I'd say)? Revert the change?

It's been this long and nobody has complained, I think we should just update
blkparse.

-- 
Jens Axboe

Re: Commit 35fe6d7632 breaks blktrace API

[Jan Kara]

On Thu 30-04-20 07:10:47, Jens Axboe wrote:
> On 4/30/20 5:47 AM, Jan Kara wrote:
> > Hello,
> > 
> > I was investigating a performance issue with BFQ IO scheduler and I was
> > pondering why I'm not seeing informational messages from BFQ. After quite
> > some debugging I have found out that commit 35fe6d763229 "block: use
> > standard blktrace API to output cgroup info for debug notes" broke standard
> > blktrace API - namely the informational messages logged by bfq_log_bfqq()
> > are no longer displayed by blkparse(8) tool. This is because these messages
> > have now __BLK_TA_CGROUP bit set and that breaks flags checking in
> > blkparse(8). It isn't that hard to fix blkparse once you know what the
> > problem is but I've wasted couple hours on this...
> > 
> > Also apparently nobody tested the patch with blkparse(8) since 4.14
> > days? Admittedly this requires CONFIG_BFQ_GROUP_IOSCHED and having cgroups
> > set up for the cgroup info to get emitted which probably is not that common
> > with non-production machines.
> > 
> > Anyway, what to do now? Update blkparse(8) and hope nobody else is using
> > the blktrace API (likely I'd say)? Revert the change?
> 
> It's been this long and nobody has complained, I think we should just update
> blkparse.

Fine by me although I would note that e.g. most of our customer are running
on 4.4 or 4.12-based kernels so if any of them has their custom scripting
using blktrace API (I seriously doubt that though), they'll probably notice
the problem only in couple of years... But as I said I doubt there's
anybody doing this and blktrace is a debugging API so I'm fine with just
fixing blkparse and crossing my fingers ;) I'll send the patch after some
polishing (I just quickly hacked it up so that I can proceed with my
analysis).

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: Commit 35fe6d7632 breaks blktrace API

[Jens Axboe]

On 4/30/20 9:59 AM, Jan Kara wrote:
> On Thu 30-04-20 07:10:47, Jens Axboe wrote:
>> On 4/30/20 5:47 AM, Jan Kara wrote:
>>> Hello,
>>>
>>> I was investigating a performance issue with BFQ IO scheduler and I was
>>> pondering why I'm not seeing informational messages from BFQ. After quite
>>> some debugging I have found out that commit 35fe6d763229 "block: use
>>> standard blktrace API to output cgroup info for debug notes" broke standard
>>> blktrace API - namely the informational messages logged by bfq_log_bfqq()
>>> are no longer displayed by blkparse(8) tool. This is because these messages
>>> have now __BLK_TA_CGROUP bit set and that breaks flags checking in
>>> blkparse(8). It isn't that hard to fix blkparse once you know what the
>>> problem is but I've wasted couple hours on this...
>>>
>>> Also apparently nobody tested the patch with blkparse(8) since 4.14
>>> days? Admittedly this requires CONFIG_BFQ_GROUP_IOSCHED and having cgroups
>>> set up for the cgroup info to get emitted which probably is not that common
>>> with non-production machines.
>>>
>>> Anyway, what to do now? Update blkparse(8) and hope nobody else is using
>>> the blktrace API (likely I'd say)? Revert the change?
>>
>> It's been this long and nobody has complained, I think we should just update
>> blkparse.
> 
> Fine by me although I would note that e.g. most of our customer are running
> on 4.4 or 4.12-based kernels so if any of them has their custom scripting
> using blktrace API (I seriously doubt that though), they'll probably notice
> the problem only in couple of years... But as I said I doubt there's
> anybody doing this and blktrace is a debugging API so I'm fine with just
> fixing blkparse and crossing my fingers ;) I'll send the patch after some
> polishing (I just quickly hacked it up so that I can proceed with my
> analysis).

In any case, much easier to push a blktrace/blkparse update! Thanks, I'll
keep an eye out for your patch.

-- 
Jens Axboe