Re: [PATCH 3/4] UBI: Fastmap: Care about the protection queue

[Artem Bityutskiy <dedekind1@gmail.com>]

On Mon, 2014-10-13 at 16:30 +0200, Richard Weinberger wrote:
> Am 13.10.2014 um 15:17 schrieb Artem Bityutskiy:
> > On Fri, 2014-10-03 at 21:06 +0200, Richard Weinberger wrote:
> >> Fastmap needs basically access to all internal state of UBI, which
> >> lives mostly
> >> within wl.c
> > 
> > Sounds like a very strong assertion, smells a bit fishy, need the
> > details.
> 
> Fastmap need to know the exact state of each PEB.
> I.e. is it free, used, scheduled for erase, etc...
> 
> >> It needs to iterate over the used, free, erase, scrub RB-trees, the
> >> protection queue, etc. to
> >> collect the exact state of all PEBs.
> > 
> > When? In 'ubi_write_fastmap()' when forming the FM pool data structures?
> 
> Yes.
> 
> > I think you can just add a function like this to wl.c:
> > 
> > int ubi_wl_get_peb_info(int pnum, struct ubi_wl_peb_info *pebinfo)
> > 
> > Yoy will give it the PEB number you are interested in, it will return
> > you the information you need in 'pebinfo'. The information will contain
> > the EC, the state (used, free, etc).
> > 
> > If you need just the EC, then you do not even need the ubi_wl_peb_info
> > data structure.
> > 
> > Then you just iterate over all PEBs and fill the FM pool data
> > structures.
> > 
> > Would something like this work?
> 
> The interface would work but some work in wl.c is needed.
> For example if I want to find out in which state PEB 1 is wl.c would have to
> look int free free, used free, protection queue, etc.. to tell me the state. This is slow.

Well, used and free are RB-trees, looking them up is slow.

If what you need is to go through all used and free PEBs, then you can
introduce some kind of

struct ubi_wl_entry *ubi_wl_get_next_used(struct ubi_wl_entry *prev)

function, and similar functions for the free.

I would return you the next entry (or NULL would indicate the end), and
it would take the previous entry as the input, or NULL for the first
call.

We'd need to take the locks before calling this function. This is
cleaner than what we do now, right?

> But we could add the state information to struct ubi_wl_entry by adding a single integer attribute called "state" or "flags".

But there is a price - memory consumption. We do not want to pay it just
for making the inter-subsystems boundaries better, there ought to be a
better reason.

Say, for an (imaginary) 8GiB NAND chip with 128KiB PEB size this would
cost 256KiB of RAM.

Squeezing the state into the last 2 bits a memory reference would be
another possibility, BTW. Not elegant, though...

> Would this make you happy? :)

Not very, I'd save this for the last resort solution.