| Message ID | 20200915053532.63279-15-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: add read-only support for subpage sector size | expand |
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
On Tue, Sep 15, 2020 at 01:35:27PM +0800, Qu Wenruo wrote: > Btree inode is pretty special compared to all other inode extent io > tree, although it has a btrfs inode, it doesn't have the track_uptodate > bit at all. > > This means a lot of things like extent locking doesn't even need to be > applied to btree io tree. > > Since it's so special, adds a new owner value for it to make debuging a > little easier. > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > fs/btrfs/disk-io.c | 2 +- > fs/btrfs/extent-io-tree.h | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index 1ba16951ccaa..82a841bd0702 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -2126,7 +2126,7 @@ static void btrfs_init_btree_inode(struct btrfs_fs_info *fs_info) > > RB_CLEAR_NODE(&BTRFS_I(inode)->rb_node); > extent_io_tree_init(fs_info, &BTRFS_I(inode)->io_tree, > - IO_TREE_INODE_IO, inode); > + IO_TREE_BTREE_IO, inode); This looks like an independent patch, so it could be taken separately.
On 2020/9/17 上午12:06, David Sterba wrote: > On Tue, Sep 15, 2020 at 01:35:27PM +0800, Qu Wenruo wrote: >> Btree inode is pretty special compared to all other inode extent io >> tree, although it has a btrfs inode, it doesn't have the track_uptodate >> bit at all. >> >> This means a lot of things like extent locking doesn't even need to be >> applied to btree io tree. >> >> Since it's so special, adds a new owner value for it to make debuging a >> little easier. >> >> Signed-off-by: Qu Wenruo <wqu@suse.com> >> --- >> fs/btrfs/disk-io.c | 2 +- >> fs/btrfs/extent-io-tree.h | 1 + >> 2 files changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c >> index 1ba16951ccaa..82a841bd0702 100644 >> --- a/fs/btrfs/disk-io.c >> +++ b/fs/btrfs/disk-io.c >> @@ -2126,7 +2126,7 @@ static void btrfs_init_btree_inode(struct btrfs_fs_info *fs_info) >> >> RB_CLEAR_NODE(&BTRFS_I(inode)->rb_node); >> extent_io_tree_init(fs_info, &BTRFS_I(inode)->io_tree, >> - IO_TREE_INODE_IO, inode); >> + IO_TREE_BTREE_IO, inode); > > This looks like an independent patch, so it could be taken separately. > Errr, why? We added a new owner for btree inode io tree, and utilize that new owner in the same patch looks completely sane to me. Or did I miss something? Thanks, Qu
On Thu, Sep 17, 2020 at 08:02:05AM +0800, Qu Wenruo wrote: > > > On 2020/9/17 上午12:06, David Sterba wrote: > > On Tue, Sep 15, 2020 at 01:35:27PM +0800, Qu Wenruo wrote: > >> Btree inode is pretty special compared to all other inode extent io > >> tree, although it has a btrfs inode, it doesn't have the track_uptodate > >> bit at all. > >> > >> This means a lot of things like extent locking doesn't even need to be > >> applied to btree io tree. > >> > >> Since it's so special, adds a new owner value for it to make debuging a > >> little easier. > >> > >> Signed-off-by: Qu Wenruo <wqu@suse.com> > >> --- > >> fs/btrfs/disk-io.c | 2 +- > >> fs/btrfs/extent-io-tree.h | 1 + > >> 2 files changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > >> index 1ba16951ccaa..82a841bd0702 100644 > >> --- a/fs/btrfs/disk-io.c > >> +++ b/fs/btrfs/disk-io.c > >> @@ -2126,7 +2126,7 @@ static void btrfs_init_btree_inode(struct btrfs_fs_info *fs_info) > >> > >> RB_CLEAR_NODE(&BTRFS_I(inode)->rb_node); > >> extent_io_tree_init(fs_info, &BTRFS_I(inode)->io_tree, > >> - IO_TREE_INODE_IO, inode); > >> + IO_TREE_BTREE_IO, inode); > > > > This looks like an independent patch, so it could be taken separately. > > > Errr, why? > > We added a new owner for btree inode io tree, and utilize that new owner > in the same patch looks completely sane to me. > > Or did I miss something? The IO_TREE_* ids are only for debugging and IO_TREE_INODE_IO is supposed to be used for data inodes. But the btree_inode has that too, which does not seem to follow the purpose of the tree id, you fix that in this patch and it applies to current code too.
On 2020/9/17 下午8:50, David Sterba wrote: > On Thu, Sep 17, 2020 at 08:02:05AM +0800, Qu Wenruo wrote: >> >> >> On 2020/9/17 上午12:06, David Sterba wrote: >>> On Tue, Sep 15, 2020 at 01:35:27PM +0800, Qu Wenruo wrote: >>>> Btree inode is pretty special compared to all other inode extent io >>>> tree, although it has a btrfs inode, it doesn't have the track_uptodate >>>> bit at all. >>>> >>>> This means a lot of things like extent locking doesn't even need to be >>>> applied to btree io tree. >>>> >>>> Since it's so special, adds a new owner value for it to make debuging a >>>> little easier. >>>> >>>> Signed-off-by: Qu Wenruo <wqu@suse.com> >>>> --- >>>> fs/btrfs/disk-io.c | 2 +- >>>> fs/btrfs/extent-io-tree.h | 1 + >>>> 2 files changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c >>>> index 1ba16951ccaa..82a841bd0702 100644 >>>> --- a/fs/btrfs/disk-io.c >>>> +++ b/fs/btrfs/disk-io.c >>>> @@ -2126,7 +2126,7 @@ static void btrfs_init_btree_inode(struct btrfs_fs_info *fs_info) >>>> >>>> RB_CLEAR_NODE(&BTRFS_I(inode)->rb_node); >>>> extent_io_tree_init(fs_info, &BTRFS_I(inode)->io_tree, >>>> - IO_TREE_INODE_IO, inode); >>>> + IO_TREE_BTREE_IO, inode); >>> >>> This looks like an independent patch, so it could be taken separately. >>> >> Errr, why? >> >> We added a new owner for btree inode io tree, and utilize that new owner >> in the same patch looks completely sane to me. >> >> Or did I miss something? > > The IO_TREE_* ids are only for debugging and IO_TREE_INODE_IO is > supposed to be used for data inodes. But the btree_inode has that too, > which does not seem to follow the purpose of the tree id, you fix that > in this patch and it applies to current code too. > Oh, I didn't see it as a fix. But your point still stands, and makes sense. And this patch would be the base stone for later btree io tree specific re-mapping bits. So if this patch can be applied to current code, it would only be a good thing. Thanks, Qu
On Fri, Sep 18, 2020 at 04:18:27PM +0800, Qu Wenruo wrote: > >>> This looks like an independent patch, so it could be taken separately. > >>> > >> Errr, why? > >> > >> We added a new owner for btree inode io tree, and utilize that new owner > >> in the same patch looks completely sane to me. > >> > >> Or did I miss something? > > > > The IO_TREE_* ids are only for debugging and IO_TREE_INODE_IO is > > supposed to be used for data inodes. But the btree_inode has that too, > > which does not seem to follow the purpose of the tree id, you fix that > > in this patch and it applies to current code too. > > > Oh, I didn't see it as a fix. But your point still stands, and makes sense. > > And this patch would be the base stone for later btree io tree specific > re-mapping bits. > So if this patch can be applied to current code, it would only be a good > thing. Thanks for confirming, I'll add it to misc-next.
On Tue, Sep 15, 2020 at 01:35:27PM +0800, Qu Wenruo wrote: > Btree inode is pretty special compared to all other inode extent io > tree, although it has a btrfs inode, it doesn't have the track_uptodate > bit at all. > > This means a lot of things like extent locking doesn't even need to be > applied to btree io tree. > > Since it's so special, adds a new owner value for it to make debuging a > little easier. > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > fs/btrfs/disk-io.c | 2 +- > fs/btrfs/extent-io-tree.h | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index 1ba16951ccaa..82a841bd0702 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -2126,7 +2126,7 @@ static void btrfs_init_btree_inode(struct btrfs_fs_info *fs_info) > > RB_CLEAR_NODE(&BTRFS_I(inode)->rb_node); > extent_io_tree_init(fs_info, &BTRFS_I(inode)->io_tree, > - IO_TREE_INODE_IO, inode); > + IO_TREE_BTREE_IO, inode); > BTRFS_I(inode)->io_tree.track_uptodate = false; > extent_map_tree_init(&BTRFS_I(inode)->extent_tree); > > diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h > index 219a09a2b734..21d128383bfd 100644 > --- a/fs/btrfs/extent-io-tree.h > +++ b/fs/btrfs/extent-io-tree.h > @@ -40,6 +40,7 @@ struct io_failure_record; > enum { > IO_TREE_FS_PINNED_EXTENTS, > IO_TREE_FS_EXCLUDED_EXTENTS, > + IO_TREE_BTREE_IO, I've renamed it to IO_TREE_BTREE_INODE_IO, and btw don't forget to check if enums aren't exported to tracepoints. This is and needs to be added there as well. > IO_TREE_INODE_IO, > IO_TREE_INODE_IO_FAILURE, > IO_TREE_RELOC_BLOCKS, > -- > 2.28.0
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 1ba16951ccaa..82a841bd0702 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -2126,7 +2126,7 @@ static void btrfs_init_btree_inode(struct btrfs_fs_info *fs_info) RB_CLEAR_NODE(&BTRFS_I(inode)->rb_node); extent_io_tree_init(fs_info, &BTRFS_I(inode)->io_tree, - IO_TREE_INODE_IO, inode); + IO_TREE_BTREE_IO, inode); BTRFS_I(inode)->io_tree.track_uptodate = false; extent_map_tree_init(&BTRFS_I(inode)->extent_tree); diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h index 219a09a2b734..21d128383bfd 100644 --- a/fs/btrfs/extent-io-tree.h +++ b/fs/btrfs/extent-io-tree.h @@ -40,6 +40,7 @@ struct io_failure_record; enum { IO_TREE_FS_PINNED_EXTENTS, IO_TREE_FS_EXCLUDED_EXTENTS, + IO_TREE_BTREE_IO, IO_TREE_INODE_IO, IO_TREE_INODE_IO_FAILURE, IO_TREE_RELOC_BLOCKS,
Btree inode is pretty special compared to all other inode extent io tree, although it has a btrfs inode, it doesn't have the track_uptodate bit at all. This means a lot of things like extent locking doesn't even need to be applied to btree io tree. Since it's so special, adds a new owner value for it to make debuging a little easier. Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/disk-io.c | 2 +- fs/btrfs/extent-io-tree.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)