Re: [External] Re: [RFC PATCH 1/3] zram: charge the compressed RAM to the page's memcgroup

[David Hildenbrand <david@redhat.com>]

On 15.06.23 15:40, 贺中坤 wrote:
>> I suspect for the swap->zram we should always get charged pages, because
>> we're effectively writing out charged anon/shmem pages only -- without
>> any buffer in between.
> 
> Hi David，the charged memory will be released in swap->zram. New pages
> are allocated by alloc_zspage(), and we did not charge the page directly，but
> the objects(like  slab), because the zspage are shared by any memcg.
> 
>>
>> For the fs->zram or direct zram access device case I'm not so sure. It
>> highly depends on what gets mapped into the bio (e.g., a kernel buffer,
>> zeropage, ...). If it's a pagecache page, that should be charged and
>> we're good. No so sure about fs metadata or some other fs cases (e.g.,
>> write() to a file that bypass the pagecache).
>>
> 
> Yes， the pagecaches are charged in fs->zram, but  will be released if
> we drop the cache. the compressed objects are not charged.

Yes. But just to stress again, one issue I see is that if there is a 
page in the BIO that is not charged, you cannot charge the compressed page.

Assume you have some FS on that zram block device and  you want to make 
sure it gets properly charged to whoever is reading/writing a file on 
that filesystem. (imagine something like a compress shmem)

If a user (or the filesystem?) can trigger a BIO that has an uncharged 
page in it, it would not get charged accordingly.

The "easy" reproducer would have been O_DIRECT write() using the shared 
zeropage, but zram_write_page() is smart enough to optimize for that 
case (page_same_filled()). :)

Maybe I'm over-thinking this (well, the we do have partial I/O support, 
so something seems to be able to trigger such cases), and it would be 
great if someone with more FS->BIO experience could comment.

I'll note that this is fundamentally different to zswap, because with 
zswap you don't get arbitrary BIOs, you get an anon or shmem page (that 
should be charged).

-- 
Cheers,

David / dhildenb