Re: [PATCH v2 3/8] blktrace: use strreplace in do_blk_trace_setup

[Jens Axboe]

On 06/08/2015 05:26 PM, Rasmus Villemoes wrote:
> Part of the disassembly of do_blk_trace_setup:
>
>      231b:       e8 00 00 00 00          callq  2320 <do_blk_trace_setup+0x50>
>                          231c: R_X86_64_PC32     strlen+0xfffffffffffffffc
>      2320:       eb 0a                   jmp    232c <do_blk_trace_setup+0x5c>
>      2322:       66 0f 1f 44 00 00       nopw   0x0(%rax,%rax,1)
>      2328:       48 83 c3 01             add    $0x1,%rbx
>      232c:       48 39 d8                cmp    %rbx,%rax
>      232f:       76 47                   jbe    2378 <do_blk_trace_setup+0xa8>
>      2331:       41 80 3c 1c 2f          cmpb   $0x2f,(%r12,%rbx,1)
>      2336:       75 f0                   jne    2328 <do_blk_trace_setup+0x58>
>      2338:       41 c6 04 1c 5f          movb   $0x5f,(%r12,%rbx,1)
>      233d:       4c 89 e7                mov    %r12,%rdi
>      2340:       e8 00 00 00 00          callq  2345 <do_blk_trace_setup+0x75>
>                          2341: R_X86_64_PC32     strlen+0xfffffffffffffffc
>      2345:       eb e1                   jmp    2328 <do_blk_trace_setup+0x58>
>
> Yep, that's right: gcc isn't smart enough to realize that replacing
> '/' by '_' cannot change the strlen(), so we call it again and again
> (at least when a '/' is found). Even if gcc were that smart, this
> construction would still loop over the string twice, once for the
> initial strlen() call and then the open-coded loop.
>
> Let's simply use strreplace() instead.

Patch looks fine to me, but there's no strreplace in in Linus' tree. 
Dependencies like that should be noted in the patch. Please send 
followup patches like this when the main patch is in Linus tree, and I'd 
be happy to apply it.

--
Jens Axboe

--
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/