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