[linux-next:pending-fixes 207/319] fs/btrfs/qgroup.c:2014:6: error: use of undeclared identifier 'bytenr'

[linux-next:pending-fixes 207/319] fs/btrfs/qgroup.c:2014:6: error: use of undeclared identifier 'bytenr'

[kernel test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git pending-fixes
head:   47fa0de9b07c6214b57458e965646a3e25656b69
commit: 5c0ea18d992fffc948e087c0002560d747e6edd9 [207/319] btrfs: use sector numbers as keys for the dirty extents xarray
config: arm-randconfig-002-20241009 (https://download.01.org/0day-ci/archive/20241009/202410091038.SI34ZULQ-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241009/202410091038.SI34ZULQ-lkp@intel.com/reproduce)

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@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410091038.SI34ZULQ-lkp@intel.com/

All errors (new ones prefixed by >>):

>> fs/btrfs/qgroup.c:2014:6: error: use of undeclared identifier 'bytenr'
           if (bytenr >= MAX_LFS_FILESIZE) {
               ^
   fs/btrfs/qgroup.c:2017:9: error: use of undeclared identifier 'bytenr'
                                bytenr);
                                ^
   2 errors generated.


vim +/bytenr +2014 fs/btrfs/qgroup.c

  1991	
  1992	/*
  1993	 * Inform qgroup to trace one dirty extent, its info is recorded in @record.
  1994	 * So qgroup can account it at transaction committing time.
  1995	 *
  1996	 * No lock version, caller must acquire delayed ref lock and allocated memory,
  1997	 * then call btrfs_qgroup_trace_extent_post() after exiting lock context.
  1998	 *
  1999	 * Return 0 for success insert
  2000	 * Return >0 for existing record, caller can free @record safely.
  2001	 * Return <0 for insertion failure, caller can free @record safely.
  2002	 */
  2003	int btrfs_qgroup_trace_extent_nolock(struct btrfs_fs_info *fs_info,
  2004					struct btrfs_delayed_ref_root *delayed_refs,
  2005					struct btrfs_qgroup_extent_record *record)
  2006	{
  2007		struct btrfs_qgroup_extent_record *existing, *ret;
  2008		const unsigned long index = (record->bytenr >> fs_info->sectorsize_bits);
  2009	
  2010		if (!btrfs_qgroup_full_accounting(fs_info))
  2011			return 1;
  2012	
  2013	#if BITS_PER_LONG == 32
> 2014		if (bytenr >= MAX_LFS_FILESIZE) {
  2015			btrfs_err_rl(fs_info,
  2016	"qgroup record for extent at %llu is beyond 32bit page cache and xarray index limit",
  2017				     bytenr);
  2018			btrfs_err_32bit_limit(fs_info);
  2019			return -EOVERFLOW;
  2020		}
  2021	#endif
  2022	
  2023		lockdep_assert_held(&delayed_refs->lock);
  2024		trace_btrfs_qgroup_trace_extent(fs_info, record);
  2025	
  2026		xa_lock(&delayed_refs->dirty_extents);
  2027		existing = xa_load(&delayed_refs->dirty_extents, index);
  2028		if (existing) {
  2029			if (record->data_rsv && !existing->data_rsv) {
  2030				existing->data_rsv = record->data_rsv;
  2031				existing->data_rsv_refroot = record->data_rsv_refroot;
  2032			}
  2033			xa_unlock(&delayed_refs->dirty_extents);
  2034			return 1;
  2035		}
  2036	
  2037		ret = __xa_store(&delayed_refs->dirty_extents, index, record, GFP_ATOMIC);
  2038		xa_unlock(&delayed_refs->dirty_extents);
  2039		if (xa_is_err(ret)) {
  2040			qgroup_mark_inconsistent(fs_info);
  2041			return xa_err(ret);
  2042		}
  2043	
  2044		return 0;
  2045	}
  2046	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [linux-next:pending-fixes 207/319] fs/btrfs/qgroup.c:2014:6: error: use of undeclared identifier 'bytenr'

[Filipe Manana]

On Wed, Oct 9, 2024 at 3:55 AM kernel test robot <lkp@intel.com> wrote:
>
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git pending-fixes
> head:   47fa0de9b07c6214b57458e965646a3e25656b69
> commit: 5c0ea18d992fffc948e087c0002560d747e6edd9 [207/319] btrfs: use sector numbers as keys for the dirty extents xarray
> config: arm-randconfig-002-20241009 (https://download.01.org/0day-ci/archive/20241009/202410091038.SI34ZULQ-lkp@intel.com/config)
> compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241009/202410091038.SI34ZULQ-lkp@intel.com/reproduce)
>
> 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@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202410091038.SI34ZULQ-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
> >> fs/btrfs/qgroup.c:2014:6: error: use of undeclared identifier 'bytenr'
>            if (bytenr >= MAX_LFS_FILESIZE) {

Ah this should be record->bytenr.
The issue is actually fixed in a later patch of the same patchset,
where bytenr is a function argument.

I can update the patch but then it will require updating the patches
that come next from the same patchset and rebasing the for-next
branch.
And I see it's already in the next-fixes branch.

David, how do you prefer to proceed here? Do you want me to fix this
and resend the patchset?

Thanks.


>                ^
>    fs/btrfs/qgroup.c:2017:9: error: use of undeclared identifier 'bytenr'
>                                 bytenr);
>                                 ^
>    2 errors generated.
>
>
> vim +/bytenr +2014 fs/btrfs/qgroup.c
>
>   1991
>   1992  /*
>   1993   * Inform qgroup to trace one dirty extent, its info is recorded in @record.
>   1994   * So qgroup can account it at transaction committing time.
>   1995   *
>   1996   * No lock version, caller must acquire delayed ref lock and allocated memory,
>   1997   * then call btrfs_qgroup_trace_extent_post() after exiting lock context.
>   1998   *
>   1999   * Return 0 for success insert
>   2000   * Return >0 for existing record, caller can free @record safely.
>   2001   * Return <0 for insertion failure, caller can free @record safely.
>   2002   */
>   2003  int btrfs_qgroup_trace_extent_nolock(struct btrfs_fs_info *fs_info,
>   2004                                  struct btrfs_delayed_ref_root *delayed_refs,
>   2005                                  struct btrfs_qgroup_extent_record *record)
>   2006  {
>   2007          struct btrfs_qgroup_extent_record *existing, *ret;
>   2008          const unsigned long index = (record->bytenr >> fs_info->sectorsize_bits);
>   2009
>   2010          if (!btrfs_qgroup_full_accounting(fs_info))
>   2011                  return 1;
>   2012
>   2013  #if BITS_PER_LONG == 32
> > 2014          if (bytenr >= MAX_LFS_FILESIZE) {
>   2015                  btrfs_err_rl(fs_info,
>   2016  "qgroup record for extent at %llu is beyond 32bit page cache and xarray index limit",
>   2017                               bytenr);
>   2018                  btrfs_err_32bit_limit(fs_info);
>   2019                  return -EOVERFLOW;
>   2020          }
>   2021  #endif
>   2022
>   2023          lockdep_assert_held(&delayed_refs->lock);
>   2024          trace_btrfs_qgroup_trace_extent(fs_info, record);
>   2025
>   2026          xa_lock(&delayed_refs->dirty_extents);
>   2027          existing = xa_load(&delayed_refs->dirty_extents, index);
>   2028          if (existing) {
>   2029                  if (record->data_rsv && !existing->data_rsv) {
>   2030                          existing->data_rsv = record->data_rsv;
>   2031                          existing->data_rsv_refroot = record->data_rsv_refroot;
>   2032                  }
>   2033                  xa_unlock(&delayed_refs->dirty_extents);
>   2034                  return 1;
>   2035          }
>   2036
>   2037          ret = __xa_store(&delayed_refs->dirty_extents, index, record, GFP_ATOMIC);
>   2038          xa_unlock(&delayed_refs->dirty_extents);
>   2039          if (xa_is_err(ret)) {
>   2040                  qgroup_mark_inconsistent(fs_info);
>   2041                  return xa_err(ret);
>   2042          }
>   2043
>   2044          return 0;
>   2045  }
>   2046
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki

Re: [linux-next:pending-fixes 207/319] fs/btrfs/qgroup.c:2014:6: error: use of undeclared identifier 'bytenr'

[David Sterba]

On Wed, Oct 09, 2024 at 11:09:13AM +0100, Filipe Manana wrote:
> On Wed, Oct 9, 2024 at 3:55 AM kernel test robot <lkp@intel.com> wrote:
> >
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git pending-fixes
> > head:   47fa0de9b07c6214b57458e965646a3e25656b69
> > commit: 5c0ea18d992fffc948e087c0002560d747e6edd9 [207/319] btrfs: use sector numbers as keys for the dirty extents xarray
> > config: arm-randconfig-002-20241009 (https://download.01.org/0day-ci/archive/20241009/202410091038.SI34ZULQ-lkp@intel.com/config)
> > compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241009/202410091038.SI34ZULQ-lkp@intel.com/reproduce)
> >
> > 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@intel.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202410091038.SI34ZULQ-lkp@intel.com/
> >
> > All errors (new ones prefixed by >>):
> >
> > >> fs/btrfs/qgroup.c:2014:6: error: use of undeclared identifier 'bytenr'
> >            if (bytenr >= MAX_LFS_FILESIZE) {
> 
> Ah this should be record->bytenr.
> The issue is actually fixed in a later patch of the same patchset,
> where bytenr is a function argument.
> 
> I can update the patch but then it will require updating the patches
> that come next from the same patchset and rebasing the for-next
> branch.
> And I see it's already in the next-fixes branch.
> 
> David, how do you prefer to proceed here? Do you want me to fix this
> and resend the patchset?

No need to resend, I'll fix it locally, the patch in next-fixes was the
old one so I'll sync it with what's in our development for-next (and
also check the others just in case).  I'll also check and update all the
branches involved in the for-next pulled by linux-next.

Re: [linux-next:pending-fixes 207/319] fs/btrfs/qgroup.c:2014:6: error: use of undeclared identifier 'bytenr'

[Filipe Manana]

On Wed, Oct 9, 2024 at 12:30 PM David Sterba <dsterba@suse.cz> wrote:
>
> On Wed, Oct 09, 2024 at 11:09:13AM +0100, Filipe Manana wrote:
> > On Wed, Oct 9, 2024 at 3:55 AM kernel test robot <lkp@intel.com> wrote:
> > >
> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git pending-fixes
> > > head:   47fa0de9b07c6214b57458e965646a3e25656b69
> > > commit: 5c0ea18d992fffc948e087c0002560d747e6edd9 [207/319] btrfs: use sector numbers as keys for the dirty extents xarray
> > > config: arm-randconfig-002-20241009 (https://download.01.org/0day-ci/archive/20241009/202410091038.SI34ZULQ-lkp@intel.com/config)
> > > compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
> > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241009/202410091038.SI34ZULQ-lkp@intel.com/reproduce)
> > >
> > > 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@intel.com>
> > > | Closes: https://lore.kernel.org/oe-kbuild-all/202410091038.SI34ZULQ-lkp@intel.com/
> > >
> > > All errors (new ones prefixed by >>):
> > >
> > > >> fs/btrfs/qgroup.c:2014:6: error: use of undeclared identifier 'bytenr'
> > >            if (bytenr >= MAX_LFS_FILESIZE) {
> >
> > Ah this should be record->bytenr.
> > The issue is actually fixed in a later patch of the same patchset,
> > where bytenr is a function argument.
> >
> > I can update the patch but then it will require updating the patches
> > that come next from the same patchset and rebasing the for-next
> > branch.
> > And I see it's already in the next-fixes branch.
> >
> > David, how do you prefer to proceed here? Do you want me to fix this
> > and resend the patchset?
>
> No need to resend, I'll fix it locally, the patch in next-fixes was the
> old one so I'll sync it with what's in our development for-next (and
> also check the others just in case).  I'll also check and update all the
> branches involved in the for-next pulled by linux-next.

Btw, I noticed that only the 2nd patch ("btrfs: use sector numbers as
keys for the dirty extents xarray") was updated,
so now it doesn't break 32 bits builds anymore.

However the next patch in the series ("btrfs: qgroups: remove bytenr
field from struct btrfs_qgroup_extent_record") wasn't updated,
and it now breaks 32 bits builds. At the same location, it needs to
use "bytenr" instead of "record->bytenr".

Do you want me to update the patches and send them to the list?

There's one more change that needs to be squashed (to that last patch)
that triggers a use-after-free reported by syzbot, which is:

https://lore.kernel.org/linux-btrfs/02fc507b62b19be2348fc08de8b13bd7af1a440e.1728922973.git.fdmanana@suse.com/

Thanks.

Re: [linux-next:pending-fixes 207/319] fs/btrfs/qgroup.c:2014:6: error: use of undeclared identifier 'bytenr'

[David Sterba]

On Mon, Oct 14, 2024 at 05:26:26PM +0100, Filipe Manana wrote:
> > > David, how do you prefer to proceed here? Do you want me to fix this
> > > and resend the patchset?
> >
> > No need to resend, I'll fix it locally, the patch in next-fixes was the
> > old one so I'll sync it with what's in our development for-next (and
> > also check the others just in case).  I'll also check and update all the
> > branches involved in the for-next pulled by linux-next.
> 
> Btw, I noticed that only the 2nd patch ("btrfs: use sector numbers as
> keys for the dirty extents xarray") was updated,
> so now it doesn't break 32 bits builds anymore.
> 
> However the next patch in the series ("btrfs: qgroups: remove bytenr
> field from struct btrfs_qgroup_extent_record") wasn't updated,
> and it now breaks 32 bits builds. At the same location, it needs to
> use "bytenr" instead of "record->bytenr".

My bad, I manually tested the build combinations in the first patch only
as it went to Linus' tree. I did not get any other linux-next build
failures though.

> Do you want me to update the patches and send them to the list?
> 
> There's one more change that needs to be squashed (to that last patch)
> that triggers a use-after-free reported by syzbot, which is:
> 
> https://lore.kernel.org/linux-btrfs/02fc507b62b19be2348fc08de8b13bd7af1a440e.1728922973.git.fdmanana@suse.com/

Ok, please send it and feel free to update the patch in for-next too.
Thanks.