From: Darrick J. Wong <djwong@kernel.org>
To: kbuild-all@lists.01.org
Subject: Re: [djwong-xfs:vectorized-scrub 99/334] fs/xfs/scrub/fscounters.c:198 xchk_fscounters_freeze() warn: inconsistent returns '&sb->s_umount'.
Date: Tue, 18 Jan 2022 20:37:01 -0800 [thread overview]
Message-ID: <20220119043701.GC13563@magnolia> (raw)
In-Reply-To: <202201170928.GcIhOWMI-lkp@intel.com>
[-- Attachment #1: Type: text/plain, Size: 7147 bytes --]
On Wed, Jan 19, 2022 at 06:58:29AM +0300, Dan Carpenter wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git vectorized-scrub
> head: 8427da8e62fbcf9a04e5b2663fe60b97d6911417
> commit: 8dd594d12f08acc6c6fa388b2cae3e270bf8effc [99/334] xfs: stabilize fs summary counters for online fsck
> config: ia64-randconfig-m031-20220116 (https://download.01.org/0day-ci/archive/20220117/202201170928.GcIhOWMI-lkp(a)intel.com/config)
> compiler: ia64-linux-gcc (GCC) 11.2.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> smatch warnings:
> fs/xfs/scrub/fscounters.c:198 xchk_fscounters_freeze() warn: inconsistent returns '&sb->s_umount'.
>
> vim +198 fs/xfs/scrub/fscounters.c
>
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 125 STATIC int
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 126 xchk_fscounters_freeze(
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 127 struct xfs_scrub *sc)
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 128 {
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 129 struct xchk_fscounters *fsc = sc->buf;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 130 struct xfs_mount *mp = sc->mp;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 131 struct super_block *sb = mp->m_super;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 132 int level;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 133 int error = 0;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 134
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 135 if (sc->flags & XCHK_HAVE_FREEZE_PROT) {
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 136 sc->flags &= ~XCHK_HAVE_FREEZE_PROT;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 137 mnt_drop_write_file(sc->file);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 138 }
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 139
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 140 /* Wait until we're ready to freeze or give up. */
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 141 down_write(&sb->s_umount);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 142 while (sb->s_writers.frozen != SB_UNFROZEN) {
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 143 up_write(&sb->s_umount);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 144
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 145 if (xchk_should_terminate(sc, &error))
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 146 return error;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 147
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 148 delay(HZ / 10);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 149 down_write(&sb->s_umount);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 150 }
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 151
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 152 if (sb_rdonly(sb)) {
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 153 sb->s_writers.frozen = SB_FREEZE_EXCLUSIVE;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 154 goto done;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 155 }
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 156
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 157 sb->s_writers.frozen = SB_FREEZE_WRITE;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 158 /* Release s_umount to preserve sb_start_write -> s_umount ordering */
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 159 up_write(&sb->s_umount);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 160 percpu_down_write(sb->s_writers.rw_sem + SB_FREEZE_WRITE - 1);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 161 down_write(&sb->s_umount);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 162
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 163 /* Now we go and block page faults... */
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 164 sb->s_writers.frozen = SB_FREEZE_PAGEFAULT;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 165 percpu_down_write(sb->s_writers.rw_sem + SB_FREEZE_PAGEFAULT - 1);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 166
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 167 /*
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 168 * All writers are done so after syncing there won't be dirty data.
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 169 * Let xfs_fs_sync_fs flush dirty data so the VFS won't start writeback
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 170 * and to disable the background gc workers.
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 171 */
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 172 error = sync_filesystem(sb);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 173 if (error) {
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 174 sb->s_writers.frozen = SB_UNFROZEN;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 175 for (level = SB_FREEZE_PAGEFAULT; level >= 0; level--)
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 176 percpu_up_write(sb->s_writers.rw_sem + level);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 177 wake_up(&sb->s_writers.wait_unfrozen);
>
> Smatch wanted an up_write(&sb->s_umount); but this looks intentional?
Nope, that's a bug. Thanks for catching that!
--D
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 178 return error;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 179 }
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 180
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 181 /* Now wait for internal filesystem counter */
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 182 sb->s_writers.frozen = SB_FREEZE_FS;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 183 percpu_down_write(sb->s_writers.rw_sem + SB_FREEZE_FS - 1);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 184
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 185 /*
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 186 * We do not need to quiesce the log to check the summary counters, so
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 187 * skip the call to xfs_fs_freeze here. To prevent anyone else from
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 188 * unfreezing us, set the VFS freeze level to one higher than
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 189 * FREEZE_COMPLETE.
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 190 */
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 191 sb->s_writers.frozen = SB_FREEZE_EXCLUSIVE;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 192 for (level = SB_FREEZE_LEVELS - 1; level >= 0; level--)
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 193 percpu_rwsem_release(sb->s_writers.rw_sem + level, 0,
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 194 _THIS_IP_);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 195 done:
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 196 fsc->frozen = true;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 197 up_write(&sb->s_umount);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 @198 return 0;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 199 }
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
>
WARNING: multiple messages have this Message-ID (diff)
From: "Darrick J. Wong" <djwong@kernel.org>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: kbuild@lists.01.org, lkp@intel.com, kbuild-all@lists.01.org,
"Darrick J. Wong" <darrick.wong@oracle.com>,
linux-kernel@vger.kernel.org
Subject: Re: [djwong-xfs:vectorized-scrub 99/334] fs/xfs/scrub/fscounters.c:198 xchk_fscounters_freeze() warn: inconsistent returns '&sb->s_umount'.
Date: Tue, 18 Jan 2022 20:37:01 -0800 [thread overview]
Message-ID: <20220119043701.GC13563@magnolia> (raw)
In-Reply-To: <202201170928.GcIhOWMI-lkp@intel.com>
On Wed, Jan 19, 2022 at 06:58:29AM +0300, Dan Carpenter wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git vectorized-scrub
> head: 8427da8e62fbcf9a04e5b2663fe60b97d6911417
> commit: 8dd594d12f08acc6c6fa388b2cae3e270bf8effc [99/334] xfs: stabilize fs summary counters for online fsck
> config: ia64-randconfig-m031-20220116 (https://download.01.org/0day-ci/archive/20220117/202201170928.GcIhOWMI-lkp@intel.com/config)
> compiler: ia64-linux-gcc (GCC) 11.2.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> smatch warnings:
> fs/xfs/scrub/fscounters.c:198 xchk_fscounters_freeze() warn: inconsistent returns '&sb->s_umount'.
>
> vim +198 fs/xfs/scrub/fscounters.c
>
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 125 STATIC int
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 126 xchk_fscounters_freeze(
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 127 struct xfs_scrub *sc)
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 128 {
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 129 struct xchk_fscounters *fsc = sc->buf;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 130 struct xfs_mount *mp = sc->mp;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 131 struct super_block *sb = mp->m_super;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 132 int level;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 133 int error = 0;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 134
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 135 if (sc->flags & XCHK_HAVE_FREEZE_PROT) {
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 136 sc->flags &= ~XCHK_HAVE_FREEZE_PROT;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 137 mnt_drop_write_file(sc->file);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 138 }
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 139
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 140 /* Wait until we're ready to freeze or give up. */
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 141 down_write(&sb->s_umount);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 142 while (sb->s_writers.frozen != SB_UNFROZEN) {
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 143 up_write(&sb->s_umount);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 144
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 145 if (xchk_should_terminate(sc, &error))
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 146 return error;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 147
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 148 delay(HZ / 10);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 149 down_write(&sb->s_umount);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 150 }
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 151
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 152 if (sb_rdonly(sb)) {
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 153 sb->s_writers.frozen = SB_FREEZE_EXCLUSIVE;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 154 goto done;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 155 }
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 156
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 157 sb->s_writers.frozen = SB_FREEZE_WRITE;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 158 /* Release s_umount to preserve sb_start_write -> s_umount ordering */
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 159 up_write(&sb->s_umount);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 160 percpu_down_write(sb->s_writers.rw_sem + SB_FREEZE_WRITE - 1);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 161 down_write(&sb->s_umount);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 162
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 163 /* Now we go and block page faults... */
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 164 sb->s_writers.frozen = SB_FREEZE_PAGEFAULT;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 165 percpu_down_write(sb->s_writers.rw_sem + SB_FREEZE_PAGEFAULT - 1);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 166
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 167 /*
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 168 * All writers are done so after syncing there won't be dirty data.
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 169 * Let xfs_fs_sync_fs flush dirty data so the VFS won't start writeback
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 170 * and to disable the background gc workers.
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 171 */
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 172 error = sync_filesystem(sb);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 173 if (error) {
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 174 sb->s_writers.frozen = SB_UNFROZEN;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 175 for (level = SB_FREEZE_PAGEFAULT; level >= 0; level--)
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 176 percpu_up_write(sb->s_writers.rw_sem + level);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 177 wake_up(&sb->s_writers.wait_unfrozen);
>
> Smatch wanted an up_write(&sb->s_umount); but this looks intentional?
Nope, that's a bug. Thanks for catching that!
--D
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 178 return error;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 179 }
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 180
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 181 /* Now wait for internal filesystem counter */
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 182 sb->s_writers.frozen = SB_FREEZE_FS;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 183 percpu_down_write(sb->s_writers.rw_sem + SB_FREEZE_FS - 1);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 184
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 185 /*
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 186 * We do not need to quiesce the log to check the summary counters, so
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 187 * skip the call to xfs_fs_freeze here. To prevent anyone else from
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 188 * unfreezing us, set the VFS freeze level to one higher than
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 189 * FREEZE_COMPLETE.
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 190 */
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 191 sb->s_writers.frozen = SB_FREEZE_EXCLUSIVE;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 192 for (level = SB_FREEZE_LEVELS - 1; level >= 0; level--)
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 193 percpu_rwsem_release(sb->s_writers.rw_sem + level, 0,
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 194 _THIS_IP_);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 195 done:
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 196 fsc->frozen = true;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 197 up_write(&sb->s_umount);
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 @198 return 0;
> 8dd594d12f08acc Darrick J. Wong 2022-01-06 199 }
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>
next prev parent reply other threads:[~2022-01-19 4:37 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-17 1:48 [djwong-xfs:vectorized-scrub 99/334] fs/xfs/scrub/fscounters.c:198 xchk_fscounters_freeze() warn: inconsistent returns '&sb->s_umount' kernel test robot
2022-01-19 3:58 ` Dan Carpenter
2022-01-19 3:58 ` Dan Carpenter
2022-01-19 4:37 ` Darrick J. Wong [this message]
2022-01-19 4:37 ` Darrick J. Wong
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=20220119043701.GC13563@magnolia \
--to=djwong@kernel.org \
--cc=kbuild-all@lists.01.org \
/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.