Re: [PATCH] UBI: add ubi_err() to report the failure of leb read

[hujianyang <hujianyang@huawei.com>]

On 2014/12/16 16:58, Richard Weinberger wrote:
> Am 16.12.2014 um 08:54 schrieb hujianyang:
>> If an error occur while reading from PEBs, for example, an ECC error,
>> ubi_io_read() will print some error messages. But it's not enough for
>> debugging. These messages don't show the mapping info for a read from
>> UBIFS layer.
>>
>> Although UBIFS will soon print its error messages after catching the
>> return value from UBI layer,  multi-path reading will confuse the
>> relationship between LEBs and PEBs showed by these messages.
>>
>> This patch adds an ubi_err() to report reading errors in the function
>> ubi_eba_read_leb(). The mapping info of LEB and PEB is showed by
>> this error message.
>>
>> Signed-off-by: hujianyang <hujianyang@huawei.com>
>> ---
>>  drivers/mtd/ubi/eba.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
>> index b698534..b4e69e1 100644
>> --- a/drivers/mtd/ubi/eba.c
>> +++ b/drivers/mtd/ubi/eba.c
>> @@ -477,6 +477,8 @@ out_free:
>>  	ubi_free_vid_hdr(ubi, vid_hdr);
>>  out_unlock:
>>  	leb_read_unlock(ubi, vol_id, lnum);
>> +	ubi_err(ubi, "err %d while reading %d bytes from offset %d of LEB %d:%d, PEB %d",
>> +		err, len, offset, vol_id, lnum, pnum);
> 
> This label will also be reached if we run out of memory.

Yes, but I think it's OK. Because a read operation is really failed
by some errors. We can print these errors not only in case of MTD
errors but also in case of CRC errors, ENOMEM or others.

> Please make sure that the new ubi_err() can only be reached in case of an MTD error.
> Also make sure that the function prints only one message and not two.

Er, At first, I want to print just MTD error, perform ubi_err() in
mtd_is_eccerr() case, for example. But I found it's not easy to print
it only once. The *retry* label makes it difficulty to determine when
this function, ubi_eba_read_leb() returns, this message will print
two times or none.

So at last, I move this ubi_err() to the end of error handling path
for easy. ^.^

What's your opinion?

Hu