linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
* [f2fs-dev] [PATCH v2] f2fs: don't return vmalloc() memory from f2fs_kmalloc()
@ 2020-06-05  4:57 Eric Biggers
  2020-06-05  5:03 ` Jaegeuk Kim
  2020-06-09  1:36 ` Chao Yu
  0 siblings, 2 replies; 5+ messages in thread
From: Eric Biggers @ 2020-06-05  4:57 UTC (permalink / raw)
  To: linux-f2fs-devel

From: Eric Biggers <ebiggers@google.com>

kmalloc() returns kmalloc'ed memory, and kvmalloc() returns either
kmalloc'ed or vmalloc'ed memory.  But the f2fs wrappers, f2fs_kmalloc()
and f2fs_kvmalloc(), both return both kinds of memory.

It's redundant to have two functions that do the same thing, and also
breaking the standard naming convention is causing bugs since people
assume it's safe to kfree() memory allocated by f2fs_kmalloc().  See
e.g. the various allocations in fs/f2fs/compress.c.

Fix this by making f2fs_kmalloc() just use kmalloc().  And to avoid
re-introducing the allocation failures that the vmalloc fallback was
intended to fix, convert the largest allocations to use f2fs_kvmalloc().

Signed-off-by: Eric Biggers <ebiggers@google.com>
---

v2: also use f2fs_kvzalloc() in init_blkz_info()

 fs/f2fs/checkpoint.c | 4 ++--
 fs/f2fs/f2fs.h       | 8 +-------
 fs/f2fs/node.c       | 8 ++++----
 fs/f2fs/super.c      | 2 +-
 4 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 3dc3ac6fe14324..23606493025165 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -895,8 +895,8 @@ int f2fs_get_valid_checkpoint(struct f2fs_sb_info *sbi)
 	int i;
 	int err;
 
-	sbi->ckpt = f2fs_kzalloc(sbi, array_size(blk_size, cp_blks),
-				 GFP_KERNEL);
+	sbi->ckpt = f2fs_kvzalloc(sbi, array_size(blk_size, cp_blks),
+				  GFP_KERNEL);
 	if (!sbi->ckpt)
 		return -ENOMEM;
 	/*
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 50e6cdf20b7331..c812fb8e2d9c7a 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2998,18 +2998,12 @@ static inline bool f2fs_may_extent_tree(struct inode *inode)
 static inline void *f2fs_kmalloc(struct f2fs_sb_info *sbi,
 					size_t size, gfp_t flags)
 {
-	void *ret;
-
 	if (time_to_inject(sbi, FAULT_KMALLOC)) {
 		f2fs_show_injection_info(sbi, FAULT_KMALLOC);
 		return NULL;
 	}
 
-	ret = kmalloc(size, flags);
-	if (ret)
-		return ret;
-
-	return kvmalloc(size, flags);
+	return kmalloc(size, flags);
 }
 
 static inline void *f2fs_kzalloc(struct f2fs_sb_info *sbi,
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index e0bb0f7e0506ee..03e24df1c84f5c 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -2993,7 +2993,7 @@ static int __get_nat_bitmaps(struct f2fs_sb_info *sbi)
 		return 0;
 
 	nm_i->nat_bits_blocks = F2FS_BLK_ALIGN((nat_bits_bytes << 1) + 8);
-	nm_i->nat_bits = f2fs_kzalloc(sbi,
+	nm_i->nat_bits = f2fs_kvzalloc(sbi,
 			nm_i->nat_bits_blocks << F2FS_BLKSIZE_BITS, GFP_KERNEL);
 	if (!nm_i->nat_bits)
 		return -ENOMEM;
@@ -3126,9 +3126,9 @@ static int init_free_nid_cache(struct f2fs_sb_info *sbi)
 	int i;
 
 	nm_i->free_nid_bitmap =
-		f2fs_kzalloc(sbi, array_size(sizeof(unsigned char *),
-					     nm_i->nat_blocks),
-			     GFP_KERNEL);
+		f2fs_kvzalloc(sbi, array_size(sizeof(unsigned char *),
+					      nm_i->nat_blocks),
+			      GFP_KERNEL);
 	if (!nm_i->free_nid_bitmap)
 		return -ENOMEM;
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index a71da699cb2d55..f3c15116954218 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3033,7 +3033,7 @@ static int init_blkz_info(struct f2fs_sb_info *sbi, int devi)
 	if (nr_sectors & (bdev_zone_sectors(bdev) - 1))
 		FDEV(devi).nr_blkz++;
 
-	FDEV(devi).blkz_seq = f2fs_kzalloc(sbi,
+	FDEV(devi).blkz_seq = f2fs_kvzalloc(sbi,
 					BITS_TO_LONGS(FDEV(devi).nr_blkz)
 					* sizeof(unsigned long),
 					GFP_KERNEL);
-- 
2.26.2



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: don't return vmalloc() memory from f2fs_kmalloc()
  2020-06-05  4:57 [f2fs-dev] [PATCH v2] f2fs: don't return vmalloc() memory from f2fs_kmalloc() Eric Biggers
@ 2020-06-05  5:03 ` Jaegeuk Kim
  2020-06-09  1:36 ` Chao Yu
  1 sibling, 0 replies; 5+ messages in thread
From: Jaegeuk Kim @ 2020-06-05  5:03 UTC (permalink / raw)
  To: Eric Biggers; +Cc: linux-f2fs-devel

On 06/04, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> kmalloc() returns kmalloc'ed memory, and kvmalloc() returns either
> kmalloc'ed or vmalloc'ed memory.  But the f2fs wrappers, f2fs_kmalloc()
> and f2fs_kvmalloc(), both return both kinds of memory.
> 
> It's redundant to have two functions that do the same thing, and also
> breaking the standard naming convention is causing bugs since people
> assume it's safe to kfree() memory allocated by f2fs_kmalloc().  See
> e.g. the various allocations in fs/f2fs/compress.c.
> 
> Fix this by making f2fs_kmalloc() just use kmalloc().  And to avoid
> re-introducing the allocation failures that the vmalloc fallback was
> intended to fix, convert the largest allocations to use f2fs_kvmalloc().
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> 
> v2: also use f2fs_kvzalloc() in init_blkz_info()

Thanks, applied.

> 
>  fs/f2fs/checkpoint.c | 4 ++--
>  fs/f2fs/f2fs.h       | 8 +-------
>  fs/f2fs/node.c       | 8 ++++----
>  fs/f2fs/super.c      | 2 +-
>  4 files changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index 3dc3ac6fe14324..23606493025165 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -895,8 +895,8 @@ int f2fs_get_valid_checkpoint(struct f2fs_sb_info *sbi)
>  	int i;
>  	int err;
>  
> -	sbi->ckpt = f2fs_kzalloc(sbi, array_size(blk_size, cp_blks),
> -				 GFP_KERNEL);
> +	sbi->ckpt = f2fs_kvzalloc(sbi, array_size(blk_size, cp_blks),
> +				  GFP_KERNEL);
>  	if (!sbi->ckpt)
>  		return -ENOMEM;
>  	/*
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 50e6cdf20b7331..c812fb8e2d9c7a 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2998,18 +2998,12 @@ static inline bool f2fs_may_extent_tree(struct inode *inode)
>  static inline void *f2fs_kmalloc(struct f2fs_sb_info *sbi,
>  					size_t size, gfp_t flags)
>  {
> -	void *ret;
> -
>  	if (time_to_inject(sbi, FAULT_KMALLOC)) {
>  		f2fs_show_injection_info(sbi, FAULT_KMALLOC);
>  		return NULL;
>  	}
>  
> -	ret = kmalloc(size, flags);
> -	if (ret)
> -		return ret;
> -
> -	return kvmalloc(size, flags);
> +	return kmalloc(size, flags);
>  }
>  
>  static inline void *f2fs_kzalloc(struct f2fs_sb_info *sbi,
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index e0bb0f7e0506ee..03e24df1c84f5c 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -2993,7 +2993,7 @@ static int __get_nat_bitmaps(struct f2fs_sb_info *sbi)
>  		return 0;
>  
>  	nm_i->nat_bits_blocks = F2FS_BLK_ALIGN((nat_bits_bytes << 1) + 8);
> -	nm_i->nat_bits = f2fs_kzalloc(sbi,
> +	nm_i->nat_bits = f2fs_kvzalloc(sbi,
>  			nm_i->nat_bits_blocks << F2FS_BLKSIZE_BITS, GFP_KERNEL);
>  	if (!nm_i->nat_bits)
>  		return -ENOMEM;
> @@ -3126,9 +3126,9 @@ static int init_free_nid_cache(struct f2fs_sb_info *sbi)
>  	int i;
>  
>  	nm_i->free_nid_bitmap =
> -		f2fs_kzalloc(sbi, array_size(sizeof(unsigned char *),
> -					     nm_i->nat_blocks),
> -			     GFP_KERNEL);
> +		f2fs_kvzalloc(sbi, array_size(sizeof(unsigned char *),
> +					      nm_i->nat_blocks),
> +			      GFP_KERNEL);
>  	if (!nm_i->free_nid_bitmap)
>  		return -ENOMEM;
>  
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index a71da699cb2d55..f3c15116954218 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -3033,7 +3033,7 @@ static int init_blkz_info(struct f2fs_sb_info *sbi, int devi)
>  	if (nr_sectors & (bdev_zone_sectors(bdev) - 1))
>  		FDEV(devi).nr_blkz++;
>  
> -	FDEV(devi).blkz_seq = f2fs_kzalloc(sbi,
> +	FDEV(devi).blkz_seq = f2fs_kvzalloc(sbi,
>  					BITS_TO_LONGS(FDEV(devi).nr_blkz)
>  					* sizeof(unsigned long),
>  					GFP_KERNEL);
> -- 
> 2.26.2
> 
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: don't return vmalloc() memory from f2fs_kmalloc()
  2020-06-05  4:57 [f2fs-dev] [PATCH v2] f2fs: don't return vmalloc() memory from f2fs_kmalloc() Eric Biggers
  2020-06-05  5:03 ` Jaegeuk Kim
@ 2020-06-09  1:36 ` Chao Yu
  2020-06-09  2:11   ` Eric Biggers
  1 sibling, 1 reply; 5+ messages in thread
From: Chao Yu @ 2020-06-09  1:36 UTC (permalink / raw)
  To: Eric Biggers, linux-f2fs-devel

On 2020/6/5 12:57, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> kmalloc() returns kmalloc'ed memory, and kvmalloc() returns either
> kmalloc'ed or vmalloc'ed memory.  But the f2fs wrappers, f2fs_kmalloc()
> and f2fs_kvmalloc(), both return both kinds of memory.
> 
> It's redundant to have two functions that do the same thing, and also
> breaking the standard naming convention is causing bugs since people
> assume it's safe to kfree() memory allocated by f2fs_kmalloc().  See
> e.g. the various allocations in fs/f2fs/compress.c.
> 
> Fix this by making f2fs_kmalloc() just use kmalloc().  And to avoid
> re-introducing the allocation failures that the vmalloc fallback was
> intended to fix, convert the largest allocations to use f2fs_kvmalloc().
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> 
> v2: also use f2fs_kvzalloc() in init_blkz_info()
> 
>  fs/f2fs/checkpoint.c | 4 ++--
>  fs/f2fs/f2fs.h       | 8 +-------
>  fs/f2fs/node.c       | 8 ++++----
>  fs/f2fs/super.c      | 2 +-
>  4 files changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index 3dc3ac6fe14324..23606493025165 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -895,8 +895,8 @@ int f2fs_get_valid_checkpoint(struct f2fs_sb_info *sbi)
>  	int i;
>  	int err;
>  
> -	sbi->ckpt = f2fs_kzalloc(sbi, array_size(blk_size, cp_blks),
> -				 GFP_KERNEL);
> +	sbi->ckpt = f2fs_kvzalloc(sbi, array_size(blk_size, cp_blks),
> +				  GFP_KERNEL);
>  	if (!sbi->ckpt)
>  		return -ENOMEM;
>  	/*
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 50e6cdf20b7331..c812fb8e2d9c7a 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2998,18 +2998,12 @@ static inline bool f2fs_may_extent_tree(struct inode *inode)
>  static inline void *f2fs_kmalloc(struct f2fs_sb_info *sbi,
>  					size_t size, gfp_t flags)
>  {
> -	void *ret;
> -
>  	if (time_to_inject(sbi, FAULT_KMALLOC)) {
>  		f2fs_show_injection_info(sbi, FAULT_KMALLOC);
>  		return NULL;
>  	}
>  
> -	ret = kmalloc(size, flags);
> -	if (ret)
> -		return ret;
> -
> -	return kvmalloc(size, flags);
> +	return kmalloc(size, flags);

Then could we revert to use kfree instead of kvfree if memory was allocated
from f2fs_kmalloc()? though there is actual problem w/o reverting.

Thanks,

>  }
>  
>  static inline void *f2fs_kzalloc(struct f2fs_sb_info *sbi,
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index e0bb0f7e0506ee..03e24df1c84f5c 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -2993,7 +2993,7 @@ static int __get_nat_bitmaps(struct f2fs_sb_info *sbi)
>  		return 0;
>  
>  	nm_i->nat_bits_blocks = F2FS_BLK_ALIGN((nat_bits_bytes << 1) + 8);
> -	nm_i->nat_bits = f2fs_kzalloc(sbi,
> +	nm_i->nat_bits = f2fs_kvzalloc(sbi,
>  			nm_i->nat_bits_blocks << F2FS_BLKSIZE_BITS, GFP_KERNEL);
>  	if (!nm_i->nat_bits)
>  		return -ENOMEM;
> @@ -3126,9 +3126,9 @@ static int init_free_nid_cache(struct f2fs_sb_info *sbi)
>  	int i;
>  
>  	nm_i->free_nid_bitmap =
> -		f2fs_kzalloc(sbi, array_size(sizeof(unsigned char *),
> -					     nm_i->nat_blocks),
> -			     GFP_KERNEL);
> +		f2fs_kvzalloc(sbi, array_size(sizeof(unsigned char *),
> +					      nm_i->nat_blocks),
> +			      GFP_KERNEL);
>  	if (!nm_i->free_nid_bitmap)
>  		return -ENOMEM;
>  
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index a71da699cb2d55..f3c15116954218 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -3033,7 +3033,7 @@ static int init_blkz_info(struct f2fs_sb_info *sbi, int devi)
>  	if (nr_sectors & (bdev_zone_sectors(bdev) - 1))
>  		FDEV(devi).nr_blkz++;
>  
> -	FDEV(devi).blkz_seq = f2fs_kzalloc(sbi,
> +	FDEV(devi).blkz_seq = f2fs_kvzalloc(sbi,
>  					BITS_TO_LONGS(FDEV(devi).nr_blkz)
>  					* sizeof(unsigned long),
>  					GFP_KERNEL);
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: don't return vmalloc() memory from f2fs_kmalloc()
  2020-06-09  1:36 ` Chao Yu
@ 2020-06-09  2:11   ` Eric Biggers
  2020-06-09  2:20     ` Chao Yu
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Biggers @ 2020-06-09  2:11 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel

On Tue, Jun 09, 2020 at 09:36:50AM +0800, Chao Yu wrote:
> On 2020/6/5 12:57, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > kmalloc() returns kmalloc'ed memory, and kvmalloc() returns either
> > kmalloc'ed or vmalloc'ed memory.  But the f2fs wrappers, f2fs_kmalloc()
> > and f2fs_kvmalloc(), both return both kinds of memory.
> > 
> > It's redundant to have two functions that do the same thing, and also
> > breaking the standard naming convention is causing bugs since people
> > assume it's safe to kfree() memory allocated by f2fs_kmalloc().  See
> > e.g. the various allocations in fs/f2fs/compress.c.
> > 
> > Fix this by making f2fs_kmalloc() just use kmalloc().  And to avoid
> > re-introducing the allocation failures that the vmalloc fallback was
> > intended to fix, convert the largest allocations to use f2fs_kvmalloc().
> > 
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> > 
> > v2: also use f2fs_kvzalloc() in init_blkz_info()
> > 
> >  fs/f2fs/checkpoint.c | 4 ++--
> >  fs/f2fs/f2fs.h       | 8 +-------
> >  fs/f2fs/node.c       | 8 ++++----
> >  fs/f2fs/super.c      | 2 +-
> >  4 files changed, 8 insertions(+), 14 deletions(-)
> > 
> > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> > index 3dc3ac6fe14324..23606493025165 100644
> > --- a/fs/f2fs/checkpoint.c
> > +++ b/fs/f2fs/checkpoint.c
> > @@ -895,8 +895,8 @@ int f2fs_get_valid_checkpoint(struct f2fs_sb_info *sbi)
> >  	int i;
> >  	int err;
> >  
> > -	sbi->ckpt = f2fs_kzalloc(sbi, array_size(blk_size, cp_blks),
> > -				 GFP_KERNEL);
> > +	sbi->ckpt = f2fs_kvzalloc(sbi, array_size(blk_size, cp_blks),
> > +				  GFP_KERNEL);
> >  	if (!sbi->ckpt)
> >  		return -ENOMEM;
> >  	/*
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 50e6cdf20b7331..c812fb8e2d9c7a 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -2998,18 +2998,12 @@ static inline bool f2fs_may_extent_tree(struct inode *inode)
> >  static inline void *f2fs_kmalloc(struct f2fs_sb_info *sbi,
> >  					size_t size, gfp_t flags)
> >  {
> > -	void *ret;
> > -
> >  	if (time_to_inject(sbi, FAULT_KMALLOC)) {
> >  		f2fs_show_injection_info(sbi, FAULT_KMALLOC);
> >  		return NULL;
> >  	}
> >  
> > -	ret = kmalloc(size, flags);
> > -	if (ret)
> > -		return ret;
> > -
> > -	return kvmalloc(size, flags);
> > +	return kmalloc(size, flags);
> 
> Then could we revert to use kfree instead of kvfree if memory was allocated
> from f2fs_kmalloc()? though there is actual problem w/o reverting.
> 

Yes, I think we should prefer kfree() when the memory was allocated with
f2fs_kmalloc().  It's not critical though, since kvfree() works on kmalloc'ed
memory.  So it should be a separate patch later.

- Eric


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: don't return vmalloc() memory from f2fs_kmalloc()
  2020-06-09  2:11   ` Eric Biggers
@ 2020-06-09  2:20     ` Chao Yu
  0 siblings, 0 replies; 5+ messages in thread
From: Chao Yu @ 2020-06-09  2:20 UTC (permalink / raw)
  To: Eric Biggers; +Cc: linux-f2fs-devel

On 2020/6/9 10:11, Eric Biggers wrote:
> On Tue, Jun 09, 2020 at 09:36:50AM +0800, Chao Yu wrote:
>> On 2020/6/5 12:57, Eric Biggers wrote:
>>> From: Eric Biggers <ebiggers@google.com>
>>>
>>> kmalloc() returns kmalloc'ed memory, and kvmalloc() returns either
>>> kmalloc'ed or vmalloc'ed memory.  But the f2fs wrappers, f2fs_kmalloc()
>>> and f2fs_kvmalloc(), both return both kinds of memory.
>>>
>>> It's redundant to have two functions that do the same thing, and also
>>> breaking the standard naming convention is causing bugs since people
>>> assume it's safe to kfree() memory allocated by f2fs_kmalloc().  See
>>> e.g. the various allocations in fs/f2fs/compress.c.
>>>
>>> Fix this by making f2fs_kmalloc() just use kmalloc().  And to avoid
>>> re-introducing the allocation failures that the vmalloc fallback was
>>> intended to fix, convert the largest allocations to use f2fs_kvmalloc().
>>>
>>> Signed-off-by: Eric Biggers <ebiggers@google.com>
>>> ---
>>>
>>> v2: also use f2fs_kvzalloc() in init_blkz_info()
>>>
>>>  fs/f2fs/checkpoint.c | 4 ++--
>>>  fs/f2fs/f2fs.h       | 8 +-------
>>>  fs/f2fs/node.c       | 8 ++++----
>>>  fs/f2fs/super.c      | 2 +-
>>>  4 files changed, 8 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>>> index 3dc3ac6fe14324..23606493025165 100644
>>> --- a/fs/f2fs/checkpoint.c
>>> +++ b/fs/f2fs/checkpoint.c
>>> @@ -895,8 +895,8 @@ int f2fs_get_valid_checkpoint(struct f2fs_sb_info *sbi)
>>>  	int i;
>>>  	int err;
>>>  
>>> -	sbi->ckpt = f2fs_kzalloc(sbi, array_size(blk_size, cp_blks),
>>> -				 GFP_KERNEL);
>>> +	sbi->ckpt = f2fs_kvzalloc(sbi, array_size(blk_size, cp_blks),
>>> +				  GFP_KERNEL);
>>>  	if (!sbi->ckpt)
>>>  		return -ENOMEM;
>>>  	/*
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index 50e6cdf20b7331..c812fb8e2d9c7a 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -2998,18 +2998,12 @@ static inline bool f2fs_may_extent_tree(struct inode *inode)
>>>  static inline void *f2fs_kmalloc(struct f2fs_sb_info *sbi,
>>>  					size_t size, gfp_t flags)
>>>  {
>>> -	void *ret;
>>> -
>>>  	if (time_to_inject(sbi, FAULT_KMALLOC)) {
>>>  		f2fs_show_injection_info(sbi, FAULT_KMALLOC);
>>>  		return NULL;
>>>  	}
>>>  
>>> -	ret = kmalloc(size, flags);
>>> -	if (ret)
>>> -		return ret;
>>> -
>>> -	return kvmalloc(size, flags);
>>> +	return kmalloc(size, flags);
>>
>> Then could we revert to use kfree instead of kvfree if memory was allocated
>> from f2fs_kmalloc()? though there is actual problem w/o reverting.
>>
> 
> Yes, I think we should prefer kfree() when the memory was allocated with
> f2fs_kmalloc().  It's not critical though, since kvfree() works on kmalloc'ed
> memory.  So it should be a separate patch later.

It's okay to me to clean up them later.

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

> 
> - Eric
> .
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-06-09  2:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-05  4:57 [f2fs-dev] [PATCH v2] f2fs: don't return vmalloc() memory from f2fs_kmalloc() Eric Biggers
2020-06-05  5:03 ` Jaegeuk Kim
2020-06-09  1:36 ` Chao Yu
2020-06-09  2:11   ` Eric Biggers
2020-06-09  2:20     ` Chao Yu

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).