Re: [PATCH 3/3] zram: add mem_used_max via sysfs

[Minchan Kim]

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@xxxxxxxx> wrote:
> > On Wed, Aug 13, 2014 at 9:12 PM, Minchan Kim <minchan@xxxxxxxxxx> 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@xxxxxxxxxx  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>

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