On Thu, Mar 17, 2016 at 08:09:50PM +0800, YiPing Xu wrote: > > > On 2016/3/15 14:51, Minchan Kim wrote: > >On Tue, Mar 15, 2016 at 03:40:53PM +0900, Sergey Senozhatsky wrote: > >>On (03/11/16 16:30), Minchan Kim wrote: > >>>-static void *location_to_obj(struct page *page, unsigned long obj_idx) > >>>+static void objidx_to_page_and_ofs(struct size_class *class, > >>>+ struct page *first_page, > >>>+ unsigned long obj_idx, > >>>+ struct page **obj_page, > >>>+ unsigned long *ofs_in_page) > >> > >>this looks big; 5 params, function "returning" both page and offset... > >>any chance to split it in two steps, perhaps? > > > >Yes, it's rather ugly but I don't have a good idea. > >Feel free to suggest if you have a better idea. > > > >> > >>besides, it is more intuitive (at least to me) when 'offset' > >>shortened to 'offt', not 'ofs'. > > the purpose to get 'obj_page' and 'ofs_in_page' is to map the page > and get the meta-data pointer in the page, so, we can finish this in > a single function. > > just like this, and maybe we could have a better function name > > static unsigned long *map_handle(struct size_class *class, > struct page *first_page, unsigned long obj_idx) > { > struct page *cursor = first_page; > unsigned long offset = obj_idx * class->size; > int nr_page = offset >> PAGE_SHIFT; > unsigned long offset_in_page = offset & ~PAGE_MASK; > void *addr; > int i; > > if (class->huge) { > VM_BUG_ON_PAGE(!is_first_page(page), page); > return &page_private(page); > } > > for (i = 0; i < nr_page; i++) > cursor = get_next_page(cursor); > > addr = kmap_atomic(cursor); > > return addr + offset_in_page; > } > > static void unmap_handle(unsigned long *addr) > { > if (class->huge) { > return; > } > > kunmap_atomic(addr & ~PAGE_MASK); > } > > all functions called "objidx_to_page_and_ofs" could use it like > this, for example: > > static unsigned long handle_from_obj(struct size_class *class, > struct page *first_page, int obj_idx) > { > unsigned long *head = map_handle(class, first_page, obj_idx); > > if (*head & OBJ_ALLOCATED_TAG) > handle = *head & ~OBJ_ALLOCATED_TAG; > > unmap_handle(*head); > > return handle; > } > > 'freeze_zspage', u'nfreeze_zspage' use it in the same way. > > but in 'obj_malloc', we still have to get the page to get obj. > > obj = location_to_obj(m_page, obj); Yes, That's why I didn't use such pattern. I didn't want to add unnecessary overhead in that hot path. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>