Hyeonggon Yoo <42.hyeyoo@xxxxxxxxx> 于2024年12月12日周四 08:12写道： > > On Wed, Dec 11, 2024 at 1:37 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > > > On Wed, Dec 11, 2024 at 12:46:43AM +0900, Hyeonggon Yoo wrote: > > > Talking about updating comments and code by replacing 'page' with 'zpdesc', > > > I'm not sure if it makes sense to replace all instances of 'page' with 'zpdesc'. > > > A zpdesc is a descriptor, not a page that contains data (at least that's > > > what I have been thinking while writing the initial patch series). > > > > Agreed. > > > > > In that context I'm still not sure if saying "a sub-zpdesc of a zspage", "lock zpdesc", > > > or "migrate zpdesc" makes sense because replacing 'page descriptor (aka. struct page)' with > > > 'zpool descriptor (aka. zpdesc)' doesn't mean zsmalloc is throwing away the conecept of pages. > > > > sub-zpdesc is a silly thing to say. subpage is equally silly. If it's > > a page, call it a page. > > Agreed. > > > However, locking the zpdesc does make sense -- we're locking the > > descriptor. > > That makes sense. > > > > > /* > > > > * Following is how we use various fields and flags of underlying > > > > - * struct page(s) to form a zspage. > > > > + * struct zpdesc(page) to form a zspage. > > > > * > > > > - * Usage of struct page fields: > > > > - * page->private: points to zspage > > > > - * page->index: links together all component pages of a zspage > > > > + * Usage of struct zpdesc fields: > > > > + * zpdesc->zspage: points to zspage > > > > + * zpdesc->next: links together all component zpdescs of a zspage > > > > * For the huge page, this is always 0, so we use this field > > > > * to store handle. > > > > - * page->page_type: PGTY_zsmalloc, lower 24 bits locate the first object > > > > - * offset in a subpage of a zspage > > > > - * > > > > - * Usage of struct page flags: > > > > - * PG_private: identifies the first component page > > > > - * PG_owner_priv_1: identifies the huge component page > > > > + * zpdesc->first_obj_offset: PGTY_zsmalloc, lower 24 bits locate the first > > > > + * object offset in a subpage of a zspage > > > > * > > > > + * Usage of struct zpdesc(page) flags: > > > > + * PG_private: identifies the first component zpdesc > > > > + * PG_lock: lock all component zpdescs for a zspage free, serialize with > > > > */ > > > > > > I think this comment is unnecessary, as it's already documented in mm/zpdesc.h. > > > It can be removed in patch 01. > > > > Agreed. I did think about doing that, so if you want to do it too, > > that's two votes for doing it. > > Will do in v9. > > > > > @@ -1351,7 +1353,7 @@ static unsigned long obj_malloc(struct zs_pool *pool, > > > > /* record handle in the header of allocated chunk */ > > > > link->handle = handle | OBJ_ALLOCATED_TAG; > > > > else > > > > - /* record handle to page->index */ > > > > + /* record handle to zpdesc->handle */ > > > > zspage->first_zpdesc->handle = handle | OBJ_ALLOCATED_TAG; > > > > > > the name of the field is already 'handle', > > > so we don't need a comment to explain it. > > > > Agreed. > > > > Do you want to take on producing v9 or do you want me to fold in your > > suggestions and send it? > > I will send v9 with my feedback adjusted this weekend. > Thank you for rebasing and pushing this forward. Very glad to see it is pushed forward. I am changing to a new career recently, so sorry for not putting some effort into it. Thanks a lot for you guys! > > Best, > Hyeonggon
