[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZyCLo51ZyjxX7eQK@rli9-mobl>
Date: Tue, 29 Oct 2024 15:15:47 +0800
From: Philip Li <philip.li@...el.com>
To: Matthew Sakai <msakai@...hat.com>, Dan Carpenter
<dan.carpenter@...aro.org>
CC: Dan Carpenter <dan.carpenter@...aro.org>, <oe-kbuild@...ts.linux.dev>,
Mike Snitzer <snitzer@...nel.org>, <lkp@...el.com>,
<oe-kbuild-all@...ts.linux.dev>, <linux-kernel@...r.kernel.org>
Subject: Re: drivers/md/dm-vdo/data-vio.c:976 vdo_launch_bio() warn:
inconsistent returns '&pool->lock'.
On Tue, Oct 29, 2024 at 11:03:07AM +0800, Philip Li wrote:
> On Mon, Oct 28, 2024 at 07:00:40PM -0400, Matthew Sakai wrote:
> > This should be addressed upstream by commit
> > 872564c501b72ae0c84af51084753e8652e4a84b ("dm vdo data-vio: silence sparse
> > warnings about locking context imbalances")
> >
> > That commit is from February. Would it be possible for these checks to use a
> > more up-to-date version of the code before warning us about things that have
> > already been addressed?
>
> Sorry about this Matt, the bot side will check why this happens and fix
> the issue asap to avoid meaningless report.
Hi Matt and Dan, would you mind do a further check of this, per the re-test,
smatch warns as below on v6.12-rc3
drivers/md/dm-vdo/data-vio.c:982 vdo_launch_bio() warn: inconsistent returns '&pool->lock'.
Locked on : 972,977
Unlocked on: 982
The corresponding code of drivers/md/dm-vdo/data-vio.c is below
942 static void wait_permit(struct limiter *limiter, struct bio *bio)
943 __releases(&limiter->pool->lock)
944 {
945 DEFINE_WAIT(wait);
946
947 bio_list_add(&limiter->new_waiters, bio);
948 prepare_to_wait_exclusive(&limiter->blocked_threads, &wait,
949 TASK_UNINTERRUPTIBLE);
950 spin_unlock(&limiter->pool->lock);
951 io_schedule();
952 finish_wait(&limiter->blocked_threads, &wait);
953 }
954
955 /**
956 * vdo_launch_bio() - Acquire a data_vio from the pool, assign the bio to it, and launch it.
957 *
958 * This will block if data_vios or discard permits are not available.
959 */
960 void vdo_launch_bio(struct data_vio_pool *pool, struct bio *bio)
961 {
962 struct data_vio *data_vio;
963
964 VDO_ASSERT_LOG_ONLY(!vdo_is_state_quiescent(&pool->state),
965 "data_vio_pool not quiescent on acquire");
966
967 bio->bi_private = (void *) jiffies;
968 spin_lock(&pool->lock);
969 if ((bio_op(bio) == REQ_OP_DISCARD) &&
970 !acquire_permit(&pool->discard_limiter)) {
971 wait_permit(&pool->discard_limiter, bio);
972 return;
973 }
974
975 if (!acquire_permit(&pool->limiter)) {
976 wait_permit(&pool->limiter, bio);
977 return;
978 }
979
980 data_vio = get_available_data_vio(pool);
981 spin_unlock(&pool->lock);
982 launch_bio(pool->completion.vdo, data_vio, bio);
983 }
Thanks
>
> >
> > Matt
> >
> > On 10/21/24 3:19 AM, Dan Carpenter wrote:
> > > Hi Mike,
> > >
> > > First bad commit (maybe != root cause):
> > >
> > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > > head: 3d5ad2d4eca337e80f38df77de89614aa5aaceb9
> > > commit: f36b1d3ba533d21b5b793623f05761b0297d114e dm vdo: use a proper Makefile for dm-vdo
> > > config: x86_64-randconfig-161-20241019 (https://download.01.org/0day-ci/archive/20241019/202410191209.osrd8z9H-lkp@intel.com/config)
> > > compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
> > >
> > > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > > the same patch/commit), kindly add following tags
> > > | Reported-by: kernel test robot <lkp@...el.com>
> > > | Reported-by: Dan Carpenter <dan.carpenter@...aro.org>
> > > | Closes: https://lore.kernel.org/r/202410191209.osrd8z9H-lkp@intel.com/
> > >
> > > smatch warnings:
> > > drivers/md/dm-vdo/data-vio.c:976 vdo_launch_bio() warn: inconsistent returns '&pool->lock'.
> > >
> > > vim +976 drivers/md/dm-vdo/data-vio.c
> > >
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 958 void vdo_launch_bio(struct data_vio_pool *pool, struct bio *bio)
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 959 {
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 960 struct data_vio *data_vio;
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 961
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 962 ASSERT_LOG_ONLY(!vdo_is_state_quiescent(&pool->state),
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 963 "data_vio_pool not quiescent on acquire");
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 964
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 965 bio->bi_private = (void *) jiffies;
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 966 spin_lock(&pool->lock);
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 967 if ((bio_op(bio) == REQ_OP_DISCARD) &&
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 968 !acquire_permit(&pool->discard_limiter, bio))
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 969 return;
> > > ^^^^^^
> > >
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 970
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 971 if (!acquire_permit(&pool->limiter, bio))
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 972 return;
> > > ^^^^^^
> > > Still holding pool->lock on these paths.
> > >
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 973
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 974 data_vio = get_available_data_vio(pool);
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 975 spin_unlock(&pool->lock);
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 @976 launch_bio(pool->completion.vdo, data_vio, bio);
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 977 }
> > > 79535a7881c0cb Matthew Sakai 2023-11-16 978
> > >
> >
> >
Powered by blists - more mailing lists