Messages in this thread | | | Subject | Re: [PATCH v2 11/17] UBI: simplify recover_peb() code | From | Richard Weinberger <> | Date | Fri, 16 Sep 2016 15:23:31 +0200 |
| |
Boris,
On 16.09.2016 13:23, Boris Brezillon wrote: > On Fri, 16 Sep 2016 12:14:04 +0200 > Richard Weinberger <richard@nod.at> wrote: > >> Boris, >> >> On 05.09.2016 17:05, Boris Brezillon wrote: >>> + * This function is called in case of a write failure and moves all good data >>> + * from the potentially bad physical eraseblock to a good physical eraseblock. >>> + * This function also writes the data which was not written due to the failure. >>> + * Returns 0 in case of success, and a negative error code in case of failure. >>> + * This function tries %UBI_IO_RETRIES before giving up. >>> + */ >>> +static int recover_peb(struct ubi_device *ubi, int pnum, int vol_id, int lnum, >>> + const void *buf, int offset, int len) >>> +{ >>> + int err, idx = vol_id2idx(ubi, vol_id), tries; >>> + struct ubi_volume *vol = ubi->volumes[idx]; >>> + struct ubi_vid_hdr *vid_hdr; >>> + >>> + vid_hdr = ubi_zalloc_vid_hdr(ubi, GFP_NOFS); >>> + if (!vid_hdr) >>> + return -ENOMEM; >>> + >>> + for (tries = 0; tries <= UBI_IO_RETRIES; tries++) { >>> + err = try_recover_peb(vol, pnum, lnum, buf, offset, len, >>> + vid_hdr); >>> + if (!err || err == -ENOSPC) >>> + break; >> >> Why do you handle ENOSPC as fatal error? Since the loop is bound by UBI_IO_RETRIES >> IMHO we can retry also upon ENOSPC. > > I was just trying to mimic the existing behavior: if ubi_wl_get_peb() > fails to return a free PEB it returns -ENOSPC, and the current > implementation does not retry in this case.
Whoops. I thought the current code does retry upon -ENOSPC.
> I also realize that we should not retry if the error happened when > reading from the source PEB. > > Maybe we should have an extra 'bool *retry' parameter to let > recover_peb() know whether the operation should be retried or not. > What do you think?
Since your change does not change the current behaviour, let's keep it as-is.
Thanks, //richard
| |