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 <[email protected]> > > > > 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 <[email protected]> > > --- > > > > 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 [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
