* [PATCH] btrfs: use set functions to update latest refs to the SB @ 2018-02-12 15:37 Anand Jain 2018-02-12 16:34 ` David Sterba 2018-02-13 3:00 ` [PATCH v2] btrfs: fix endianness compatibility during the SB RW Anand Jain 0 siblings, 2 replies; 12+ messages in thread From: Anand Jain @ 2018-02-12 15:37 UTC (permalink / raw) To: linux-btrfs We have btrfs_set_super...() to set the various values, so just use it. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- fs/btrfs/transaction.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c index 04f07144b45c..9220f004001c 100644 --- a/fs/btrfs/transaction.c +++ b/fs/btrfs/transaction.c @@ -1722,19 +1722,23 @@ static void update_super_roots(struct btrfs_fs_info *fs_info) super = fs_info->super_copy; + /* update latest btrfs_super_block::chunk_root refs */ root_item = &fs_info->chunk_root->root_item; - super->chunk_root = root_item->bytenr; - super->chunk_root_generation = root_item->generation; - super->chunk_root_level = root_item->level; + btrfs_set_super_chunk_root(super, root_item->bytenr); + btrfs_set_super_chunk_root_generation(super, root_item->generation); + btrfs_set_super_chunk_root_level(super, root_item->level); + /* update latest btrfs_super_block::root refs */ root_item = &fs_info->tree_root->root_item; - super->root = root_item->bytenr; - super->generation = root_item->generation; - super->root_level = root_item->level; + btrfs_set_super_root(super, root_item->bytenr); + btrfs_set_super_generation(super, root_item->generation); + btrfs_set_super_root_level(super, root_item->level); + if (btrfs_test_opt(fs_info, SPACE_CACHE)) - super->cache_generation = root_item->generation; + btrfs_set_super_cache_generation(super, root_item->generation); if (test_bit(BTRFS_FS_UPDATE_UUID_TREE_GEN, &fs_info->flags)) - super->uuid_tree_generation = root_item->generation; + btrfs_set_super_uuid_tree_generation(super, + root_item->generation); } int btrfs_transaction_in_commit(struct btrfs_fs_info *info) -- 2.15.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] btrfs: use set functions to update latest refs to the SB 2018-02-12 15:37 [PATCH] btrfs: use set functions to update latest refs to the SB Anand Jain @ 2018-02-12 16:34 ` David Sterba 2018-02-13 2:56 ` Anand Jain 2018-02-13 3:00 ` [PATCH v2] btrfs: fix endianness compatibility during the SB RW Anand Jain 1 sibling, 1 reply; 12+ messages in thread From: David Sterba @ 2018-02-12 16:34 UTC (permalink / raw) To: Anand Jain; +Cc: linux-btrfs On Mon, Feb 12, 2018 at 11:37:49PM +0800, Anand Jain wrote: > We have btrfs_set_super...() to set the various values, so just use it. Sorry but this is not "just use it", as it fixes an endianness bug and should be documented in the changelog. And there are probably more instances of the bug while accessing fs_info::super or fs_info::super_copy directly. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] btrfs: use set functions to update latest refs to the SB 2018-02-12 16:34 ` David Sterba @ 2018-02-13 2:56 ` Anand Jain 0 siblings, 0 replies; 12+ messages in thread From: Anand Jain @ 2018-02-13 2:56 UTC (permalink / raw) To: dsterba, linux-btrfs On 02/13/2018 12:34 AM, David Sterba wrote: > On Mon, Feb 12, 2018 at 11:37:49PM +0800, Anand Jain wrote: >> We have btrfs_set_super...() to set the various values, so just use it. > > Sorry but this is not "just use it", as it fixes an endianness bug and > should be documented in the changelog. And there are probably more > instances of the bug while accessing fs_info::super or > fs_info::super_copy directly. Will fix. Also checked rest of the code for similar misses in reading/writing of fs_info::super_copy and fs_info::super_for_commit values, but didn't find any further or did I miss something ? Sent V2. With new title: btrfs: fix endianness compatibility during the SB RW Thanks, Anand ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] btrfs: fix endianness compatibility during the SB RW 2018-02-12 15:37 [PATCH] btrfs: use set functions to update latest refs to the SB Anand Jain 2018-02-12 16:34 ` David Sterba @ 2018-02-13 3:00 ` Anand Jain 2018-02-13 7:04 ` Nikolay Borisov ` (2 more replies) 1 sibling, 3 replies; 12+ messages in thread From: Anand Jain @ 2018-02-13 3:00 UTC (permalink / raw) To: linux-btrfs Fixes the endianness bug in the fs_info::super_copy by using its btrfs_set_super...() function to set values in the SB, as these functions manage the endianness compatibility nicely. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- v1->v2: Update change log. Update $Subject. Old: [PATCH] btrfs: use set functions to update latest refs to the SB fs/btrfs/transaction.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c index 04f07144b45c..9220f004001c 100644 --- a/fs/btrfs/transaction.c +++ b/fs/btrfs/transaction.c @@ -1722,19 +1722,23 @@ static void update_super_roots(struct btrfs_fs_info *fs_info) super = fs_info->super_copy; + /* update latest btrfs_super_block::chunk_root refs */ root_item = &fs_info->chunk_root->root_item; - super->chunk_root = root_item->bytenr; - super->chunk_root_generation = root_item->generation; - super->chunk_root_level = root_item->level; + btrfs_set_super_chunk_root(super, root_item->bytenr); + btrfs_set_super_chunk_root_generation(super, root_item->generation); + btrfs_set_super_chunk_root_level(super, root_item->level); + /* update latest btrfs_super_block::root refs */ root_item = &fs_info->tree_root->root_item; - super->root = root_item->bytenr; - super->generation = root_item->generation; - super->root_level = root_item->level; + btrfs_set_super_root(super, root_item->bytenr); + btrfs_set_super_generation(super, root_item->generation); + btrfs_set_super_root_level(super, root_item->level); + if (btrfs_test_opt(fs_info, SPACE_CACHE)) - super->cache_generation = root_item->generation; + btrfs_set_super_cache_generation(super, root_item->generation); if (test_bit(BTRFS_FS_UPDATE_UUID_TREE_GEN, &fs_info->flags)) - super->uuid_tree_generation = root_item->generation; + btrfs_set_super_uuid_tree_generation(super, + root_item->generation); } int btrfs_transaction_in_commit(struct btrfs_fs_info *info) -- 2.15.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] btrfs: fix endianness compatibility during the SB RW 2018-02-13 3:00 ` [PATCH v2] btrfs: fix endianness compatibility during the SB RW Anand Jain @ 2018-02-13 7:04 ` Nikolay Borisov 2018-02-13 9:01 ` Qu Wenruo 2018-02-20 17:16 ` Liu Bo 2 siblings, 0 replies; 12+ messages in thread From: Nikolay Borisov @ 2018-02-13 7:04 UTC (permalink / raw) To: Anand Jain, linux-btrfs On 13.02.2018 05:00, Anand Jain wrote: > Fixes the endianness bug in the fs_info::super_copy by using its > btrfs_set_super...() function to set values in the SB, as these > functions manage the endianness compatibility nicely. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> Reviewed-by: Nikolay Borisov <nborisov@suse.com> > --- > v1->v2: Update change log. Update $Subject. > Old: > [PATCH] btrfs: use set functions to update latest refs to the SB > fs/btrfs/transaction.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c > index 04f07144b45c..9220f004001c 100644 > --- a/fs/btrfs/transaction.c > +++ b/fs/btrfs/transaction.c > @@ -1722,19 +1722,23 @@ static void update_super_roots(struct btrfs_fs_info *fs_info) > > super = fs_info->super_copy; > > + /* update latest btrfs_super_block::chunk_root refs */ > root_item = &fs_info->chunk_root->root_item; > - super->chunk_root = root_item->bytenr; > - super->chunk_root_generation = root_item->generation; > - super->chunk_root_level = root_item->level; > + btrfs_set_super_chunk_root(super, root_item->bytenr); > + btrfs_set_super_chunk_root_generation(super, root_item->generation); > + btrfs_set_super_chunk_root_level(super, root_item->level); > > + /* update latest btrfs_super_block::root refs */ > root_item = &fs_info->tree_root->root_item; > - super->root = root_item->bytenr; > - super->generation = root_item->generation; > - super->root_level = root_item->level; > + btrfs_set_super_root(super, root_item->bytenr); > + btrfs_set_super_generation(super, root_item->generation); > + btrfs_set_super_root_level(super, root_item->level); > + > if (btrfs_test_opt(fs_info, SPACE_CACHE)) > - super->cache_generation = root_item->generation; > + btrfs_set_super_cache_generation(super, root_item->generation); > if (test_bit(BTRFS_FS_UPDATE_UUID_TREE_GEN, &fs_info->flags)) > - super->uuid_tree_generation = root_item->generation; > + btrfs_set_super_uuid_tree_generation(super, > + root_item->generation); > } > > int btrfs_transaction_in_commit(struct btrfs_fs_info *info) > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] btrfs: fix endianness compatibility during the SB RW 2018-02-13 3:00 ` [PATCH v2] btrfs: fix endianness compatibility during the SB RW Anand Jain 2018-02-13 7:04 ` Nikolay Borisov @ 2018-02-13 9:01 ` Qu Wenruo 2018-02-13 10:27 ` Anand Jain 2018-02-20 17:16 ` Liu Bo 2 siblings, 1 reply; 12+ messages in thread From: Qu Wenruo @ 2018-02-13 9:01 UTC (permalink / raw) To: Anand Jain, linux-btrfs [-- Attachment #1.1: Type: text/plain, Size: 2764 bytes --] On 2018年02月13日 11:00, Anand Jain wrote: > Fixes the endianness bug in the fs_info::super_copy by using its > btrfs_set_super...() function to set values in the SB, as these > functions manage the endianness compatibility nicely. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> Also went through all btrfs_super_block SETGET functions, greping using \><member name>, seems that there are still some left here: fs/btrfs/sysfs.c: In both btrfs_sectorsize_show() and btrfs_clone_alignment_show(): return snprintf(buf, PAGE_SIZE, "%u\n", fs_info->super_copy->sectorsize); In btrfs_nodesize_show(): return snprintf(buf, PAGE_SIZE, "%u\n", fs_info->super_copy->nodesize); And what about cc this to stable kernel? IIRC it's a very critical problem for btrfs. Maybe cc: stable@vger.kernel.org # v3.2+? Thanks, Qu > --- > v1->v2: Update change log. Update $Subject. > Old: > [PATCH] btrfs: use set functions to update latest refs to the SB > fs/btrfs/transaction.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c > index 04f07144b45c..9220f004001c 100644 > --- a/fs/btrfs/transaction.c > +++ b/fs/btrfs/transaction.c > @@ -1722,19 +1722,23 @@ static void update_super_roots(struct btrfs_fs_info *fs_info) > > super = fs_info->super_copy; > > + /* update latest btrfs_super_block::chunk_root refs */ > root_item = &fs_info->chunk_root->root_item; > - super->chunk_root = root_item->bytenr; > - super->chunk_root_generation = root_item->generation; > - super->chunk_root_level = root_item->level; > + btrfs_set_super_chunk_root(super, root_item->bytenr); > + btrfs_set_super_chunk_root_generation(super, root_item->generation); > + btrfs_set_super_chunk_root_level(super, root_item->level); > > + /* update latest btrfs_super_block::root refs */ > root_item = &fs_info->tree_root->root_item; > - super->root = root_item->bytenr; > - super->generation = root_item->generation; > - super->root_level = root_item->level; > + btrfs_set_super_root(super, root_item->bytenr); > + btrfs_set_super_generation(super, root_item->generation); > + btrfs_set_super_root_level(super, root_item->level); > + > if (btrfs_test_opt(fs_info, SPACE_CACHE)) > - super->cache_generation = root_item->generation; > + btrfs_set_super_cache_generation(super, root_item->generation); > if (test_bit(BTRFS_FS_UPDATE_UUID_TREE_GEN, &fs_info->flags)) > - super->uuid_tree_generation = root_item->generation; > + btrfs_set_super_uuid_tree_generation(super, > + root_item->generation); > } > > int btrfs_transaction_in_commit(struct btrfs_fs_info *info) > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 520 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] btrfs: fix endianness compatibility during the SB RW 2018-02-13 9:01 ` Qu Wenruo @ 2018-02-13 10:27 ` Anand Jain 2018-02-13 10:39 ` Qu Wenruo 2018-02-13 17:55 ` David Sterba 0 siblings, 2 replies; 12+ messages in thread From: Anand Jain @ 2018-02-13 10:27 UTC (permalink / raw) To: Qu Wenruo, linux-btrfs On 02/13/2018 05:01 PM, Qu Wenruo wrote: > > > On 2018年02月13日 11:00, Anand Jain wrote: >> Fixes the endianness bug in the fs_info::super_copy by using its >> btrfs_set_super...() function to set values in the SB, as these >> functions manage the endianness compatibility nicely. >> >> Signed-off-by: Anand Jain <anand.jain@oracle.com> > > Also went through all btrfs_super_block SETGET functions, greping using > \><member name>, seems that there are still some left here: > > fs/btrfs/sysfs.c: > In both btrfs_sectorsize_show() and btrfs_clone_alignment_show(): > return snprintf(buf, PAGE_SIZE, "%u\n", > fs_info->super_copy->sectorsize); > > In btrfs_nodesize_show(): > return snprintf(buf, PAGE_SIZE, "%u\n", fs_info->super_copy->nodesize); Oh. Thanks. Will fix. Maybe it's a good idea to add sysfs fixes into a new patch. > And what about cc this to stable kernel? > IIRC it's a very critical problem for btrfs. > > Maybe cc: stable@vger.kernel.org # v3.2+? Thanks for the suggestion. Will do. Any idea what if the patch which applied on mainline ends up conflict on LTS, so write a separate patch to stable? Thanks, Anand > Thanks, > Qu > >> --- >> v1->v2: Update change log. Update $Subject. >> Old: >> [PATCH] btrfs: use set functions to update latest refs to the SB >> fs/btrfs/transaction.c | 20 ++++++++++++-------- >> 1 file changed, 12 insertions(+), 8 deletions(-) >> >> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c >> index 04f07144b45c..9220f004001c 100644 >> --- a/fs/btrfs/transaction.c >> +++ b/fs/btrfs/transaction.c >> @@ -1722,19 +1722,23 @@ static void update_super_roots(struct btrfs_fs_info *fs_info) >> >> super = fs_info->super_copy; >> >> + /* update latest btrfs_super_block::chunk_root refs */ >> root_item = &fs_info->chunk_root->root_item; >> - super->chunk_root = root_item->bytenr; >> - super->chunk_root_generation = root_item->generation; >> - super->chunk_root_level = root_item->level; >> + btrfs_set_super_chunk_root(super, root_item->bytenr); >> + btrfs_set_super_chunk_root_generation(super, root_item->generation); >> + btrfs_set_super_chunk_root_level(super, root_item->level); >> >> + /* update latest btrfs_super_block::root refs */ >> root_item = &fs_info->tree_root->root_item; >> - super->root = root_item->bytenr; >> - super->generation = root_item->generation; >> - super->root_level = root_item->level; >> + btrfs_set_super_root(super, root_item->bytenr); >> + btrfs_set_super_generation(super, root_item->generation); >> + btrfs_set_super_root_level(super, root_item->level); >> + >> if (btrfs_test_opt(fs_info, SPACE_CACHE)) >> - super->cache_generation = root_item->generation; >> + btrfs_set_super_cache_generation(super, root_item->generation); >> if (test_bit(BTRFS_FS_UPDATE_UUID_TREE_GEN, &fs_info->flags)) >> - super->uuid_tree_generation = root_item->generation; >> + btrfs_set_super_uuid_tree_generation(super, >> + root_item->generation); >> } >> >> int btrfs_transaction_in_commit(struct btrfs_fs_info *info) >> > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] btrfs: fix endianness compatibility during the SB RW 2018-02-13 10:27 ` Anand Jain @ 2018-02-13 10:39 ` Qu Wenruo 2018-02-13 17:55 ` David Sterba 1 sibling, 0 replies; 12+ messages in thread From: Qu Wenruo @ 2018-02-13 10:39 UTC (permalink / raw) To: Anand Jain, linux-btrfs [-- Attachment #1.1: Type: text/plain, Size: 3986 bytes --] On 2018年02月13日 18:27, Anand Jain wrote: > > > On 02/13/2018 05:01 PM, Qu Wenruo wrote: >> >> >> On 2018年02月13日 11:00, Anand Jain wrote: >>> Fixes the endianness bug in the fs_info::super_copy by using its >>> btrfs_set_super...() function to set values in the SB, as these >>> functions manage the endianness compatibility nicely. >>> >>> Signed-off-by: Anand Jain <anand.jain@oracle.com> >> >> Also went through all btrfs_super_block SETGET functions, greping using >> \><member name>, seems that there are still some left here: >> >> fs/btrfs/sysfs.c: >> In both btrfs_sectorsize_show() and btrfs_clone_alignment_show(): >> return snprintf(buf, PAGE_SIZE, "%u\n", >> fs_info->super_copy->sectorsize); >> >> In btrfs_nodesize_show(): >> return snprintf(buf, PAGE_SIZE, "%u\n", >> fs_info->super_copy->nodesize); > > Oh. Thanks. Will fix. Maybe it's a good idea to add sysfs fixes > into a new patch. > >> And what about cc this to stable kernel? >> IIRC it's a very critical problem for btrfs. >> >> Maybe cc: stable@vger.kernel.org # v3.2+? > > Thanks for the suggestion. Will do. Any idea what if the patch which > applied on mainline ends up conflict on LTS, so write a separate patch > to stable? In fact I'm not sure about this either. But considering much of the offending code doesn't change since 2009, it wouldn't cause much conflict IIRC. Thanks, Qu > > Thanks, Anand > > >> Thanks, >> Qu >> >>> --- >>> v1->v2: Update change log. Update $Subject. >>> Old: >>> [PATCH] btrfs: use set functions to update latest refs to >>> the SB >>> fs/btrfs/transaction.c | 20 ++++++++++++-------- >>> 1 file changed, 12 insertions(+), 8 deletions(-) >>> >>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c >>> index 04f07144b45c..9220f004001c 100644 >>> --- a/fs/btrfs/transaction.c >>> +++ b/fs/btrfs/transaction.c >>> @@ -1722,19 +1722,23 @@ static void update_super_roots(struct >>> btrfs_fs_info *fs_info) >>> super = fs_info->super_copy; >>> + /* update latest btrfs_super_block::chunk_root refs */ >>> root_item = &fs_info->chunk_root->root_item; >>> - super->chunk_root = root_item->bytenr; >>> - super->chunk_root_generation = root_item->generation; >>> - super->chunk_root_level = root_item->level; >>> + btrfs_set_super_chunk_root(super, root_item->bytenr); >>> + btrfs_set_super_chunk_root_generation(super, >>> root_item->generation); >>> + btrfs_set_super_chunk_root_level(super, root_item->level); >>> + /* update latest btrfs_super_block::root refs */ >>> root_item = &fs_info->tree_root->root_item; >>> - super->root = root_item->bytenr; >>> - super->generation = root_item->generation; >>> - super->root_level = root_item->level; >>> + btrfs_set_super_root(super, root_item->bytenr); >>> + btrfs_set_super_generation(super, root_item->generation); >>> + btrfs_set_super_root_level(super, root_item->level); >>> + >>> if (btrfs_test_opt(fs_info, SPACE_CACHE)) >>> - super->cache_generation = root_item->generation; >>> + btrfs_set_super_cache_generation(super, root_item->generation); >>> if (test_bit(BTRFS_FS_UPDATE_UUID_TREE_GEN, &fs_info->flags)) >>> - super->uuid_tree_generation = root_item->generation; >>> + btrfs_set_super_uuid_tree_generation(super, >>> + root_item->generation); >>> } >>> int btrfs_transaction_in_commit(struct btrfs_fs_info *info) >>> >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 520 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] btrfs: fix endianness compatibility during the SB RW 2018-02-13 10:27 ` Anand Jain 2018-02-13 10:39 ` Qu Wenruo @ 2018-02-13 17:55 ` David Sterba 2018-02-14 14:53 ` Anand Jain 1 sibling, 1 reply; 12+ messages in thread From: David Sterba @ 2018-02-13 17:55 UTC (permalink / raw) To: Anand Jain; +Cc: Qu Wenruo, linux-btrfs On Tue, Feb 13, 2018 at 06:27:13PM +0800, Anand Jain wrote: > On 02/13/2018 05:01 PM, Qu Wenruo wrote: > > On 2018年02月13日 11:00, Anand Jain wrote: > >> Fixes the endianness bug in the fs_info::super_copy by using its > >> btrfs_set_super...() function to set values in the SB, as these > >> functions manage the endianness compatibility nicely. > >> > >> Signed-off-by: Anand Jain <anand.jain@oracle.com> > > > > Also went through all btrfs_super_block SETGET functions, greping using > > \><member name>, seems that there are still some left here: > > > > fs/btrfs/sysfs.c: > > In both btrfs_sectorsize_show() and btrfs_clone_alignment_show(): > > return snprintf(buf, PAGE_SIZE, "%u\n", > > fs_info->super_copy->sectorsize); > > > > In btrfs_nodesize_show(): > > return snprintf(buf, PAGE_SIZE, "%u\n", fs_info->super_copy->nodesize); > > Oh. Thanks. Will fix. Maybe it's a good idea to add sysfs fixes > into a new patch. I'd prefer a single patch as it fixes the same problem for one structure, the context of use is not that important to justify 2 patches. I went through the possible uses of superblock again and did not find anything else than the update_super_roots and sysfs read handlers. There are some direct uses of super block members, like label, sys_array, uuid that are passed unconverted and must be accessed via the set/get helpers. In some places the superblock is put to a temporary variable so simple grep may miss these. > > And what about cc this to stable kernel? > > IIRC it's a very critical problem for btrfs. If the filesystem is always used on a same endianity host, this will not be a problem. Moving between opposite endianity hosts will report bogus numbers in sysfs and the backup root would not be restored correctly. As this is not common, I'd rate thats as a bugfix for stable, but "only" a serious one. > > Maybe cc: stable@vger.kernel.org # v3.2+? > > Thanks for the suggestion. Will do. Any idea what if the patch which > applied on mainline ends up conflict on LTS, so write a separate patch > to stable? If the patch does not apply to some older stable branch, all involved people get a mail from stable team and have an opportunity to send an updated version of the patch. Regarding the long-term branches, I would consider 4.4 and up. Anything older is a plus but fixing merge conflicts is more likely there so I think the respective maintainers would either fix it by themselves or ask for help. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] btrfs: fix endianness compatibility during the SB RW 2018-02-13 17:55 ` David Sterba @ 2018-02-14 14:53 ` Anand Jain 2018-02-15 16:49 ` David Sterba 0 siblings, 1 reply; 12+ messages in thread From: Anand Jain @ 2018-02-14 14:53 UTC (permalink / raw) To: dsterba, Qu Wenruo, linux-btrfs On 02/14/2018 01:55 AM, David Sterba wrote: > On Tue, Feb 13, 2018 at 06:27:13PM +0800, Anand Jain wrote: >> On 02/13/2018 05:01 PM, Qu Wenruo wrote: >>> On 2018年02月13日 11:00, Anand Jain wrote: >>>> Fixes the endianness bug in the fs_info::super_copy by using its >>>> btrfs_set_super...() function to set values in the SB, as these >>>> functions manage the endianness compatibility nicely. >>>> >>>> Signed-off-by: Anand Jain <anand.jain@oracle.com> >>> >>> Also went through all btrfs_super_block SETGET functions, greping using >>> \><member name>, seems that there are still some left here: >>> >>> fs/btrfs/sysfs.c: >>> In both btrfs_sectorsize_show() and btrfs_clone_alignment_show(): >>> return snprintf(buf, PAGE_SIZE, "%u\n", >>> fs_info->super_copy->sectorsize); >>> >>> In btrfs_nodesize_show(): >>> return snprintf(buf, PAGE_SIZE, "%u\n", fs_info->super_copy->nodesize); Thinking again on the printf they are fine without the endianness functions, because it is printing the values from the memory to buf/stdout, which was previously read from the disk (which possibly written by the opposite endianness system). Here, there is no endianness changes that will be required for it to be written out using printf. >> Oh. Thanks. Will fix. Maybe it's a good idea to add sysfs fixes >> into a new patch. > I'd prefer a single patch as it fixes the same problem for one > structure, the context of use is not that important to justify 2 > patches. Agree. > I went through the possible uses of superblock again and did not find > anything else than the update_super_roots and sysfs read handlers. There > are some direct uses of super block members, like label, sys_array, uuid > that are passed unconverted and must be accessed via the set/get > helpers. AFAIK byte arrays btrfs_super_block::(label, sys_chunk_array, fsid) are not affected by endianness, as memory access for the array starts from the zero in both endianness. And the lowest granularity affected by the endianness is a byte. > In some places the superblock is put to a temporary variable so simple > grep may miss these. > >>> And what about cc this to stable kernel? >>> IIRC it's a very critical problem for btrfs. > > If the filesystem is always used on a same endianity host, this will not > be a problem. Moving between opposite endianity hosts will report bogus > numbers in sysfs and the backup root would not be restored correctly. > > As this is not common, I'd rate thats as a bugfix for stable, but "only" > a serious one. agree. >>> Maybe cc: stable@vger.kernel.org # v3.2+? >> >> Thanks for the suggestion. Will do. Any idea what if the patch which >> applied on mainline ends up conflict on LTS, so write a separate patch >> to stable? > > If the patch does not apply to some older stable branch, all involved > people get a mail from stable team and have an opportunity to send an > updated version of the patch. > > Regarding the long-term branches, I would consider 4.4 and up. Anything > older is a plus but fixing merge conflicts is more likely there so I > think the respective maintainers would either fix it by themselves or > ask for help. ok. Thanks, Anand > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] btrfs: fix endianness compatibility during the SB RW 2018-02-14 14:53 ` Anand Jain @ 2018-02-15 16:49 ` David Sterba 0 siblings, 0 replies; 12+ messages in thread From: David Sterba @ 2018-02-15 16:49 UTC (permalink / raw) To: Anand Jain; +Cc: dsterba, Qu Wenruo, linux-btrfs On Wed, Feb 14, 2018 at 10:53:19PM +0800, Anand Jain wrote: > > > On 02/14/2018 01:55 AM, David Sterba wrote: > > On Tue, Feb 13, 2018 at 06:27:13PM +0800, Anand Jain wrote: > >> On 02/13/2018 05:01 PM, Qu Wenruo wrote: > >>> On 2018年02月13日 11:00, Anand Jain wrote: > >>>> Fixes the endianness bug in the fs_info::super_copy by using its > >>>> btrfs_set_super...() function to set values in the SB, as these > >>>> functions manage the endianness compatibility nicely. > >>>> > >>>> Signed-off-by: Anand Jain <anand.jain@oracle.com> > >>> > >>> Also went through all btrfs_super_block SETGET functions, greping using > >>> \><member name>, seems that there are still some left here: > >>> > >>> fs/btrfs/sysfs.c: > >>> In both btrfs_sectorsize_show() and btrfs_clone_alignment_show(): > >>> return snprintf(buf, PAGE_SIZE, "%u\n", > >>> fs_info->super_copy->sectorsize); > >>> > >>> In btrfs_nodesize_show(): > >>> return snprintf(buf, PAGE_SIZE, "%u\n", fs_info->super_copy->nodesize); > > Thinking again on the printf they are fine without the endianness > functions, because it is printing the values from the memory to > buf/stdout, which was previously read from the disk (which > possibly written by the opposite endianness system). Here, there > is no endianness changes that will be required for it to be written > out using printf. We need to fix the value for printf for the same reason this patch was needed: https://git.kernel.org/linus/bea7eafdbda3ba1d4b2ccb9cca829eefb7989bb9 and we can actually use the fs_info-> copies here too. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] btrfs: fix endianness compatibility during the SB RW 2018-02-13 3:00 ` [PATCH v2] btrfs: fix endianness compatibility during the SB RW Anand Jain 2018-02-13 7:04 ` Nikolay Borisov 2018-02-13 9:01 ` Qu Wenruo @ 2018-02-20 17:16 ` Liu Bo 2 siblings, 0 replies; 12+ messages in thread From: Liu Bo @ 2018-02-20 17:16 UTC (permalink / raw) To: Anand Jain; +Cc: linux-btrfs On Tue, Feb 13, 2018 at 11:00:46AM +0800, Anand Jain wrote: > Fixes the endianness bug in the fs_info::super_copy by using its > btrfs_set_super...() function to set values in the SB, as these > functions manage the endianness compatibility nicely. Reviewed-by: Liu Bo <bo.li.liu@oracle.com> Thanks, -liubo > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > --- > v1->v2: Update change log. Update $Subject. > Old: > [PATCH] btrfs: use set functions to update latest refs to the SB > fs/btrfs/transaction.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c > index 04f07144b45c..9220f004001c 100644 > --- a/fs/btrfs/transaction.c > +++ b/fs/btrfs/transaction.c > @@ -1722,19 +1722,23 @@ static void update_super_roots(struct btrfs_fs_info *fs_info) > > super = fs_info->super_copy; > > + /* update latest btrfs_super_block::chunk_root refs */ > root_item = &fs_info->chunk_root->root_item; > - super->chunk_root = root_item->bytenr; > - super->chunk_root_generation = root_item->generation; > - super->chunk_root_level = root_item->level; > + btrfs_set_super_chunk_root(super, root_item->bytenr); > + btrfs_set_super_chunk_root_generation(super, root_item->generation); > + btrfs_set_super_chunk_root_level(super, root_item->level); > > + /* update latest btrfs_super_block::root refs */ > root_item = &fs_info->tree_root->root_item; > - super->root = root_item->bytenr; > - super->generation = root_item->generation; > - super->root_level = root_item->level; > + btrfs_set_super_root(super, root_item->bytenr); > + btrfs_set_super_generation(super, root_item->generation); > + btrfs_set_super_root_level(super, root_item->level); > + > if (btrfs_test_opt(fs_info, SPACE_CACHE)) > - super->cache_generation = root_item->generation; > + btrfs_set_super_cache_generation(super, root_item->generation); > if (test_bit(BTRFS_FS_UPDATE_UUID_TREE_GEN, &fs_info->flags)) > - super->uuid_tree_generation = root_item->generation; > + btrfs_set_super_uuid_tree_generation(super, > + root_item->generation); > } > > int btrfs_transaction_in_commit(struct btrfs_fs_info *info) > -- > 2.15.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-02-20 18:19 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-12 15:37 [PATCH] btrfs: use set functions to update latest refs to the SB Anand Jain 2018-02-12 16:34 ` David Sterba 2018-02-13 2:56 ` Anand Jain 2018-02-13 3:00 ` [PATCH v2] btrfs: fix endianness compatibility during the SB RW Anand Jain 2018-02-13 7:04 ` Nikolay Borisov 2018-02-13 9:01 ` Qu Wenruo 2018-02-13 10:27 ` Anand Jain 2018-02-13 10:39 ` Qu Wenruo 2018-02-13 17:55 ` David Sterba 2018-02-14 14:53 ` Anand Jain 2018-02-15 16:49 ` David Sterba 2018-02-20 17:16 ` Liu Bo
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.