[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20150608120330.4ecb73b7@gandalf.local.home>
Date: Mon, 8 Jun 2015 12:03:30 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Rasmus Villemoes <linux@...musvillemoes.dk>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
Jens Axboe <axboe@...nel.dk>
Subject: Re: [RFC/PATCH 3/8] blktrace: use strreplace in do_blk_trace_setup
blktrace is maintained by Jens Axboe (added to the Cc), although I also
maintain it for cleanups such as this. That's the reason it doesn't
have a "trace_" in its name like the other files do (a bit of trivia for
you ;-)
kernel/trace/blktrace.c should probably be included in the MAINTAINERS
file under BLOCK LAYER.
On Thu, 4 Jun 2015 11:37:10 +0200
Rasmus Villemoes <linux@...musvillemoes.dk> 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.
>
> Signed-off-by: Rasmus Villemoes <linux@...musvillemoes.dk>
> ---
> kernel/trace/blktrace.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
> index 483cecfa5c17..4eeae4674b5a 100644
> --- a/kernel/trace/blktrace.c
> +++ b/kernel/trace/blktrace.c
> @@ -439,7 +439,7 @@ int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
> {
> struct blk_trace *old_bt, *bt = NULL;
> struct dentry *dir = NULL;
> - int ret, i;
> + int ret;
>
> if (!buts->buf_size || !buts->buf_nr)
> return -EINVAL;
> @@ -451,9 +451,7 @@ int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
> * some device names have larger paths - convert the slashes
> * to underscores for this to work as expected
> */
> - for (i = 0; i < strlen(buts->name); i++)
Wow! I'm surprised this wasn't:
for (i = 0; buts->name[i]; i++)
> - if (buts->name[i] == '/')
> - buts->name[i] = '_';
> + strreplace(buts->name, '/', '_');
>
> bt = kzalloc(sizeof(*bt), GFP_KERNEL);
> if (!bt)
Acked-by: Steven Rostedt <rostedt@...dmis.org>
-- Steve
--
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