All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
To: Richard Weinberger <richard@nod.at>,
	hch@infradead.org, axboe@fb.com, dedekind1@gmail.com
Cc: linux-mtd@lists.infradead.org, computersforpeace@gmail.com,
	dwmw2@infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] UBI: Block: Add blk-mq support
Date: Sun, 02 Nov 2014 19:27:54 -0300	[thread overview]
Message-ID: <5456AFEA.6000507@free-electrons.com> (raw)
In-Reply-To: <5456AE72.1010409@nod.at>

[-- Attachment #1: Type: text/plain, Size: 2108 bytes --]

On 11/02/2014 07:21 PM, Richard Weinberger wrote:
> Ezequiel,
> 
> Am 02.11.2014 um 22:52 schrieb Ezequiel Garcia:
>> Maybe you can explain a bit better what's this all about?
> 
> In short, blk-mq is the future and the current blk interface will be legacy. :-)
> Christoph asked me to convert the MTD block drivers to blk-mq.
> 

Ah, OK. That makes sense then.

>> Both the commit that introduces blk-mq and the paper on it talk about
>> high IOPS devices, multi-core, NUMA systems. I'm not sure this is the
>> case for UBI-based devices.
>>
>> Probably some numbers would help us decide. Does the patch increases the
>> dynamic memory footprint? Is there any performance benefit?
> 
> I did a very rough micro benchmark:
> 
> root@(none):~# dd if=/dev/ubiblock0_0 of=/dev/null bs=1M
> 121+1 records in
> 121+1 records out
> 127420416 bytes (127 MB) copied, 1.59056 s, 80.1 MB/s
> 
> vs.
> 
> root@(none):~# dd if=/dev/ubiblock0_0 of=/dev/null bs=1M
> 121+1 records in
> 121+1 records out
> 127420416 bytes (127 MB) copied, 0.916117 s, 139 MB/s
> 
> So, yes there is a performance gain.
> 

Wow. Where did you run this and on top of what storage device?

I'm still interested in the memory footprint, UBI is already heavy enough.

>> I kind of like the negative diffstat, but the code doesn't look cleaner
>> or simpler.
>>
>> In other words, we need a good reason before we agree on making this
>> "zen style" driver more complex.
> 
> After reading my patch again I think we could move ubiblock_read_to_sg()
> to kapi.c or io.c. It is rather generic and maybe we can tun more UBI users to
> scattergather such that less vmalloc()s are needed.
> 
> This would also make the diffstat nicer...
> 

Yes, any additional effort to make the current patch any simpler would
be great. In its current form it seems rather cumbersome to me.

If you can re-submit something better and put a more verbose commit log,
I'd really appreciate it :)
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
To: Richard Weinberger <richard@nod.at>,
	hch@infradead.org, axboe@fb.com, dedekind1@gmail.com
Cc: dwmw2@infradead.org, computersforpeace@gmail.com,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] UBI: Block: Add blk-mq support
Date: Sun, 02 Nov 2014 19:27:54 -0300	[thread overview]
Message-ID: <5456AFEA.6000507@free-electrons.com> (raw)
In-Reply-To: <5456AE72.1010409@nod.at>

[-- Attachment #1: Type: text/plain, Size: 2108 bytes --]

On 11/02/2014 07:21 PM, Richard Weinberger wrote:
> Ezequiel,
> 
> Am 02.11.2014 um 22:52 schrieb Ezequiel Garcia:
>> Maybe you can explain a bit better what's this all about?
> 
> In short, blk-mq is the future and the current blk interface will be legacy. :-)
> Christoph asked me to convert the MTD block drivers to blk-mq.
> 

Ah, OK. That makes sense then.

>> Both the commit that introduces blk-mq and the paper on it talk about
>> high IOPS devices, multi-core, NUMA systems. I'm not sure this is the
>> case for UBI-based devices.
>>
>> Probably some numbers would help us decide. Does the patch increases the
>> dynamic memory footprint? Is there any performance benefit?
> 
> I did a very rough micro benchmark:
> 
> root@(none):~# dd if=/dev/ubiblock0_0 of=/dev/null bs=1M
> 121+1 records in
> 121+1 records out
> 127420416 bytes (127 MB) copied, 1.59056 s, 80.1 MB/s
> 
> vs.
> 
> root@(none):~# dd if=/dev/ubiblock0_0 of=/dev/null bs=1M
> 121+1 records in
> 121+1 records out
> 127420416 bytes (127 MB) copied, 0.916117 s, 139 MB/s
> 
> So, yes there is a performance gain.
> 

Wow. Where did you run this and on top of what storage device?

I'm still interested in the memory footprint, UBI is already heavy enough.

>> I kind of like the negative diffstat, but the code doesn't look cleaner
>> or simpler.
>>
>> In other words, we need a good reason before we agree on making this
>> "zen style" driver more complex.
> 
> After reading my patch again I think we could move ubiblock_read_to_sg()
> to kapi.c or io.c. It is rather generic and maybe we can tun more UBI users to
> scattergather such that less vmalloc()s are needed.
> 
> This would also make the diffstat nicer...
> 

Yes, any additional effort to make the current patch any simpler would
be great. In its current form it seems rather cumbersome to me.

If you can re-submit something better and put a more verbose commit log,
I'd really appreciate it :)
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  reply	other threads:[~2014-11-02 22:30 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-02 13:00 [PATCH] UBI: Block: Add blk-mq support Richard Weinberger
2014-11-02 13:00 ` Richard Weinberger
2014-11-02 21:52 ` Ezequiel Garcia
2014-11-02 21:52   ` Ezequiel Garcia
2014-11-02 22:21   ` Richard Weinberger
2014-11-02 22:21     ` Richard Weinberger
2014-11-02 22:27     ` Ezequiel Garcia [this message]
2014-11-02 22:27       ` Ezequiel Garcia
2014-11-02 22:49       ` Richard Weinberger
2014-11-02 22:49         ` Richard Weinberger
2014-11-03  1:20         ` Jens Axboe
2014-11-03  1:20           ` Jens Axboe
2014-11-03  1:18   ` Jens Axboe
2014-11-03  1:18     ` Jens Axboe
2014-11-03  5:03 ` Ming Lei
2014-11-03  5:03   ` Ming Lei
2014-11-03 13:58   ` Ezequiel Garcia
2014-11-03 13:58     ` Ezequiel Garcia
2014-11-03 18:22     ` Richard Weinberger
2014-11-03 18:22       ` Richard Weinberger
2014-11-03  8:18 ` Christoph Hellwig
2014-11-03  8:18   ` Christoph Hellwig
2014-11-03  8:23   ` Richard Weinberger
2014-11-03  8:23     ` Richard Weinberger
2014-11-07  9:46     ` Artem Bityutskiy
2014-11-07  9:46       ` Artem Bityutskiy
2014-11-07  9:53       ` Richard Weinberger
2014-11-07  9:53         ` Richard Weinberger
2014-11-07  9:56         ` Artem Bityutskiy
2014-11-07  9:56           ` Artem Bityutskiy

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=5456AFEA.6000507@free-electrons.com \
    --to=ezequiel.garcia@free-electrons.com \
    --cc=axboe@fb.com \
    --cc=computersforpeace@gmail.com \
    --cc=dedekind1@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=richard@nod.at \
    /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.