Boris, On 16.09.2016 13:23, Boris Brezillon wrote: > On Fri, 16 Sep 2016 12:14:04 +0200 > Richard Weinberger <[email protected]> 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
