All of lore.kernel.org
 help / color / mirror / Atom feed
From: Minchan Kim <minchan@kernel.org>
To: Dan Streetman <ddstreet@ieee.org>
Cc: Linux-MM <linux-mm@kvack.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Nitin Gupta <ngupta@vflare.org>,
	Seth Jennings <sjennings@variantweb.net>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 01/10] zsmalloc: fix init_zspage free obj linking
Date: Mon, 15 Sep 2014 08:24:27 +0900	[thread overview]
Message-ID: <20140914232427.GD2160@bbox> (raw)
In-Reply-To: <CALZtONAuJhgZLJECxwQOyKPj2n02d+521d+eHCkqLjjc=Ba9FQ@mail.gmail.com>

On Fri, Sep 12, 2014 at 12:43:22PM -0400, Dan Streetman wrote:
> On Fri, Sep 12, 2014 at 12:59 AM, Minchan Kim <minchan@kernel.org> wrote:
> > On Thu, Sep 11, 2014 at 04:53:52PM -0400, Dan Streetman wrote:
> >> When zsmalloc creates a new zspage, it initializes each object it contains
> >> with a link to the next object, so that the zspage has a singly-linked list
> >> of its free objects.  However, the logic that sets up the links is wrong,
> >> and in the case of objects that are precisely aligned with the page boundries
> >> (e.g. a zspage with objects that are 1/2 PAGE_SIZE) the first object on the
> >> next page is skipped, due to incrementing the offset twice.  The logic can be
> >> simplified, as it doesn't need to calculate how many objects can fit on the
> >> current page; simply checking the offset for each object is enough.
> >
> > If objects are precisely aligned with the page boundary, pages_per_zspage
> > should be 1 so there is no next page.
> 
> ah, ok.  I wonder if it should be changed anyway so it doesn't rely on
> that detail, in case that's ever changed in the future.  It's not
> obvious the existing logic relies on that for correct operation.  And
> this simplifies the logic too.

Correct description and resend if you want.

-- 
Kind regards,
Minchan Kim

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Minchan Kim <minchan@kernel.org>
To: Dan Streetman <ddstreet@ieee.org>
Cc: Linux-MM <linux-mm@kvack.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Nitin Gupta <ngupta@vflare.org>,
	Seth Jennings <sjennings@variantweb.net>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 01/10] zsmalloc: fix init_zspage free obj linking
Date: Mon, 15 Sep 2014 08:24:27 +0900	[thread overview]
Message-ID: <20140914232427.GD2160@bbox> (raw)
In-Reply-To: <CALZtONAuJhgZLJECxwQOyKPj2n02d+521d+eHCkqLjjc=Ba9FQ@mail.gmail.com>

On Fri, Sep 12, 2014 at 12:43:22PM -0400, Dan Streetman wrote:
> On Fri, Sep 12, 2014 at 12:59 AM, Minchan Kim <minchan@kernel.org> wrote:
> > On Thu, Sep 11, 2014 at 04:53:52PM -0400, Dan Streetman wrote:
> >> When zsmalloc creates a new zspage, it initializes each object it contains
> >> with a link to the next object, so that the zspage has a singly-linked list
> >> of its free objects.  However, the logic that sets up the links is wrong,
> >> and in the case of objects that are precisely aligned with the page boundries
> >> (e.g. a zspage with objects that are 1/2 PAGE_SIZE) the first object on the
> >> next page is skipped, due to incrementing the offset twice.  The logic can be
> >> simplified, as it doesn't need to calculate how many objects can fit on the
> >> current page; simply checking the offset for each object is enough.
> >
> > If objects are precisely aligned with the page boundary, pages_per_zspage
> > should be 1 so there is no next page.
> 
> ah, ok.  I wonder if it should be changed anyway so it doesn't rely on
> that detail, in case that's ever changed in the future.  It's not
> obvious the existing logic relies on that for correct operation.  And
> this simplifies the logic too.

Correct description and resend if you want.

-- 
Kind regards,
Minchan Kim

  reply	other threads:[~2014-09-14 23:24 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-11 20:53 [PATCH 00/10] implement zsmalloc shrinking Dan Streetman
2014-09-11 20:53 ` Dan Streetman
2014-09-11 20:53 ` [PATCH 01/10] zsmalloc: fix init_zspage free obj linking Dan Streetman
2014-09-11 20:53   ` Dan Streetman
2014-09-12  3:16   ` Seth Jennings
2014-09-12  4:59   ` Minchan Kim
2014-09-12  4:59     ` Minchan Kim
2014-09-12 16:43     ` Dan Streetman
2014-09-12 16:43       ` Dan Streetman
2014-09-14 23:24       ` Minchan Kim [this message]
2014-09-14 23:24         ` Minchan Kim
2014-09-15 20:58         ` [PATCH] zsmalloc: simplify " Dan Streetman
2014-09-15 20:58           ` Dan Streetman
2014-09-16  2:41           ` Minchan Kim
2014-09-16  2:41             ` Minchan Kim
2014-09-11 20:53 ` [PATCH 02/10] zsmalloc: add fullness group list for ZS_FULL zspages Dan Streetman
2014-09-11 20:53   ` Dan Streetman
2014-09-11 20:53 ` [PATCH 03/10] zsmalloc: always update lru ordering of each zspage Dan Streetman
2014-09-11 20:53   ` Dan Streetman
2014-09-12  3:20   ` Seth Jennings
2014-09-11 20:53 ` [PATCH 04/10] zsmalloc: move zspage obj freeing to separate function Dan Streetman
2014-09-11 20:53   ` Dan Streetman
2014-09-11 20:53 ` [PATCH 05/10] zsmalloc: add atomic index to find zspage to reclaim Dan Streetman
2014-09-11 20:53   ` Dan Streetman
2014-09-11 20:53 ` [PATCH 06/10] zsmalloc: add zs_ops to zs_pool Dan Streetman
2014-09-11 20:53   ` Dan Streetman
2014-09-11 20:53 ` [PATCH 07/10] zsmalloc: add obj_handle_is_free() Dan Streetman
2014-09-11 20:53   ` Dan Streetman
2014-09-11 20:53 ` [PATCH 08/10] zsmalloc: add reclaim_zspage() Dan Streetman
2014-09-11 20:53   ` Dan Streetman
2014-09-11 20:54 ` [PATCH 09/10] zsmalloc: add zs_shrink() Dan Streetman
2014-09-11 20:54   ` Dan Streetman
2014-09-11 20:54 ` [PATCH 10/10] zsmalloc: implement zs_zpool_shrink() with zs_shrink() Dan Streetman
2014-09-11 20:54   ` Dan Streetman
2014-09-12  3:14 ` [PATCH 00/10] implement zsmalloc shrinking Seth Jennings
2014-09-12  5:46 ` Minchan Kim
2014-09-12  5:46   ` Minchan Kim
2014-09-12 17:05   ` Dan Streetman
2014-09-12 17:05     ` Dan Streetman
2014-09-15  0:00     ` Minchan Kim
2014-09-15  0:00       ` Minchan Kim
2014-09-15 14:29       ` Dan Streetman
2014-09-15 14:29         ` Dan Streetman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140914232427.GD2160@bbox \
    --to=minchan@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=ddstreet@ieee.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ngupta@vflare.org \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=sjennings@variantweb.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.