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 [off-list ref] wrote:

quoted

On Wed, Sep 26, 2012 at 06:55:03AM +0530, Raghavendra D Prabhu wrote:

quoted

Hi,


* On Sat, Sep 22, 2012 at 08:43:37PM +0800, Fengguang Wu [off-list ref] wrote:

quoted

On Sat, Sep 22, 2012 at 04:03:10PM +0530, raghu.prabhu13@gmail.com wrote:

quoted

From: Raghavendra D Prabhu <redacted>

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

quoted

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@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>