Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755134AbcCOGjk (ORCPT ); Tue, 15 Mar 2016 02:39:40 -0400 Received: from mail-pf0-f179.google.com ([209.85.192.179]:34112 "EHLO mail-pf0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750823AbcCOGjb (ORCPT ); Tue, 15 Mar 2016 02:39:31 -0400 Date: Tue, 15 Mar 2016 15:40:53 +0900 From: Sergey Senozhatsky To: Minchan Kim Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, jlayton@poochiereds.net, bfields@fieldses.org, Vlastimil Babka , Joonsoo Kim , koct9i@gmail.com, aquini@redhat.com, virtualization@lists.linux-foundation.org, Mel Gorman , Hugh Dickins , Sergey Senozhatsky , rknize@motorola.com, Rik van Riel , Gioh Kim Subject: Re: [PATCH v1 11/19] zsmalloc: squeeze freelist into page->mapping Message-ID: <20160315064053.GF1464@swordfish> References: <1457681423-26664-1-git-send-email-minchan@kernel.org> <1457681423-26664-12-git-send-email-minchan@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1457681423-26664-12-git-send-email-minchan@kernel.org> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1077 Lines: 42 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? besides, it is more intuitive (at least to me) when 'offset' shortened to 'offt', not 'ofs'. -ss > { > - unsigned long obj; > + int i; > + unsigned long ofs; > + struct page *cursor; > + int nr_page; > > - if (!page) { > - VM_BUG_ON(obj_idx); > - return NULL; > - } > + ofs = obj_idx * class->size; > + cursor = first_page; > + nr_page = ofs >> PAGE_SHIFT; > > - obj = page_to_pfn(page) << OBJ_INDEX_BITS; > - obj |= ((obj_idx) & OBJ_INDEX_MASK); > - obj <<= OBJ_TAG_BITS; > + *ofs_in_page = ofs & ~PAGE_MASK; > + > + for (i = 0; i < nr_page; i++) > + cursor = get_next_page(cursor); > > - return (void *)obj; > + *obj_page = cursor; > }