Re: Re: Re: [PATCH 1/5] mm/readahead: Check return value of read_pages

[Raghavendra D Prabhu <raghu.prabhu13@xxxxxxxxx>]

Hi,


* On Fri, Sep 28, 2012 at 07:54:05PM +0800, Fengguang Wu <fengguang.wu@xxxxxxxxx> wrote:
> On Wed, Sep 26, 2012 at 06:55:03AM +0530, Raghavendra D Prabhu wrote:
> > Hi,
> >
> >
> > * On Sat, Sep 22, 2012 at 08:43:37PM +0800, Fengguang Wu <fengguang.wu@xxxxxxxxx> wrote:
> > >On Sat, Sep 22, 2012 at 04:03:10PM +0530, raghu.prabhu13@xxxxxxxxx wrote:
> > >>From: Raghavendra D Prabhu <rprabhu@xxxxxxxxxxx>
> > >>
> > >>Return value of a_ops->readpage will be propagated to return value of read_pages
> > >>and __do_page_cache_readahead.
> > >
> > >That does not explain the intention and benefit of this patch..
> >
> > I noticed that force_page_cache_readahead checks return value of
> > __do_page_cache_readahead but the actual error if any is never
> > propagated.
>
> force_page_cache_readahead()'s return value, in turn, is never used by
> its callers..
Yes, it is not called by its callers, however, since it is called 
in a loop, shouldn't we bail out if force_page_cache_readahead 
fails once? Without the appropriate return value, it will 
continue  and  in 

force_page_cache_readahead


		if (err < 0) {
			ret = err;
			break;
		}

	is never hit.
Nor does the other __do_page_cache_readahead() callers
> care about the error state. So until we find an actual user of the
> error code, I'd recommend to avoid changing the current code.
>
> Thanks,
> Fengguang




Regards,
--
Raghavendra Prabhu
GPG Id : 0xD72BE977
Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977
www: wnohang.net

Attachment: pgplVrUJdJZGl.pgp
Description: PGP signature