| Message ID | 20220420115401.186147-1-nborisov@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: Improve error reporting in lookup_inline_extent_backref | expand |
On Wed, Apr 20, 2022 at 02:54:00PM +0300, Nikolay Borisov wrote: > When iterating the backrefs in an extent item if the ptr to the > 'current' backref record goes beyond the extent item a warning is > generated and -ENOENT is returned. However what's more appropriate to > debug such cases would be to return EUCLEAN and also print the in-memory > state of the offending leaf. Agreed, EUCLEAN makese sense. Added to misc-next, thanks.
On Wed, Apr 20, 2022 at 02:54:00PM +0300, Nikolay Borisov wrote: > When iterating the backrefs in an extent item if the ptr to the > 'current' backref record goes beyond the extent item a warning is > generated and -ENOENT is returned. However what's more appropriate to > debug such cases would be to return EUCLEAN and also print the in-memory > state of the offending leaf. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/extent-tree.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index 963160a0c393..5a53bcfdca83 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -895,7 +895,10 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans, > err = -ENOENT; > while (1) { > if (ptr >= end) { > - WARN_ON(ptr > end); > + if (WARN_ON(ptr > end)) { > + err = -EUCLEAN; > + btrfs_print_leaf(path->slots[0]); This gives a warning, the slots array does not contain the extent buffer pointer so this should have been path->nodes[0] but this is already in 'leaf', fixed.
On 20.04.22 г. 18:54 ч., David Sterba wrote: > On Wed, Apr 20, 2022 at 02:54:00PM +0300, Nikolay Borisov wrote: >> When iterating the backrefs in an extent item if the ptr to the >> 'current' backref record goes beyond the extent item a warning is >> generated and -ENOENT is returned. However what's more appropriate to >> debug such cases would be to return EUCLEAN and also print the in-memory >> state of the offending leaf. >> >> Signed-off-by: Nikolay Borisov <nborisov@suse.com> >> --- >> fs/btrfs/extent-tree.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c >> index 963160a0c393..5a53bcfdca83 100644 >> --- a/fs/btrfs/extent-tree.c >> +++ b/fs/btrfs/extent-tree.c >> @@ -895,7 +895,10 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans, >> err = -ENOENT; >> while (1) { >> if (ptr >= end) { >> - WARN_ON(ptr > end); >> + if (WARN_ON(ptr > end)) { >> + err = -EUCLEAN; >> + btrfs_print_leaf(path->slots[0]); > > This gives a warning, the slots array does not contain the extent buffer > pointer so this should have been path->nodes[0] but this is already in > 'leaf', fixed. > Ah you are right... should have been ->nodes[0] not slot...
Hi Nikolay,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on kdave/for-next]
[also build test WARNING on v5.18-rc3 next-20220420]
[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/intel-lab-lkp/linux/commits/Nikolay-Borisov/btrfs-Improve-error-reporting-in-lookup_inline_extent_backref/20220420-195528
base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
config: hexagon-randconfig-r041-20220420 (https://download.01.org/0day-ci/archive/20220420/202204202351.jCXN37xH-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project bac6cd5bf85669e3376610cfc4c4f9ca015e7b9b)
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/intel-lab-lkp/linux/commit/a77df79d2f67b166cb64a21808ec432edcbf5bba
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Nikolay-Borisov/btrfs-Improve-error-reporting-in-lookup_inline_extent_backref/20220420-195528
git checkout a77df79d2f67b166cb64a21808ec432edcbf5bba
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash fs/btrfs/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> fs/btrfs/extent-tree.c:900:22: warning: incompatible integer to pointer conversion passing 'int' to parameter of type 'struct extent_buffer *' [-Wint-conversion]
btrfs_print_leaf(path->slots[0]);
^~~~~~~~~~~~~~
fs/btrfs/print-tree.h:12:45: note: passing argument to parameter 'l' here
void btrfs_print_leaf(struct extent_buffer *l);
^
1 warning generated.
vim +900 fs/btrfs/extent-tree.c
770
771 /*
772 * look for inline back ref. if back ref is found, *ref_ret is set
773 * to the address of inline back ref, and 0 is returned.
774 *
775 * if back ref isn't found, *ref_ret is set to the address where it
776 * should be inserted, and -ENOENT is returned.
777 *
778 * if insert is true and there are too many inline back refs, the path
779 * points to the extent item, and -EAGAIN is returned.
780 *
781 * NOTE: inline back refs are ordered in the same way that back ref
782 * items in the tree are ordered.
783 */
784 static noinline_for_stack
785 int lookup_inline_extent_backref(struct btrfs_trans_handle *trans,
786 struct btrfs_path *path,
787 struct btrfs_extent_inline_ref **ref_ret,
788 u64 bytenr, u64 num_bytes,
789 u64 parent, u64 root_objectid,
790 u64 owner, u64 offset, int insert)
791 {
792 struct btrfs_fs_info *fs_info = trans->fs_info;
793 struct btrfs_root *root = btrfs_extent_root(fs_info, bytenr);
794 struct btrfs_key key;
795 struct extent_buffer *leaf;
796 struct btrfs_extent_item *ei;
797 struct btrfs_extent_inline_ref *iref;
798 u64 flags;
799 u64 item_size;
800 unsigned long ptr;
801 unsigned long end;
802 int extra_size;
803 int type;
804 int want;
805 int ret;
806 int err = 0;
807 bool skinny_metadata = btrfs_fs_incompat(fs_info, SKINNY_METADATA);
808 int needed;
809
810 key.objectid = bytenr;
811 key.type = BTRFS_EXTENT_ITEM_KEY;
812 key.offset = num_bytes;
813
814 want = extent_ref_type(parent, owner);
815 if (insert) {
816 extra_size = btrfs_extent_inline_ref_size(want);
817 path->search_for_extension = 1;
818 path->keep_locks = 1;
819 } else
820 extra_size = -1;
821
822 /*
823 * Owner is our level, so we can just add one to get the level for the
824 * block we are interested in.
825 */
826 if (skinny_metadata && owner < BTRFS_FIRST_FREE_OBJECTID) {
827 key.type = BTRFS_METADATA_ITEM_KEY;
828 key.offset = owner;
829 }
830
831 again:
832 ret = btrfs_search_slot(trans, root, &key, path, extra_size, 1);
833 if (ret < 0) {
834 err = ret;
835 goto out;
836 }
837
838 /*
839 * We may be a newly converted file system which still has the old fat
840 * extent entries for metadata, so try and see if we have one of those.
841 */
842 if (ret > 0 && skinny_metadata) {
843 skinny_metadata = false;
844 if (path->slots[0]) {
845 path->slots[0]--;
846 btrfs_item_key_to_cpu(path->nodes[0], &key,
847 path->slots[0]);
848 if (key.objectid == bytenr &&
849 key.type == BTRFS_EXTENT_ITEM_KEY &&
850 key.offset == num_bytes)
851 ret = 0;
852 }
853 if (ret) {
854 key.objectid = bytenr;
855 key.type = BTRFS_EXTENT_ITEM_KEY;
856 key.offset = num_bytes;
857 btrfs_release_path(path);
858 goto again;
859 }
860 }
861
862 if (ret && !insert) {
863 err = -ENOENT;
864 goto out;
865 } else if (WARN_ON(ret)) {
866 err = -EIO;
867 goto out;
868 }
869
870 leaf = path->nodes[0];
871 item_size = btrfs_item_size(leaf, path->slots[0]);
872 if (unlikely(item_size < sizeof(*ei))) {
873 err = -EINVAL;
874 btrfs_print_v0_err(fs_info);
875 btrfs_abort_transaction(trans, err);
876 goto out;
877 }
878
879 ei = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_extent_item);
880 flags = btrfs_extent_flags(leaf, ei);
881
882 ptr = (unsigned long)(ei + 1);
883 end = (unsigned long)ei + item_size;
884
885 if (flags & BTRFS_EXTENT_FLAG_TREE_BLOCK && !skinny_metadata) {
886 ptr += sizeof(struct btrfs_tree_block_info);
887 BUG_ON(ptr > end);
888 }
889
890 if (owner >= BTRFS_FIRST_FREE_OBJECTID)
891 needed = BTRFS_REF_TYPE_DATA;
892 else
893 needed = BTRFS_REF_TYPE_BLOCK;
894
895 err = -ENOENT;
896 while (1) {
897 if (ptr >= end) {
898 if (WARN_ON(ptr > end)) {
899 err = -EUCLEAN;
> 900 btrfs_print_leaf(path->slots[0]);
901 }
902 break;
903 }
904 iref = (struct btrfs_extent_inline_ref *)ptr;
905 type = btrfs_get_extent_inline_ref_type(leaf, iref, needed);
906 if (type == BTRFS_REF_TYPE_INVALID) {
907 err = -EUCLEAN;
908 goto out;
909 }
910
911 if (want < type)
912 break;
913 if (want > type) {
914 ptr += btrfs_extent_inline_ref_size(type);
915 continue;
916 }
917
918 if (type == BTRFS_EXTENT_DATA_REF_KEY) {
919 struct btrfs_extent_data_ref *dref;
920 dref = (struct btrfs_extent_data_ref *)(&iref->offset);
921 if (match_extent_data_ref(leaf, dref, root_objectid,
922 owner, offset)) {
923 err = 0;
924 break;
925 }
926 if (hash_extent_data_ref_item(leaf, dref) <
927 hash_extent_data_ref(root_objectid, owner, offset))
928 break;
929 } else {
930 u64 ref_offset;
931 ref_offset = btrfs_extent_inline_ref_offset(leaf, iref);
932 if (parent > 0) {
933 if (parent == ref_offset) {
934 err = 0;
935 break;
936 }
937 if (ref_offset < parent)
938 break;
939 } else {
940 if (root_objectid == ref_offset) {
941 err = 0;
942 break;
943 }
944 if (ref_offset < root_objectid)
945 break;
946 }
947 }
948 ptr += btrfs_extent_inline_ref_size(type);
949 }
950 if (err == -ENOENT && insert) {
951 if (item_size + extra_size >=
952 BTRFS_MAX_EXTENT_ITEM_SIZE(root)) {
953 err = -EAGAIN;
954 goto out;
955 }
956 /*
957 * To add new inline back ref, we have to make sure
958 * there is no corresponding back ref item.
959 * For simplicity, we just do not add new inline back
960 * ref if there is any kind of item for this block
961 */
962 if (find_next_key(path, 0, &key) == 0 &&
963 key.objectid == bytenr &&
964 key.type < BTRFS_BLOCK_GROUP_ITEM_KEY) {
965 err = -EAGAIN;
966 goto out;
967 }
968 }
969 *ref_ret = (struct btrfs_extent_inline_ref *)ptr;
970 out:
971 if (insert) {
972 path->keep_locks = 0;
973 path->search_for_extension = 0;
974 btrfs_unlock_up_safe(path, 1);
975 }
976 return err;
977 }
978
Hi Nikolay,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on kdave/for-next]
[also build test WARNING on v5.18-rc3 next-20220420]
[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/intel-lab-lkp/linux/commits/Nikolay-Borisov/btrfs-Improve-error-reporting-in-lookup_inline_extent_backref/20220420-195528
base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
config: alpha-randconfig-r014-20220420 (https://download.01.org/0day-ci/archive/20220421/202204210142.BF3uWUQj-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.2.0
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/intel-lab-lkp/linux/commit/a77df79d2f67b166cb64a21808ec432edcbf5bba
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Nikolay-Borisov/btrfs-Improve-error-reporting-in-lookup_inline_extent_backref/20220420-195528
git checkout a77df79d2f67b166cb64a21808ec432edcbf5bba
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross W=1 O=build_dir ARCH=alpha SHELL=/bin/bash fs/btrfs/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
fs/btrfs/extent-tree.c: In function 'lookup_inline_extent_backref':
>> fs/btrfs/extent-tree.c:900:61: warning: passing argument 1 of 'btrfs_print_leaf' makes pointer from integer without a cast [-Wint-conversion]
900 | btrfs_print_leaf(path->slots[0]);
| ~~~~~~~~~~~^~~
| |
| int
In file included from fs/btrfs/extent-tree.c:22:
fs/btrfs/print-tree.h:12:45: note: expected 'struct extent_buffer *' but argument is of type 'int'
12 | void btrfs_print_leaf(struct extent_buffer *l);
| ~~~~~~~~~~~~~~~~~~~~~~^
vim +/btrfs_print_leaf +900 fs/btrfs/extent-tree.c
770
771 /*
772 * look for inline back ref. if back ref is found, *ref_ret is set
773 * to the address of inline back ref, and 0 is returned.
774 *
775 * if back ref isn't found, *ref_ret is set to the address where it
776 * should be inserted, and -ENOENT is returned.
777 *
778 * if insert is true and there are too many inline back refs, the path
779 * points to the extent item, and -EAGAIN is returned.
780 *
781 * NOTE: inline back refs are ordered in the same way that back ref
782 * items in the tree are ordered.
783 */
784 static noinline_for_stack
785 int lookup_inline_extent_backref(struct btrfs_trans_handle *trans,
786 struct btrfs_path *path,
787 struct btrfs_extent_inline_ref **ref_ret,
788 u64 bytenr, u64 num_bytes,
789 u64 parent, u64 root_objectid,
790 u64 owner, u64 offset, int insert)
791 {
792 struct btrfs_fs_info *fs_info = trans->fs_info;
793 struct btrfs_root *root = btrfs_extent_root(fs_info, bytenr);
794 struct btrfs_key key;
795 struct extent_buffer *leaf;
796 struct btrfs_extent_item *ei;
797 struct btrfs_extent_inline_ref *iref;
798 u64 flags;
799 u64 item_size;
800 unsigned long ptr;
801 unsigned long end;
802 int extra_size;
803 int type;
804 int want;
805 int ret;
806 int err = 0;
807 bool skinny_metadata = btrfs_fs_incompat(fs_info, SKINNY_METADATA);
808 int needed;
809
810 key.objectid = bytenr;
811 key.type = BTRFS_EXTENT_ITEM_KEY;
812 key.offset = num_bytes;
813
814 want = extent_ref_type(parent, owner);
815 if (insert) {
816 extra_size = btrfs_extent_inline_ref_size(want);
817 path->search_for_extension = 1;
818 path->keep_locks = 1;
819 } else
820 extra_size = -1;
821
822 /*
823 * Owner is our level, so we can just add one to get the level for the
824 * block we are interested in.
825 */
826 if (skinny_metadata && owner < BTRFS_FIRST_FREE_OBJECTID) {
827 key.type = BTRFS_METADATA_ITEM_KEY;
828 key.offset = owner;
829 }
830
831 again:
832 ret = btrfs_search_slot(trans, root, &key, path, extra_size, 1);
833 if (ret < 0) {
834 err = ret;
835 goto out;
836 }
837
838 /*
839 * We may be a newly converted file system which still has the old fat
840 * extent entries for metadata, so try and see if we have one of those.
841 */
842 if (ret > 0 && skinny_metadata) {
843 skinny_metadata = false;
844 if (path->slots[0]) {
845 path->slots[0]--;
846 btrfs_item_key_to_cpu(path->nodes[0], &key,
847 path->slots[0]);
848 if (key.objectid == bytenr &&
849 key.type == BTRFS_EXTENT_ITEM_KEY &&
850 key.offset == num_bytes)
851 ret = 0;
852 }
853 if (ret) {
854 key.objectid = bytenr;
855 key.type = BTRFS_EXTENT_ITEM_KEY;
856 key.offset = num_bytes;
857 btrfs_release_path(path);
858 goto again;
859 }
860 }
861
862 if (ret && !insert) {
863 err = -ENOENT;
864 goto out;
865 } else if (WARN_ON(ret)) {
866 err = -EIO;
867 goto out;
868 }
869
870 leaf = path->nodes[0];
871 item_size = btrfs_item_size(leaf, path->slots[0]);
872 if (unlikely(item_size < sizeof(*ei))) {
873 err = -EINVAL;
874 btrfs_print_v0_err(fs_info);
875 btrfs_abort_transaction(trans, err);
876 goto out;
877 }
878
879 ei = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_extent_item);
880 flags = btrfs_extent_flags(leaf, ei);
881
882 ptr = (unsigned long)(ei + 1);
883 end = (unsigned long)ei + item_size;
884
885 if (flags & BTRFS_EXTENT_FLAG_TREE_BLOCK && !skinny_metadata) {
886 ptr += sizeof(struct btrfs_tree_block_info);
887 BUG_ON(ptr > end);
888 }
889
890 if (owner >= BTRFS_FIRST_FREE_OBJECTID)
891 needed = BTRFS_REF_TYPE_DATA;
892 else
893 needed = BTRFS_REF_TYPE_BLOCK;
894
895 err = -ENOENT;
896 while (1) {
897 if (ptr >= end) {
898 if (WARN_ON(ptr > end)) {
899 err = -EUCLEAN;
> 900 btrfs_print_leaf(path->slots[0]);
901 }
902 break;
903 }
904 iref = (struct btrfs_extent_inline_ref *)ptr;
905 type = btrfs_get_extent_inline_ref_type(leaf, iref, needed);
906 if (type == BTRFS_REF_TYPE_INVALID) {
907 err = -EUCLEAN;
908 goto out;
909 }
910
911 if (want < type)
912 break;
913 if (want > type) {
914 ptr += btrfs_extent_inline_ref_size(type);
915 continue;
916 }
917
918 if (type == BTRFS_EXTENT_DATA_REF_KEY) {
919 struct btrfs_extent_data_ref *dref;
920 dref = (struct btrfs_extent_data_ref *)(&iref->offset);
921 if (match_extent_data_ref(leaf, dref, root_objectid,
922 owner, offset)) {
923 err = 0;
924 break;
925 }
926 if (hash_extent_data_ref_item(leaf, dref) <
927 hash_extent_data_ref(root_objectid, owner, offset))
928 break;
929 } else {
930 u64 ref_offset;
931 ref_offset = btrfs_extent_inline_ref_offset(leaf, iref);
932 if (parent > 0) {
933 if (parent == ref_offset) {
934 err = 0;
935 break;
936 }
937 if (ref_offset < parent)
938 break;
939 } else {
940 if (root_objectid == ref_offset) {
941 err = 0;
942 break;
943 }
944 if (ref_offset < root_objectid)
945 break;
946 }
947 }
948 ptr += btrfs_extent_inline_ref_size(type);
949 }
950 if (err == -ENOENT && insert) {
951 if (item_size + extra_size >=
952 BTRFS_MAX_EXTENT_ITEM_SIZE(root)) {
953 err = -EAGAIN;
954 goto out;
955 }
956 /*
957 * To add new inline back ref, we have to make sure
958 * there is no corresponding back ref item.
959 * For simplicity, we just do not add new inline back
960 * ref if there is any kind of item for this block
961 */
962 if (find_next_key(path, 0, &key) == 0 &&
963 key.objectid == bytenr &&
964 key.type < BTRFS_BLOCK_GROUP_ITEM_KEY) {
965 err = -EAGAIN;
966 goto out;
967 }
968 }
969 *ref_ret = (struct btrfs_extent_inline_ref *)ptr;
970 out:
971 if (insert) {
972 path->keep_locks = 0;
973 path->search_for_extension = 0;
974 btrfs_unlock_up_safe(path, 1);
975 }
976 return err;
977 }
978
Hi Nikolay,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on kdave/for-next]
[also build test WARNING on v5.18-rc3 next-20220420]
[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/intel-lab-lkp/linux/commits/Nikolay-Borisov/btrfs-Improve-error-reporting-in-lookup_inline_extent_backref/20220420-195528
base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
config: i386-randconfig-s002 (https://download.01.org/0day-ci/archive/20220421/202204210801.4G6iUKKd-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/intel-lab-lkp/linux/commit/a77df79d2f67b166cb64a21808ec432edcbf5bba
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Nikolay-Borisov/btrfs-Improve-error-reporting-in-lookup_inline_extent_backref/20220420-195528
git checkout a77df79d2f67b166cb64a21808ec432edcbf5bba
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash fs/btrfs/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> fs/btrfs/extent-tree.c:900:61: sparse: sparse: incorrect type in argument 1 (different base types) @@ expected struct extent_buffer *l @@ got int @@
fs/btrfs/extent-tree.c:900:61: sparse: expected struct extent_buffer *l
fs/btrfs/extent-tree.c:900:61: sparse: got int
fs/btrfs/extent-tree.c:1784:9: sparse: sparse: context imbalance in 'run_and_cleanup_extent_op' - unexpected unlock
fs/btrfs/extent-tree.c:1838:28: sparse: sparse: context imbalance in 'cleanup_ref_head' - unexpected unlock
fs/btrfs/extent-tree.c:1917:36: sparse: sparse: context imbalance in 'btrfs_run_delayed_refs_for_head' - unexpected unlock
fs/btrfs/extent-tree.c:1982:21: sparse: sparse: context imbalance in '__btrfs_run_delayed_refs' - wrong count at exit
vim +900 fs/btrfs/extent-tree.c
770
771 /*
772 * look for inline back ref. if back ref is found, *ref_ret is set
773 * to the address of inline back ref, and 0 is returned.
774 *
775 * if back ref isn't found, *ref_ret is set to the address where it
776 * should be inserted, and -ENOENT is returned.
777 *
778 * if insert is true and there are too many inline back refs, the path
779 * points to the extent item, and -EAGAIN is returned.
780 *
781 * NOTE: inline back refs are ordered in the same way that back ref
782 * items in the tree are ordered.
783 */
784 static noinline_for_stack
785 int lookup_inline_extent_backref(struct btrfs_trans_handle *trans,
786 struct btrfs_path *path,
787 struct btrfs_extent_inline_ref **ref_ret,
788 u64 bytenr, u64 num_bytes,
789 u64 parent, u64 root_objectid,
790 u64 owner, u64 offset, int insert)
791 {
792 struct btrfs_fs_info *fs_info = trans->fs_info;
793 struct btrfs_root *root = btrfs_extent_root(fs_info, bytenr);
794 struct btrfs_key key;
795 struct extent_buffer *leaf;
796 struct btrfs_extent_item *ei;
797 struct btrfs_extent_inline_ref *iref;
798 u64 flags;
799 u64 item_size;
800 unsigned long ptr;
801 unsigned long end;
802 int extra_size;
803 int type;
804 int want;
805 int ret;
806 int err = 0;
807 bool skinny_metadata = btrfs_fs_incompat(fs_info, SKINNY_METADATA);
808 int needed;
809
810 key.objectid = bytenr;
811 key.type = BTRFS_EXTENT_ITEM_KEY;
812 key.offset = num_bytes;
813
814 want = extent_ref_type(parent, owner);
815 if (insert) {
816 extra_size = btrfs_extent_inline_ref_size(want);
817 path->search_for_extension = 1;
818 path->keep_locks = 1;
819 } else
820 extra_size = -1;
821
822 /*
823 * Owner is our level, so we can just add one to get the level for the
824 * block we are interested in.
825 */
826 if (skinny_metadata && owner < BTRFS_FIRST_FREE_OBJECTID) {
827 key.type = BTRFS_METADATA_ITEM_KEY;
828 key.offset = owner;
829 }
830
831 again:
832 ret = btrfs_search_slot(trans, root, &key, path, extra_size, 1);
833 if (ret < 0) {
834 err = ret;
835 goto out;
836 }
837
838 /*
839 * We may be a newly converted file system which still has the old fat
840 * extent entries for metadata, so try and see if we have one of those.
841 */
842 if (ret > 0 && skinny_metadata) {
843 skinny_metadata = false;
844 if (path->slots[0]) {
845 path->slots[0]--;
846 btrfs_item_key_to_cpu(path->nodes[0], &key,
847 path->slots[0]);
848 if (key.objectid == bytenr &&
849 key.type == BTRFS_EXTENT_ITEM_KEY &&
850 key.offset == num_bytes)
851 ret = 0;
852 }
853 if (ret) {
854 key.objectid = bytenr;
855 key.type = BTRFS_EXTENT_ITEM_KEY;
856 key.offset = num_bytes;
857 btrfs_release_path(path);
858 goto again;
859 }
860 }
861
862 if (ret && !insert) {
863 err = -ENOENT;
864 goto out;
865 } else if (WARN_ON(ret)) {
866 err = -EIO;
867 goto out;
868 }
869
870 leaf = path->nodes[0];
871 item_size = btrfs_item_size(leaf, path->slots[0]);
872 if (unlikely(item_size < sizeof(*ei))) {
873 err = -EINVAL;
874 btrfs_print_v0_err(fs_info);
875 btrfs_abort_transaction(trans, err);
876 goto out;
877 }
878
879 ei = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_extent_item);
880 flags = btrfs_extent_flags(leaf, ei);
881
882 ptr = (unsigned long)(ei + 1);
883 end = (unsigned long)ei + item_size;
884
885 if (flags & BTRFS_EXTENT_FLAG_TREE_BLOCK && !skinny_metadata) {
886 ptr += sizeof(struct btrfs_tree_block_info);
887 BUG_ON(ptr > end);
888 }
889
890 if (owner >= BTRFS_FIRST_FREE_OBJECTID)
891 needed = BTRFS_REF_TYPE_DATA;
892 else
893 needed = BTRFS_REF_TYPE_BLOCK;
894
895 err = -ENOENT;
896 while (1) {
897 if (ptr >= end) {
898 if (WARN_ON(ptr > end)) {
899 err = -EUCLEAN;
> 900 btrfs_print_leaf(path->slots[0]);
901 }
902 break;
903 }
904 iref = (struct btrfs_extent_inline_ref *)ptr;
905 type = btrfs_get_extent_inline_ref_type(leaf, iref, needed);
906 if (type == BTRFS_REF_TYPE_INVALID) {
907 err = -EUCLEAN;
908 goto out;
909 }
910
911 if (want < type)
912 break;
913 if (want > type) {
914 ptr += btrfs_extent_inline_ref_size(type);
915 continue;
916 }
917
918 if (type == BTRFS_EXTENT_DATA_REF_KEY) {
919 struct btrfs_extent_data_ref *dref;
920 dref = (struct btrfs_extent_data_ref *)(&iref->offset);
921 if (match_extent_data_ref(leaf, dref, root_objectid,
922 owner, offset)) {
923 err = 0;
924 break;
925 }
926 if (hash_extent_data_ref_item(leaf, dref) <
927 hash_extent_data_ref(root_objectid, owner, offset))
928 break;
929 } else {
930 u64 ref_offset;
931 ref_offset = btrfs_extent_inline_ref_offset(leaf, iref);
932 if (parent > 0) {
933 if (parent == ref_offset) {
934 err = 0;
935 break;
936 }
937 if (ref_offset < parent)
938 break;
939 } else {
940 if (root_objectid == ref_offset) {
941 err = 0;
942 break;
943 }
944 if (ref_offset < root_objectid)
945 break;
946 }
947 }
948 ptr += btrfs_extent_inline_ref_size(type);
949 }
950 if (err == -ENOENT && insert) {
951 if (item_size + extra_size >=
952 BTRFS_MAX_EXTENT_ITEM_SIZE(root)) {
953 err = -EAGAIN;
954 goto out;
955 }
956 /*
957 * To add new inline back ref, we have to make sure
958 * there is no corresponding back ref item.
959 * For simplicity, we just do not add new inline back
960 * ref if there is any kind of item for this block
961 */
962 if (find_next_key(path, 0, &key) == 0 &&
963 key.objectid == bytenr &&
964 key.type < BTRFS_BLOCK_GROUP_ITEM_KEY) {
965 err = -EAGAIN;
966 goto out;
967 }
968 }
969 *ref_ret = (struct btrfs_extent_inline_ref *)ptr;
970 out:
971 if (insert) {
972 path->keep_locks = 0;
973 path->search_for_extension = 0;
974 btrfs_unlock_up_safe(path, 1);
975 }
976 return err;
977 }
978
Hi Nikolay,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on kdave/for-next]
[also build test WARNING on v5.18-rc3 next-20220420]
[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/intel-lab-lkp/linux/commits/Nikolay-Borisov/btrfs-Improve-error-reporting-in-lookup_inline_extent_backref/20220420-195528
base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
config: alpha-randconfig-s032-20220420 (https://download.01.org/0day-ci/archive/20220421/202204210906.VK0dZtcM-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/intel-lab-lkp/linux/commit/a77df79d2f67b166cb64a21808ec432edcbf5bba
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Nikolay-Borisov/btrfs-Improve-error-reporting-in-lookup_inline_extent_backref/20220420-195528
git checkout a77df79d2f67b166cb64a21808ec432edcbf5bba
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=alpha SHELL=/bin/bash fs/btrfs/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
fs/btrfs/extent-tree.c:900:61: sparse: sparse: incorrect type in argument 1 (different base types) @@ expected struct extent_buffer *l @@ got int @@
fs/btrfs/extent-tree.c:900:61: sparse: expected struct extent_buffer *l
fs/btrfs/extent-tree.c:900:61: sparse: got int
>> fs/btrfs/extent-tree.c:900:50: sparse: sparse: non size-preserving integer to pointer cast
fs/btrfs/extent-tree.c:1784:9: sparse: sparse: context imbalance in 'run_and_cleanup_extent_op' - unexpected unlock
fs/btrfs/extent-tree.c:1838:28: sparse: sparse: context imbalance in 'cleanup_ref_head' - unexpected unlock
fs/btrfs/extent-tree.c:1917:36: sparse: sparse: context imbalance in 'btrfs_run_delayed_refs_for_head' - unexpected unlock
fs/btrfs/extent-tree.c:1982:21: sparse: sparse: context imbalance in '__btrfs_run_delayed_refs' - wrong count at exit
vim +900 fs/btrfs/extent-tree.c
770
771 /*
772 * look for inline back ref. if back ref is found, *ref_ret is set
773 * to the address of inline back ref, and 0 is returned.
774 *
775 * if back ref isn't found, *ref_ret is set to the address where it
776 * should be inserted, and -ENOENT is returned.
777 *
778 * if insert is true and there are too many inline back refs, the path
779 * points to the extent item, and -EAGAIN is returned.
780 *
781 * NOTE: inline back refs are ordered in the same way that back ref
782 * items in the tree are ordered.
783 */
784 static noinline_for_stack
785 int lookup_inline_extent_backref(struct btrfs_trans_handle *trans,
786 struct btrfs_path *path,
787 struct btrfs_extent_inline_ref **ref_ret,
788 u64 bytenr, u64 num_bytes,
789 u64 parent, u64 root_objectid,
790 u64 owner, u64 offset, int insert)
791 {
792 struct btrfs_fs_info *fs_info = trans->fs_info;
793 struct btrfs_root *root = btrfs_extent_root(fs_info, bytenr);
794 struct btrfs_key key;
795 struct extent_buffer *leaf;
796 struct btrfs_extent_item *ei;
797 struct btrfs_extent_inline_ref *iref;
798 u64 flags;
799 u64 item_size;
800 unsigned long ptr;
801 unsigned long end;
802 int extra_size;
803 int type;
804 int want;
805 int ret;
806 int err = 0;
807 bool skinny_metadata = btrfs_fs_incompat(fs_info, SKINNY_METADATA);
808 int needed;
809
810 key.objectid = bytenr;
811 key.type = BTRFS_EXTENT_ITEM_KEY;
812 key.offset = num_bytes;
813
814 want = extent_ref_type(parent, owner);
815 if (insert) {
816 extra_size = btrfs_extent_inline_ref_size(want);
817 path->search_for_extension = 1;
818 path->keep_locks = 1;
819 } else
820 extra_size = -1;
821
822 /*
823 * Owner is our level, so we can just add one to get the level for the
824 * block we are interested in.
825 */
826 if (skinny_metadata && owner < BTRFS_FIRST_FREE_OBJECTID) {
827 key.type = BTRFS_METADATA_ITEM_KEY;
828 key.offset = owner;
829 }
830
831 again:
832 ret = btrfs_search_slot(trans, root, &key, path, extra_size, 1);
833 if (ret < 0) {
834 err = ret;
835 goto out;
836 }
837
838 /*
839 * We may be a newly converted file system which still has the old fat
840 * extent entries for metadata, so try and see if we have one of those.
841 */
842 if (ret > 0 && skinny_metadata) {
843 skinny_metadata = false;
844 if (path->slots[0]) {
845 path->slots[0]--;
846 btrfs_item_key_to_cpu(path->nodes[0], &key,
847 path->slots[0]);
848 if (key.objectid == bytenr &&
849 key.type == BTRFS_EXTENT_ITEM_KEY &&
850 key.offset == num_bytes)
851 ret = 0;
852 }
853 if (ret) {
854 key.objectid = bytenr;
855 key.type = BTRFS_EXTENT_ITEM_KEY;
856 key.offset = num_bytes;
857 btrfs_release_path(path);
858 goto again;
859 }
860 }
861
862 if (ret && !insert) {
863 err = -ENOENT;
864 goto out;
865 } else if (WARN_ON(ret)) {
866 err = -EIO;
867 goto out;
868 }
869
870 leaf = path->nodes[0];
871 item_size = btrfs_item_size(leaf, path->slots[0]);
872 if (unlikely(item_size < sizeof(*ei))) {
873 err = -EINVAL;
874 btrfs_print_v0_err(fs_info);
875 btrfs_abort_transaction(trans, err);
876 goto out;
877 }
878
879 ei = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_extent_item);
880 flags = btrfs_extent_flags(leaf, ei);
881
882 ptr = (unsigned long)(ei + 1);
883 end = (unsigned long)ei + item_size;
884
885 if (flags & BTRFS_EXTENT_FLAG_TREE_BLOCK && !skinny_metadata) {
886 ptr += sizeof(struct btrfs_tree_block_info);
887 BUG_ON(ptr > end);
888 }
889
890 if (owner >= BTRFS_FIRST_FREE_OBJECTID)
891 needed = BTRFS_REF_TYPE_DATA;
892 else
893 needed = BTRFS_REF_TYPE_BLOCK;
894
895 err = -ENOENT;
896 while (1) {
897 if (ptr >= end) {
898 if (WARN_ON(ptr > end)) {
899 err = -EUCLEAN;
> 900 btrfs_print_leaf(path->slots[0]);
901 }
902 break;
903 }
904 iref = (struct btrfs_extent_inline_ref *)ptr;
905 type = btrfs_get_extent_inline_ref_type(leaf, iref, needed);
906 if (type == BTRFS_REF_TYPE_INVALID) {
907 err = -EUCLEAN;
908 goto out;
909 }
910
911 if (want < type)
912 break;
913 if (want > type) {
914 ptr += btrfs_extent_inline_ref_size(type);
915 continue;
916 }
917
918 if (type == BTRFS_EXTENT_DATA_REF_KEY) {
919 struct btrfs_extent_data_ref *dref;
920 dref = (struct btrfs_extent_data_ref *)(&iref->offset);
921 if (match_extent_data_ref(leaf, dref, root_objectid,
922 owner, offset)) {
923 err = 0;
924 break;
925 }
926 if (hash_extent_data_ref_item(leaf, dref) <
927 hash_extent_data_ref(root_objectid, owner, offset))
928 break;
929 } else {
930 u64 ref_offset;
931 ref_offset = btrfs_extent_inline_ref_offset(leaf, iref);
932 if (parent > 0) {
933 if (parent == ref_offset) {
934 err = 0;
935 break;
936 }
937 if (ref_offset < parent)
938 break;
939 } else {
940 if (root_objectid == ref_offset) {
941 err = 0;
942 break;
943 }
944 if (ref_offset < root_objectid)
945 break;
946 }
947 }
948 ptr += btrfs_extent_inline_ref_size(type);
949 }
950 if (err == -ENOENT && insert) {
951 if (item_size + extra_size >=
952 BTRFS_MAX_EXTENT_ITEM_SIZE(root)) {
953 err = -EAGAIN;
954 goto out;
955 }
956 /*
957 * To add new inline back ref, we have to make sure
958 * there is no corresponding back ref item.
959 * For simplicity, we just do not add new inline back
960 * ref if there is any kind of item for this block
961 */
962 if (find_next_key(path, 0, &key) == 0 &&
963 key.objectid == bytenr &&
964 key.type < BTRFS_BLOCK_GROUP_ITEM_KEY) {
965 err = -EAGAIN;
966 goto out;
967 }
968 }
969 *ref_ret = (struct btrfs_extent_inline_ref *)ptr;
970 out:
971 if (insert) {
972 path->keep_locks = 0;
973 path->search_for_extension = 0;
974 btrfs_unlock_up_safe(path, 1);
975 }
976 return err;
977 }
978
On Wed, Apr 20, 2022 at 04:52:04PM +0200, David Sterba wrote: > On Wed, Apr 20, 2022 at 02:54:00PM +0300, Nikolay Borisov wrote: > > When iterating the backrefs in an extent item if the ptr to the > > 'current' backref record goes beyond the extent item a warning is > > generated and -ENOENT is returned. However what's more appropriate to > > debug such cases would be to return EUCLEAN and also print the in-memory > > state of the offending leaf. How does printing only the leaf helps debugging anything? You get the leaf dumped, but how do you know what you should be looking for? Which key in the leaf, and for which inline backref are we searching for? Dumping the leaf alone is not really useful, unless we also mention what we are searching for... So this should print as well: 1) The key, which tells us which key to look at, the extent's bytenr, type and size or owner; 2) The type of reference we are looking for, stored in the variable 'want'; 3) The values of the variables 'root_objectid', 'parent' and 'offset'. These are used to search for the backreference we want, once we find one with the type we want. Thanks. > > Agreed, EUCLEAN makese sense. Added to misc-next, thanks.
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 963160a0c393..5a53bcfdca83 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -895,7 +895,10 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans, err = -ENOENT; while (1) { if (ptr >= end) { - WARN_ON(ptr > end); + if (WARN_ON(ptr > end)) { + err = -EUCLEAN; + btrfs_print_leaf(path->slots[0]); + } break; } iref = (struct btrfs_extent_inline_ref *)ptr;
When iterating the backrefs in an extent item if the ptr to the 'current' backref record goes beyond the extent item a warning is generated and -ENOENT is returned. However what's more appropriate to debug such cases would be to return EUCLEAN and also print the in-memory state of the offending leaf. Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- fs/btrfs/extent-tree.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)