linux-kernel - Re: [PATCH 3/3] zram: add mem_used

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] [day] [month] [year] [list]

Message-ID: <20140817235324.GE11367@bbox>
Date:	Mon, 18 Aug 2014 08:53:24 +0900
From:	Minchan Kim <minchan@...nel.org>
To:	David Horner <ds2horner@...il.com>
Cc:	Dan Streetman <ddstreet@...e.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linux-MM <linux-mm@...ck.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
	Jerome Marchand <jmarchan@...hat.com>, juno.choi@....com,
	seungho1.park@....com, Luigi Semenzato <semenzato@...gle.com>,
	Nitin Gupta <ngupta@...are.org>,
	Seth Jennings <sjennings@...iantweb.net>
Subject: Re: [PATCH 3/3] zram: add mem_used_max via sysfs

On Thu, Aug 14, 2014 at 11:32:36AM -0400, David Horner wrote:
> On Thu, Aug 14, 2014 at 11:09 AM, Dan Streetman <ddstreet@...e.org> wrote:
> > On Wed, Aug 13, 2014 at 9:12 PM, Minchan Kim <minchan@...nel.org> wrote:
> >> -       if (zram->limit_bytes &&
> >> -               zs_get_total_size_bytes(meta->mem_pool) > zram->limit_bytes) {
> >> +       total_bytes = zs_get_total_size_bytes(meta->mem_pool);
> >> +       if (zram->limit_bytes && total_bytes > zram->limit_bytes) {
> >
> > do you need to take the init_lock to read limit_bytes here?  It could
> > be getting changed between these checks...
> 
> There is no real danger in freeing with an error.
> It is more timing than a race.
> 
> The max calculation is still ok because committed allocations are
> added atomically.

There is one problem in below code piece.

zram->max_used_bytes = max(zram->max_used_bytes, total_bytes);

so we should consider this case.

if (zram->max_used_bytes < total_bytes)
        zram->max_used_bytes = total_bytes;

And we could make the situation like this.


if (zram->max_used_bytes < total_bytes)
        IRQ happen;
        zram->max_used_bytes = total_bytes

During IRQ, other CPU could consume a lot of zsmalloc memory so that
zram->max_used_bytes would be increased under the foot so when IRQ is
finshed, zram->max_used_bytes could be reset with old total_bytes.

To prevent it, we should use the lock I posted RFC version or retry
logic with atomic opeartion(ie, cmpxchg) and my approach makes it simple
first and fix it if we see the trouble in future so my preference is
new spin lock at the moment.

Any comments?



> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@...ck.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@...ck.org"> email@...ck.org </a>

-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/