All of lore.kernel.org
 help / color / mirror / Atom feed
From: Artem Bityutskiy <dedekind1@gmail.com>
To: Ezequiel Garcia <elezegarcia@gmail.com>
Cc: Michael Opdenacker <michael.opdenacker@free-electrons.com>,
	David Woodhouse <dwmw2@infradead.org>,
	linux-mtd@lists.infradead.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Tim Bird <tim.bird@am.sony.com>
Subject: Re: [RFC/PATCH 0/1] ubi: Add ubiblock driver
Date: Fri, 30 Nov 2012 13:08:17 +0200	[thread overview]
Message-ID: <1354273697.30168.87.camel@sauron.fi.intel.com> (raw)
In-Reply-To: <CALF0-+WuaUn5b=2XrzqW7pvM=N1MfQf=sFwLFOrokTkHbOXh3w@mail.gmail.com>

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

Hi, without the reveiw, I can say that overall this sounds good, thanks!

On Tue, 2012-11-20 at 19:39 -0300, Ezequiel Garcia wrote:
> Also, I've decided to make block devices get automatically created for
> each ubi volume present.
> This has been done to match gluebi behavior of automatically create an
> mtd device
> per ubi volume, and to save us the usertool trouble.
> 
> The latter is the most important reason: a new usertool means an added
> complexity
> for the user and yet more crap to maintain.

> I don't know how many ubi volumes a user typically creates, but I
> expect not to be too many.

I think I saw something like 8-10 in some peoples' reports.

> * Read/write support
> 
> Yes, this implementation supports read/write access.
> It's expected to work fairly well because the request queue at block elevator
> is suppose to order block transfers to be space-effective.
> In other words, it's expected that reads and writes gets ordered
> to point to the same LEB (see Artem's hint at [1]).
> 
> To help this and reduce access to the UBI volume, a 1-LEB size
> write-back cache has been implemented (similar to the one at mtdblock.c).
> 
> Every read and every write, goes through this cache and the write is only
> done when a request arrives to read or write to a different LEB or when
> the device is released, when the last file handle is closed.

Sounds good, but you should make sure you flush the cache when the
file-system syncs a file. You can consider this as a disk cache.
File-systems usually sends I/O barriers when the disk cache has to be
flushed. I guess this is what you should also do.

> This cache is 1-LEB bytes, vmalloced at open() and freed at release().

Is it per-block device? Then I am not sure it is a good idea to
automatically create them for every volume...

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Artem Bityutskiy <dedekind1@gmail.com>
To: Ezequiel Garcia <elezegarcia@gmail.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-mtd@lists.infradead.org,
	David Woodhouse <dwmw2@infradead.org>,
	Tim Bird <tim.bird@am.sony.com>,
	Michael Opdenacker <michael.opdenacker@free-electrons.com>
Subject: Re: [RFC/PATCH 0/1] ubi: Add ubiblock driver
Date: Fri, 30 Nov 2012 13:08:17 +0200	[thread overview]
Message-ID: <1354273697.30168.87.camel@sauron.fi.intel.com> (raw)
In-Reply-To: <CALF0-+WuaUn5b=2XrzqW7pvM=N1MfQf=sFwLFOrokTkHbOXh3w@mail.gmail.com>

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

Hi, without the reveiw, I can say that overall this sounds good, thanks!

On Tue, 2012-11-20 at 19:39 -0300, Ezequiel Garcia wrote:
> Also, I've decided to make block devices get automatically created for
> each ubi volume present.
> This has been done to match gluebi behavior of automatically create an
> mtd device
> per ubi volume, and to save us the usertool trouble.
> 
> The latter is the most important reason: a new usertool means an added
> complexity
> for the user and yet more crap to maintain.

> I don't know how many ubi volumes a user typically creates, but I
> expect not to be too many.

I think I saw something like 8-10 in some peoples' reports.

> * Read/write support
> 
> Yes, this implementation supports read/write access.
> It's expected to work fairly well because the request queue at block elevator
> is suppose to order block transfers to be space-effective.
> In other words, it's expected that reads and writes gets ordered
> to point to the same LEB (see Artem's hint at [1]).
> 
> To help this and reduce access to the UBI volume, a 1-LEB size
> write-back cache has been implemented (similar to the one at mtdblock.c).
> 
> Every read and every write, goes through this cache and the write is only
> done when a request arrives to read or write to a different LEB or when
> the device is released, when the last file handle is closed.

Sounds good, but you should make sure you flush the cache when the
file-system syncs a file. You can consider this as a disk cache.
File-systems usually sends I/O barriers when the disk cache has to be
flushed. I guess this is what you should also do.

> This cache is 1-LEB bytes, vmalloced at open() and freed at release().

Is it per-block device? Then I am not sure it is a good idea to
automatically create them for every volume...

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

  parent reply	other threads:[~2012-11-30 11:07 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-20 22:39 [RFC/PATCH 0/1] ubi: Add ubiblock driver Ezequiel Garcia
2012-11-21  5:28 ` Ricard Wanderlof
2012-11-21  9:56   ` Thomas Petazzoni
2012-11-21 10:00     ` Ricard Wanderlof
2012-11-21 10:05       ` Thomas Petazzoni
2012-11-24 21:02         ` Ezequiel Garcia
2012-11-25  7:36           ` Shmulik Ladkani
2012-11-30 11:24             ` Artem Bityutskiy
2012-12-02 17:39               ` Shmulik Ladkani
2012-12-03  0:58                 ` Ezequiel Garcia
2012-12-03 19:33                   ` Ezequiel Garcia
2012-12-03 21:03                     ` Shmulik Ladkani
2012-11-30 11:18         ` Artem Bityutskiy
2012-11-30 11:15   ` Artem Bityutskiy
2012-11-21 10:00 ` Thomas Petazzoni
2012-11-21 10:00   ` Thomas Petazzoni
2012-11-21 10:42   ` Ezequiel Garcia
2012-11-21 10:42     ` Ezequiel Garcia
2012-11-30 11:25   ` Artem Bityutskiy
2012-11-30 11:25     ` Artem Bityutskiy
2012-11-30 11:08 ` Artem Bityutskiy [this message]
2012-11-30 11:08   ` Artem Bityutskiy
2012-11-30 20:43   ` Ezequiel Garcia
2012-11-30 20:43     ` Ezequiel Garcia

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=1354273697.30168.87.camel@sauron.fi.intel.com \
    --to=dedekind1@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=elezegarcia@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=michael.opdenacker@free-electrons.com \
    --cc=tim.bird@am.sony.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.