[Devel] [PATCH RHEL7 COMMIT] block: store exec_ub on struct request and use it

Konstantin Khorenko khorenko at virtuozzo.com
Mon Aug 27 17:53:24 MSK 2018


The commit is pushed to "branch-rh7-3.10.0-862.11.6.vz7.71.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-862.11.6.vz7.71.4
------>
commit 9c40dfcf14a4fd132cb0bf8ab50b9e0cd46741d7
Author: Konstantin Khorenko <khorenko at virtuozzo.com>
Date:   Thu Aug 2 17:11:18 2018 +0300

    block: store exec_ub on struct request and use it
    
    Good calltrace: we set proper exec_ub in ploop_req_state_process()
    and later process requests - accounting goes to correct ub.
    
    loop35172  6275 [002]   755.716683:  probe:ub_writeback_io_3:
                            (ffffffff813547aa) ub=0xffff880394a04000
                      5547ab deadline_add_request
                      52e76a __elv_add_request
                      536350 blk_flush_plug_list
                      5365aa blk_queue_bio
                      5345bb generic_make_request
                      534843 submit_bio
                        43e4 dio_submit ([pio_direct])
                        616f ploop_entry_request ([ploop])
                        7209 ploop_req_state_process ([ploop])
                        7735 ploop_thread ([ploop])
    
    Bad calltrace: due to task->plug io batching we may miss calling
    ploop_req_state_process() and process requests with ploop thread's ub == ub0:
    
    ploop35172  6275 [002]   755.716763:  probe:ub_writeback_io_2:
                             (ffffffff81354785) ub=0xffffffff81f7b880 (ub0!!!)
                      554786 deadline_add_request
                      52e76a __elv_add_request
                      536350 blk_flush_plug_list
                      536714 blk_finish_plug
                        1a93 ploop_wait ([ploop])
                        77a1 ploop_thread ([ploop])
    
    => need to store exec_ub in struct request as well.
    
    We are safe to check for NULL request:req_ub to detect set it or not
    because request is zeroed on creation:
    
       get_request
        __get_request
         blk_rq_init
          memset(rq, 0, sizeof(*rq));
    
    https://jira.sw.ru/browse/PSBM-86910
    
    Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
    
    v2 changes:
     - store exec_ub to request in __get_request() instead of
       blk_queue_bio(), this covers more potential cases
     - __elv_add_request() must get/put ub because request can be freed
       before the end of __elv_add_request() execution
    v3 changes:
     - move blk_rq_set_ub() declaration into blk-core.c to avoid
       including beancounter.h into lot of other files
---
 block/blk-core.c       | 21 +++++++++++++++++++++
 block/elevator.c       | 18 ++++++++++++++++++
 include/linux/blkdev.h |  1 +
 3 files changed, 40 insertions(+)

diff --git a/block/blk-core.c b/block/blk-core.c
index 45e2d9a93fc8..3bab718b3f5f 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -950,6 +950,12 @@ EXPORT_SYMBOL(blk_get_queue);
 
 static inline void blk_free_request(struct request_list *rl, struct request *rq)
 {
+#ifdef CONFIG_BEANCOUNTERS
+	if (rq->req_ub) {
+		put_beancounter(rq->req_ub);
+		rq->req_ub = NULL;
+	}
+#endif
 	if (rq->cmd_flags & REQ_ELVPRIV) {
 		elv_put_request(rl->q, rq);
 		if (rq->elv.icq)
@@ -1093,6 +1099,20 @@ static bool blk_rq_should_init_elevator(struct bio *bio)
 	return true;
 }
 
+/**
+ * blk_rq_set_ub - associate a request with a user beancounter
+ * @rq: request of interest
+ *
+ * Store current exec_ub in @rq so later we can use it while adding the request
+ * to elevator.
+ */
+static inline void blk_rq_set_ub(struct request *rq)
+{
+#ifdef CONFIG_BEANCOUNTERS
+	rq->req_ub = get_beancounter(get_exec_ub());
+#endif
+}
+
 /**
  * __get_request - get a free request
  * @rl: request list to allocate from
@@ -1197,6 +1217,7 @@ static struct request *__get_request(struct request_list *rl, int rw_flags,
 
 	blk_rq_init(q, rq);
 	blk_rq_set_rl(rq, rl);
+	blk_rq_set_ub(rq);
 	rq->cmd_flags = rw_flags | REQ_ALLOCED;
 	if (flags & BLK_MQ_REQ_PREEMPT)
 		rq->cmd_flags |= REQ_PREEMPT;
diff --git a/block/elevator.c b/block/elevator.c
index e81087c3e72d..8163ec4fd63d 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -35,6 +35,7 @@
 #include <linux/hash.h>
 #include <linux/uaccess.h>
 #include <linux/pm_runtime.h>
+#include <bc/beancounter.h>
 
 #include <trace/events/block.h>
 
@@ -658,6 +659,17 @@ void elv_drain_elevator(struct request_queue *q)
 
 void __elv_add_request(struct request_queue *q, struct request *rq, int where)
 {
+#ifdef CONFIG_BEANCOUNTERS
+	struct user_beancounter *ub = NULL;
+	if (rq->req_ub) {
+		/*
+		 * Request can be already freed at the end of func,
+		 * so get beancounter.
+		 */
+		get_beancounter(rq->req_ub);
+		ub = set_exec_ub(rq->req_ub);
+	}
+#endif
 	trace_block_rq_insert(q, rq);
 
 	blk_pm_add_request(q, rq);
@@ -734,6 +746,12 @@ void __elv_add_request(struct request_queue *q, struct request *rq, int where)
 		       __func__, where);
 		BUG();
 	}
+#ifdef CONFIG_BEANCOUNTERS
+	if (ub) {
+		ub = set_exec_ub(ub);
+		put_beancounter(ub);
+	}
+#endif
 }
 EXPORT_SYMBOL(__elv_add_request);
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 55cba7563e57..49bb13fa4d4f 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -200,6 +200,7 @@ struct request {
 #endif
 
 	unsigned short ioprio;
+	struct user_beancounter *req_ub;
 
 	void *special;		/* opaque pointer available for LLD use */
 	char *buffer;		/* kaddr of the current segment if available */


More information about the Devel mailing list