Messages in this thread | | | Date | Fri, 16 Sep 2016 13:23:55 +0200 | From | Boris Brezillon <> | Subject | Re: [PATCH v2 11/17] UBI: simplify recover_peb() code |
| |
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.
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?
| |