* [PATCH] btrfs: Use struct inode* to replace extent_io_tree::private_data
@ 2019-02-25 5:50 Qu Wenruo
2019-02-25 10:00 ` Filipe Manana
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Qu Wenruo @ 2019-02-25 5:50 UTC (permalink / raw)
To: linux-btrfs
All users of extent_io_tree::private_data are expecting struct inode*.
So just use struct inode* to replace extent_io_tree::private_data, and
this should provide better type check.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/extent_io.c | 36 +++++++++++++++++-------------------
fs/btrfs/extent_io.h | 4 ++--
fs/btrfs/inode.c | 2 +-
3 files changed, 20 insertions(+), 22 deletions(-)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index e0a96f74e81e..2fd78b10830b 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -89,7 +89,7 @@ void btrfs_leak_debug_check(void)
static inline void __btrfs_debug_check_extent_io_range(const char *caller,
struct extent_io_tree *tree, u64 start, u64 end)
{
- struct inode *inode = tree->private_data;
+ struct inode *inode = tree->inode;
u64 isize;
if (!inode || !is_data_inode(inode))
@@ -201,13 +201,13 @@ void __cold extent_io_exit(void)
}
void extent_io_tree_init(struct extent_io_tree *tree,
- void *private_data)
+ struct inode *inode)
{
tree->state = RB_ROOT;
tree->ops = NULL;
tree->dirty_bytes = 0;
spin_lock_init(&tree->lock);
- tree->private_data = private_data;
+ tree->inode = inode;
}
static struct extent_state *alloc_extent_state(gfp_t mask)
@@ -376,9 +376,8 @@ static void merge_state(struct extent_io_tree *tree,
other = rb_entry(other_node, struct extent_state, rb_node);
if (other->end == state->start - 1 &&
other->state == state->state) {
- if (tree->private_data &&
- is_data_inode(tree->private_data))
- btrfs_merge_delalloc_extent(tree->private_data,
+ if (tree->inode && is_data_inode(tree->inode))
+ btrfs_merge_delalloc_extent(tree->inode,
state, other);
state->start = other->start;
rb_erase(&other->rb_node, &tree->state);
@@ -391,9 +390,8 @@ static void merge_state(struct extent_io_tree *tree,
other = rb_entry(other_node, struct extent_state, rb_node);
if (other->start == state->end + 1 &&
other->state == state->state) {
- if (tree->private_data &&
- is_data_inode(tree->private_data))
- btrfs_merge_delalloc_extent(tree->private_data,
+ if (tree->inode && is_data_inode(tree->inode))
+ btrfs_merge_delalloc_extent(tree->inode,
state, other);
state->end = other->end;
rb_erase(&other->rb_node, &tree->state);
@@ -464,8 +462,8 @@ static int split_state(struct extent_io_tree *tree, struct extent_state *orig,
{
struct rb_node *node;
- if (tree->private_data && is_data_inode(tree->private_data))
- btrfs_split_delalloc_extent(tree->private_data, orig, split);
+ if (tree->inode && is_data_inode(tree->inode))
+ btrfs_split_delalloc_extent(tree->inode, orig, split);
prealloc->start = orig->start;
prealloc->end = split - 1;
@@ -512,8 +510,8 @@ static struct extent_state *clear_state_bit(struct extent_io_tree *tree,
tree->dirty_bytes -= range;
}
- if (tree->private_data && is_data_inode(tree->private_data))
- btrfs_clear_delalloc_extent(tree->private_data, state, bits);
+ if (tree->inode && is_data_inode(tree->inode))
+ btrfs_clear_delalloc_extent(tree->inode, state, bits);
ret = add_extent_changeset(state, bits_to_clear, changeset, 0);
BUG_ON(ret < 0);
@@ -547,7 +545,7 @@ alloc_extent_state_atomic(struct extent_state *prealloc)
static void extent_io_tree_panic(struct extent_io_tree *tree, int err)
{
- struct inode *inode = tree->private_data;
+ struct inode *inode = tree->inode;
btrfs_panic(btrfs_sb(inode->i_sb), err,
"locking error: extent tree was modified by another thread while locked");
@@ -791,8 +789,8 @@ static void set_state_bits(struct extent_io_tree *tree,
unsigned bits_to_set = *bits & ~EXTENT_CTLBITS;
int ret;
- if (tree->private_data && is_data_inode(tree->private_data))
- btrfs_set_delalloc_extent(tree->private_data, state, bits);
+ if (tree->inode && is_data_inode(tree->inode))
+ btrfs_set_delalloc_extent(tree->inode, state, bits);
if ((bits_to_set & EXTENT_DIRTY) && !(state->state & EXTENT_DIRTY)) {
u64 range = state->end - state->start + 1;
@@ -2378,8 +2376,8 @@ static int bio_readpage_error(struct bio *failed_bio, u64 phy_offset,
"Repair Read Error: submitting new read[%#x] to this_mirror=%d, in_validation=%d",
read_mode, failrec->this_mirror, failrec->in_validation);
- status = tree->ops->submit_bio_hook(tree->private_data, bio, failrec->this_mirror,
- failrec->bio_flags, 0);
+ status = tree->ops->submit_bio_hook(tree->inode, bio,
+ failrec->this_mirror, failrec->bio_flags, 0);
if (status) {
free_io_failure(failure_tree, tree, failrec);
bio_put(bio);
@@ -2706,7 +2704,7 @@ static int __must_check submit_one_bio(struct bio *bio, int mirror_num,
bio->bi_private = NULL;
if (tree->ops)
- ret = tree->ops->submit_bio_hook(tree->private_data, bio,
+ ret = tree->ops->submit_bio_hook(tree->inode, bio,
mirror_num, bio_flags, start);
else
btrfsic_submit_bio(bio);
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 9673be3f3d1f..b656d65bca83 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -109,7 +109,7 @@ struct extent_io_ops {
struct extent_io_tree {
struct rb_root state;
- void *private_data;
+ struct inode *inode;
u64 dirty_bytes;
int track_uptodate;
spinlock_t lock;
@@ -240,7 +240,7 @@ typedef struct extent_map *(get_extent_t)(struct btrfs_inode *inode,
u64 start, u64 len,
int create);
-void extent_io_tree_init(struct extent_io_tree *tree, void *private_data);
+void extent_io_tree_init(struct extent_io_tree *tree, struct inode *inode);
int try_release_extent_mapping(struct page *page, gfp_t mask);
int try_release_extent_buffer(struct page *page);
int lock_extent_bits(struct extent_io_tree *tree, u64 start, u64 end,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 5c349667c761..4fc82f582fa5 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -10403,7 +10403,7 @@ static int btrfs_tmpfile(struct inode *dir, struct dentry *dentry, umode_t mode)
void btrfs_set_range_writeback(struct extent_io_tree *tree, u64 start, u64 end)
{
- struct inode *inode = tree->private_data;
+ struct inode *inode = tree->inode;
unsigned long index = start >> PAGE_SHIFT;
unsigned long end_index = end >> PAGE_SHIFT;
struct page *page;
--
2.20.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] btrfs: Use struct inode* to replace extent_io_tree::private_data
2019-02-25 5:50 [PATCH] btrfs: Use struct inode* to replace extent_io_tree::private_data Qu Wenruo
@ 2019-02-25 10:00 ` Filipe Manana
2019-02-25 11:10 ` Johannes Thumshirn
2019-02-25 12:15 ` David Sterba
2 siblings, 0 replies; 9+ messages in thread
From: Filipe Manana @ 2019-02-25 10:00 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Mon, Feb 25, 2019 at 5:53 AM Qu Wenruo <wqu@suse.com> wrote:
>
> All users of extent_io_tree::private_data are expecting struct inode*.
> So just use struct inode* to replace extent_io_tree::private_data, and
> this should provide better type check.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Looks good.
> ---
> fs/btrfs/extent_io.c | 36 +++++++++++++++++-------------------
> fs/btrfs/extent_io.h | 4 ++--
> fs/btrfs/inode.c | 2 +-
> 3 files changed, 20 insertions(+), 22 deletions(-)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index e0a96f74e81e..2fd78b10830b 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -89,7 +89,7 @@ void btrfs_leak_debug_check(void)
> static inline void __btrfs_debug_check_extent_io_range(const char *caller,
> struct extent_io_tree *tree, u64 start, u64 end)
> {
> - struct inode *inode = tree->private_data;
> + struct inode *inode = tree->inode;
> u64 isize;
>
> if (!inode || !is_data_inode(inode))
> @@ -201,13 +201,13 @@ void __cold extent_io_exit(void)
> }
>
> void extent_io_tree_init(struct extent_io_tree *tree,
> - void *private_data)
> + struct inode *inode)
> {
> tree->state = RB_ROOT;
> tree->ops = NULL;
> tree->dirty_bytes = 0;
> spin_lock_init(&tree->lock);
> - tree->private_data = private_data;
> + tree->inode = inode;
> }
>
> static struct extent_state *alloc_extent_state(gfp_t mask)
> @@ -376,9 +376,8 @@ static void merge_state(struct extent_io_tree *tree,
> other = rb_entry(other_node, struct extent_state, rb_node);
> if (other->end == state->start - 1 &&
> other->state == state->state) {
> - if (tree->private_data &&
> - is_data_inode(tree->private_data))
> - btrfs_merge_delalloc_extent(tree->private_data,
> + if (tree->inode && is_data_inode(tree->inode))
> + btrfs_merge_delalloc_extent(tree->inode,
> state, other);
> state->start = other->start;
> rb_erase(&other->rb_node, &tree->state);
> @@ -391,9 +390,8 @@ static void merge_state(struct extent_io_tree *tree,
> other = rb_entry(other_node, struct extent_state, rb_node);
> if (other->start == state->end + 1 &&
> other->state == state->state) {
> - if (tree->private_data &&
> - is_data_inode(tree->private_data))
> - btrfs_merge_delalloc_extent(tree->private_data,
> + if (tree->inode && is_data_inode(tree->inode))
> + btrfs_merge_delalloc_extent(tree->inode,
> state, other);
> state->end = other->end;
> rb_erase(&other->rb_node, &tree->state);
> @@ -464,8 +462,8 @@ static int split_state(struct extent_io_tree *tree, struct extent_state *orig,
> {
> struct rb_node *node;
>
> - if (tree->private_data && is_data_inode(tree->private_data))
> - btrfs_split_delalloc_extent(tree->private_data, orig, split);
> + if (tree->inode && is_data_inode(tree->inode))
> + btrfs_split_delalloc_extent(tree->inode, orig, split);
>
> prealloc->start = orig->start;
> prealloc->end = split - 1;
> @@ -512,8 +510,8 @@ static struct extent_state *clear_state_bit(struct extent_io_tree *tree,
> tree->dirty_bytes -= range;
> }
>
> - if (tree->private_data && is_data_inode(tree->private_data))
> - btrfs_clear_delalloc_extent(tree->private_data, state, bits);
> + if (tree->inode && is_data_inode(tree->inode))
> + btrfs_clear_delalloc_extent(tree->inode, state, bits);
>
> ret = add_extent_changeset(state, bits_to_clear, changeset, 0);
> BUG_ON(ret < 0);
> @@ -547,7 +545,7 @@ alloc_extent_state_atomic(struct extent_state *prealloc)
>
> static void extent_io_tree_panic(struct extent_io_tree *tree, int err)
> {
> - struct inode *inode = tree->private_data;
> + struct inode *inode = tree->inode;
>
> btrfs_panic(btrfs_sb(inode->i_sb), err,
> "locking error: extent tree was modified by another thread while locked");
> @@ -791,8 +789,8 @@ static void set_state_bits(struct extent_io_tree *tree,
> unsigned bits_to_set = *bits & ~EXTENT_CTLBITS;
> int ret;
>
> - if (tree->private_data && is_data_inode(tree->private_data))
> - btrfs_set_delalloc_extent(tree->private_data, state, bits);
> + if (tree->inode && is_data_inode(tree->inode))
> + btrfs_set_delalloc_extent(tree->inode, state, bits);
>
> if ((bits_to_set & EXTENT_DIRTY) && !(state->state & EXTENT_DIRTY)) {
> u64 range = state->end - state->start + 1;
> @@ -2378,8 +2376,8 @@ static int bio_readpage_error(struct bio *failed_bio, u64 phy_offset,
> "Repair Read Error: submitting new read[%#x] to this_mirror=%d, in_validation=%d",
> read_mode, failrec->this_mirror, failrec->in_validation);
>
> - status = tree->ops->submit_bio_hook(tree->private_data, bio, failrec->this_mirror,
> - failrec->bio_flags, 0);
> + status = tree->ops->submit_bio_hook(tree->inode, bio,
> + failrec->this_mirror, failrec->bio_flags, 0);
> if (status) {
> free_io_failure(failure_tree, tree, failrec);
> bio_put(bio);
> @@ -2706,7 +2704,7 @@ static int __must_check submit_one_bio(struct bio *bio, int mirror_num,
> bio->bi_private = NULL;
>
> if (tree->ops)
> - ret = tree->ops->submit_bio_hook(tree->private_data, bio,
> + ret = tree->ops->submit_bio_hook(tree->inode, bio,
> mirror_num, bio_flags, start);
> else
> btrfsic_submit_bio(bio);
> diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
> index 9673be3f3d1f..b656d65bca83 100644
> --- a/fs/btrfs/extent_io.h
> +++ b/fs/btrfs/extent_io.h
> @@ -109,7 +109,7 @@ struct extent_io_ops {
>
> struct extent_io_tree {
> struct rb_root state;
> - void *private_data;
> + struct inode *inode;
> u64 dirty_bytes;
> int track_uptodate;
> spinlock_t lock;
> @@ -240,7 +240,7 @@ typedef struct extent_map *(get_extent_t)(struct btrfs_inode *inode,
> u64 start, u64 len,
> int create);
>
> -void extent_io_tree_init(struct extent_io_tree *tree, void *private_data);
> +void extent_io_tree_init(struct extent_io_tree *tree, struct inode *inode);
> int try_release_extent_mapping(struct page *page, gfp_t mask);
> int try_release_extent_buffer(struct page *page);
> int lock_extent_bits(struct extent_io_tree *tree, u64 start, u64 end,
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 5c349667c761..4fc82f582fa5 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -10403,7 +10403,7 @@ static int btrfs_tmpfile(struct inode *dir, struct dentry *dentry, umode_t mode)
>
> void btrfs_set_range_writeback(struct extent_io_tree *tree, u64 start, u64 end)
> {
> - struct inode *inode = tree->private_data;
> + struct inode *inode = tree->inode;
> unsigned long index = start >> PAGE_SHIFT;
> unsigned long end_index = end >> PAGE_SHIFT;
> struct page *page;
> --
> 2.20.1
>
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] btrfs: Use struct inode* to replace extent_io_tree::private_data
2019-02-25 5:50 [PATCH] btrfs: Use struct inode* to replace extent_io_tree::private_data Qu Wenruo
2019-02-25 10:00 ` Filipe Manana
@ 2019-02-25 11:10 ` Johannes Thumshirn
2019-02-25 12:15 ` David Sterba
2 siblings, 0 replies; 9+ messages in thread
From: Johannes Thumshirn @ 2019-02-25 11:10 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
Johannes Thumshirn SUSE Labs Filesystems
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] btrfs: Use struct inode* to replace extent_io_tree::private_data
2019-02-25 5:50 [PATCH] btrfs: Use struct inode* to replace extent_io_tree::private_data Qu Wenruo
2019-02-25 10:00 ` Filipe Manana
2019-02-25 11:10 ` Johannes Thumshirn
@ 2019-02-25 12:15 ` David Sterba
2019-02-25 12:17 ` Qu Wenruo
2 siblings, 1 reply; 9+ messages in thread
From: David Sterba @ 2019-02-25 12:15 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, josef
On Mon, Feb 25, 2019 at 01:50:43PM +0800, Qu Wenruo wrote:
> All users of extent_io_tree::private_data are expecting struct inode*.
> So just use struct inode* to replace extent_io_tree::private_data, and
> this should provide better type check.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> struct extent_io_tree {
> struct rb_root state;
> - void *private_data;
> + struct inode *inode;
> u64 dirty_bytes;
> int track_uptodate;
> spinlock_t lock;
So this is effectively reverting c6100a4b4e3d1650deafd ("Btrfs: replace
tree->mapping with tree->private_data"), that seems to be preparatory
work for btree_inode removal. I haven't heared any news about that work
for a long time though, so if this is going to land any time soon then
we can keep it there. Otherwise, well, ack for the patch.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] btrfs: Use struct inode* to replace extent_io_tree::private_data
2019-02-25 12:15 ` David Sterba
@ 2019-02-25 12:17 ` Qu Wenruo
2019-02-25 12:26 ` David Sterba
0 siblings, 1 reply; 9+ messages in thread
From: Qu Wenruo @ 2019-02-25 12:17 UTC (permalink / raw)
To: dsterba, Qu Wenruo, linux-btrfs, josef
[-- Attachment #1.1: Type: text/plain, Size: 1067 bytes --]
On 2019/2/25 下午8:15, David Sterba wrote:
> On Mon, Feb 25, 2019 at 01:50:43PM +0800, Qu Wenruo wrote:
>> All users of extent_io_tree::private_data are expecting struct inode*.
>> So just use struct inode* to replace extent_io_tree::private_data, and
>> this should provide better type check.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> struct extent_io_tree {
>> struct rb_root state;
>> - void *private_data;
>> + struct inode *inode;
>> u64 dirty_bytes;
>> int track_uptodate;
>> spinlock_t lock;
>
> So this is effectively reverting c6100a4b4e3d1650deafd ("Btrfs: replace
> tree->mapping with tree->private_data"),
That commit message doesn't explain why this is needed for btree_inode
removal.
Any idea what the extra type would be used in that case?
Thanks,
Qu
> that seems to be preparatory
> work for btree_inode removal. I haven't heared any news about that work
> for a long time though, so if this is going to land any time soon then
> we can keep it there. Otherwise, well, ack for the patch.
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] btrfs: Use struct inode* to replace extent_io_tree::private_data
2019-02-25 12:17 ` Qu Wenruo
@ 2019-02-25 12:26 ` David Sterba
2019-02-25 15:46 ` Josef Bacik
0 siblings, 1 reply; 9+ messages in thread
From: David Sterba @ 2019-02-25 12:26 UTC (permalink / raw)
To: Qu Wenruo; +Cc: dsterba, Qu Wenruo, linux-btrfs, josef
On Mon, Feb 25, 2019 at 08:17:52PM +0800, Qu Wenruo wrote:
>
>
> On 2019/2/25 下午8:15, David Sterba wrote:
> > On Mon, Feb 25, 2019 at 01:50:43PM +0800, Qu Wenruo wrote:
> >> All users of extent_io_tree::private_data are expecting struct inode*.
> >> So just use struct inode* to replace extent_io_tree::private_data, and
> >> this should provide better type check.
> >>
> >> Signed-off-by: Qu Wenruo <wqu@suse.com>
> >> struct extent_io_tree {
> >> struct rb_root state;
> >> - void *private_data;
> >> + struct inode *inode;
> >> u64 dirty_bytes;
> >> int track_uptodate;
> >> spinlock_t lock;
> >
> > So this is effectively reverting c6100a4b4e3d1650deafd ("Btrfs: replace
> > tree->mapping with tree->private_data"),
>
> That commit message doesn't explain why this is needed for btree_inode
> removal.
>
> Any idea what the extra type would be used in that case?
I don't know, Josef in CC to answer that.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] btrfs: Use struct inode* to replace extent_io_tree::private_data
2019-02-25 12:26 ` David Sterba
@ 2019-02-25 15:46 ` Josef Bacik
2019-02-26 4:07 ` Qu Wenruo
0 siblings, 1 reply; 9+ messages in thread
From: Josef Bacik @ 2019-02-25 15:46 UTC (permalink / raw)
To: David Sterba; +Cc: Qu Wenruo, Qu Wenruo, linux-btrfs, josef
On Mon, Feb 25, 2019 at 01:26:57PM +0100, David Sterba wrote:
> On Mon, Feb 25, 2019 at 08:17:52PM +0800, Qu Wenruo wrote:
> >
> >
> > On 2019/2/25 下午8:15, David Sterba wrote:
> > > On Mon, Feb 25, 2019 at 01:50:43PM +0800, Qu Wenruo wrote:
> > >> All users of extent_io_tree::private_data are expecting struct inode*.
> > >> So just use struct inode* to replace extent_io_tree::private_data, and
> > >> this should provide better type check.
> > >>
> > >> Signed-off-by: Qu Wenruo <wqu@suse.com>
> > >> struct extent_io_tree {
> > >> struct rb_root state;
> > >> - void *private_data;
> > >> + struct inode *inode;
> > >> u64 dirty_bytes;
> > >> int track_uptodate;
> > >> spinlock_t lock;
> > >
> > > So this is effectively reverting c6100a4b4e3d1650deafd ("Btrfs: replace
> > > tree->mapping with tree->private_data"),
> >
> > That commit message doesn't explain why this is needed for btree_inode
> > removal.
> >
> > Any idea what the extra type would be used in that case?
>
> I don't know, Josef in CC to answer that.
Yeah there's a mapping tree thing I make to keep track of the metadata, it holds
the radix tree for the eb's and a bunch of other stuff. This needs to stay a
void *. Thanks,
Josef
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] btrfs: Use struct inode* to replace extent_io_tree::private_data
2019-02-25 15:46 ` Josef Bacik
@ 2019-02-26 4:07 ` Qu Wenruo
2019-02-26 13:35 ` Josef Bacik
0 siblings, 1 reply; 9+ messages in thread
From: Qu Wenruo @ 2019-02-26 4:07 UTC (permalink / raw)
To: Josef Bacik, David Sterba; +Cc: Qu Wenruo, linux-btrfs
[-- Attachment #1.1: Type: text/plain, Size: 1471 bytes --]
On 2019/2/25 下午11:46, Josef Bacik wrote:
> On Mon, Feb 25, 2019 at 01:26:57PM +0100, David Sterba wrote:
>> On Mon, Feb 25, 2019 at 08:17:52PM +0800, Qu Wenruo wrote:
>>>
>>>
>>> On 2019/2/25 下午8:15, David Sterba wrote:
>>>> On Mon, Feb 25, 2019 at 01:50:43PM +0800, Qu Wenruo wrote:
>>>>> All users of extent_io_tree::private_data are expecting struct inode*.
>>>>> So just use struct inode* to replace extent_io_tree::private_data, and
>>>>> this should provide better type check.
>>>>>
>>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>>>> struct extent_io_tree {
>>>>> struct rb_root state;
>>>>> - void *private_data;
>>>>> + struct inode *inode;
>>>>> u64 dirty_bytes;
>>>>> int track_uptodate;
>>>>> spinlock_t lock;
>>>>
>>>> So this is effectively reverting c6100a4b4e3d1650deafd ("Btrfs: replace
>>>> tree->mapping with tree->private_data"),
>>>
>>> That commit message doesn't explain why this is needed for btree_inode
>>> removal.
>>>
>>> Any idea what the extra type would be used in that case?
>>
>> I don't know, Josef in CC to answer that.
>
> Yeah there's a mapping tree thing I make to keep track of the metadata, it holds
> the radix tree for the eb's and a bunch of other stuff. This needs to stay a
> void *. Thanks,
Then what about an union here?
BTW, for no btree_inode case, how do we distinguish regular inode
pointer with yours? An extra member?
Thanks,
Qu
>
> Josef
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] btrfs: Use struct inode* to replace extent_io_tree::private_data
2019-02-26 4:07 ` Qu Wenruo
@ 2019-02-26 13:35 ` Josef Bacik
0 siblings, 0 replies; 9+ messages in thread
From: Josef Bacik @ 2019-02-26 13:35 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Josef Bacik, David Sterba, Qu Wenruo, linux-btrfs
On Tue, Feb 26, 2019 at 12:07:37PM +0800, Qu Wenruo wrote:
>
>
> On 2019/2/25 下午11:46, Josef Bacik wrote:
> > On Mon, Feb 25, 2019 at 01:26:57PM +0100, David Sterba wrote:
> >> On Mon, Feb 25, 2019 at 08:17:52PM +0800, Qu Wenruo wrote:
> >>>
> >>>
> >>> On 2019/2/25 下午8:15, David Sterba wrote:
> >>>> On Mon, Feb 25, 2019 at 01:50:43PM +0800, Qu Wenruo wrote:
> >>>>> All users of extent_io_tree::private_data are expecting struct inode*.
> >>>>> So just use struct inode* to replace extent_io_tree::private_data, and
> >>>>> this should provide better type check.
> >>>>>
> >>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> >>>>> struct extent_io_tree {
> >>>>> struct rb_root state;
> >>>>> - void *private_data;
> >>>>> + struct inode *inode;
> >>>>> u64 dirty_bytes;
> >>>>> int track_uptodate;
> >>>>> spinlock_t lock;
> >>>>
> >>>> So this is effectively reverting c6100a4b4e3d1650deafd ("Btrfs: replace
> >>>> tree->mapping with tree->private_data"),
> >>>
> >>> That commit message doesn't explain why this is needed for btree_inode
> >>> removal.
> >>>
> >>> Any idea what the extra type would be used in that case?
> >>
> >> I don't know, Josef in CC to answer that.
> >
> > Yeah there's a mapping tree thing I make to keep track of the metadata, it holds
> > the radix tree for the eb's and a bunch of other stuff. This needs to stay a
> > void *. Thanks,
>
> Then what about an union here?
>
> BTW, for no btree_inode case, how do we distinguish regular inode
> pointer with yours? An extra member?
>
We don't have to, the only place the inode is used is when doing the callbacks.
The callbacks are implementation specific, so the btree extent_io callbacks know
that the void *private_data is their struct btrfs_eb_info, and the inode ones
know it's their struct inode *. Thanks,
Josef
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-02-26 13:35 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-25 5:50 [PATCH] btrfs: Use struct inode* to replace extent_io_tree::private_data Qu Wenruo
2019-02-25 10:00 ` Filipe Manana
2019-02-25 11:10 ` Johannes Thumshirn
2019-02-25 12:15 ` David Sterba
2019-02-25 12:17 ` Qu Wenruo
2019-02-25 12:26 ` David Sterba
2019-02-25 15:46 ` Josef Bacik
2019-02-26 4:07 ` Qu Wenruo
2019-02-26 13:35 ` Josef Bacik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).