2023-12-06 09:34:22

by Julia Lawall

[permalink] [raw]
Subject: Re: [PATCH] zram: Using GFP_ATOMIC instead of GFP_KERNEL to allocate bitmap memory in backing_dev_store (fwd)

Hello,

The warning is because kvfree is used to free memory that was allocated
using kmalloc; kfree would be fine. But I think that the only way you can
get to out is with bitmap being NULL, so there is no need to free it at
all.

Furthermore, it could be safer in the long term to use different labels
for the different amounts of things that need to be freed, as done in most
other kernel code, rather than using a single label "out".

thanks,
julia

---------- Forwarded message ----------
Date: Wed, 6 Dec 2023 16:08:49 +0800
From: kernel test robot <[email protected]>
To: [email protected]
Cc: [email protected], Julia Lawall <[email protected]>
Subject: Re: [PATCH] zram: Using GFP_ATOMIC instead of GFP_KERNEL to allocate
bitmap memory in backing_dev_store

BCC: [email protected]
CC: [email protected]
In-Reply-To: <[email protected]>
References: <[email protected]>
TO: Dongyun Liu <[email protected]>
TO: [email protected]
TO: [email protected]
TO: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: Dongyun Liu <[email protected]>

Hi Dongyun,

kernel test robot noticed the following build warnings:

[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on linus/master v6.7-rc4 next-20231206]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Dongyun-Liu/zram-Using-GFP_ATOMIC-instead-of-GFP_KERNEL-to-allocate-bitmap-memory-in-backing_dev_store/20231130-233042
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link: https://lore.kernel.org/r/20231130152047.200169-1-dongyun.liu%40transsion.com
patch subject: [PATCH] zram: Using GFP_ATOMIC instead of GFP_KERNEL to allocate bitmap memory in backing_dev_store
:::::: branch date: 6 days ago
:::::: commit date: 6 days ago
config: i386-randconfig-051-20231201 (https://download.01.org/0day-ci/archive/20231206/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231206/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Julia Lawall <[email protected]>
| Closes: https://lore.kernel.org/r/[email protected]/

cocci warnings: (new ones prefixed by >>)
>> drivers/block/zram/zram_drv.c:536:14-15: WARNING kmalloc is used to allocate this memory at line 517

vim +536 drivers/block/zram/zram_drv.c

013bf95a83ec76 Minchan Kim 2017-09-06 459
013bf95a83ec76 Minchan Kim 2017-09-06 460 static ssize_t backing_dev_store(struct device *dev,
013bf95a83ec76 Minchan Kim 2017-09-06 461 struct device_attribute *attr, const char *buf, size_t len)
013bf95a83ec76 Minchan Kim 2017-09-06 462 {
013bf95a83ec76 Minchan Kim 2017-09-06 463 char *file_name;
c8bd134a4bddaf Peter Kalauskas 2018-08-21 464 size_t sz;
013bf95a83ec76 Minchan Kim 2017-09-06 465 struct file *backing_dev = NULL;
013bf95a83ec76 Minchan Kim 2017-09-06 466 struct inode *inode;
013bf95a83ec76 Minchan Kim 2017-09-06 467 struct address_space *mapping;
ee763e2143e79f Christoph Hellwig 2020-11-16 468 unsigned int bitmap_sz;
1363d4662a0d28 Minchan Kim 2017-09-06 469 unsigned long nr_pages, *bitmap = NULL;
eed993a0910338 Jan Kara 2023-09-27 470 struct bdev_handle *bdev_handle = NULL;
013bf95a83ec76 Minchan Kim 2017-09-06 471 int err;
013bf95a83ec76 Minchan Kim 2017-09-06 472 struct zram *zram = dev_to_zram(dev);
013bf95a83ec76 Minchan Kim 2017-09-06 473
013bf95a83ec76 Minchan Kim 2017-09-06 474 file_name = kmalloc(PATH_MAX, GFP_KERNEL);
013bf95a83ec76 Minchan Kim 2017-09-06 475 if (!file_name)
013bf95a83ec76 Minchan Kim 2017-09-06 476 return -ENOMEM;
013bf95a83ec76 Minchan Kim 2017-09-06 477
013bf95a83ec76 Minchan Kim 2017-09-06 478 down_write(&zram->init_lock);
013bf95a83ec76 Minchan Kim 2017-09-06 479 if (init_done(zram)) {
013bf95a83ec76 Minchan Kim 2017-09-06 480 pr_info("Can't setup backing device for initialized device\n");
013bf95a83ec76 Minchan Kim 2017-09-06 481 err = -EBUSY;
013bf95a83ec76 Minchan Kim 2017-09-06 482 goto out;
013bf95a83ec76 Minchan Kim 2017-09-06 483 }
013bf95a83ec76 Minchan Kim 2017-09-06 484
e55e1b4831563e Wolfram Sang 2022-08-18 485 strscpy(file_name, buf, PATH_MAX);
c8bd134a4bddaf Peter Kalauskas 2018-08-21 486 /* ignore trailing newline */
c8bd134a4bddaf Peter Kalauskas 2018-08-21 487 sz = strlen(file_name);
c8bd134a4bddaf Peter Kalauskas 2018-08-21 488 if (sz > 0 && file_name[sz - 1] == '\n')
c8bd134a4bddaf Peter Kalauskas 2018-08-21 489 file_name[sz - 1] = 0x00;
013bf95a83ec76 Minchan Kim 2017-09-06 490
013bf95a83ec76 Minchan Kim 2017-09-06 491 backing_dev = filp_open(file_name, O_RDWR|O_LARGEFILE, 0);
013bf95a83ec76 Minchan Kim 2017-09-06 492 if (IS_ERR(backing_dev)) {
013bf95a83ec76 Minchan Kim 2017-09-06 493 err = PTR_ERR(backing_dev);
013bf95a83ec76 Minchan Kim 2017-09-06 494 backing_dev = NULL;
013bf95a83ec76 Minchan Kim 2017-09-06 495 goto out;
013bf95a83ec76 Minchan Kim 2017-09-06 496 }
013bf95a83ec76 Minchan Kim 2017-09-06 497
013bf95a83ec76 Minchan Kim 2017-09-06 498 mapping = backing_dev->f_mapping;
013bf95a83ec76 Minchan Kim 2017-09-06 499 inode = mapping->host;
013bf95a83ec76 Minchan Kim 2017-09-06 500
013bf95a83ec76 Minchan Kim 2017-09-06 501 /* Support only block device in this moment */
013bf95a83ec76 Minchan Kim 2017-09-06 502 if (!S_ISBLK(inode->i_mode)) {
013bf95a83ec76 Minchan Kim 2017-09-06 503 err = -ENOTBLK;
013bf95a83ec76 Minchan Kim 2017-09-06 504 goto out;
013bf95a83ec76 Minchan Kim 2017-09-06 505 }
013bf95a83ec76 Minchan Kim 2017-09-06 506
eed993a0910338 Jan Kara 2023-09-27 507 bdev_handle = bdev_open_by_dev(inode->i_rdev,
eed993a0910338 Jan Kara 2023-09-27 508 BLK_OPEN_READ | BLK_OPEN_WRITE, zram, NULL);
eed993a0910338 Jan Kara 2023-09-27 509 if (IS_ERR(bdev_handle)) {
eed993a0910338 Jan Kara 2023-09-27 510 err = PTR_ERR(bdev_handle);
eed993a0910338 Jan Kara 2023-09-27 511 bdev_handle = NULL;
013bf95a83ec76 Minchan Kim 2017-09-06 512 goto out;
5547932dc67a48 Minchan Kim 2018-12-28 513 }
013bf95a83ec76 Minchan Kim 2017-09-06 514
1363d4662a0d28 Minchan Kim 2017-09-06 515 nr_pages = i_size_read(inode) >> PAGE_SHIFT;
1363d4662a0d28 Minchan Kim 2017-09-06 516 bitmap_sz = BITS_TO_LONGS(nr_pages) * sizeof(long);
e51f2361fc7fd9 Dongyun Liu 2023-11-30 @517 bitmap = kmalloc(bitmap_sz, GFP_ATOMIC);
1363d4662a0d28 Minchan Kim 2017-09-06 518 if (!bitmap) {
1363d4662a0d28 Minchan Kim 2017-09-06 519 err = -ENOMEM;
1363d4662a0d28 Minchan Kim 2017-09-06 520 goto out;
1363d4662a0d28 Minchan Kim 2017-09-06 521 }
1363d4662a0d28 Minchan Kim 2017-09-06 522
013bf95a83ec76 Minchan Kim 2017-09-06 523 reset_bdev(zram);
013bf95a83ec76 Minchan Kim 2017-09-06 524
eed993a0910338 Jan Kara 2023-09-27 525 zram->bdev_handle = bdev_handle;
013bf95a83ec76 Minchan Kim 2017-09-06 526 zram->backing_dev = backing_dev;
1363d4662a0d28 Minchan Kim 2017-09-06 527 zram->bitmap = bitmap;
1363d4662a0d28 Minchan Kim 2017-09-06 528 zram->nr_pages = nr_pages;
013bf95a83ec76 Minchan Kim 2017-09-06 529 up_write(&zram->init_lock);
013bf95a83ec76 Minchan Kim 2017-09-06 530
013bf95a83ec76 Minchan Kim 2017-09-06 531 pr_info("setup backing device %s\n", file_name);
013bf95a83ec76 Minchan Kim 2017-09-06 532 kfree(file_name);
013bf95a83ec76 Minchan Kim 2017-09-06 533
013bf95a83ec76 Minchan Kim 2017-09-06 534 return len;
013bf95a83ec76 Minchan Kim 2017-09-06 535 out:
1363d4662a0d28 Minchan Kim 2017-09-06 @536 kvfree(bitmap);
1363d4662a0d28 Minchan Kim 2017-09-06 537
eed993a0910338 Jan Kara 2023-09-27 538 if (bdev_handle)
eed993a0910338 Jan Kara 2023-09-27 539 bdev_release(bdev_handle);
013bf95a83ec76 Minchan Kim 2017-09-06 540
013bf95a83ec76 Minchan Kim 2017-09-06 541 if (backing_dev)
013bf95a83ec76 Minchan Kim 2017-09-06 542 filp_close(backing_dev, NULL);
013bf95a83ec76 Minchan Kim 2017-09-06 543
013bf95a83ec76 Minchan Kim 2017-09-06 544 up_write(&zram->init_lock);
013bf95a83ec76 Minchan Kim 2017-09-06 545
013bf95a83ec76 Minchan Kim 2017-09-06 546 kfree(file_name);
013bf95a83ec76 Minchan Kim 2017-09-06 547
013bf95a83ec76 Minchan Kim 2017-09-06 548 return err;
013bf95a83ec76 Minchan Kim 2017-09-06 549 }
013bf95a83ec76 Minchan Kim 2017-09-06 550

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


2023-12-06 09:53:34

by Sergey Senozhatsky

[permalink] [raw]
Subject: Re: [PATCH] zram: Using GFP_ATOMIC instead of GFP_KERNEL to allocate bitmap memory in backing_dev_store (fwd)

Hi,

On (23/12/06 10:33), Julia Lawall wrote:
> Hello,
>
> The warning is because kvfree is used to free memory that was allocated
> using kmalloc; kfree would be fine. But I think that the only way you can
> get to out is with bitmap being NULL, so there is no need to free it at
> all.
>
> Furthermore, it could be safer in the long term to use different labels
> for the different amounts of things that need to be freed, as done in most
> other kernel code, rather than using a single label "out".

[..]

> Date: Wed, 6 Dec 2023 16:08:49 +0800
> From: kernel test robot <[email protected]>
> To: [email protected]
> Cc: [email protected], Julia Lawall <[email protected]>
> Subject: Re: [PATCH] zram: Using GFP_ATOMIC instead of GFP_KERNEL to allocate
> bitmap memory in backing_dev_store
>
> BCC: [email protected]
> CC: [email protected]
> In-Reply-To: <[email protected]>
> References: <[email protected]>
> TO: Dongyun Liu <[email protected]>
> TO: [email protected]
> TO: [email protected]
> TO: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: Dongyun Liu <[email protected]>
>
> Hi Dongyun,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on axboe-block/for-next]
> [also build test WARNING on linus/master v6.7-rc4 next-20231206]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Dongyun-Liu/zram-Using-GFP_ATOMIC-instead-of-GFP_KERNEL-to-allocate-bitmap-memory-in-backing_dev_store/20231130-233042
> base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
> patch link: https://lore.kernel.org/r/20231130152047.200169-1-dongyun.liu%40transsion.com
> patch subject: [PATCH] zram: Using GFP_ATOMIC instead of GFP_KERNEL to allocate bitmap memory in backing_dev_store

This patch won't land upstream. It was NAK-ed.