From: kernel test robot <lkp@intel.com>
To: oe-kbuild@lists.linux.dev
Cc: lkp@intel.com, Dan Carpenter <error27@gmail.com>
Subject: Re: [PATCH 17/17] block, bfq: remove unnecessary local variable __bfqq in bfq_setup_merge
Date: Mon, 20 Feb 2023 08:48:23 +0800 [thread overview]
Message-ID: <202302200841.9zinyY7i-lkp@intel.com> (raw)
BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20230219104309.1511562-18-shikemeng@huaweicloud.com>
References: <20230219104309.1511562-18-shikemeng@huaweicloud.com>
TO: Kemeng Shi <shikemeng@huaweicloud.com>
TO: paolo.valente@linaro.org
TO: axboe@kernel.dk
TO: jack@suse.cz
CC: linux-block@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: shikemeng@huaweicloud.com
Hi Kemeng,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on next-20230217]
[cannot apply to linus/master v6.2-rc8]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
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
:::::: branch date: 22 hours ago
:::::: commit date: 22 hours ago
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 2750
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 }
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);
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 }
36eca894832351 Arianna Avanzini 2017-04-12 2824
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
WARNING: multiple messages have this Message-ID (diff)
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)
Message-ID: <20230223054812.JVqH_Bg8xJHCpqu__zVH6lA7ZlR0dmW9kW_bGf8lzc4@z> (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 reply other threads:[~2023-02-20 0:48 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-20 0:48 kernel test robot [this message]
2023-02-23 5:48 ` [PATCH 17/17] block, bfq: remove unnecessary local variable __bfqq in bfq_setup_merge Dan Carpenter
2023-02-23 6:34 ` Kemeng Shi
-- strict thread matches above, loose matches on Subject: below --
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-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=lkp@intel.com \
--cc=error27@gmail.com \
--cc=oe-kbuild@lists.linux.dev \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.