public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Theodore Tso <tytso@mit.edu>
To: Artem Bityutskiy <dedekind@infradead.org>
Cc: Matt Mackall <mpm@selenic.com>,
	Josh Boyer <jwboyer@linux.vnet.ibm.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Frank Haverkamp <haver@vnet.ibm.com>,
	Christoph Hellwig <hch@infradead.org>,
	David Woodhouse <dwmw2@infradead.org>
Subject: Re: [PATCH 00/22 take 3] UBI: Unsorted Block Images
Date: Tue, 20 Mar 2007 09:52:31 -0400	[thread overview]
Message-ID: <20070320135231.GA6043@thunk.org> (raw)
In-Reply-To: <1174393549.17249.101.camel@sauron>

On Tue, Mar 20, 2007 at 02:25:49PM +0200, Artem Bityutskiy wrote:
> You failed to clearly define what is block until now, then you blame me
> that I do not understand you. So I see block = eraseblock, lets assume
> for further conversation.
> 
> OK. Suppose we have done what you say, although I _do not_ think it is
> makes a lot of sense. So, now we have a block device, with 128KiB block
> size. We have LVM, dm-wl or whatever stuff. Fine.
> 
> Do you realize that 128KiB is _huge_ block size, and performance will
> suck, and suck a lot if you utilize say, ext2 or whatever block device
> FS.

As a suggestion, let's stop right here and see if we can get both
sides talking in a more constructive fashion.  Maybe it's just me, but
I see both sides talking past each other in a rather dramatic fashion.

Linux seems to allow multiple implementations of things at the edge
(such as filesystems), but not at the core (devicemapper when in,
device mapper didn't; it's unlikely that we would have two competing
block device layers or two VFS layers, etc.).  The question then is
whether UBI and dm are close enough or not that should be one
subsystem or not.

There a number of red herrings that have been introduced in this
discussion; of *course* the existing block device layer can handle
FLASH devices; Matt is proposing that they be extended.  And of
*course* you woulnd't propose to use ext2 on top of an 128k blocksize,
anymore than you would force a flash filesystem to use a 4k or 512
byte blocksize; there are plenty of configurations that won't make
sense, and by itself this isn't an indictment of the core idea that
the block device layer and dm should be augmented to encompass flash
functionality.

As far as who gets to do the work, unfortunately sometimes the people
submitting the new code have to make the changes suggested by the
reviewer.  That's one of the prices that gets paid for mainline
inclusion.  It's different when someone asks for a completely new
feature, especially for code that is already in mainline; then, "feel
free to send a patch" is perfectly accepted.  But if it's a matter of
refactoring the code to fit in some other framework, that's often up
to the submitter to do, not the reviewer. 

Of course, it remains to be seen whether or not this is a good idea to
do in the first place; but some of the arguments being used to shoot
down Matt's suggestion aren't really good ones to begin with.

> To make it faster I have to have a way to do finer grained I/O:
> read/write to different positions of 128KiB block. Do you realize how
> much you will abuse all the generic block device infrastructure if you
> try to add this? Note, all levels up to LVM will need to have this. A
> believe it is braindead ((c) tglx) to add this feature.

OK, and this could be it.  But I suspect one of the things which may
be missing that make it easier for you to explain why what UBI is
doing is so different from the dm and block device stack is to include
the contents of:

	http://www.linux-mtd.infradead.org/faq/ubi.html
	http://www.linux-mtd.infradead.org/doc/ubi.html

and some system level documentation in a Documentation/ubi.txt file as
part of the patch set.  I don't think people completely understand the
high-level architecture of what UBI is trying to achieve.  What are
the interfaces at the top and the bottom of the stack?  For example,
the fact that UBI exports Logical Erase blocks that are not a
power-of-two (possibly 128k minus 128 bytes) means that it certainly
might not be a good match for the dm stack.  But why is that the case?
I can imagine good reasons for it, but a high-level description of the
design decisions would be very useful.

It would also help people understand why there are so many "units" in
UBI, since hopefully the high-level documentation would explain why
they fit together, and perhaps why some of the units weren't folded
together.  What value do they add as separate components?

There are hints of the overall system architecture in some of the
indivdual comments for data structures, but even reading all of those,
there isn't quite enough for people to figure out what it is; and that
may be causing some of these comments of people saying there's too
much code to evaluate, or why didn't you do it *this* way?   

Regards,

						- Ted

  reply	other threads:[~2007-03-20 13:53 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-03-14 15:19 [PATCH 00/22 take 3] UBI: Unsorted Block Images Artem Bityutskiy
2007-03-14 15:19 ` [PATCH 01/22 take 3] UBI: on-flash data structures header Artem Bityutskiy
2007-03-14 15:19 ` [PATCH 02/22 take 3] UBI: user-space API header Artem Bityutskiy
2007-03-14 15:19 ` [PATCH 03/22 take 3] UBI: kernel-space " Artem Bityutskiy
2007-03-14 15:19 ` [PATCH 04/22 take 3] UBI: internal header Artem Bityutskiy
2007-03-14 15:19 ` [PATCH 05/22 take 3] UBI: startup code Artem Bityutskiy
2007-03-14 15:20 ` [PATCH 06/22 take 3] UBI: scanning unit Artem Bityutskiy
2007-03-14 15:20 ` [PATCH 07/22 take 3] UBI: I/O unit Artem Bityutskiy
2007-03-14 15:20 ` [PATCH 08/22 take 3] UBI: volume table unit Artem Bityutskiy
2007-03-14 15:20 ` [PATCH 09/22 take 3] UBI: wear-leveling unit Artem Bityutskiy
2007-03-14 15:20 ` [PATCH 10/22 take 3] UBI: EBA unit Artem Bityutskiy
2007-03-15 19:07   ` Andrew Morton
2007-03-15 21:24     ` Randy Dunlap
2007-03-15 23:29       ` Josh Boyer
2007-03-16  1:49         ` Randy Dunlap
2007-03-16 10:23           ` Artem Bityutskiy
2007-03-16 10:21       ` Artem Bityutskiy
2007-03-16 14:55         ` Randy Dunlap
2007-03-16 10:14     ` Artem Bityutskiy
2007-03-14 15:20 ` [PATCH 11/22 take 3] UBI: user-interfaces unit Artem Bityutskiy
2007-03-14 15:20 ` [PATCH 12/22 take 3] UBI: update functionality Artem Bityutskiy
2007-03-14 15:20 ` [PATCH 13/22 take 3] UBI: accounting unit Artem Bityutskiy
2007-03-14 15:20 ` [PATCH 14/22 take 3] UBI: volume management functionality Artem Bityutskiy
2007-03-14 15:20 ` [PATCH 15/22 take 3] UBI: sysfs functionality Artem Bityutskiy
2007-03-14 15:20 ` [PATCH 16/22 take 3] UBI: character devices functionality Artem Bityutskiy
2007-03-14 15:21 ` [PATCH 17/22 take 3] UBI: gluebi functionality Artem Bityutskiy
2007-03-14 15:21 ` [PATCH 18/22 take 3] UBI: misc stuff Artem Bityutskiy
2007-03-14 15:21 ` [PATCH 19/22 take 3] UBI: debugging stuff Artem Bityutskiy
2007-03-14 15:21 ` [PATCH 20/22 take 3] UBI: JFFS2 UBI support Artem Bityutskiy
2007-03-14 15:21 ` [PATCH 21/22 take 3] UBI: update MAINTAINERS Artem Bityutskiy
2007-03-14 15:21 ` [PATCH 22/22 take 3] UBI: Linux build integration Artem Bityutskiy
2007-03-18 16:27 ` [PATCH 00/22 take 3] UBI: Unsorted Block Images Matt Mackall
2007-03-18 16:49   ` Artem Bityutskiy
2007-03-18 19:18     ` Matt Mackall
2007-03-18 20:31       ` Josh Boyer
2007-03-19 17:08         ` Matt Mackall
2007-03-19 18:16           ` Josh Boyer
2007-03-19 19:54             ` Matt Mackall
2007-03-19 20:18               ` Artem Bityutskiy
2007-03-19 21:05               ` Thomas Gleixner
2007-03-19 22:32                 ` Matt Mackall
2007-03-20  0:42                   ` Thomas Gleixner
2007-03-20  1:05                     ` Matt Mackall
2007-03-20  6:28                       ` Thomas Gleixner
2007-03-21 11:05                     ` Jörn Engel
2007-03-21 11:25                       ` Thomas Gleixner
2007-03-21 11:35                         ` Jörn Engel
2007-03-21 11:57                           ` Thomas Gleixner
2007-03-21 12:31                             ` Jörn Engel
2007-03-21 12:39                               ` Artem Bityutskiy
2007-03-21 11:36                         ` Artem Bityutskiy
2007-03-25 20:08                         ` Jörn Engel
2007-03-25 21:49                           ` David Lang
2007-03-25 22:55                             ` Jörn Engel
2007-03-25 23:46                               ` David Woodhouse
2007-03-26  0:01                                 ` Jörn Engel
2007-03-26  0:21                                   ` David Woodhouse
2007-03-26  1:04                                     ` Jörn Engel
2007-03-26  9:45                                       ` David Woodhouse
2007-03-26  9:51                                         ` Jörn Engel
2007-03-26 10:07                                           ` David Woodhouse
2007-03-26 10:02                                         ` Thomas Gleixner
2007-03-26 10:49                           ` Artem Bityutskiy
2007-03-26 11:30                             ` Jörn Engel
2007-03-19 21:06               ` Artem Bityutskiy
2007-03-19 21:36                 ` Matt Mackall
2007-03-20  0:43                   ` Thomas Gleixner
2007-03-20 12:25                   ` Artem Bityutskiy
2007-03-20 13:52                     ` Theodore Tso [this message]
2007-03-20 15:14                       ` Artem Bityutskiy
2007-03-20 15:59                       ` Josh Boyer
2007-03-20 18:58                         ` David Lang
2007-03-20 20:05                           ` Artem Bityutskiy
2007-03-20 21:36                             ` David Woodhouse
2007-03-21  8:54                               ` Artem Bityutskiy
2007-03-20 21:32                           ` David Woodhouse
2007-03-21 13:03                             ` Jörn Engel
2007-03-20 22:03                         ` Theodore Tso
2007-03-21  8:44                           ` Artem Bityutskiy
2007-03-21 13:50                             ` Theodore Tso
2007-03-21 13:59                               ` Josh Boyer
2007-03-21 14:02                               ` Artem Bityutskiy
2007-03-21 15:38                               ` Frank Haverkamp
2007-03-21 20:26                                 ` David Lang
2007-03-20 12:13               ` Josh Boyer
2007-03-19 19:03           ` Thomas Gleixner
2007-03-19 20:12             ` Matt Mackall
2007-03-19 21:04               ` Thomas Gleixner

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=20070320135231.GA6043@thunk.org \
    --to=tytso@mit.edu \
    --cc=dedekind@infradead.org \
    --cc=dwmw2@infradead.org \
    --cc=haver@vnet.ibm.com \
    --cc=hch@infradead.org \
    --cc=jwboyer@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpm@selenic.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox