[GIT PULL] logfs: bug fixes

[Linus Torvalds]

On Tue, Jan 31, 2012 at 9:02 AM, Prasad Joshi
<prasadjoshi.linux at gmail.com> wrote:
>
> I have pushed the signed tag on the master branch as suggested by you

Ok, this works, but please don't put auto-generated content with no
semantics in the tag message. The shortlog and diffstat don't actually
tell me anything new - I can see those from the git details directly.

I want to see them in the *email* - so that I know what I'm pulling
beforehand, and so that I know that you are aware of what you are
pushing me - but there's no point in putting them in the signed tag.

So please put your own explanations for what is going on (if there is
anything half-way interesting) in the signed tag message, but not
anything that can be seen automatically from git itself. You signed
it, that signature covers the contents, but explanations for humans
are interesting.

Anyway, it does have a conflict. And I can (and did) easily resolve
the conflict that I get, even if I can't really test my end result.

HOWEVER: the conflict has a major semantic component to it that I want
people to discuss and be aware of my resolution.

So the conflict is between

 - commit f2933e86ad93: "Logfs: Allow NULL block_isbad() methods"

 - commits 7086c19d0742: "mtd: introduce mtd_block_isbad interface"
and d58b27ed58a3: "logfs: do not use 'mtd->block_isbad' directly"

both of which change how mtd->block_isbad' is used (the first
semantically by logfs, the others change the interface)

Now, my reaction and resolution to the conflict is to actually take
the *semantics* from commit f2933e86ad93, and just make
mtd_block_isbad() return zero (false) if the 'block_isbad' function is
NULL. But that also means that now "mtd_can_have_bb()" always returns
0.

At the same time, "mtd_block_markbad()" will obviously return an error
if the low-level driver doesn't support bad blocks, so this is
somewhat non-symmetric, but it actually makes sense if a NULL
"block_isbad" function is considered to mean "I assume that all my
blocks are always good".

Anyway, it is possible that the MTD people react very negatively to
this semantic change, but quite frankly, 99% of all uses of the new
mtd_block_isbad() function treated it as a boolean, so returning an
error from it didn't actually *work* correctly - and also made that
whole mtd_can_have_bb() function kind of pointless.

Essentially nobody checked it anyway with a *very* few exceptions. And
the mtd_can_have_bb() function was imnsho clearly misdesigned for the
whole concept.

So I think my resolution is reasonable, but I also think that you guys
should look at

 - getting rid of mtd_can_have_bb() entirely as being stupid
(mtd_block_isbad() does the right thing regardless - if the MTD
supports bad blocks it will do the right thing, and if it doesn't, it
will do the best thing it can)

 - instead introduce a "mtd_can_mark_blockbad()" and use that as a
"does this MTD device support *explicit* bad blocks" thing.

Some uses may care about that "does it support bad blocks explicitly",
but nobody should ever care about "can_have_bb()". Possibly somebody
could care about the "inverse", in the sense that some filesystem
might want to only support devices that are defined to be perfect, but
that doesn't sound much like a MTD filesystem.

Anyway, I think you guys should have discussed this conflict when you
saw it in -next (or when the MTD people changed the fs/logfs code - I
see that Jörn was cc'd).

As it is, I just did what I think was the "RightThing(tm)". Please
discuss, and feel free to send me patches/pull requests for whatever
resolution you get to.

                        Linus