* Re: [PATCH RFC] btrfs: sysfs: add <uuid>/debug/io_accounting/ directory
@ 2022-01-25 19:17 kernel test robot
2022-01-27 6:09 ` kernel test robot
0 siblings, 1 reply; 7+ messages in thread
From: kernel test robot @ 2022-01-25 19:17 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 13528 bytes --]
CC: llvm(a)lists.linux.dev
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20220121052445.37692-1-wqu@suse.com>
References: <20220121052445.37692-1-wqu@suse.com>
TO: Qu Wenruo <wqu@suse.com>
Hi Qu,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on kdave/for-next]
[also build test WARNING on v5.17-rc1 next-20220125]
[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]
url: https://github.com/0day-ci/linux/commits/Qu-Wenruo/btrfs-sysfs-add-uuid-debug-io_accounting-directory/20220121-132629
base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
:::::: branch date: 5 days ago
:::::: commit date: 5 days ago
config: x86_64-randconfig-c007-20220124 (https://download.01.org/0day-ci/archive/20220126/202201260130.4VVHtyiM-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 997e128e2a78f5a5434fc75997441ae1ee76f8a4)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/879aebce4c7d6a684b93b92545978dc75bf4abd5
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Qu-Wenruo/btrfs-sysfs-add-uuid-debug-io_accounting-directory/20220121-132629
git checkout 879aebce4c7d6a684b93b92545978dc75bf4abd5
# save the config file to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 clang-analyzer
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
clang-analyzer warnings: (new ones prefixed by >>)
^ ~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:3784:7: warning: Access to field 'flags' results in a dereference of a null pointer (loaded from variable 'bargs') [clang-analyzer-core.NullDereference]
if ((bargs->flags & BTRFS_BALANCE_ARGS_PROFILES) &&
^
fs/btrfs/volumes.c:3875:6: note: Assuming 'path' is non-null
if (!path) {
^~~~~
fs/btrfs/volumes.c:3875:2: note: Taking false branch
if (!path) {
^
fs/btrfs/volumes.c:3885:7: note: 'counting' is true
if (!counting) {
^~~~~~~~
fs/btrfs/volumes.c:3885:2: note: Taking false branch
if (!counting) {
^
fs/btrfs/volumes.c:3898:2: note: Loop condition is true. Entering loop body
while (1) {
^
fs/btrfs/volumes.c:3899:9: note: 'counting' is true
if ((!counting && atomic_read(&fs_info->balance_pause_req)) ||
^~~~~~~~
fs/btrfs/volumes.c:3899:18: note: Left side of '&&' is false
if ((!counting && atomic_read(&fs_info->balance_pause_req)) ||
^
fs/btrfs/volumes.c:3900:7: note: Assuming the condition is false
atomic_read(&fs_info->balance_cancel_req)) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:3899:3: note: Taking false branch
if ((!counting && atomic_read(&fs_info->balance_pause_req)) ||
^
fs/btrfs/volumes.c:3907:7: note: Assuming 'ret' is >= 0
if (ret < 0) {
^~~~~~~
fs/btrfs/volumes.c:3907:3: note: Taking false branch
if (ret < 0) {
^
fs/btrfs/volumes.c:3916:7: note: Assuming 'ret' is not equal to 0
if (ret == 0)
^~~~~~~~
fs/btrfs/volumes.c:3916:3: note: Taking false branch
if (ret == 0)
^
fs/btrfs/volumes.c:3921:7: note: Assuming 'ret' is 0
if (ret) {
^~~
fs/btrfs/volumes.c:3921:3: note: Taking false branch
if (ret) {
^
fs/btrfs/volumes.c:3931:7: note: Assuming 'found_key.objectid' is equal to 'key.objectid'
if (found_key.objectid != key.objectid) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:3931:3: note: Taking false branch
if (found_key.objectid != key.objectid) {
^
fs/btrfs/volumes.c:3939:8: note: 'counting' is true
if (!counting) {
^~~~~~~~
fs/btrfs/volumes.c:3939:3: note: Taking false branch
if (!counting) {
^
fs/btrfs/volumes.c:3945:9: note: Calling 'should_balance_chunk'
ret = should_balance_chunk(leaf, chunk, found_key.offset);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:3767:2: note: 'bargs' initialized to a null pointer value
struct btrfs_balance_args *bargs = NULL;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:3771:6: note: Assuming the condition is false
if (!((chunk_type & BTRFS_BLOCK_GROUP_TYPE_MASK) &
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:3771:2: note: Taking false branch
if (!((chunk_type & BTRFS_BLOCK_GROUP_TYPE_MASK) &
^
fs/btrfs/volumes.c:3776:6: note: Assuming the condition is false
if (chunk_type & BTRFS_BLOCK_GROUP_DATA)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:3776:2: note: Taking false branch
if (chunk_type & BTRFS_BLOCK_GROUP_DATA)
^
fs/btrfs/volumes.c:3778:11: note: Assuming the condition is false
else if (chunk_type & BTRFS_BLOCK_GROUP_SYSTEM)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:3778:7: note: Taking false branch
else if (chunk_type & BTRFS_BLOCK_GROUP_SYSTEM)
^
fs/btrfs/volumes.c:3780:11: note: Assuming the condition is false
else if (chunk_type & BTRFS_BLOCK_GROUP_METADATA)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:3780:7: note: Taking false branch
else if (chunk_type & BTRFS_BLOCK_GROUP_METADATA)
^
fs/btrfs/volumes.c:3784:7: note: Access to field 'flags' results in a dereference of a null pointer (loaded from variable 'bargs')
if ((bargs->flags & BTRFS_BALANCE_ARGS_PROFILES) &&
^~~~~
fs/btrfs/volumes.c:4903:4: warning: Value stored to 'ret' is never read [clang-analyzer-deadcode.DeadStores]
ret = 0;
^ ~
fs/btrfs/volumes.c:4903:4: note: Value stored to 'ret' is never read
ret = 0;
^ ~
>> fs/btrfs/volumes.c:6780:6: warning: Value stored to 'length' during its initialization is never read [clang-analyzer-deadcode.DeadStores]
u32 length = bio->bi_iter.bi_size;
^~~~~~ ~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:6780:6: note: Value stored to 'length' during its initialization is never read
u32 length = bio->bi_iter.bi_size;
^~~~~~ ~~~~~~~~~~~~~~~~~~~~
>> fs/btrfs/volumes.c:6781:7: warning: Value stored to 'metadata' during its initialization is never read [clang-analyzer-deadcode.DeadStores]
bool metadata = bio->bi_opf & REQ_META;
^~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:6781:7: note: Value stored to 'metadata' during its initialization is never read
bool metadata = bio->bi_opf & REQ_META;
^~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:6855:9: warning: Use of memory after it is freed [clang-analyzer-unix.Malloc]
dev = bioc->stripes[dev_nr].dev;
^~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:6821:6: note: Assuming 'ret' is 0
if (ret) {
^~~
fs/btrfs/volumes.c:6821:2: note: Taking false branch
if (ret) {
^
fs/btrfs/volumes.c:6832:7: note: Assuming the condition is false
if ((bioc->map_type & BTRFS_BLOCK_GROUP_RAID56_MASK) &&
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:6832:55: note: Left side of '&&' is false
if ((bioc->map_type & BTRFS_BLOCK_GROUP_RAID56_MASK) &&
^
fs/btrfs/volumes.c:6847:6: note: Assuming 'map_length' is >= 'length'
if (map_length < length) {
^~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:6847:2: note: Taking false branch
if (map_length < length) {
^
fs/btrfs/volumes.c:6854:19: note: Assuming 'dev_nr' is < 'total_devs'
for (dev_nr = 0; dev_nr < total_devs; dev_nr++) {
^~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:6854:2: note: Loop condition is true. Entering loop body
for (dev_nr = 0; dev_nr < total_devs; dev_nr++) {
^
fs/btrfs/volumes.c:6856:7: note: Assuming 'dev' is null
if (!dev || !dev->bdev || test_bit(BTRFS_DEV_STATE_MISSING,
^~~~
fs/btrfs/volumes.c:6856:12: note: Left side of '||' is true
if (!dev || !dev->bdev || test_bit(BTRFS_DEV_STATE_MISSING,
^
fs/btrfs/volumes.c:6860:4: note: Calling 'bioc_error'
bioc_error(bioc, first_bio, logical);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:6764:6: note: Assuming the condition is true
if (atomic_dec_and_test(&bioc->stripes_pending)) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:6764:2: note: Taking true branch
if (atomic_dec_and_test(&bioc->stripes_pending)) {
^
fs/btrfs/volumes.c:6766:3: note: Taking false branch
WARN_ON(bio != bioc->orig_bio);
^
include/asm-generic/bug.h:122:2: note: expanded from macro 'WARN_ON'
if (unlikely(__ret_warn_on)) \
^
fs/btrfs/volumes.c:6770:7: note: Assuming the condition is false
if (atomic_read(&bioc->error) > bioc->max_errors)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:6770:3: note: Taking false branch
if (atomic_read(&bioc->error) > bioc->max_errors)
^
fs/btrfs/volumes.c:6774:3: note: Calling 'btrfs_end_bioc'
btrfs_end_bioc(bioc, bio);
^~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:6668:2: note: Calling 'btrfs_put_bioc'
btrfs_put_bioc(bioc);
^~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:5947:7: note: 'bioc' is non-null
if (!bioc)
^~~~
fs/btrfs/volumes.c:5947:2: note: Taking false branch
if (!bioc)
^
fs/btrfs/volumes.c:5949:2: note: Taking true branch
if (refcount_dec_and_test(&bioc->refs))
^
fs/btrfs/volumes.c:5950:3: note: Memory is released
kfree(bioc);
^~~~~~~~~~~
fs/btrfs/volumes.c:6668:2: note: Returning; memory was released via 1st parameter
btrfs_put_bioc(bioc);
^~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:6774:3: note: Returning; memory was released via 1st parameter
btrfs_end_bioc(bioc, bio);
^~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:6860:4: note: Returning; memory was released via 1st parameter
bioc_error(bioc, first_bio, logical);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:6861:4: note: Execution continues on line 6854
continue;
^
fs/btrfs/volumes.c:6854:19: note: Assuming 'dev_nr' is < 'total_devs'
for (dev_nr = 0; dev_nr < total_devs; dev_nr++) {
^~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:6854:2: note: Loop condition is true. Entering loop body
for (dev_nr = 0; dev_nr < total_devs; dev_nr++) {
^
fs/btrfs/volumes.c:6855:9: note: Use of memory after it is freed
dev = bioc->stripes[dev_nr].dev;
^~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/volumes.c:8280:4: warning: Value stored to 'ret' is never read [clang-analyzer-deadcode.DeadStores]
ret = 0;
vim +/length +6780 fs/btrfs/volumes.c
de1ee92ac3bce4 Josef Bacik 2012-10-19 6777
879aebce4c7d6a Qu Wenruo 2022-01-21 6778 static void update_io_accounting(struct btrfs_fs_info *fs_info, struct bio *bio)
879aebce4c7d6a Qu Wenruo 2022-01-21 6779 {
879aebce4c7d6a Qu Wenruo 2022-01-21 @6780 u32 length = bio->bi_iter.bi_size;
879aebce4c7d6a Qu Wenruo 2022-01-21 @6781 bool metadata = bio->bi_opf & REQ_META;
879aebce4c7d6a Qu Wenruo 2022-01-21 6782
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH RFC] btrfs: sysfs: add <uuid>/debug/io_accounting/ directory
2022-01-25 19:17 [PATCH RFC] btrfs: sysfs: add <uuid>/debug/io_accounting/ directory kernel test robot
@ 2022-01-27 6:09 ` kernel test robot
2022-01-27 8:36 ` Qu Wenruo
0 siblings, 1 reply; 7+ messages in thread
From: kernel test robot @ 2022-01-27 6:09 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2791 bytes --]
Hi Wenruo,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on kdave/for-next]
[also build test WARNING on v5.17-rc1 next-20220125]
[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]
url: https://github.com/0day-ci/linux/commits/Qu-Wenruo/btrfs-sysfs-add-uuid-debug-io_accounting-directory/20220121-132629
base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
config: x86_64-randconfig-c007-20220124 (https://download.01.org/0day-ci/archive/20220126/202201260130.4VVHtyiM-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 997e128e2a78f5a5434fc75997441ae1ee76f8a4)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/879aebce4c7d6a684b93b92545978dc75bf4abd5
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Qu-Wenruo/btrfs-sysfs-add-uuid-debug-io_accounting-directory/20220121-132629
git checkout 879aebce4c7d6a684b93b92545978dc75bf4abd5
# save the config file to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 clang-analyzer
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
clang-analyzer warnings: (new ones prefixed by >>)
>> fs/btrfs/volumes.c:6780:6: warning: Value stored to 'length' during its initialization is never read [clang-analyzer-deadcode.DeadStores]
u32 length = bio->bi_iter.bi_size;
^~~~~~ ~~~~~~~~~~~~~~~~~~~~
>> fs/btrfs/volumes.c:6781:7: warning: Value stored to 'metadata' during its initialization is never read [clang-analyzer-deadcode.DeadStores]
bool metadata = bio->bi_opf & REQ_META;
^~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~
vim +/length +6780 fs/btrfs/volumes.c
de1ee92ac3bce4 Josef Bacik 2012-10-19 6777
879aebce4c7d6a Qu Wenruo 2022-01-21 6778 static void update_io_accounting(struct btrfs_fs_info *fs_info, struct bio *bio)
879aebce4c7d6a Qu Wenruo 2022-01-21 6779 {
879aebce4c7d6a Qu Wenruo 2022-01-21 @6780 u32 length = bio->bi_iter.bi_size;
879aebce4c7d6a Qu Wenruo 2022-01-21 @6781 bool metadata = bio->bi_opf & REQ_META;
879aebce4c7d6a Qu Wenruo 2022-01-21 6782
## this build has CONFIG_BTRFS_DEBUG=n ##
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH RFC] btrfs: sysfs: add <uuid>/debug/io_accounting/ directory
2022-01-27 6:09 ` kernel test robot
@ 2022-01-27 8:36 ` Qu Wenruo
0 siblings, 0 replies; 7+ messages in thread
From: Qu Wenruo @ 2022-01-27 8:36 UTC (permalink / raw)
To: kernel test robot; +Cc: kbuild-all, linux-btrfs@vger.kernel.org
On 2022/1/27 14:09, kernel test robot wrote:
> Hi Wenruo,
>
> [FYI, it's a private test report for your RFC patch.]
> [auto build test WARNING on kdave/for-next]
> [also build test WARNING on v5.17-rc1 next-20220125]
> [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]
>
> url:
> https://github.com/0day-ci/linux/commits/Qu-Wenruo/btrfs-sysfs-add-uuid-debug-io_accounting-directory/20220121-132629
>
> base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git
> for-next
> config: x86_64-randconfig-c007-20220124
> (https://download.01.org/0day-ci/archive/20220126/202201260130.4VVHtyiM-lkp@intel.com/config)
>
> compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project
> 997e128e2a78f5a5434fc75997441ae1ee76f8a4)
> reproduce (this is a W=1 build):
> wget
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> #
> https://github.com/0day-ci/linux/commit/879aebce4c7d6a684b93b92545978dc75bf4abd5
>
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review
> Qu-Wenruo/btrfs-sysfs-add-uuid-debug-io_accounting-directory/20220121-132629
>
> git checkout 879aebce4c7d6a684b93b92545978dc75bf4abd5
> # save the config file to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross
> ARCH=x86_64 clang-analyzer
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
>
> clang-analyzer warnings: (new ones prefixed by >>)
>
> >> fs/btrfs/volumes.c:6780:6: warning: Value stored to 'length' during
> its initialization is never read [clang-analyzer-deadcode.DeadStores]
> u32 length = bio->bi_iter.bi_size;
> ^~~~~~ ~~~~~~~~~~~~~~~~~~~~
> >> fs/btrfs/volumes.c:6781:7: warning: Value stored to 'metadata'
> during its initialization is never read
> [clang-analyzer-deadcode.DeadStores]
> bool metadata = bio->bi_opf & REQ_META;
> ^~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~
>
> vim +/length +6780 fs/btrfs/volumes.c
>
> de1ee92ac3bce4 Josef Bacik 2012-10-19 6777
> 879aebce4c7d6a Qu Wenruo 2022-01-21 6778 static void
> update_io_accounting(struct btrfs_fs_info *fs_info, struct bio *bio)
> 879aebce4c7d6a Qu Wenruo 2022-01-21 6779 {
> 879aebce4c7d6a Qu Wenruo 2022-01-21 @6780 u32 length =
> bio->bi_iter.bi_size;
> 879aebce4c7d6a Qu Wenruo 2022-01-21 @6781 bool metadata =
> bio->bi_opf & REQ_META;
> 879aebce4c7d6a Qu Wenruo 2022-01-21 6782
>
> ## this build has CONFIG_BTRFS_DEBUG=n ##
No wonder, I'd put the initialization inside CONFIG_BTRFS_DEBUG branch.
Thanks for catching it.
Qu
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH RFC] btrfs: sysfs: add <uuid>/debug/io_accounting/ directory
@ 2022-01-27 8:36 ` Qu Wenruo
0 siblings, 0 replies; 7+ messages in thread
From: Qu Wenruo @ 2022-01-27 8:36 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3280 bytes --]
On 2022/1/27 14:09, kernel test robot wrote:
> Hi Wenruo,
>
> [FYI, it's a private test report for your RFC patch.]
> [auto build test WARNING on kdave/for-next]
> [also build test WARNING on v5.17-rc1 next-20220125]
> [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]
>
> url:
> https://github.com/0day-ci/linux/commits/Qu-Wenruo/btrfs-sysfs-add-uuid-debug-io_accounting-directory/20220121-132629
>
> base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git
> for-next
> config: x86_64-randconfig-c007-20220124
> (https://download.01.org/0day-ci/archive/20220126/202201260130.4VVHtyiM-lkp(a)intel.com/config)
>
> compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project
> 997e128e2a78f5a5434fc75997441ae1ee76f8a4)
> reproduce (this is a W=1 build):
> wget
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> #
> https://github.com/0day-ci/linux/commit/879aebce4c7d6a684b93b92545978dc75bf4abd5
>
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review
> Qu-Wenruo/btrfs-sysfs-add-uuid-debug-io_accounting-directory/20220121-132629
>
> git checkout 879aebce4c7d6a684b93b92545978dc75bf4abd5
> # save the config file to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross
> ARCH=x86_64 clang-analyzer
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
>
> clang-analyzer warnings: (new ones prefixed by >>)
>
> >> fs/btrfs/volumes.c:6780:6: warning: Value stored to 'length' during
> its initialization is never read [clang-analyzer-deadcode.DeadStores]
> u32 length = bio->bi_iter.bi_size;
> ^~~~~~ ~~~~~~~~~~~~~~~~~~~~
> >> fs/btrfs/volumes.c:6781:7: warning: Value stored to 'metadata'
> during its initialization is never read
> [clang-analyzer-deadcode.DeadStores]
> bool metadata = bio->bi_opf & REQ_META;
> ^~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~
>
> vim +/length +6780 fs/btrfs/volumes.c
>
> de1ee92ac3bce4 Josef Bacik 2012-10-19 6777
> 879aebce4c7d6a Qu Wenruo 2022-01-21 6778 static void
> update_io_accounting(struct btrfs_fs_info *fs_info, struct bio *bio)
> 879aebce4c7d6a Qu Wenruo 2022-01-21 6779 {
> 879aebce4c7d6a Qu Wenruo 2022-01-21 @6780 u32 length =
> bio->bi_iter.bi_size;
> 879aebce4c7d6a Qu Wenruo 2022-01-21 @6781 bool metadata =
> bio->bi_opf & REQ_META;
> 879aebce4c7d6a Qu Wenruo 2022-01-21 6782
>
> ## this build has CONFIG_BTRFS_DEBUG=n ##
No wonder, I'd put the initialization inside CONFIG_BTRFS_DEBUG branch.
Thanks for catching it.
Qu
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH RFC] btrfs: sysfs: add <uuid>/debug/io_accounting/ directory
@ 2022-01-21 5:24 Qu Wenruo
2022-01-21 9:14 ` Anand Jain
0 siblings, 1 reply; 7+ messages in thread
From: Qu Wenruo @ 2022-01-21 5:24 UTC (permalink / raw)
To: linux-btrfs, dm-devel
[BACKGROUND]
There is a series of btrfs autodefrag bugs in v5.16, that would cause
way more IO than previous kernels.
Unfortunately there isn't any test case covering autodefrag, nor there
is anyway to show the io accounting of a btrfs.
[WORKAROUND]
I originally want to implement a dm target to do the io accounting for
all filesystems, but can not find a good enough interface (the status
interface has only 3 pre-defined workload).
Thus I turned to btrfs specific io accounting first.
One thing specific to btrfs is its integrated volume management/RAID.
Without proper profiles specification, default profile will cause
metadata IO to be accounted twice (DUP profile) and only data IO is
accounted correctly.
So for btrfs this patch will introduce a new sysfs directory in
/sys/fs/btrfs/<uuid>/debug/io_accounting/
And have the following files:
- meta_read: Metadata bytes read
- meta_write: Metadata bytes written
- data_read: Data bytes read
- data_write: Data bytes written
(including both zoned append and regular write)
And all these accounting is in logical address space, meaning profile
will not affect the values.
All those values can be reset by simply "echo 0".
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
Reason for RFC:
- (To DM guys) Is there any good way to implement a dm target to do the IO
accounting?
A more generic one can help more filesystems.
- (To Btrfs guys) Is the sysfs interface fine?
---
fs/btrfs/ctree.h | 11 +++++++
fs/btrfs/disk-io.c | 1 +
fs/btrfs/sysfs.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++
fs/btrfs/volumes.c | 24 +++++++++++++++
4 files changed, 113 insertions(+)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index b4a9b1c58d22..3983bceaef7f 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1035,10 +1035,21 @@ struct btrfs_fs_info {
#ifdef CONFIG_BTRFS_DEBUG
struct kobject *debug_kobj;
struct kobject *discard_debug_kobj;
+ struct kobject *io_accounting_debug_kobj;
struct list_head allocated_roots;
spinlock_t eb_leak_lock;
struct list_head allocated_ebs;
+
+ spinlock_t io_accounting_lock;
+ /*
+ * The IO accounting unit are all in byte, and are in logical address
+ * space, which is before the RAID/DUP mapping.
+ */
+ u64 meta_read;
+ u64 meta_write;
+ u64 data_read;
+ u64 data_write;
#endif
};
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 87a5addbedf6..41b56fde6e97 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3065,6 +3065,7 @@ void btrfs_init_fs_info(struct btrfs_fs_info *fs_info)
INIT_LIST_HEAD(&fs_info->allocated_roots);
INIT_LIST_HEAD(&fs_info->allocated_ebs);
spin_lock_init(&fs_info->eb_leak_lock);
+ spin_lock_init(&fs_info->io_accounting_lock);
#endif
extent_map_tree_init(&fs_info->mapping_tree);
btrfs_init_block_rsv(&fs_info->global_block_rsv,
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index beb7f72d50b8..dfdef93bdeab 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -40,6 +40,7 @@
* btrfs_debug_feature_attrs /sys/fs/btrfs/debug
* btrfs_debug_mount_attrs /sys/fs/btrfs/<uuid>/debug
* discard_debug_attrs /sys/fs/btrfs/<uuid>/debug/discard
+ * io_accounting_debug_attrs /sys/fs/btrfs/<uuid>/debug/io_accounting
*/
struct btrfs_feature_attr {
@@ -616,6 +617,62 @@ static const struct attribute_group btrfs_debug_feature_attr_group = {
.attrs = btrfs_debug_feature_attrs,
};
+/* IO accounting */
+#define io_accounting_to_fs_info(_kobj) to_fs_info((_kobj)->parent->parent)
+
+#define DECLARE_IO_ACCOUNTING_OPS(name) \
+static ssize_t io_accounting_##name##_show(struct kobject *kobj, \
+ struct kobj_attribute *a, \
+ char *buf) \
+{ \
+ struct btrfs_fs_info *fs_info = io_accounting_to_fs_info(kobj); \
+ u64 result; \
+ \
+ spin_lock(&fs_info->io_accounting_lock); \
+ result = fs_info->name; \
+ spin_unlock(&fs_info->io_accounting_lock); \
+ return sysfs_emit(buf, "%llu\n", result); \
+} \
+static ssize_t io_accounting_##name##_store(struct kobject *kobj, \
+ struct kobj_attribute *a, \
+ const char *buf, \
+ size_t count) \
+{ \
+ struct btrfs_fs_info *fs_info = io_accounting_to_fs_info(kobj); \
+ u64 value; \
+ int ret; \
+ \
+ ret = kstrtoull(skip_spaces(buf), 0, &value); \
+ if (ret) \
+ return ret; \
+ spin_lock(&fs_info->io_accounting_lock); \
+ fs_info->name = value; \
+ spin_unlock(&fs_info->io_accounting_lock); \
+ return count; \
+}
+
+DECLARE_IO_ACCOUNTING_OPS(meta_read);
+DECLARE_IO_ACCOUNTING_OPS(meta_write);
+DECLARE_IO_ACCOUNTING_OPS(data_read);
+DECLARE_IO_ACCOUNTING_OPS(data_write);
+
+BTRFS_ATTR_RW(io_accounting, meta_read, io_accounting_meta_read_show,
+ io_accounting_meta_read_store);
+BTRFS_ATTR_RW(io_accounting, meta_write, io_accounting_meta_write_show,
+ io_accounting_meta_write_store);
+BTRFS_ATTR_RW(io_accounting, data_read, io_accounting_data_read_show,
+ io_accounting_data_read_store);
+BTRFS_ATTR_RW(io_accounting, data_write, io_accounting_data_write_show,
+ io_accounting_data_write_store);
+
+static const struct attribute *io_accounting_debug_attrs[] = {
+ BTRFS_ATTR_PTR(io_accounting, meta_read),
+ BTRFS_ATTR_PTR(io_accounting, meta_write),
+ BTRFS_ATTR_PTR(io_accounting, data_read),
+ BTRFS_ATTR_PTR(io_accounting, data_write),
+ NULL,
+};
+
#endif
static ssize_t btrfs_show_u64(u64 *value_ptr, spinlock_t *lock, char *buf)
@@ -1219,6 +1276,12 @@ void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
kobject_del(fs_info->discard_debug_kobj);
kobject_put(fs_info->discard_debug_kobj);
}
+ if (fs_info->io_accounting_debug_kobj) {
+ sysfs_remove_files(fs_info->io_accounting_debug_kobj,
+ io_accounting_debug_attrs);
+ kobject_del(fs_info->io_accounting_debug_kobj);
+ kobject_put(fs_info->io_accounting_debug_kobj);
+ }
if (fs_info->debug_kobj) {
sysfs_remove_files(fs_info->debug_kobj, btrfs_debug_mount_attrs);
kobject_del(fs_info->debug_kobj);
@@ -1804,6 +1867,20 @@ int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info)
discard_debug_attrs);
if (error)
goto failure;
+
+ /* io_accounting directory */
+ fs_info->io_accounting_debug_kobj =
+ kobject_create_and_add("io_accounting",fs_info->debug_kobj);
+ if (!fs_info->io_accounting_debug_kobj) {
+ error = -ENOMEM;
+ goto failure;
+ }
+
+ error = sysfs_create_files(fs_info->io_accounting_debug_kobj,
+ io_accounting_debug_attrs);
+ if (error)
+ goto failure;
+
#endif
error = addrm_unknown_feature_attrs(fs_info, true);
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 2332e3026efa..58f2ec0a611a 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6763,6 +6763,29 @@ static void bioc_error(struct btrfs_io_context *bioc, struct bio *bio, u64 logic
}
}
+static void update_io_accounting(struct btrfs_fs_info *fs_info, struct bio *bio)
+{
+ u32 length = bio->bi_iter.bi_size;
+ bool metadata = bio->bi_opf & REQ_META;
+
+#ifdef CONFIG_BTRFS_DEBUG
+ spin_lock(&fs_info->io_accounting_lock);
+ if (bio_op(bio) == REQ_OP_READ) {
+ if (metadata)
+ fs_info->meta_read += length;
+ else
+ fs_info->data_read += length;
+ } else {
+ if (metadata)
+ fs_info->meta_write += length;
+ else
+ fs_info->data_write += length;
+ }
+ spin_unlock(&fs_info->io_accounting_lock);
+#endif
+ return;
+}
+
blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio,
int mirror_num)
{
@@ -6776,6 +6799,7 @@ blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio,
int total_devs;
struct btrfs_io_context *bioc = NULL;
+ update_io_accounting(fs_info, bio);
length = bio->bi_iter.bi_size;
map_length = length;
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH RFC] btrfs: sysfs: add <uuid>/debug/io_accounting/ directory
2022-01-21 5:24 Qu Wenruo
@ 2022-01-21 9:14 ` Anand Jain
2022-01-21 9:20 ` Qu Wenruo
0 siblings, 1 reply; 7+ messages in thread
From: Anand Jain @ 2022-01-21 9:14 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs, dm-devel
On 21/01/2022 13:24, Qu Wenruo wrote:
> [BACKGROUND]
> There is a series of btrfs autodefrag bugs in v5.16, that would cause
> way more IO than previous kernels.
>
> Unfortunately there isn't any test case covering autodefrag, nor there
> is anyway to show the io accounting of a btrfs.
>
> [WORKAROUND]
> I originally want to implement a dm target to do the io accounting for
> all filesystems, but can not find a good enough interface (the status
> interface has only 3 pre-defined workload).
>
> Thus I turned to btrfs specific io accounting first.
> One thing specific to btrfs is its integrated volume management/RAID.
>
> Without proper profiles specification, default profile will cause
> metadata IO to be accounted twice (DUP profile) and only data IO is
> accounted correctly.
>
> So for btrfs this patch will introduce a new sysfs directory in
> /sys/fs/btrfs/<uuid>/debug/io_accounting/
>
> And have the following files:
>
> - meta_read: Metadata bytes read
> - meta_write: Metadata bytes written
> - data_read: Data bytes read
> - data_write: Data bytes written
> (including both zoned append and regular write)
>
> And all these accounting is in logical address space, meaning profile
> will not affect the values.
>
> All those values can be reset by simply "echo 0".
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> Reason for RFC:
>
> - (To DM guys) Is there any good way to implement a dm target to do the IO
> accounting?
> A more generic one can help more filesystems.
>
> - (To Btrfs guys) Is the sysfs interface fine?
I am in for it. It can be a non debug feature IMO.
More below.
> ---
> fs/btrfs/ctree.h | 11 +++++++
> fs/btrfs/disk-io.c | 1 +
> fs/btrfs/sysfs.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++
> fs/btrfs/volumes.c | 24 +++++++++++++++
> 4 files changed, 113 insertions(+)
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index b4a9b1c58d22..3983bceaef7f 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -1035,10 +1035,21 @@ struct btrfs_fs_info {
> #ifdef CONFIG_BTRFS_DEBUG
> struct kobject *debug_kobj;
> struct kobject *discard_debug_kobj;
> + struct kobject *io_accounting_debug_kobj;
> struct list_head allocated_roots;
>
> spinlock_t eb_leak_lock;
> struct list_head allocated_ebs;
> +
> + spinlock_t io_accounting_lock;
> + /*
> + * The IO accounting unit are all in byte, and are in logical address
> + * space, which is before the RAID/DUP mapping.
> + */
> + u64 meta_read;
> + u64 meta_write;
> + u64 data_read;
> + u64 data_write;
> #endif
> };
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 87a5addbedf6..41b56fde6e97 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3065,6 +3065,7 @@ void btrfs_init_fs_info(struct btrfs_fs_info *fs_info)
> INIT_LIST_HEAD(&fs_info->allocated_roots);
> INIT_LIST_HEAD(&fs_info->allocated_ebs);
> spin_lock_init(&fs_info->eb_leak_lock);
> + spin_lock_init(&fs_info->io_accounting_lock);
> #endif
> extent_map_tree_init(&fs_info->mapping_tree);
> btrfs_init_block_rsv(&fs_info->global_block_rsv,
> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
> index beb7f72d50b8..dfdef93bdeab 100644
> --- a/fs/btrfs/sysfs.c
> +++ b/fs/btrfs/sysfs.c
> @@ -40,6 +40,7 @@
> * btrfs_debug_feature_attrs /sys/fs/btrfs/debug
> * btrfs_debug_mount_attrs /sys/fs/btrfs/<uuid>/debug
> * discard_debug_attrs /sys/fs/btrfs/<uuid>/debug/discard
> + * io_accounting_debug_attrs /sys/fs/btrfs/<uuid>/debug/io_accounting
> */
>
> struct btrfs_feature_attr {
> @@ -616,6 +617,62 @@ static const struct attribute_group btrfs_debug_feature_attr_group = {
> .attrs = btrfs_debug_feature_attrs,
> };
>
> +/* IO accounting */
> +#define io_accounting_to_fs_info(_kobj) to_fs_info((_kobj)->parent->parent)
> +
> +#define DECLARE_IO_ACCOUNTING_OPS(name) \
> +static ssize_t io_accounting_##name##_show(struct kobject *kobj, \
> + struct kobj_attribute *a, \
> + char *buf) \
> +{ \
> + struct btrfs_fs_info *fs_info = io_accounting_to_fs_info(kobj); \
> + u64 result; \
> + \
> + spin_lock(&fs_info->io_accounting_lock); \
> + result = fs_info->name; \
> + spin_unlock(&fs_info->io_accounting_lock); \
> + return sysfs_emit(buf, "%llu\n", result); \
> +} \
> +static ssize_t io_accounting_##name##_store(struct kobject *kobj, \
> + struct kobj_attribute *a, \
> + const char *buf, \
> + size_t count) \
> +{ \
> + struct btrfs_fs_info *fs_info = io_accounting_to_fs_info(kobj); \
> + u64 value; \
> + int ret; \
> + \
> + ret = kstrtoull(skip_spaces(buf), 0, &value); \
> + if (ret) \
> + return ret; \
> + spin_lock(&fs_info->io_accounting_lock); \
> + fs_info->name = value; \
> + spin_unlock(&fs_info->io_accounting_lock); \
> + return count; \
> +}
> +
> +DECLARE_IO_ACCOUNTING_OPS(meta_read);
> +DECLARE_IO_ACCOUNTING_OPS(meta_write);
> +DECLARE_IO_ACCOUNTING_OPS(data_read);
> +DECLARE_IO_ACCOUNTING_OPS(data_write);
> +
> +BTRFS_ATTR_RW(io_accounting, meta_read, io_accounting_meta_read_show,
> + io_accounting_meta_read_store);
> +BTRFS_ATTR_RW(io_accounting, meta_write, io_accounting_meta_write_show,
> + io_accounting_meta_write_store);
> +BTRFS_ATTR_RW(io_accounting, data_read, io_accounting_data_read_show,
> + io_accounting_data_read_store);
> +BTRFS_ATTR_RW(io_accounting, data_write, io_accounting_data_write_show,
> + io_accounting_data_write_store);
> +
> +static const struct attribute *io_accounting_debug_attrs[] = {
> + BTRFS_ATTR_PTR(io_accounting, meta_read),
> + BTRFS_ATTR_PTR(io_accounting, meta_write),
> + BTRFS_ATTR_PTR(io_accounting, data_read),
> + BTRFS_ATTR_PTR(io_accounting, data_write),
> + NULL,
> +};
> +
> #endif
>
> static ssize_t btrfs_show_u64(u64 *value_ptr, spinlock_t *lock, char *buf)
> @@ -1219,6 +1276,12 @@ void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
> kobject_del(fs_info->discard_debug_kobj);
> kobject_put(fs_info->discard_debug_kobj);
> }
> + if (fs_info->io_accounting_debug_kobj) {
> + sysfs_remove_files(fs_info->io_accounting_debug_kobj,
> + io_accounting_debug_attrs);
> + kobject_del(fs_info->io_accounting_debug_kobj);
> + kobject_put(fs_info->io_accounting_debug_kobj);
> + }
> if (fs_info->debug_kobj) {
> sysfs_remove_files(fs_info->debug_kobj, btrfs_debug_mount_attrs);
> kobject_del(fs_info->debug_kobj);
> @@ -1804,6 +1867,20 @@ int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info)
> discard_debug_attrs);
> if (error)
> goto failure;
> +
> + /* io_accounting directory */
> + fs_info->io_accounting_debug_kobj =
> + kobject_create_and_add("io_accounting",fs_info->debug_kobj);
> + if (!fs_info->io_accounting_debug_kobj) {
> + error = -ENOMEM;
> + goto failure;
> + }
> +
> + error = sysfs_create_files(fs_info->io_accounting_debug_kobj,
> + io_accounting_debug_attrs);
> + if (error)
> + goto failure;
> +
> #endif
>
> error = addrm_unknown_feature_attrs(fs_info, true);
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 2332e3026efa..58f2ec0a611a 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -6763,6 +6763,29 @@ static void bioc_error(struct btrfs_io_context *bioc, struct bio *bio, u64 logic
> }
> }
>
> +static void update_io_accounting(struct btrfs_fs_info *fs_info, struct bio *bio)
> +{
> + u32 length = bio->bi_iter.bi_size;
> + bool metadata = bio->bi_opf & REQ_META;
> +
> +#ifdef CONFIG_BTRFS_DEBUG
> + spin_lock(&fs_info->io_accounting_lock);
> + if (bio_op(bio) == REQ_OP_READ) {
> + if (metadata)
> + fs_info->meta_read += length;
> + else
> + fs_info->data_read += length;
> + } else {
> + if (metadata)
> + fs_info->meta_write += length;
> + else
> + fs_info->data_write += length;
> + }
> + spin_unlock(&fs_info->io_accounting_lock);
> +#endif
> + return;
> +}
> +
> blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio,
> int mirror_num)
> {
> @@ -6776,6 +6799,7 @@ blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio,
>
> int total_devs;
> struct btrfs_io_context *bioc = NULL;
>
> + update_io_accounting(fs_info, bio);
Generally, speaking accounting should go at the end of IO.
Other than that looks good.
Also, need a way to reset the values.
Thanks, Anand
> length = bio->bi_iter.bi_size;
> map_length = length;
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH RFC] btrfs: sysfs: add <uuid>/debug/io_accounting/ directory
2022-01-21 9:14 ` Anand Jain
@ 2022-01-21 9:20 ` Qu Wenruo
0 siblings, 0 replies; 7+ messages in thread
From: Qu Wenruo @ 2022-01-21 9:20 UTC (permalink / raw)
To: Anand Jain, Qu Wenruo, linux-btrfs, dm-devel
On 2022/1/21 17:14, Anand Jain wrote:
> On 21/01/2022 13:24, Qu Wenruo wrote:
>> [BACKGROUND]
>> There is a series of btrfs autodefrag bugs in v5.16, that would cause
>> way more IO than previous kernels.
>>
>> Unfortunately there isn't any test case covering autodefrag, nor there
>> is anyway to show the io accounting of a btrfs.
>>
>> [WORKAROUND]
>> I originally want to implement a dm target to do the io accounting for
>> all filesystems, but can not find a good enough interface (the status
>> interface has only 3 pre-defined workload).
>>
>> Thus I turned to btrfs specific io accounting first.
>> One thing specific to btrfs is its integrated volume management/RAID.
>>
>> Without proper profiles specification, default profile will cause
>> metadata IO to be accounted twice (DUP profile) and only data IO is
>> accounted correctly.
>>
>> So for btrfs this patch will introduce a new sysfs directory in
>> /sys/fs/btrfs/<uuid>/debug/io_accounting/
>>
>> And have the following files:
>>
>> - meta_read: Metadata bytes read
>> - meta_write: Metadata bytes written
>> - data_read: Data bytes read
>> - data_write: Data bytes written
>> (including both zoned append and regular write)
>>
>> And all these accounting is in logical address space, meaning profile
>> will not affect the values.
>>
>> All those values can be reset by simply "echo 0".
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> Reason for RFC:
>>
>> - (To DM guys) Is there any good way to implement a dm target to do
>> the IO
>> accounting?
>> A more generic one can help more filesystems.
>>
>> - (To Btrfs guys) Is the sysfs interface fine?
>
> I am in for it. It can be a non debug feature IMO.
> More below.
>
>> ---
>> fs/btrfs/ctree.h | 11 +++++++
>> fs/btrfs/disk-io.c | 1 +
>> fs/btrfs/sysfs.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++
>> fs/btrfs/volumes.c | 24 +++++++++++++++
>> 4 files changed, 113 insertions(+)
>>
>> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
>> index b4a9b1c58d22..3983bceaef7f 100644
>> --- a/fs/btrfs/ctree.h
>> +++ b/fs/btrfs/ctree.h
>> @@ -1035,10 +1035,21 @@ struct btrfs_fs_info {
>> #ifdef CONFIG_BTRFS_DEBUG
>> struct kobject *debug_kobj;
>> struct kobject *discard_debug_kobj;
>> + struct kobject *io_accounting_debug_kobj;
>> struct list_head allocated_roots;
>> spinlock_t eb_leak_lock;
>> struct list_head allocated_ebs;
>> +
>> + spinlock_t io_accounting_lock;
>> + /*
>> + * The IO accounting unit are all in byte, and are in logical
>> address
>> + * space, which is before the RAID/DUP mapping.
>> + */
>> + u64 meta_read;
>> + u64 meta_write;
>> + u64 data_read;
>> + u64 data_write;
>> #endif
>> };
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 87a5addbedf6..41b56fde6e97 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -3065,6 +3065,7 @@ void btrfs_init_fs_info(struct btrfs_fs_info
>> *fs_info)
>> INIT_LIST_HEAD(&fs_info->allocated_roots);
>> INIT_LIST_HEAD(&fs_info->allocated_ebs);
>> spin_lock_init(&fs_info->eb_leak_lock);
>> + spin_lock_init(&fs_info->io_accounting_lock);
>> #endif
>> extent_map_tree_init(&fs_info->mapping_tree);
>> btrfs_init_block_rsv(&fs_info->global_block_rsv,
>> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
>> index beb7f72d50b8..dfdef93bdeab 100644
>> --- a/fs/btrfs/sysfs.c
>> +++ b/fs/btrfs/sysfs.c
>> @@ -40,6 +40,7 @@
>> * btrfs_debug_feature_attrs /sys/fs/btrfs/debug
>> * btrfs_debug_mount_attrs /sys/fs/btrfs/<uuid>/debug
>> * discard_debug_attrs /sys/fs/btrfs/<uuid>/debug/discard
>> + * io_accounting_debug_attrs
>> /sys/fs/btrfs/<uuid>/debug/io_accounting
>> */
>> struct btrfs_feature_attr {
>> @@ -616,6 +617,62 @@ static const struct attribute_group
>> btrfs_debug_feature_attr_group = {
>> .attrs = btrfs_debug_feature_attrs,
>> };
>> +/* IO accounting */
>> +#define io_accounting_to_fs_info(_kobj)
>> to_fs_info((_kobj)->parent->parent)
>> +
>> +#define DECLARE_IO_ACCOUNTING_OPS(name) \
>> +static ssize_t io_accounting_##name##_show(struct kobject *kobj, \
>> + struct kobj_attribute *a, \
>> + char *buf) \
>> +{ \
>> + struct btrfs_fs_info *fs_info = io_accounting_to_fs_info(kobj); \
>> + u64 result; \
>> + \
>> + spin_lock(&fs_info->io_accounting_lock); \
>> + result = fs_info->name; \
>> + spin_unlock(&fs_info->io_accounting_lock); \
>> + return sysfs_emit(buf, "%llu\n", result); \
>> +} \
>> +static ssize_t io_accounting_##name##_store(struct kobject *kobj, \
>> + struct kobj_attribute *a, \
>> + const char *buf, \
>> + size_t count) \
>> +{ \
>> + struct btrfs_fs_info *fs_info = io_accounting_to_fs_info(kobj); \
>> + u64 value; \
>> + int ret; \
>> + \
>> + ret = kstrtoull(skip_spaces(buf), 0, &value); \
>> + if (ret) \
>> + return ret; \
>> + spin_lock(&fs_info->io_accounting_lock); \
>> + fs_info->name = value; \
>> + spin_unlock(&fs_info->io_accounting_lock); \
>> + return count; \
>> +}
>> +
>> +DECLARE_IO_ACCOUNTING_OPS(meta_read);
>> +DECLARE_IO_ACCOUNTING_OPS(meta_write);
>> +DECLARE_IO_ACCOUNTING_OPS(data_read);
>> +DECLARE_IO_ACCOUNTING_OPS(data_write);
>> +
>> +BTRFS_ATTR_RW(io_accounting, meta_read, io_accounting_meta_read_show,
>> + io_accounting_meta_read_store);
>> +BTRFS_ATTR_RW(io_accounting, meta_write, io_accounting_meta_write_show,
>> + io_accounting_meta_write_store);
>> +BTRFS_ATTR_RW(io_accounting, data_read, io_accounting_data_read_show,
>> + io_accounting_data_read_store);
>> +BTRFS_ATTR_RW(io_accounting, data_write, io_accounting_data_write_show,
>> + io_accounting_data_write_store);
>> +
>> +static const struct attribute *io_accounting_debug_attrs[] = {
>> + BTRFS_ATTR_PTR(io_accounting, meta_read),
>> + BTRFS_ATTR_PTR(io_accounting, meta_write),
>> + BTRFS_ATTR_PTR(io_accounting, data_read),
>> + BTRFS_ATTR_PTR(io_accounting, data_write),
>> + NULL,
>> +};
>> +
>> #endif
>> static ssize_t btrfs_show_u64(u64 *value_ptr, spinlock_t *lock, char
>> *buf)
>> @@ -1219,6 +1276,12 @@ void btrfs_sysfs_remove_mounted(struct
>> btrfs_fs_info *fs_info)
>> kobject_del(fs_info->discard_debug_kobj);
>> kobject_put(fs_info->discard_debug_kobj);
>> }
>> + if (fs_info->io_accounting_debug_kobj) {
>> + sysfs_remove_files(fs_info->io_accounting_debug_kobj,
>> + io_accounting_debug_attrs);
>> + kobject_del(fs_info->io_accounting_debug_kobj);
>> + kobject_put(fs_info->io_accounting_debug_kobj);
>> + }
>> if (fs_info->debug_kobj) {
>> sysfs_remove_files(fs_info->debug_kobj,
>> btrfs_debug_mount_attrs);
>> kobject_del(fs_info->debug_kobj);
>> @@ -1804,6 +1867,20 @@ int btrfs_sysfs_add_mounted(struct
>> btrfs_fs_info *fs_info)
>> discard_debug_attrs);
>> if (error)
>> goto failure;
>> +
>> + /* io_accounting directory */
>> + fs_info->io_accounting_debug_kobj =
>> + kobject_create_and_add("io_accounting",fs_info->debug_kobj);
>> + if (!fs_info->io_accounting_debug_kobj) {
>> + error = -ENOMEM;
>> + goto failure;
>> + }
>> +
>> + error = sysfs_create_files(fs_info->io_accounting_debug_kobj,
>> + io_accounting_debug_attrs);
>> + if (error)
>> + goto failure;
>> +
>> #endif
>> error = addrm_unknown_feature_attrs(fs_info, true);
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index 2332e3026efa..58f2ec0a611a 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -6763,6 +6763,29 @@ static void bioc_error(struct btrfs_io_context
>> *bioc, struct bio *bio, u64 logic
>> }
>> }
>> +static void update_io_accounting(struct btrfs_fs_info *fs_info,
>> struct bio *bio)
>> +{
>> + u32 length = bio->bi_iter.bi_size;
>> + bool metadata = bio->bi_opf & REQ_META;
>> +
>> +#ifdef CONFIG_BTRFS_DEBUG
>> + spin_lock(&fs_info->io_accounting_lock);
>> + if (bio_op(bio) == REQ_OP_READ) {
>> + if (metadata)
>> + fs_info->meta_read += length;
>> + else
>> + fs_info->data_read += length;
>> + } else {
>> + if (metadata)
>> + fs_info->meta_write += length;
>> + else
>> + fs_info->data_write += length;
>> + }
>> + spin_unlock(&fs_info->io_accounting_lock);
>> +#endif
>> + return;
>> +}
>> +
>> blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio
>> *bio,
>> int mirror_num)
>> {
>> @@ -6776,6 +6799,7 @@ blk_status_t btrfs_map_bio(struct btrfs_fs_info
>> *fs_info, struct bio *bio,
> >
>> int total_devs;
>> struct btrfs_io_context *bioc = NULL;
>> + update_io_accounting(fs_info, bio);
>
> Generally, speaking accounting should go at the end of IO.
> Other than that looks good.
That is a little more complex than this solution.
At endio time, we no longer have reliable bi_iter->bi_size.
And if we go bio_for_each_segment(), it will go conflicts with recent
bio split patchset.
Thus I prefer to do the accounting before the IO.
>
> Also, need a way to reset the values.
Check io_accounting_##name##_store(), which will reset the value to the
provided one.
Normally we only need to "echo 0" to reset.
Thanks,
Qu
>
> Thanks, Anand
>
>> length = bio->bi_iter.bi_size;
>> map_length = length;
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-01-27 8:37 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-25 19:17 [PATCH RFC] btrfs: sysfs: add <uuid>/debug/io_accounting/ directory kernel test robot
2022-01-27 6:09 ` kernel test robot
2022-01-27 8:36 ` Qu Wenruo
2022-01-27 8:36 ` Qu Wenruo
-- strict thread matches above, loose matches on Subject: below --
2022-01-21 5:24 Qu Wenruo
2022-01-21 9:14 ` Anand Jain
2022-01-21 9:20 ` Qu Wenruo
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.