Message-ID: <dede2f5b-2ae5-6fa3-c0d5-3ce7fba11694@redhat.com>
Date:   Fri, 16 Jun 2023 10:37:09 +0200
From:   David Hildenbrand <david@...hat.com>
To:     Yosry Ahmed <yosryahmed@...gle.com>
Cc:     贺中坤 <hezhongkun.hzk@...edance.com>,
        Yu Zhao <yuzhao@...gle.com>, minchan@...nel.org,
        senozhatsky@...omium.org, mhocko@...e.com, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org,
        Andrea Arcangeli <aarcange@...hat.com>,
        Fabian Deutsch <fdeutsch@...hat.com>
Subject: Re: [External] Re: [RFC PATCH 1/3] zram: charge the compressed RAM to
 the page's memcgroup

On 16.06.23 10:04, Yosry Ahmed wrote:
> On Fri, Jun 16, 2023 at 12:57 AM David Hildenbrand <david@...hat.com> wrote:
>>
>> On 16.06.23 09:37, Yosry Ahmed wrote:
>>> On Thu, Jun 15, 2023 at 9:41 PM 贺中坤 <hezhongkun.hzk@...edance.com> wrote:
>>>>
>>>>> Thanks Fabian for tagging me.
>>>>>
>>>>> I am not familiar with #1, so I will speak to #2. Zhongkun, There are
>>>>> a few parts that I do not understand -- hopefully you can help me out
>>>>> here:
>>>>>
>>>>> (1) If I understand correctly in this patch we set the active memcg
>>>>> trying to charge any pages allocated in a zspage to the current memcg,
>>>>> yet that zspage will contain multiple compressed object slots, not
>>>>> just the one used by this memcg. Aren't we overcharging the memcg?
>>>>> Basically the first memcg that happens to allocate the zspage will pay
>>>>> for all the objects in this zspage, even after it stops using the
>>>>> zspage completely?
>>>>
>>>> It will not overcharge.  As you said below, we are not using
>>>> __GFP_ACCOUNT and charging the compressed slots to the memcgs.
>>>>
>>>>>
>>>>> (2) Patch 3 seems to be charging the compressed slots to the memcgs,
>>>>> yet this patch is trying to charge the entire zspage. Aren't we double
>>>>> charging the zspage? I am guessing this isn't happening because (as
>>>>> Michal pointed out) we are not using __GFP_ACCOUNT here anyway, so
>>>>> this patch may be NOP, and the actual charging is coming from patch 3
>>>>> only.
>>>>
>>>> YES， the actual charging is coming from patch 3. This patch just
>>>> delivers the BIO page's  memcg to the current task which is not the
>>>> consumer.
>>>>
>>>>>
>>>>> (3) Zswap recently implemented per-memcg charging of compressed
>>>>> objects in a much simpler way. If your main interest is #2 (which is
>>>>> what I understand from the commit log), it seems like zswap might be
>>>>> providing this already? Why can't you use zswap? Is it the fact that
>>>>> zswap requires a backing swapfile?
>>>>
>>>> Thanks for your reply and review. Yes, the zswap requires a backing
>>>> swapfile. The I/O path is very complex, sometimes it will throttle the
>>>> whole system if some resources are short , so we hope to use zram.
>>>
>>> Is the only problem with zswap for you the requirement of a backing swapfile?
>>>
>>> If yes, I am in the early stages of developing a solution to make
>>> zswap work without a backing swapfile. This was discussed in LSF/MM
>>> [1]. Would this make zswap usable in for your use case?
>>
>> Out of curiosity, are there any other known pros/cons when using
>> zswap-without-swap instead of zram?
>>
>> I know that zram requires sizing (size of the virtual block device) and
>> consumes metadata, zswap doesn't.
> 
> We don't use zram in our data centers so I am not an expert about
> zram, but off the top of my head there are a few more advantages to
> zswap:

Thanks!

> (1) Better memcg support (which this series is attempting to address
> in zram, although in a much more complicated way).

Right. I think this patch also misses to update apply the charging in the recompress
case. (only triggered by user space IIUC)

> 
> (2) We internally have incompressible memory handling on top of zswap,
> which is something that we would like to upstream when
> zswap-without-swap is supported. Basically if a page does not compress
> well enough to save memory we reject it from zswap and make it
> unevictable (if there is no backing swapfile). The existence of zswap
> in the MM layer helps with this. Since zram is a block device from the
> MM perspective, it's more difficult to do something like this.
> Incompressible pages just sit in zram AFAICT.

I see. With ZRAM_HUGE we still have to store the uncompressed page
(because, it's a block device and has to hold that data).

> 
> (3) Writeback support. If you're running out of memory to store
> compressed pages you can add a swapfile in runtime and zswap will
> start writing to it freeing up space to compress more pages. This
> wouldn't be possible in the same way in zram. Zram supports writing to
> a backing device but in a more manual way (userspace has to write to
> an interface to tell zram to write some pages).

Right, that zram backing device stuff is really sub-optimal and only useful
in corner cases (most probably not datacenters).

What one can do with zram is to add a second swap device with lower priority.
Looking at my Fedora machine:

  $ cat /proc/swaps
Filename                                Type            Size            Used            Priority
/dev/dm-2                               partition       16588796        0               -2
/dev/zram0                              partition       8388604         0               100


Guess the difference here is that you won't be writing out the compressed
data to the disk, but anything the gets swapped out afterwards will
end up on the disk. I can see how the zswap behavior might be better in that case
(instead of swapping out some additional pages you relocate the
already-swapped-out-to-zswap pages to the disk).

-- 
Cheers,

David / dhildenb

Powered by blists - more mailing lists