From: Dan Carpenter <error27@gmail.com>
To: oe-kbuild@lists.linux.dev, Kemeng Shi <shikemeng@huaweicloud.com>,
paolo.valente@linaro.org, axboe@kernel.dk, jack@suse.cz
Cc: lkp@intel.com, oe-kbuild-all@lists.linux.dev,
linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
shikemeng@huaweicloud.com
Subject: Re: [PATCH 17/17] block, bfq: remove unnecessary local variable __bfqq in bfq_setup_merge
Date: Thu, 23 Feb 2023 08:48:12 +0300 [thread overview]
Message-ID: <202302200841.9zinyY7i-lkp@intel.com> (raw)
In-Reply-To: <20230219104309.1511562-18-shikemeng@huaweicloud.com>
Hi Kemeng,
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Kemeng-Shi/block-bfq-properly-mark-bfqq-remained-idle/20230219-104312
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link: https://lore.kernel.org/r/20230219104309.1511562-18-shikemeng%40huaweicloud.com
patch subject: [PATCH 17/17] block, bfq: remove unnecessary local variable __bfqq in bfq_setup_merge
config: openrisc-randconfig-m041-20230219 (https://download.01.org/0day-ci/archive/20230220/202302200841.9zinyY7i-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 12.1.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202302200841.9zinyY7i-lkp@intel.com/
New smatch warnings:
block/bfq-iosched.c:2785 bfq_setup_merge() error: we previously assumed 'new_bfqq' could be null (see line 2766)
Old smatch warnings:
block/bfq-iosched.c:6195 __bfq_insert_request() warn: variable dereferenced before check 'bfqq' (see line 6191)
vim +/new_bfqq +2785 block/bfq-iosched.c
36eca894832351 Arianna Avanzini 2017-04-12 2751 static struct bfq_queue *
36eca894832351 Arianna Avanzini 2017-04-12 2752 bfq_setup_merge(struct bfq_queue *bfqq, struct bfq_queue *new_bfqq)
36eca894832351 Arianna Avanzini 2017-04-12 2753 {
36eca894832351 Arianna Avanzini 2017-04-12 2754 int process_refs, new_process_refs;
36eca894832351 Arianna Avanzini 2017-04-12 2755
36eca894832351 Arianna Avanzini 2017-04-12 2756 /*
36eca894832351 Arianna Avanzini 2017-04-12 2757 * If there are no process references on the new_bfqq, then it is
36eca894832351 Arianna Avanzini 2017-04-12 2758 * unsafe to follow the ->new_bfqq chain as other bfqq's in the chain
36eca894832351 Arianna Avanzini 2017-04-12 2759 * may have dropped their last reference (not just their last process
36eca894832351 Arianna Avanzini 2017-04-12 2760 * reference).
36eca894832351 Arianna Avanzini 2017-04-12 2761 */
36eca894832351 Arianna Avanzini 2017-04-12 2762 if (!bfqq_process_refs(new_bfqq))
36eca894832351 Arianna Avanzini 2017-04-12 2763 return NULL;
36eca894832351 Arianna Avanzini 2017-04-12 2764
36eca894832351 Arianna Avanzini 2017-04-12 2765 /* Avoid a circular list and skip interim queue merges. */
114533e1e26a36 Kemeng Shi 2023-02-19 @2766 while ((new_bfqq = new_bfqq->new_bfqq)) {
114533e1e26a36 Kemeng Shi 2023-02-19 2767 if (new_bfqq == bfqq)
36eca894832351 Arianna Avanzini 2017-04-12 2768 return NULL;
36eca894832351 Arianna Avanzini 2017-04-12 2769 }
This now loops until new_bfqq is NULL.
36eca894832351 Arianna Avanzini 2017-04-12 2770
36eca894832351 Arianna Avanzini 2017-04-12 2771 process_refs = bfqq_process_refs(bfqq);
36eca894832351 Arianna Avanzini 2017-04-12 2772 new_process_refs = bfqq_process_refs(new_bfqq);
What?
36eca894832351 Arianna Avanzini 2017-04-12 2773 /*
36eca894832351 Arianna Avanzini 2017-04-12 2774 * If the process for the bfqq has gone away, there is no
36eca894832351 Arianna Avanzini 2017-04-12 2775 * sense in merging the queues.
36eca894832351 Arianna Avanzini 2017-04-12 2776 */
36eca894832351 Arianna Avanzini 2017-04-12 2777 if (process_refs == 0 || new_process_refs == 0)
36eca894832351 Arianna Avanzini 2017-04-12 2778 return NULL;
36eca894832351 Arianna Avanzini 2017-04-12 2779
c1cee4ab36acef Jan Kara 2022-04-01 2780 /*
c1cee4ab36acef Jan Kara 2022-04-01 2781 * Make sure merged queues belong to the same parent. Parents could
c1cee4ab36acef Jan Kara 2022-04-01 2782 * have changed since the time we decided the two queues are suitable
c1cee4ab36acef Jan Kara 2022-04-01 2783 * for merging.
c1cee4ab36acef Jan Kara 2022-04-01 2784 */
c1cee4ab36acef Jan Kara 2022-04-01 @2785 if (new_bfqq->entity.parent != bfqq->entity.parent)
c1cee4ab36acef Jan Kara 2022-04-01 2786 return NULL;
c1cee4ab36acef Jan Kara 2022-04-01 2787
36eca894832351 Arianna Avanzini 2017-04-12 2788 bfq_log_bfqq(bfqq->bfqd, bfqq, "scheduling merge with queue %d",
36eca894832351 Arianna Avanzini 2017-04-12 2789 new_bfqq->pid);
36eca894832351 Arianna Avanzini 2017-04-12 2790
36eca894832351 Arianna Avanzini 2017-04-12 2791 /*
36eca894832351 Arianna Avanzini 2017-04-12 2792 * Merging is just a redirection: the requests of the process
36eca894832351 Arianna Avanzini 2017-04-12 2793 * owning one of the two queues are redirected to the other queue.
36eca894832351 Arianna Avanzini 2017-04-12 2794 * The latter queue, in its turn, is set as shared if this is the
36eca894832351 Arianna Avanzini 2017-04-12 2795 * first time that the requests of some process are redirected to
36eca894832351 Arianna Avanzini 2017-04-12 2796 * it.
36eca894832351 Arianna Avanzini 2017-04-12 2797 *
6fa3e8d34204d5 Paolo Valente 2017-04-12 2798 * We redirect bfqq to new_bfqq and not the opposite, because
6fa3e8d34204d5 Paolo Valente 2017-04-12 2799 * we are in the context of the process owning bfqq, thus we
6fa3e8d34204d5 Paolo Valente 2017-04-12 2800 * have the io_cq of this process. So we can immediately
6fa3e8d34204d5 Paolo Valente 2017-04-12 2801 * configure this io_cq to redirect the requests of the
6fa3e8d34204d5 Paolo Valente 2017-04-12 2802 * process to new_bfqq. In contrast, the io_cq of new_bfqq is
6fa3e8d34204d5 Paolo Valente 2017-04-12 2803 * not available any more (new_bfqq->bic == NULL).
6fa3e8d34204d5 Paolo Valente 2017-04-12 2804 *
6fa3e8d34204d5 Paolo Valente 2017-04-12 2805 * Anyway, even in case new_bfqq coincides with the in-service
6fa3e8d34204d5 Paolo Valente 2017-04-12 2806 * queue, redirecting requests the in-service queue is the
6fa3e8d34204d5 Paolo Valente 2017-04-12 2807 * best option, as we feed the in-service queue with new
6fa3e8d34204d5 Paolo Valente 2017-04-12 2808 * requests close to the last request served and, by doing so,
6fa3e8d34204d5 Paolo Valente 2017-04-12 2809 * are likely to increase the throughput.
36eca894832351 Arianna Avanzini 2017-04-12 2810 */
36eca894832351 Arianna Avanzini 2017-04-12 2811 bfqq->new_bfqq = new_bfqq;
15729ff8143f81 Paolo Valente 2021-11-25 2812 /*
15729ff8143f81 Paolo Valente 2021-11-25 2813 * The above assignment schedules the following redirections:
15729ff8143f81 Paolo Valente 2021-11-25 2814 * each time some I/O for bfqq arrives, the process that
15729ff8143f81 Paolo Valente 2021-11-25 2815 * generated that I/O is disassociated from bfqq and
15729ff8143f81 Paolo Valente 2021-11-25 2816 * associated with new_bfqq. Here we increases new_bfqq->ref
15729ff8143f81 Paolo Valente 2021-11-25 2817 * in advance, adding the number of processes that are
15729ff8143f81 Paolo Valente 2021-11-25 2818 * expected to be associated with new_bfqq as they happen to
15729ff8143f81 Paolo Valente 2021-11-25 2819 * issue I/O.
15729ff8143f81 Paolo Valente 2021-11-25 2820 */
36eca894832351 Arianna Avanzini 2017-04-12 2821 new_bfqq->ref += process_refs;
36eca894832351 Arianna Avanzini 2017-04-12 2822 return new_bfqq;
36eca894832351 Arianna Avanzini 2017-04-12 2823 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
next prev parent reply other threads:[~2023-02-23 5:48 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-19 10:42 [PATCH 00/17] Some bugfix and cleanup patches for bfq Kemeng Shi
2023-02-19 10:42 ` [PATCH 01/17] block, bfq: properly mark bfqq remained idle Kemeng Shi
2023-02-19 10:42 ` [PATCH 02/17] block, bfq: try preemption if bfqq has higher weight and the same priority class Kemeng Shi
2023-02-19 10:42 ` [PATCH 03/17] block, bfq: only preempt plugged in_service_queue if bfq_better_to_idle say no Kemeng Shi
2023-02-19 10:42 ` [PATCH 04/17] block, bfq: recover the "service hole" if enough budget is left Kemeng Shi
2023-02-19 10:42 ` [PATCH 05/17] block, bfq: Update bfqd->max_budget with bfqd->lock held Kemeng Shi
2023-02-19 10:42 ` [PATCH 06/17] block, bfq: correct bfq_max_budget and bfq_min_budget Kemeng Shi
2023-02-19 10:42 ` [PATCH 07/17] block, bfq: correct interactive weight-raise check in bfq_set_budget_timeout Kemeng Shi
2023-02-19 10:43 ` [PATCH 08/17] block, bfq: start service_from_wr accumulating of async queues correctly Kemeng Shi
2023-02-19 10:43 ` [PATCH 09/17] block, bfq: stop to detect queue as waker queue if it already is now Kemeng Shi
2023-02-19 10:43 ` [PATCH 10/17] block, bfq: fix typo in comment Kemeng Shi
2023-02-19 10:43 ` [PATCH 11/17] block, bfq: simpfy computation of bfqd->budgets_assigned Kemeng Shi
2023-02-19 10:43 ` [PATCH 12/17] block, bfq: define and use soft_rt, in_burst and wr_or_deserves_wr only low_latency is enable Kemeng Shi
2023-02-19 10:43 ` [PATCH 13/17] block, bfq: remove unnecessary "wr" part of wr_or_deserves_wr Kemeng Shi
2023-02-19 10:43 ` [PATCH 14/17] block, bfq: remove redundant oom_bfqq check for bfqq from bfq_find_close_cooperator Kemeng Shi
2023-02-19 10:43 ` [PATCH 15/17] block, bfq: some cleanups for function bfq_pos_tree_add_move Kemeng Shi
2023-02-19 10:43 ` [PATCH 16/17] block, bfq: remove unnecessary goto tag in __bfq_weights_tree_remove Kemeng Shi
2023-02-19 10:43 ` [PATCH 17/17] block, bfq: remove unnecessary local variable __bfqq in bfq_setup_merge Kemeng Shi
2023-02-23 5:48 ` Dan Carpenter [this message]
2023-02-23 6:34 ` Kemeng Shi
2023-03-14 8:45 ` kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=202302200841.9zinyY7i-lkp@intel.com \
--to=error27@gmail.com \
--cc=axboe@kernel.dk \
--cc=jack@suse.cz \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lkp@intel.com \
--cc=oe-kbuild-all@lists.linux.dev \
--cc=oe-kbuild@lists.linux.dev \
--cc=paolo.valente@linaro.org \
--cc=shikemeng@huaweicloud.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox