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

[Richard Weinberger]

Am 20.10.2014 um 17:40 schrieb Artem Bityutskiy:
> On Mon, 2014-10-20 at 17:17 +0200, Richard Weinberger wrote:
>>> That's just fastmap code not doing the right thing. We should not touch
>>> the work queue directly at all. What we _should_ do instead is to make
>>> it empty by asking the subsystem which manages it to flush it.
>>>
>>> 1. Lock the work queue to prevent anyone from submitting new jobs while
>>> we are in process of writing the fastmap.
>>> 2. Flush all works
>>> 3. do all the fastmap write stuff
>>> 4. Unlock
>>
>> Are you sure? Flushing all works before every fastmap write will slow UBI down.
>> The fastmap write is not cheap. The goal should be to make it faster.
> 
> Yes, you are right. So instead, we wanna "freeze" it, and save all PEBs
> the jobs in fastmap too.
> 
> Why 'ubi_write_fastmap()' only cares about the erase jobs, and saves the
> PEBs from the job as "must be erased".
> 
> What about the "move" jobs?

There are no move jobs. Do you mean ubi->move_from/to used in wear_leveling_worker()?
How could it happen that a fastmap is written between these? IIRC everything is done
under wl_lock. And the PEBs used have to go through the pool.

> Also, say, PEB X is in the work queue waiting for erasure. Fastmap comes
> along and saves it as "must be erased" in the fastmap. Fastmap finishes
> its job, PEB X gets erased, and I write my data there, so PEB X is
> referred to by LEB Y. Now I have power cut. Then I attach the flash
> again. Surely it is not that fastmap just erases PEB X and I lose the
> contents of LEB Y?

This cannot happen. If X is erased you cannot write data do it. I must first go
thought the pool and the pool is scanned while attaching.

Thanks,
//richard

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/