On Fri, 16 Sep 2016 15:23:31 +0200 Richard Weinberger <[email protected]> wrote:
> 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. Actually it does. Now I'm retrying even when we have errors on the reads :-(. > > Thanks, > //richard
