don't allow sys_readahead() on files opened with O_DIRECT

IMO sys_readahead() doesn't make sense if the file is opened with O_DIRECT, because the page cache is stuffed but never used. Therefore this patch changes that by letting the call return with -EINVAL.

 mm/filemap.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletion(-)

Index: experimental-jb/mm/filemap.c
===================================================================
@@ -1111,7 +1111,8 @@ static ssize_t
 do_readahead(struct address_space *mapping, struct file *filp,
 	     unsigned long index, unsigned long nr)
 {
-	if (!mapping || !mapping->a_ops || !mapping->a_ops->readpage)
+	if (!mapping || !mapping->a_ops || !mapping->a_ops->readpage
+	    || (filp->f_flags & O_DIRECT))
 		return -EINVAL;
 
 	force_page_cache_readahead(mapping, filp, index,

a) It doesn't hurt, it's just a bit of a silly thing to do.

b) posix_fadvise(POSIX_FADV_WILLNEED) should get the same treatment (and it's the preferred way of doing readahead).

c) O_DIRECT fd's should, as much as possible, offer the same ABI as buffered fd's.

d) The patch could break existing apps.

IMO it may hurt the performance.

Yes, of course.

Hmm, with XIP fd's we agreed on the following behavior: fadvise() and madvise() just return without reading anything to the page cache. Since XIP fd's are similar to O_DIRECT fd's their behavior should be similar, too. If we don't honor the advises we might also ignore the syscall. At least redhat's readahead is using them.

Maybe we should agree on one behavior that makes sense. And I don't see any point in filling the page cache when it is not needed.

Since it could break applications that are already (some kind of) broken that shouldn't be a problem.

So you think it is better to read nothing to the page cache and return zero instead? This seems like "lying" to the user-space :)

For ocfs that only allows either direct or buffered I/O on a single inode at a given time it hurts. It messes up the synchronization protocol to be exact..