[vz7,2/2] block: store exec_ub on struct request and use it

Submitted by Konstantin Khorenko on Aug. 2, 2018, 2:11 p.m.

Details

Message ID 20180802141118.1931-2-khorenko@virtuozzo.com
State New
Series "Series without cover letter"
Headers show

Commit Message

Konstantin Khorenko Aug. 2, 2018, 2:11 p.m.
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@virtuozzo.com>
---
 block/blk-core.c       | 9 +++++++++
 block/elevator.c       | 9 +++++++++
 include/linux/blkdev.h | 1 +
 3 files changed, 19 insertions(+)

Patch hide | download patch | download mbox

diff --git a/block/blk-core.c b/block/blk-core.c
index a72c3032b86d..b61cac155983 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -869,6 +869,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)
@@ -1750,6 +1756,9 @@  void blk_queue_bio(struct request_queue *q, struct bio *bio)
 	 * often, and the elevators are able to handle it.
 	 */
 	init_request_from_bio(req, bio);
+#ifdef CONFIG_BEANCOUNTERS
+	req->req_ub = get_beancounter(get_exec_ub());
+#endif
 
 	if (test_bit(QUEUE_FLAG_SAME_COMP, &q->queue_flags))
 		req->cpu = raw_smp_processor_id();
diff --git a/block/elevator.c b/block/elevator.c
index 849711de8b05..e5cbbcf43257 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -597,6 +597,11 @@  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)
+		ub = set_exec_ub(rq->req_ub);
+#endif
 	trace_block_rq_insert(q, rq);
 
 	blk_pm_add_request(q, rq);
@@ -673,6 +678,10 @@  void __elv_add_request(struct request_queue *q, struct request *rq, int where)
 		       __func__, where);
 		BUG();
 	}
+#ifdef CONFIG_BEANCOUNTERS
+	if (ub)
+		set_exec_ub(ub);
+#endif
 }
 EXPORT_SYMBOL(__elv_add_request);
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 7e9227543735..3856b4e01a87 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -198,6 +198,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 */

Comments

Andrey Ryabinin Aug. 2, 2018, 4:36 p.m.
On 08/02/2018 05:11 PM, Konstantin Khorenko wrote:
> 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@virtuozzo.com>

Reviewed-by: Andrey Ryabinin <aryabinin@virtuozzo.com>