linux-kernel - Re: [PATCH 2/2] zram: idle memory tracking

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180326095712.GA214483@rodete-desktop-imager.corp.google.com>
Date:   Mon, 26 Mar 2018 18:57:12 +0900
From:   Minchan Kim <minchan@...nel.org>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] zram: idle memory tracking

On Mon, Mar 26, 2018 at 10:15:11AM +0200, Greg KH wrote:
> On Mon, Mar 26, 2018 at 03:49:51PM +0900, Minchan Kim wrote:
> > +static int zram_debugfs_register(struct zram *zram)
> > +{
> > +	struct dentry *ret;
> > +
> > +	if (!zram_debugfs_root)
> > +		return -ENOENT;
> 
> No need to care, you should not error out if debugfs is not enabled or
> not working, your code path should be identical either way.  debugfs is
> not required for any functionality, so don't treat it like it matters :)

Glad to hear. It makes code much clean.

> 
> > +	zram->debugfs_dir = debugfs_create_dir(zram->disk->disk_name,
> > +						zram_debugfs_root);
> > +	if (!zram->debugfs_dir)
> > +		return -ENOMEM;
> 
> No need to check the return value of any debugfs call, you can always
> either use it for future debugfs calls, or ignore it.
> 
> > +	ret = debugfs_create_file("access_time", 0400, zram->debugfs_dir,
> > +				zram, &proc_zram_access_operations);
> > +	if (!ret)
> > +		return -ENOMEM;
> 
> Again, you shouldn't care :)
> 
> > +
> > +	return 0;
> 
> just return void, no need to test any of this.

Thanks for the quick review, Greg.