On Tue, Oct 16, 2012 at 11:17:05PM +0530, Raghavendra D Prabhu wrote:
> 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
That sounds all reasonable, but please don't change the meaning of
__do_page_cache_readahead()'s return value. It should always return
the number of new pages put to IO, which will be used by some
readahead tracing/accounting feature. So it will need another parameter
for passing the error code from ->readpages(). However since the major
filesystems always return 0 in ->readpages(), I'm not sure it worth
the efforts.
Thanks,
Fengguang
> >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
> >
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>