All of lore.kernel.org
 help / color / mirror / Atom feed
From: Minchan Kim <minchan@kernel.org>
To: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Linux-MM <linux-mm@kvack.org>, Nitin Gupta <ngupta@vflare.org>,
	Jerome Marchand <jmarchan@redhat.com>,
	Ganesh Mahendran <opensource.ganesh@gmail.com>
Subject: Re: [PATCH v1 2/2] zram: remove init_lock in zram_make_request
Date: Thu, 29 Jan 2015 15:35:05 +0900	[thread overview]
Message-ID: <20150129063505.GA32331@blaptop> (raw)
In-Reply-To: <20150129060604.GC2555@swordfish>

On Thu, Jan 29, 2015 at 03:06:04PM +0900, Sergey Senozhatsky wrote:
> On (01/29/15 14:28), Minchan Kim wrote:
> > > I'm still concerned about performance numbers that I see on my x86_64.
> > > it's not always, but mostly slower. I'll give it another try (disable
> > > lockdep, etc.), but if we lose 10% on average then, sorry, I'm not so
> > > positive about srcu change and will tend to vote for your initial commit
> > > that simply moved meta free() out of init_lock and left locking as is
> > > (lockdep warning would have been helpful there, because otherwise it
> > > just looked like we change code w/o any reason).
> > > 
> > > what do you thunk?
> > 
> > Surely I agreee with you. If it suffers from 10% performance regression,
> > it's absolutely no go.
> > 
> > However, I believe it should be no loss because that's one of the reason
> > from RCU birth which should be really win in read-side lock path compared
> > to other locking.
> > 
> > Please test it with dd or something for block-based test for removing
> > noise from FS. I also will test it to confirm that with real machine.
> > 
> 
> do you test with a single dd thread/process?  just dd if=foo of=bar -c... or
> you start N `dd &' processes?

I tested it with multiple dd processes.

> 
> for a single writer there should be no difference, no doubt. I'm more
> interested in multi-writer/multi-reader/mixed use cases.
> 
> the options that I use are: iozone -t 3 -R -r 16K -s 60M -I +Z
> and -I is:
> 	-I  Use VxFS VX_DIRECT, O_DIRECT,or O_DIRECTIO for all file operations
> 
> with O_DIRECT I don't think there is a lot of noise, but I'll try to use
> different benchmarks a bit later.
> 

As you told, the data was not stable.
Anyway, when I read down_read implementation, it's one atomic instruction.
Hmm, it seems te be better for srcu_read_lock which does more things.
But I guessed most of overhead are from [de]compression, memcpy, clear_page
That's why I guessed we don't have measurable difference from that.
What's the data pattern if you use iozone? I guess it's really simple
pattern compressor can do fast. I used /dev/sda for dd write so
more realistic data. Anyway, if we has 10% regression even if the data
is simple, I never want to merge it.
I will test it carefully and if it turns out lots regression,
surely, I will not go with this and send the original patch again.

Thanks.

> 
> 	-ss

-- 
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: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Linux-MM <linux-mm@kvack.org>, Nitin Gupta <ngupta@vflare.org>,
	Jerome Marchand <jmarchan@redhat.com>,
	Ganesh Mahendran <opensource.ganesh@gmail.com>
Subject: Re: [PATCH v1 2/2] zram: remove init_lock in zram_make_request
Date: Thu, 29 Jan 2015 15:35:05 +0900	[thread overview]
Message-ID: <20150129063505.GA32331@blaptop> (raw)
In-Reply-To: <20150129060604.GC2555@swordfish>

On Thu, Jan 29, 2015 at 03:06:04PM +0900, Sergey Senozhatsky wrote:
> On (01/29/15 14:28), Minchan Kim wrote:
> > > I'm still concerned about performance numbers that I see on my x86_64.
> > > it's not always, but mostly slower. I'll give it another try (disable
> > > lockdep, etc.), but if we lose 10% on average then, sorry, I'm not so
> > > positive about srcu change and will tend to vote for your initial commit
> > > that simply moved meta free() out of init_lock and left locking as is
> > > (lockdep warning would have been helpful there, because otherwise it
> > > just looked like we change code w/o any reason).
> > > 
> > > what do you thunk?
> > 
> > Surely I agreee with you. If it suffers from 10% performance regression,
> > it's absolutely no go.
> > 
> > However, I believe it should be no loss because that's one of the reason
> > from RCU birth which should be really win in read-side lock path compared
> > to other locking.
> > 
> > Please test it with dd or something for block-based test for removing
> > noise from FS. I also will test it to confirm that with real machine.
> > 
> 
> do you test with a single dd thread/process?  just dd if=foo of=bar -c... or
> you start N `dd &' processes?

I tested it with multiple dd processes.

> 
> for a single writer there should be no difference, no doubt. I'm more
> interested in multi-writer/multi-reader/mixed use cases.
> 
> the options that I use are: iozone -t 3 -R -r 16K -s 60M -I +Z
> and -I is:
> 	-I  Use VxFS VX_DIRECT, O_DIRECT,or O_DIRECTIO for all file operations
> 
> with O_DIRECT I don't think there is a lot of noise, but I'll try to use
> different benchmarks a bit later.
> 

As you told, the data was not stable.
Anyway, when I read down_read implementation, it's one atomic instruction.
Hmm, it seems te be better for srcu_read_lock which does more things.
But I guessed most of overhead are from [de]compression, memcpy, clear_page
That's why I guessed we don't have measurable difference from that.
What's the data pattern if you use iozone? I guess it's really simple
pattern compressor can do fast. I used /dev/sda for dd write so
more realistic data. Anyway, if we has 10% regression even if the data
is simple, I never want to merge it.
I will test it carefully and if it turns out lots regression,
surely, I will not go with this and send the original patch again.

Thanks.

> 
> 	-ss

-- 
Kind regards,
Minchan Kim

  reply	other threads:[~2015-01-29  6:35 UTC|newest]

Thread overview: 90+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-28  8:15 [PATCH 1/2] zram: free meta table in zram_meta_free Minchan Kim
2015-01-28  8:15 ` [PATCH v1 2/2] zram: remove init_lock in zram_make_request Minchan Kim
2015-01-28 14:56   ` Sergey Senozhatsky
2015-01-28 14:56     ` Sergey Senozhatsky
2015-01-28 15:04     ` Sergey Senozhatsky
2015-01-28 15:04       ` Sergey Senozhatsky
2015-01-28 23:33     ` Minchan Kim
2015-01-28 23:33       ` Minchan Kim
2015-01-29  1:57       ` Sergey Senozhatsky
2015-01-29  2:01         ` Minchan Kim
2015-01-29  2:01           ` Minchan Kim
2015-01-29  2:22           ` Sergey Senozhatsky
2015-01-29  2:22             ` Sergey Senozhatsky
2015-01-29  5:28             ` Minchan Kim
2015-01-29  5:28               ` Minchan Kim
2015-01-29  6:06               ` Sergey Senozhatsky
2015-01-29  6:06                 ` Sergey Senozhatsky
2015-01-29  6:35                 ` Minchan Kim [this message]
2015-01-29  6:35                   ` Minchan Kim
2015-01-29  7:08                   ` Sergey Senozhatsky
2015-01-29  7:08                     ` Sergey Senozhatsky
2015-01-30 14:41                     ` Minchan Kim
2015-01-30 14:41                       ` Minchan Kim
2015-01-31 11:31                       ` Sergey Senozhatsky
2015-01-31 11:31                         ` Sergey Senozhatsky
2015-02-01 14:50                       ` Sergey Senozhatsky
2015-02-01 14:50                         ` Sergey Senozhatsky
2015-02-01 15:04                         ` Sergey Senozhatsky
2015-02-01 15:04                           ` Sergey Senozhatsky
2015-02-02  1:43                           ` Minchan Kim
2015-02-02  1:43                             ` Minchan Kim
2015-02-02  1:59                             ` Sergey Senozhatsky
2015-02-02  1:59                               ` Sergey Senozhatsky
2015-02-02  2:45                               ` Minchan Kim
2015-02-02  2:45                                 ` Minchan Kim
2015-02-02  3:47                                 ` Sergey Senozhatsky
2015-02-02  3:47                                   ` Sergey Senozhatsky
2015-02-02  1:30                         ` Minchan Kim
2015-02-02  1:30                           ` Minchan Kim
2015-02-02  1:48                           ` Sergey Senozhatsky
2015-02-02  1:48                             ` Sergey Senozhatsky
2015-02-02  2:44                             ` Minchan Kim
2015-02-02  2:44                               ` Minchan Kim
2015-02-02  4:01                               ` Sergey Senozhatsky
2015-02-02  4:01                                 ` Sergey Senozhatsky
2015-02-02  4:28                                 ` Minchan Kim
2015-02-02  4:28                                   ` Minchan Kim
2015-02-02  5:09                                   ` Sergey Senozhatsky
2015-02-02  5:09                                     ` Sergey Senozhatsky
2015-02-02  5:18                                     ` Minchan Kim
2015-02-02  5:18                                       ` Minchan Kim
2015-02-02  5:28                                       ` Sergey Senozhatsky
2015-02-02  5:28                                         ` Sergey Senozhatsky
2015-02-02  5:10                                   ` Minchan Kim
2015-02-02  5:10                                     ` Minchan Kim
2015-01-30  0:20                   ` Sergey Senozhatsky
2015-01-29 13:48   ` Ganesh Mahendran
2015-01-29 13:48     ` Ganesh Mahendran
2015-01-29 15:12     ` Sergey Senozhatsky
2015-01-29 15:12       ` Sergey Senozhatsky
2015-01-30  7:52       ` Ganesh Mahendran
2015-01-30  7:52         ` Ganesh Mahendran
2015-01-30  8:08         ` Sergey Senozhatsky
2015-01-30  8:08           ` Sergey Senozhatsky
2015-01-31  8:50           ` Ganesh Mahendran
2015-01-31  8:50             ` Ganesh Mahendran
2015-01-31 11:07             ` Sergey Senozhatsky
2015-01-31 11:07               ` Sergey Senozhatsky
2015-01-31 12:59               ` Ganesh Mahendran
2015-01-31 12:59                 ` Ganesh Mahendran
2015-01-28 14:19 ` [PATCH 1/2] zram: free meta table in zram_meta_free Sergey Senozhatsky
2015-01-28 14:19   ` Sergey Senozhatsky
2015-01-28 23:17   ` Minchan Kim
2015-01-28 23:17     ` Minchan Kim
2015-01-29  1:49     ` Ganesh Mahendran
2015-01-29  1:49       ` Ganesh Mahendran
  -- strict thread matches above, loose matches on Subject: below --
2015-02-02  3:41 [PATCH v1 2/2] zram: remove init_lock in zram_make_request Minchan Kim
2015-02-02  3:41 ` Minchan Kim
2015-02-02  5:59 ` Sergey Senozhatsky
2015-02-02  5:59   ` Sergey Senozhatsky
2015-02-02  6:18   ` Minchan Kim
2015-02-02  6:18     ` Minchan Kim
2015-02-02  7:06     ` Sergey Senozhatsky
2015-02-02  7:06       ` Sergey Senozhatsky
2015-02-03  1:54       ` Sergey Senozhatsky
2015-02-03  1:54         ` Sergey Senozhatsky
2015-02-03  3:02         ` Minchan Kim
2015-02-03  3:02           ` Minchan Kim
2015-02-03  3:56           ` Sergey Senozhatsky
2015-02-03  3:56             ` Sergey Senozhatsky

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=20150129063505.GA32331@blaptop \
    --to=minchan@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=jmarchan@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ngupta@vflare.org \
    --cc=opensource.ganesh@gmail.com \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sergey.senozhatsky@gmail.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.