On 09/18/2013 06:56 AM, Andrew Morton wrote:
> On Wed, 21 Aug 2013 10:41:20 +0800 Chen Gang <gang.chen@xxxxxxxxxxx> wrote:
>
>> force_page_cache_readahead() may fail, so need let the related upper
>> system calls know about it by its return value.
>>
>> For system call fadvise64_64(), ignore return value because fadvise()
>> shall return success even if filesystem can't retrieve a hint.
>>
>
> Actually, force_page_cache_readahead() cannot fail - I see no code path
> via which it returns a -ve errno.
>
Hmm... except return -EINVAL, currently it is.
> Of course, that might change in the future and although readahead is
> usually a best-effort-dont-care-if-it-fails thing, I suppose that in
> the case of madvise() and sys_readahead() we should inform userspace,
> as readhead is the primary reason for thier performing the syscall.
>
Yeah.
>
> While we're there, please review...
>
> From: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Subject: mm/readahead.c:do_readhead(): don't check for ->readpage
>
> The callee force_page_cache_readahead() already does this and unlike
> do_readahead(), force_page_cache_readahead() remembers to check for
> ->readpages() as well.
>
>
>
> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> ---
>
> mm/readahead.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff -puN mm/readahead.c~a mm/readahead.c
> --- a/mm/readahead.c~a
> +++ a/mm/readahead.c
> @@ -569,7 +569,7 @@ static ssize_t
> do_readahead(struct address_space *mapping, struct file *filp,
> pgoff_t index, unsigned long nr)
> {
> - if (!mapping || !mapping->a_ops || !mapping->a_ops->readpage)
> + if (!mapping || !mapping->a_ops)
> return -EINVAL;
>
> return force_page_cache_readahead(mapping, filp, index, nr);
> _
>
>
>
At least for me, this patch sounds good.
And for the code below in "mm/readahead.c", I guess, skipping return
value of force_page_cache_readahead() is acceptable, but still better to
get some comments for it
505 void page_cache_sync_readahead(struct address_space *mapping,
506 struct file_ra_state *ra, struct file
*filp,
507 pgoff_t offset, unsigned long req_size)
508 {
509 /* no read-ahead */
510 if (!ra->ra_pages)
511 return;
512
513 /* be dumb */
514 if (filp && (filp->f_mode & FMODE_RANDOM)) {
515 force_page_cache_readahead(mapping, filp, offset,
req_size);
516 return;
517 }
518
519 /* do read-ahead */
520 ondemand_readahead(mapping, ra, filp, false, offset, req_size);
521 }
522 EXPORT_SYMBOL_GPL(page_cache_sync_readahead);
Thanks.
--
Chen Gang
--
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>