Re: [dm-devel] [PATCH 05/11] block: get rid of the trace rq insert wrapper
On Fri, Jul 03, 2020 at 11:29:25PM +, Chaitanya Kulkarni wrote:
> Christoph,
>
> On 6/29/20 4:44 PM, Chaitanya Kulkarni wrote:
> > Get rid of the wrapper for trace_block_rq_insert() and call the function
> > directly.
> >
> > Signed-off-by: Chaitanya Kulkarni
> > ---
>
> Can we re-consider adding this patch ? here are couple of reasons:-
>
> 1. Increase in the size of the text region of the object file:-
>
> By adding the trace header #include
> in io-scheduler where it is calling trace_block_rq_insert()
> increases the size of the text region of the object file
> kyber(+215) & bfq (+317) [1].
You really shouldn't have both loaded, never mind used at the same
time. It also avoid a function call for the scheduler fast path.
> 2. Mandatory io-sched built-in kernel compilation:-
>
> When testing with a different io-sched KConfig options ("*" vs "M"),
> when kyber and bfq compilation option set to "M" having this patch
> reports error[2].
See EXPORT_TRACEPOINT_SYMBOL_GPL.
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH 05/11] block: get rid of the trace rq insert wrapper
Christoph,
On 6/29/20 4:44 PM, Chaitanya Kulkarni wrote:
> Get rid of the wrapper for trace_block_rq_insert() and call the function
> directly.
>
> Signed-off-by: Chaitanya Kulkarni
> ---
Can we re-consider adding this patch ? here are couple of reasons:-
1. Increase in the size of the text region of the object file:-
By adding the trace header #include
in io-scheduler where it is calling trace_block_rq_insert()
increases the size of the text region of the object file
kyber(+215) & bfq (+317) [1].
2. Mandatory io-sched built-in kernel compilation:-
When testing with a different io-sched KConfig options ("*" vs "M"),
when kyber and bfq compilation option set to "M" having this patch
reports error[2].
If I've not missed something here then can we drop this patch ?
In case we really want to do this change it will need to have KConfig
separate patch such that if tracing is selected it will force *
selection (built-in KConfig) for schedulers in question and etc.
Do we want to do this ?
Regards,
Chaitanya
[1] Scheduler IO object size comparison :-
Without this patch :-
-
# size block/bfq-iosched.o
text data bss dec hex filename
62204 1011 32 63247f70f block/bfq-iosched.o
# size block/kyber-iosched.o
text data bss dec hex filename
14808 2699 48 175554493 block/kyber-iosched.o
With this patch :-
--
# size block/bfq-iosched.o
text data bss dec hex filename
62521 1028 32 63581f85d block/bfq-iosched.o
# size block/kyber-iosched.o
text data bss dec hex filename
15023 2716 48 17787457b block/kyber-iosched.o
[2] Error with selecting M for io-sched kyber & bfq :-
ERROR: modpost: "__tracepoint_block_rq_insert" [block/bfq.ko] undefined!
ERROR: modpost: "__tracepoint_block_rq_insert" [block/kyber-iosched.ko]
undefined!
make[2]: *** [Module.symvers] Error 1
make[2]: *** Deleting file `Module.symvers'
make[1]: *** [modules] Error 2
make[1]: *** Waiting for unfinished jobs
arch/x86/tools/insn_decoder_test: success: Decoded and checked 4932572
instructions
TESTposttest
arch/x86/tools/insn_sanity: Success: decoded and checked 100
random instructions with 0 errors (seed:0x4c6e1a40)
CC arch/x86/boot/version.o
VOFFSET arch/x86/boot/compressed/../voffset.h
OBJCOPY arch/x86/boot/compressed/vmlinux.bin
RELOCS arch/x86/boot/compressed/vmlinux.relocs
CC arch/x86/boot/compressed/kaslr.o
CC arch/x86/boot/compressed/misc.o
GZIParch/x86/boot/compressed/vmlinux.bin.gz
MKPIGGY arch/x86/boot/compressed/piggy.S
AS arch/x86/boot/compressed/piggy.o
LD arch/x86/boot/compressed/vmlinux
ZOFFSET arch/x86/boot/zoffset.h
OBJCOPY arch/x86/boot/vmlinux.bin
AS arch/x86/boot/header.o
LD arch/x86/boot/setup.elf
OBJCOPY arch/x86/boot/setup.bin
BUILD arch/x86/boot/bzImage
Setup is 15132 bytes (padded to 15360 bytes).
System is 8951 kB
CRC ff6eac72
Kernel: arch/x86/boot/bzImage is ready (#59)
make: *** [__sub-make] Error 2
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH 05/11] block: get rid of the trace rq insert wrapper
On 6/29/20 10:11 PM, Christoph Hellwig wrote: > On Mon, Jun 29, 2020 at 04:43:08PM -0700, Chaitanya Kulkarni wrote: >> Get rid of the wrapper for trace_block_rq_insert() and call the function >> directly. > I'd mention blk_mq_sched_request_inserted instead of the tracepoint > in the subject and commit message. Otherwise this looks fine. > Okay, will change the message. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 05/11] block: get rid of the trace rq insert wrapper
Get rid of the wrapper for trace_block_rq_insert() and call the function
directly.
Signed-off-by: Chaitanya Kulkarni
---
block/bfq-iosched.c | 4 +++-
block/blk-mq-sched.c | 6 --
block/blk-mq-sched.h | 1 -
block/kyber-iosched.c | 4 +++-
block/mq-deadline.c | 4 +++-
5 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 50c8f034c01c..e2b9b700ed34 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -125,6 +125,8 @@
#include
#include
+#include
+
#include "blk.h"
#include "blk-mq.h"
#include "blk-mq-tag.h"
@@ -5507,7 +5509,7 @@ static void bfq_insert_request(struct blk_mq_hw_ctx
*hctx, struct request *rq,
spin_unlock_irq(>lock);
- blk_mq_sched_request_inserted(rq);
+ trace_block_rq_insert(rq);
spin_lock_irq(>lock);
bfqq = bfq_init_rq(rq);
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index a3cade16ef80..20b6a59fbd5a 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -407,12 +407,6 @@ bool blk_mq_sched_try_insert_merge(struct request_queue
*q, struct request *rq)
}
EXPORT_SYMBOL_GPL(blk_mq_sched_try_insert_merge);
-void blk_mq_sched_request_inserted(struct request *rq)
-{
- trace_block_rq_insert(rq);
-}
-EXPORT_SYMBOL_GPL(blk_mq_sched_request_inserted);
-
static bool blk_mq_sched_bypass_insert(struct blk_mq_hw_ctx *hctx,
bool has_sched,
struct request *rq)
diff --git a/block/blk-mq-sched.h b/block/blk-mq-sched.h
index 126021fc3a11..04c40c695bf0 100644
--- a/block/blk-mq-sched.h
+++ b/block/blk-mq-sched.h
@@ -10,7 +10,6 @@ void blk_mq_sched_free_hctx_data(struct request_queue *q,
void blk_mq_sched_assign_ioc(struct request *rq);
-void blk_mq_sched_request_inserted(struct request *rq);
bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio,
unsigned int nr_segs, struct request **merged_request);
bool __blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio,
diff --git a/block/kyber-iosched.c b/block/kyber-iosched.c
index a38c5ab103d1..e42d78deee90 100644
--- a/block/kyber-iosched.c
+++ b/block/kyber-iosched.c
@@ -13,6 +13,8 @@
#include
#include
+#include
+
#include "blk.h"
#include "blk-mq.h"
#include "blk-mq-debugfs.h"
@@ -602,7 +604,7 @@ static void kyber_insert_requests(struct blk_mq_hw_ctx
*hctx,
list_move_tail(>queuelist, head);
sbitmap_set_bit(>kcq_map[sched_domain],
rq->mq_ctx->index_hw[hctx->type]);
- blk_mq_sched_request_inserted(rq);
+ trace_block_rq_insert(rq);
spin_unlock(>lock);
}
}
diff --git a/block/mq-deadline.c b/block/mq-deadline.c
index b57470e154c8..f3631a287466 100644
--- a/block/mq-deadline.c
+++ b/block/mq-deadline.c
@@ -18,6 +18,8 @@
#include
#include
+#include
+
#include "blk.h"
#include "blk-mq.h"
#include "blk-mq-debugfs.h"
@@ -496,7 +498,7 @@ static void dd_insert_request(struct blk_mq_hw_ctx *hctx,
struct request *rq,
if (blk_mq_sched_try_insert_merge(q, rq))
return;
- blk_mq_sched_request_inserted(rq);
+ trace_block_rq_insert(rq);
if (at_head || blk_rq_is_passthrough(rq)) {
if (at_head)
--
2.26.0
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH 05/11] block: get rid of the trace rq insert wrapper
On Mon, Jun 29, 2020 at 04:43:08PM -0700, Chaitanya Kulkarni wrote: > Get rid of the wrapper for trace_block_rq_insert() and call the function > directly. I'd mention blk_mq_sched_request_inserted instead of the tracepoint in the subject and commit message. Otherwise this looks fine. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
