[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <557657EE.80809@kernel.dk>
Date: Mon, 08 Jun 2015 21:05:18 -0600
From: Jens Axboe <axboe@...nel.dk>
To: Rasmus Villemoes <linux@...musvillemoes.dk>,
Andrew Morton <akpm@...ux-foundation.org>,
Steven Rostedt <rostedt@...dmis.org>,
Ingo Molnar <mingo@...hat.com>
CC: Joe Perches <joe@...ches.com>, Al Viro <viro@...IV.linux.org.uk>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/8] blktrace: use strreplace in do_blk_trace_setup
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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists