linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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).