All of lore.kernel.org
 help / color / mirror / Atom feed
From: Minchan Kim <minchan@kernel.org>
To: Weijie Yang <weijie.yang@samsung.com>
Cc: 'Joonsoo Kim' <js1304@gmail.com>,
	'Weijie Yang' <weijie.yang.kh@gmail.com>,
	'Davidlohr Bueso' <davidlohr@hp.com>,
	'Andrew Morton' <akpm@linux-foundation.org>,
	'Seth Jennings' <sjennings@variantweb.net>,
	'Nitin Gupta' <ngupta@vflare.org>,
	'Sergey Senozhatsky' <sergey.senozhatsky@gmail.com>,
	'Bob Liu' <bob.liu@oracle.com>,
	'Dan Streetman' <ddstreet@ieee.org>,
	'Heesub Shin' <heesub.shin@samsung.com>,
	'linux-kernel' <linux-kernel@vger.kernel.org>,
	'Linux-MM' <linux-mm@kvack.org>
Subject: Re: [PATCH] zram: remove global tb_lock by using lock-free CAS
Date: Mon, 12 May 2014 14:15:05 +0900	[thread overview]
Message-ID: <20140512051505.GB32617@bbox> (raw)
In-Reply-To: <000001cf6c16$afe73800$0fb5a800$%yang@samsung.com>

On Sat, May 10, 2014 at 02:10:08PM +0800, Weijie Yang wrote:
> On Thu, May 8, 2014 at 2:24 PM, Minchan Kim <minchan@kernel.org> wrote:
> > On Wed, May 07, 2014 at 11:52:59PM +0900, Joonsoo Kim wrote:
> >> >> Most popular use of zram is the in-memory swap for small embedded system
> >> >> so I don't want to increase memory footprint without good reason although
> >> >> it makes synthetic benchmark. Alhought it's 1M for 1G, it isn't small if we
> >> >> consider compression ratio and real free memory after boot
> >>
> >> We can use bit spin lock and this would not increase memory footprint for 32 bit
> >> platform.
> >
> > Sounds like a idea.
> > Weijie, Do you mind testing with bit spin lock?
> 
> Yes, I re-test them.
> This time, I test each case 10 times, and take the average(KS/s).
> (the test machine and method are same like previous mail's)
> 
> Iozone test result:
> 
>       Test       BASE     CAS   spinlock   rwlock  bit_spinlock
> --------------------------------------------------------------
>  Initial write  1381094   1425435   1422860   1423075   1421521
>        Rewrite  1529479   1641199   1668762   1672855   1654910
>           Read  8468009  11324979  11305569  11117273  10997202
>        Re-read  8467476  11260914  11248059  11145336  10906486
>   Reverse Read  6821393   8106334   8282174   8279195   8109186
>    Stride read  7191093   8994306   9153982   8961224   9004434
>    Random read  7156353   8957932   9167098   8980465   8940476
> Mixed workload  4172747   5680814   5927825   5489578   5972253
>   Random write  1483044   1605588   1594329   1600453   1596010
>         Pwrite  1276644   1303108   1311612   1314228   1300960
>          Pread  4324337   4632869   4618386   4457870   4500166
> 
> Fio test result:
> 
>     Test     base     CAS    spinlock    rwlock  bit_spinlock
> -------------------------------------------------------------
> seq-write   933789   999357   1003298    995961   1001958
>  seq-read  5634130  6577930   6380861   6243912   6230006
>    seq-rw  1405687  1638117   1640256   1633903   1634459
>   rand-rw  1386119  1614664   1617211   1609267   1612471
> 
> 
> The base is v3.15.0-rc3, the others are per-meta entry lock.
> Every optimization method shows higher performance than the base, however,
> it is hard to say which method is the most appropriate.

It's not too big between CAS and bit_spinlock so I prefer general method.

> 
> To bit_spinlock, the modified code is mainly like this:
> 
> +#define ZRAM_FLAG_SHIFT 16
> +
> enum zram_pageflags {
>  	/* Page consists entirely of zeros */
> -	ZRAM_ZERO,
> +	ZRAM_ZERO = ZRAM_FLAG_SHIFT + 1,
> +	ZRAM_ACCESS,
>  
>  	__NR_ZRAM_PAGEFLAGS,
>  };
>  
>  /* Allocated for each disk page */
>  struct table {
>  	unsigned long handle;
> -	u16 size;	/* object size (excluding header) */
> -	u8 flags;
> +	unsigned long value;

Why does we need to change flags and size "unsigned long value"?
Couldn't we use existing flags with just adding new ZRAM_TABLE_LOCK?


>  } __aligned(4);
> 
> The lower ZRAM_FLAG_SHIFT bits of table.value is size, the higher bits
> is for zram_pageflags. By this means, it doesn't increase any memory
> overhead on both 32-bit and 64-bit system.
> 
> Any complaint or suggestions are welcomed.

Anyway, I'd like to go this way.
Pz, resend formal patch with a number.

Thanks!

> 
> >>
> >> Thanks.
> >>
> >> --
> >> 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>
> >
> > --
> > 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>

-- 
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: Weijie Yang <weijie.yang@samsung.com>
Cc: "'Joonsoo Kim'" <js1304@gmail.com>,
	"'Weijie Yang'" <weijie.yang.kh@gmail.com>,
	"'Davidlohr Bueso'" <davidlohr@hp.com>,
	"'Andrew Morton'" <akpm@linux-foundation.org>,
	"'Seth Jennings'" <sjennings@variantweb.net>,
	"'Nitin Gupta'" <ngupta@vflare.org>,
	"'Sergey Senozhatsky'" <sergey.senozhatsky@gmail.com>,
	"'Bob Liu'" <bob.liu@oracle.com>,
	"'Dan Streetman'" <ddstreet@ieee.org>,
	"'Heesub Shin'" <heesub.shin@samsung.com>,
	"'linux-kernel'" <linux-kernel@vger.kernel.org>,
	"'Linux-MM'" <linux-mm@kvack.org>
Subject: Re: [PATCH] zram: remove global tb_lock by using lock-free CAS
Date: Mon, 12 May 2014 14:15:05 +0900	[thread overview]
Message-ID: <20140512051505.GB32617@bbox> (raw)
In-Reply-To: <000001cf6c16$afe73800$0fb5a800$%yang@samsung.com>

On Sat, May 10, 2014 at 02:10:08PM +0800, Weijie Yang wrote:
> On Thu, May 8, 2014 at 2:24 PM, Minchan Kim <minchan@kernel.org> wrote:
> > On Wed, May 07, 2014 at 11:52:59PM +0900, Joonsoo Kim wrote:
> >> >> Most popular use of zram is the in-memory swap for small embedded system
> >> >> so I don't want to increase memory footprint without good reason although
> >> >> it makes synthetic benchmark. Alhought it's 1M for 1G, it isn't small if we
> >> >> consider compression ratio and real free memory after boot
> >>
> >> We can use bit spin lock and this would not increase memory footprint for 32 bit
> >> platform.
> >
> > Sounds like a idea.
> > Weijie, Do you mind testing with bit spin lock?
> 
> Yes, I re-test them.
> This time, I test each case 10 times, and take the average(KS/s).
> (the test machine and method are same like previous mail's)
> 
> Iozone test result:
> 
>       Test       BASE     CAS   spinlock   rwlock  bit_spinlock
> --------------------------------------------------------------
>  Initial write  1381094   1425435   1422860   1423075   1421521
>        Rewrite  1529479   1641199   1668762   1672855   1654910
>           Read  8468009  11324979  11305569  11117273  10997202
>        Re-read  8467476  11260914  11248059  11145336  10906486
>   Reverse Read  6821393   8106334   8282174   8279195   8109186
>    Stride read  7191093   8994306   9153982   8961224   9004434
>    Random read  7156353   8957932   9167098   8980465   8940476
> Mixed workload  4172747   5680814   5927825   5489578   5972253
>   Random write  1483044   1605588   1594329   1600453   1596010
>         Pwrite  1276644   1303108   1311612   1314228   1300960
>          Pread  4324337   4632869   4618386   4457870   4500166
> 
> Fio test result:
> 
>     Test     base     CAS    spinlock    rwlock  bit_spinlock
> -------------------------------------------------------------
> seq-write   933789   999357   1003298    995961   1001958
>  seq-read  5634130  6577930   6380861   6243912   6230006
>    seq-rw  1405687  1638117   1640256   1633903   1634459
>   rand-rw  1386119  1614664   1617211   1609267   1612471
> 
> 
> The base is v3.15.0-rc3, the others are per-meta entry lock.
> Every optimization method shows higher performance than the base, however,
> it is hard to say which method is the most appropriate.

It's not too big between CAS and bit_spinlock so I prefer general method.

> 
> To bit_spinlock, the modified code is mainly like this:
> 
> +#define ZRAM_FLAG_SHIFT 16
> +
> enum zram_pageflags {
>  	/* Page consists entirely of zeros */
> -	ZRAM_ZERO,
> +	ZRAM_ZERO = ZRAM_FLAG_SHIFT + 1,
> +	ZRAM_ACCESS,
>  
>  	__NR_ZRAM_PAGEFLAGS,
>  };
>  
>  /* Allocated for each disk page */
>  struct table {
>  	unsigned long handle;
> -	u16 size;	/* object size (excluding header) */
> -	u8 flags;
> +	unsigned long value;

Why does we need to change flags and size "unsigned long value"?
Couldn't we use existing flags with just adding new ZRAM_TABLE_LOCK?


>  } __aligned(4);
> 
> The lower ZRAM_FLAG_SHIFT bits of table.value is size, the higher bits
> is for zram_pageflags. By this means, it doesn't increase any memory
> overhead on both 32-bit and 64-bit system.
> 
> Any complaint or suggestions are welcomed.

Anyway, I'd like to go this way.
Pz, resend formal patch with a number.

Thanks!

> 
> >>
> >> Thanks.
> >>
> >> --
> >> 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>
> >
> > --
> > 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>

-- 
Kind regards,
Minchan Kim

  reply	other threads:[~2014-05-12  5:12 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-05  4:01 [PATCH] zram: remove global tb_lock by using lock-free CAS Weijie Yang
2014-05-05  4:01 ` Weijie Yang
2014-05-05 10:32 ` Sergey Senozhatsky
2014-05-05 10:32   ` Sergey Senozhatsky
2014-05-05 15:20 ` Seth Jennings
2014-05-05 15:20   ` Seth Jennings
2014-05-05 18:00   ` Davidlohr Bueso
2014-05-05 18:00     ` Davidlohr Bueso
2014-05-05 20:46     ` Andrew Morton
2014-05-05 20:46       ` Andrew Morton
2014-05-05 22:22       ` Davidlohr Bueso
2014-05-05 22:22         ` Davidlohr Bueso
2014-05-07  7:51         ` Weijie Yang
2014-05-07  7:51           ` Weijie Yang
2014-05-07  8:57           ` Minchan Kim
2014-05-07  8:57             ` Minchan Kim
2014-05-07  9:16             ` Weijie Yang
2014-05-07  9:16               ` Weijie Yang
2014-05-07 14:52               ` Joonsoo Kim
2014-05-07 14:52                 ` Joonsoo Kim
2014-05-08  6:24                 ` Minchan Kim
2014-05-08  6:24                   ` Minchan Kim
2014-05-10  6:10                   ` Weijie Yang
2014-05-10  6:10                     ` Weijie Yang
2014-05-12  5:15                     ` Minchan Kim [this message]
2014-05-12  5:15                       ` Minchan Kim
2014-05-12 14:49                       ` Davidlohr Bueso
2014-05-12 14:49                         ` Davidlohr Bueso
2014-05-13  0:03                         ` Minchan Kim
2014-05-13  0:03                           ` Minchan Kim

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=20140512051505.GB32617@bbox \
    --to=minchan@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=bob.liu@oracle.com \
    --cc=davidlohr@hp.com \
    --cc=ddstreet@ieee.org \
    --cc=heesub.shin@samsung.com \
    --cc=js1304@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ngupta@vflare.org \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=sjennings@variantweb.net \
    --cc=weijie.yang.kh@gmail.com \
    --cc=weijie.yang@samsung.com \
    /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.