On Mon, Aug 7, 2023 at 11:26 PM xiujianfeng <xiujianfeng@xxxxxxxxxx> wrote:
>
> Hi Yosry,
>
> On 2023/8/8 13:59, Yosry Ahmed wrote:
> > Hi Xiu,
> >
> > On Mon, Aug 7, 2023 at 7:49 PM Xiu Jianfeng <xiujianfeng@xxxxxxxxxxxxxxx> wrote:
> >>
> >> From: Xiu Jianfeng <xiujianfeng@xxxxxxxxxx>
> >>
> >> Since commit 0bb488498c98 ("mm: zswap: remove zswap_header"), the
> >> 'offset' has been replaced by swpentry, update the comment for it,
> >> and also add comment for 'objcg'.
> >>
> >> Signed-off-by: Xiu Jianfeng <xiujianfeng@xxxxxxxxxx>
> >> ---
> >> mm/zswap.c | 3 ++-
> >> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/mm/zswap.c b/mm/zswap.c
> >> index 5b56d38e7339..d7863c139c82 100644
> >> --- a/mm/zswap.c
> >> +++ b/mm/zswap.c
> >> @@ -182,7 +182,7 @@ struct zswap_pool {
> >> * page within zswap.
> >> *
> >> * rbnode - links the entry into red-black tree for the appropriate swap type
> >> - * offset - the swap offset for the entry. Index into the red-black tree.
> >> + * swpentry - the swap entry for the entry, include 'type' and 'offset' fields
> >
> >
> > This is not the place to document that a swap entry has 'type' and
> > 'offset' fields.
> >
> > I would also avoid the repetitive 'entry'. Maybe something like:
> > swpentry - associated swap entry, the offset indexes into the red-black tree
> >
> >>
> >> * refcount - the number of outstanding reference to the entry. This is needed
> >> * to protect against premature freeing of the entry by code
> >> * concurrent calls to load, invalidate, and writeback. The lock
> >> @@ -195,6 +195,7 @@ struct zswap_pool {
> >> * pool - the zswap_pool the entry's data is in
> >> * handle - zpool allocation handle that stores the compressed page data
> >> * value - value of the same-value filled pages which have same content
> >> + * objcg - the obj_cgroup(actually memcg) this entry belongs to
> >
> >
> > Again, not the place to document that objcg is an abstraction layer
> > above a memcg.
> >
> > I would do something like:
> > objcg - the obj_cgroup that the compressed memory is charged to
>
> Thanks for your review, I've just sent out the v2 patch.
I will take a look, thanks.
(a CC would have been convenient ;) )
>
> >
> >>
> >> * lru - handle to the pool's lru used to evict pages.
> >> */
> >> struct zswap_entry {
> >> --
> >> 2.34.1
> >>
> >>
