• Jiang Ying's avatar
    ext4: fix direct I/O read error · dbb8d293
    Jiang Ying authored
    BugLink: https://bugs.launchpad.net/bugs/1892822
    
    This patch is used to fix ext4 direct I/O read error when
    the read size is not aligned with block size.
    
    Then, I will use a test to explain the error.
    
    (1) Make a file that is not aligned with block size:
    	$dd if=/dev/zero of=./test.jar bs=1000 count=3
    
    (2) I wrote a source file named "direct_io_read_file.c" as following:
    
    	#include <stdio.h>
    	#include <stdlib.h>
    	#include <unistd.h>
    	#include <sys/file.h>
    	#include <sys/types.h>
    	#include <sys/stat.h>
    	#include <string.h>
    	#define BUF_SIZE 1024
    
    	int main()
    	{
    		int fd;
    		int ret;
    
    		unsigned char *buf;
    		ret = posix_memalign((void **)&buf, 512, BUF_SIZE);
    		if (ret) {
    			perror("posix_memalign failed");
    			exit(1);
    		}
    		fd = open("./test.jar", O_RDONLY | O_DIRECT, 0755);
    		if (fd < 0){
    			perror("open ./test.jar failed");
    			exit(1);
    		}
    
    		do {
    			ret = read(fd, buf, BUF_SIZE);
    			printf("ret=%d\n",ret);
    			if (ret < 0) {
    				perror("write test.jar failed");
    			}
    		} while (ret > 0);
    
    		free(buf);
    		close(fd);
    	}
    
    (3) Compile the source file:
    	$gcc direct_io_read_file.c -D_GNU_SOURCE
    
    (4) Run the test program:
    	$./a.out
    
    	The result is as following:
    	ret=1024
    	ret=1024
    	ret=952
    	ret=-1
    	write test.jar failed: Invalid argument.
    
    I have tested this program on XFS filesystem, XFS does not have
    this problem, because XFS use iomap_dio_rw() to do direct I/O
    read. And the comparing between read offset and file size is done
    in iomap_dio_rw(), the code is as following:
    
    	if (pos < size) {
    		retval = filemap_write_and_wait_range(mapping, pos,
    				pos + iov_length(iov, nr_segs) - 1);
    
    		if (!retval) {
    			retval = mapping->a_ops->direct_IO(READ, iocb,
    						iov, pos, nr_segs);
    		}
    		...
    	}
    
    ...only when "pos < size", direct I/O can be done, or 0 will be return.
    
    I have tested the fix patch on Ext4, it is up to the mustard of
    EINVAL in man2(read) as following:
    	#include <unistd.h>
    	ssize_t read(int fd, void *buf, size_t count);
    
    	EINVAL
    		fd is attached to an object which is unsuitable for reading;
    		or the file was opened with the O_DIRECT flag, and either the
    		address specified in buf, the value specified in count, or the
    		current file offset is not suitably aligned.
    
    So I think this patch can be applied to fix ext4 direct I/O error.
    
    However Ext4 introduces direct I/O read using iomap infrastructure
    on kernel 5.5, the patch is commit <b1b4705d>
    ("ext4: introduce direct I/O read using iomap infrastructure"),
    then Ext4 will be the same as XFS, they all use iomap_dio_rw() to do direct
    I/O read. So this problem does not exist on kernel 5.5 for Ext4.
    
    >From above description, we can see this problem exists on all the kernel
    versions between kernel 3.14 and kernel 5.4. It will cause the Applications
    to fail to read. For example, when the search service downloads a new full
    index file, the search engine is loading the previous index file and is
    processing the search request, it can not use buffer io that may squeeze
    the previous index file in use from pagecache, so the serch service must
    use direct I/O read.
    
    Please apply this patch on these kernel versions, or please use the method
    on kernel 5.5 to fix this problem.
    
    Fixes: 9fe55eea ("Fix race when checking i_size on direct i/o read")
    Reviewed-by: default avatarJan Kara <jack@suse.cz>
    Reviewed-by: default avatarWang Long <wanglong19@meituan.com>
    Signed-off-by: default avatarJiang Ying <jiangying8582@126.com>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    Signed-off-by: default avatarKhalid Elmously <khalid.elmously@canonical.com>
    Signed-off-by: default avatarIan May <ian.may@canonical.com>
    Signed-off-by: default avatarKelsey Skunberg <kelsey.skunberg@canonical.com>
    dbb8d293
inode.c 165 KB