lkml.org 
[lkml]   [2015]   [Dec]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC v2 PATCH] fs: __generic_file_splice_read retry lookup on AOP_TRUNCATED_PAGE
On Wed 16-12-15 10:55:25, Abhijith Das wrote:
> > > @@ -439,14 +441,13 @@ __generic_file_splice_read(struct file *in, loff_t
> > > *ppos,
> > > error = mapping->a_ops->readpage(in, page);
> > > if (unlikely(error)) {
> > > /*
> > > - * We really should re-lookup the page here,
> > > - * but it complicates things a lot. Instead
> > > - * lets just do what we already stored, and
> > > - * we'll get it the next time we are called.
> > > + * Re-lookup the page
> > > */
> > > - if (error == AOP_TRUNCATED_PAGE)
> > > + if (error == AOP_TRUNCATED_PAGE) {
> > > error = 0;
> > > -
> > > + if (retries++ < 3)
> > > + goto retry_lookup;
> > > + }
> >
> > I don't like this retry-three-times loop. That is still leaving the
> > possibility of 0 return just much less likely (so it will lead to even
> > weirder and harded to debug failures). IMO we should just terminate the
> > loop like we did previously if spd.nr_pages > 0 and we retry indefinitely
> > if it is the first page that failed to read.
>
> The 3-retry loop was the thing I was unsure about too. With regards to the
> indefinite retry, I was wondering if there's some corner case where we might
> get into an infinite retry loop...

Well you have two options:

1) Return incorrect value from splice_read()
2) Retry indefinitely

Option two looks better to me. Also do_generic_file_read() behaves the same
way so mirroring the behavior in __generic_file_splice_read() makes sense.

> If we're doing the retry for the first page, why not for other pages too?
> Is it because we'd potentially be increasing the odds for an infinite
> loop and/or affecting performance by doing more lookups?

Yes, that was my thought. But seeing now that do_generic_file_read()
actually retries indefinitely for every page, I'd just do the same in
__generic_file_splice_read().

Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR


\
 
 \ /
  Last update: 2015-12-16 18:21    [W:0.085 / U:0.208 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site