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 <[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.
It's okay to me to clean up them later. Reviewed-by: Chao Yu <[email protected]> Thanks, > > - Eric > . > _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
