| Message ID | 20240227030045.3443702-1-chengming.zhou@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/zsmalloc: don't need to save tag bit in handle | expand |
On (24/02/27 03:00), chengming.zhou@linux.dev wrote: > > We only need to save the position (pfn + obj_idx) in the handle, don't > need to save tag bit in handle. So one more bit can be used as obj_idx. [..] > mm/zsmalloc: don't need to save tag bit in handle Does this mean "don't need to reserve LSB for tag"? We still save allocated tag in the handle, that's what handle |= OBJ_ALLOCATED_TAG; does. > Actually, the tag bit is only useful in zspage's memory space, to tell > if an object is allocated or not. I'm not completely sure if I follow this sentence.
On 2024/2/27 15:52, Sergey Senozhatsky wrote: > On (24/02/27 03:00), chengming.zhou@linux.dev wrote: >> >> We only need to save the position (pfn + obj_idx) in the handle, don't >> need to save tag bit in handle. So one more bit can be used as obj_idx. > > [..] > >> mm/zsmalloc: don't need to save tag bit in handle > > Does this mean "don't need to reserve LSB for tag"? The head of object still need to reverve LSB, to save (handle | OBJ_ALLOCATED_TAG), only the handle doesn't need to reserve LSB, which save (pfn | obj_idx). > We still save allocated tag in the handle, that's what > > handle |= OBJ_ALLOCATED_TAG; Yes, this result will be saved in the head of each allocated object. > > does. > >> Actually, the tag bit is only useful in zspage's memory space, to tell >> if an object is allocated or not. > > I'm not completely sure if I follow this sentence. What I mean is that only the head of each allocated object need to reverve LSB, which is used to check if allocated or not. handle address -> handle (pfn + obj_idx) -> object: (handle | tag), real_object start I'm not sure if this makes it clearer? Thanks.
On (24/02/27 16:16), Chengming Zhou wrote: > On 2024/2/27 15:52, Sergey Senozhatsky wrote: > > On (24/02/27 03:00), chengming.zhou@linux.dev wrote: > >> > >> We only need to save the position (pfn + obj_idx) in the handle, don't > >> need to save tag bit in handle. So one more bit can be used as obj_idx. > > > > [..] > > > >> mm/zsmalloc: don't need to save tag bit in handle > > > > Does this mean "don't need to reserve LSB for tag"? > The head of object still need to reverve LSB, to save (handle | OBJ_ALLOCATED_TAG), > only the handle doesn't need to reserve LSB, which save (pfn | obj_idx). Correct. > > We still save allocated tag in the handle, that's what > > > > handle |= OBJ_ALLOCATED_TAG; > > Yes, this result will be saved in the head of each allocated object. Right, that's what I was talking about. > >> Actually, the tag bit is only useful in zspage's memory space, to tell > >> if an object is allocated or not. > > > > I'm not completely sure if I follow this sentence. > > What I mean is that only the head of each allocated object need to reverve LSB, > which is used to check if allocated or not. > > handle address -> handle (pfn + obj_idx) -> object: (handle | tag), real_object start > > I'm not sure if this makes it clearer? Yes, thanks. I think separating handle and object header in the commit message will be helpful.
On 2024/2/28 09:54, Sergey Senozhatsky wrote: > On (24/02/27 16:16), Chengming Zhou wrote: >> On 2024/2/27 15:52, Sergey Senozhatsky wrote: >>> On (24/02/27 03:00), chengming.zhou@linux.dev wrote: >>>> >>>> We only need to save the position (pfn + obj_idx) in the handle, don't >>>> need to save tag bit in handle. So one more bit can be used as obj_idx. >>> >>> [..] >>> >>>> mm/zsmalloc: don't need to save tag bit in handle >>> >>> Does this mean "don't need to reserve LSB for tag"? >> The head of object still need to reverve LSB, to save (handle | OBJ_ALLOCATED_TAG), >> only the handle doesn't need to reserve LSB, which save (pfn | obj_idx). > > Correct. > >>> We still save allocated tag in the handle, that's what >>> >>> handle |= OBJ_ALLOCATED_TAG; >> >> Yes, this result will be saved in the head of each allocated object. > > Right, that's what I was talking about. > >>>> Actually, the tag bit is only useful in zspage's memory space, to tell >>>> if an object is allocated or not. >>> >>> I'm not completely sure if I follow this sentence. >> >> What I mean is that only the head of each allocated object need to reverve LSB, >> which is used to check if allocated or not. >> >> handle address -> handle (pfn + obj_idx) -> object: (handle | tag), real_object start >> >> I'm not sure if this makes it clearer? > > Yes, thanks. I think separating handle and object header in the commit > message will be helpful. Right, I will improve the commit message and send v2. Thanks!
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c index 63ec385cd670..7d7cb3eaabe0 100644 --- a/mm/zsmalloc.c +++ b/mm/zsmalloc.c @@ -110,7 +110,7 @@ #define OBJ_TAG_BITS 1 #define OBJ_TAG_MASK OBJ_ALLOCATED_TAG -#define OBJ_INDEX_BITS (BITS_PER_LONG - _PFN_BITS - OBJ_TAG_BITS) +#define OBJ_INDEX_BITS (BITS_PER_LONG - _PFN_BITS) #define OBJ_INDEX_MASK ((_AC(1, UL) << OBJ_INDEX_BITS) - 1) #define HUGE_BITS 1 @@ -737,14 +737,12 @@ static struct page *get_next_page(struct page *page) static void obj_to_location(unsigned long obj, struct page **page, unsigned int *obj_idx) { - obj >>= OBJ_TAG_BITS; *page = pfn_to_page(obj >> OBJ_INDEX_BITS); *obj_idx = (obj & OBJ_INDEX_MASK); } static void obj_to_page(unsigned long obj, struct page **page) { - obj >>= OBJ_TAG_BITS; *page = pfn_to_page(obj >> OBJ_INDEX_BITS); } @@ -759,7 +757,6 @@ static unsigned long location_to_obj(struct page *page, unsigned int obj_idx) obj = page_to_pfn(page) << OBJ_INDEX_BITS; obj |= obj_idx & OBJ_INDEX_MASK; - obj <<= OBJ_TAG_BITS; return obj; }