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

From: Richard Weinberger
Date: Sun Apr 12 2015 - 13:36:10 EST


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@xxxxxx> 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
--
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/