Re: [PATCH 4/4] UBI: Implement bitrot checking

[Richard Weinberger <richard@nod.at>]

Am 12.04.2015 um 19:01 schrieb Boris Brezillon:
> Hi Richard,
> 
> After the 'coding style related'/'useless' comments, now comes a real
> question related to the approach you've taken :-).
> 
> On Sun, 29 Mar 2015 14:13:17 +0200
> Richard Weinberger <richard@nod.at> wrote:
> 
> [...]
>> +
>> +/**
>> + * ubi_wl_trigger_bitrot_check - triggers a re-read of all physical erase
>> + * blocks.
>> + * @ubi: UBI device description object
>> + */
>> +void ubi_wl_trigger_bitrot_check(struct ubi_device *ubi)
>> +{
>> +	int i;
>> +	struct ubi_wl_entry *e;
>> +
>> +	ubi_msg(ubi, "Running a full read check");
>> +
>> +	for (i = 0; i < ubi->peb_count; i++) {
>> +		spin_lock(&ubi->wl_lock);
>> +		e = ubi->lookuptbl[i];
>> +		spin_unlock(&ubi->wl_lock);
>> +		if (e) {
>> +			atomic_inc(&ubi->bit_rot_work);
>> +			schedule_bitrot_check(ubi, e);
>> +		}

After re-reading this loop I realized that I've missed a possible race against
other workers. It can happen that we schedule eX and while being scheduled it
is possible that eX turns bad and some worker frees eX under us.
Not very likely but definitely possible.
Let's see if I can find a solution for that without adding new locks or refcounter to
ubi_wl_entry.

I can think of adding a check to the schedule work code which ensures that work
targeting eX is scheduled only once.


Thanks,
//richard