[PATCH] mtdblock: warn if opened on NAND

[Tudor.Ambarus at microchip.com]

Hi, Bjørn,

On 4/21/22 19:43, Bjørn Mork wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> <Tudor.Ambarus at microchip.com> writes:
> 
>> I don't see this as a fix, you just mask out the warning, you assume
>> that mtdblock is not used
> 
> No, I don't.  No real warnings are masked.  Only bogus warnings.
> 
> I make no assumption about mtdblock usage. I moved the warning to open()
> so that the driver complains when a NAND mtdblock partition is mounted.
> I am sure this does happen from time to time, and agree 100% that a
> warning is appropriate in those cases.
> 
> Commit e07403a8c6be is based on two false assumptions:
> 1) the presence of the mtdblock driver is a problem

It is a problem. Using it will wear out parts of your raw flash relatively
fast. There's no wear-leveling with mtdblock. IMO mtdblock should be used
just for testing purposes maybe, or for cases where users are forced to
use write capable block-based File Systems - which is still wrong in my
opinion.

> 2) all partitions will be mounted as mtdblock devices

Indeed, we should print only a message, not one for each partition. Even so,
wearing out parts of a partition damages your raw flash.

> 
> Extensive log noise is a problem for end users.  We can obviously argue
> if it is a bug or not.  I claim that it is because real warnings drown
> in the noise, making real bugs much harder to spot.
> 
>> but you still keep it in your config, which
>> is wrong. You should instead update your config, remove mtdblock, and
>> fix the cause if it's possible.
> 
> So if you have a device with both NOR and NAND flash, then what?  Not
> that unusual.

Sorry, I don't understand this argument. Both NAND and NOR flashes will
wear out if using mtdblock. NAND will wear faster than NOR, but NOR is
still affected.

> 
>>> start noticing now that OpenWrt is moving to 5.15.  As you may or
>>> not know, OpenWrt unconditionally includes mtdblock whether the
>>> target is NAND or NOR.  So this will cause lots of noise on any
>>
>> Why does Openwrt include mtdblock?
> 
> I don't know and I don't speak for OpenWrt, being only an end user.  But
> I assume it's because OpenWrt builds semi-generic images for both NOR
> and NAND devices, as well as devices with both flash types. They are

Neither NOR, nor NAND should use mtdblock to enable write-capable
block-based File Systems in production. Raw flashes should be handled
with UBI instead.

> also trying to maintain flash layout compatibillty with OEM firmware,
> which often does really stupid things.
> 
> Sure, OpenWrt could probably strip mtdblock from NAND only devices. But
> I guess the priority is low since it doesn't fix any problem, and it
> will complicate their build slightly. I'm sure patches are welcome
> though.
> 
>> I have encountered these messages in some configs that I'm using and I
>> ended up cleaning them. These messages helped me identify the problem and
>> ultimately to fix it. Let's give others the chance to do the same.
> 
> Others will still get the warning iff there is a problem.

It is a problem in my opinion. Your flash will be irreversibly damaged
really fast if using mtdblock.

> 
> The fix is to prevent warnings when there isn't one.  If you warn 200
> times (which IMHO is a low estimate here) for every real problem, then
> no one will care in the end. "Oh, it's just mtdblock whining again.  It
> does that all the time"
> 

I agree we should improve this and pass a single warning. I would like to
still have this warning at boot if possible. I guess people have the
mtdblock config in their defconfigs, but never use mtdblock, at least that
was my case. Having that warning will give them the chance to fix their
defconfigs.

Btw, something similar was discussed at:
https://lore.kernel.org/lkml/876982414.38679.1635274892099.JavaMail.zimbra@nod.at/

Cheers,
ta