Re: [PATCH v6 12/15] ubi: fastmap: Add all fastmap pebs into 'ai->fastmap' when fm->used_blocks>=2

[Zhihao Cheng]

Hi Richard,
> > FYI, I think I understand now our disagreement.
> > You assume that old Fastmap PEBs are *guaranteed* to be part of 
> > Fastmap's erase list.
> > That's okay and this is what Linux as of today does.
> >
> > My point is that we need to be paranoid and check carefully for old 
> > Fastmap PEBs
> > which might be *not* on the erase list.
> > I saw such issues in the wild. These were causes by old and/or buggy 
> > Fastmap
> > implementations.
> > Keep in mind that Linux is not the only system that implements UBI 
> > (and fastmap).
> Uh, that is really a point, I met UBI implemented in Vxworks ever. Now, 
> you convinced me, we should process fastmap with considering bad 
> images(caused by other implementations). Let's keep this wonky assertion 
> until a better fix.
> > So let me give the whole situation another thought on how to improve it.
> > I totally agree with you that currently there is a problem with 
> > fm->used_blocks > 1.
> > I'm just careful (maybe too careful) about changing Fastmap code.
> > I reconsider the WARNON, it can recognize the bad images and fall back 
full scanning mode early. If linux kernel encounters the WARNON, it 
means something wrong with your image(caused by bad UBI implementation). 
I begin to like this strict check.
After comparing WARNON with the assertion:
  WARN_ON(count_fastmap_pebs(ai) != ubi->peb_count - ai->bad_peb_count 
- fm->used_blocks)
  ubi_assert(ubi->good_peb_count == found_pebs)
The 'found_pebs' consists of 'ai->erase', 'ai->free', 'ai->volumes' and 
'ai->fastmap'. The count_fastmap_pebs() includes 'ai->erase', 'ai->free' 
and 'ai->volumes'. This means the number of 'ai->fastmap' equals to 
'fm->used_blocks'. So, the 'ai->fastmap' adding position in my fix is right.
In a word, can you accept the point that 'WARNON' can help us recognize 
the bad fastmap images?