Re: [PATCH] mm/readahead.c: need always return 0 when system call readahead() succeeds

[Chen Gang <gang.chen@asianux.com>]

On 10/17/2013 07:06 AM, David Rientjes wrote:
> On Tue, 15 Oct 2013, Chen Gang wrote:
> 
>> diff --git a/mm/readahead.c b/mm/readahead.c
>> index 1eee42b..83a202e 100644
>> --- a/mm/readahead.c
>> +++ b/mm/readahead.c
>> @@ -592,5 +592,5 @@ SYSCALL_DEFINE3(readahead, int, fd, loff_t, offset, size_t, count)
>>  		}
>>  		fdput(f);
>>  	}
>> -	return ret;
>> +	return ret < 0 ? ret : 0;
>>  }
> 
> This was broken by your own "mm/readahead.c: return the value which 
> force_page_cache_readahead() returns" patch in -mm, luckily Linus's tree 
> isn't affected.
> 

Of cause it is.

And every member knows about it: in my comments, already mentioned about
it in a standard way.

Hmm... isn't it enough? (it seems you don't think so)

If possible, you can help me check all my patches again (at least, it is
not a bad idea to me).  ;-)


> Nack to this and nack to the problem patch, which is absolutely pointless 
> and did nothing but introduce this error.  readahead() is supposed to 
> return 0, -EINVAL, or -EBADF and your original patch broke it.  That's 
> because your original patch was completely pointless to begin with.
> 
> 

Do you mean: in do_readahead(), we need not check the return value of
force_page_cache_readahead()?

In my opinion, the system call of readahead() wants to notice whether
force_page_cache_readahead() fails or not (may return -EINVAL), which is
the mainly callee of readahead().

Don't you think so??


Thanks.
-- 
Chen Gang

--
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>