[PATCH] erofs: support direct IO for uncompressed file

[PATCH] erofs: support direct IO for uncompressed file

[Huang Jianan]

direct IO is useful in certain scenarios for uncompressed files.
For example, it can avoid double pagecache when use the uncompressed
file to mount upper layer filesystem.

In addition, another patch adds direct IO test for the stress tool
which was mentioned here:
https://lore.kernel.org/linux-erofs/20200206135631.1491-1-hsiangkao@aol.com/

Signed-off-by: Huang Jianan <huangjianan@oppo.com>
Signed-off-by: Guo Weichao <guoweichao@oppo.com>
---
 fs/erofs/data.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index ea4f693bee22..3067aa3defff 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -6,6 +6,8 @@
  */
 #include "internal.h"
 #include <linux/prefetch.h>
+#include <linux/uio.h>
+#include <linux/blkdev.h>
 
 #include <trace/events/erofs.h>
 
@@ -312,6 +314,60 @@ static void erofs_raw_access_readahead(struct readahead_control *rac)
 		submit_bio(bio);
 }
 
+static int erofs_get_block(struct inode *inode, sector_t iblock,
+			   struct buffer_head *bh, int create)
+{
+	struct erofs_map_blocks map = {
+		.m_la = blknr_to_addr(iblock),
+	};
+	int err;
+
+	err = erofs_map_blocks(inode, &map, EROFS_GET_BLOCKS_RAW);
+	if (err)
+		return err;
+
+	if (map.m_flags & EROFS_MAP_MAPPED)
+		map_bh(bh, inode->i_sb, erofs_blknr(map.m_pa));
+
+	return err;
+}
+
+static int check_direct_IO(struct inode *inode, struct iov_iter *iter,
+			   loff_t offset)
+{
+	unsigned i_blkbits = READ_ONCE(inode->i_blkbits);
+	unsigned blkbits = i_blkbits;
+	unsigned blocksize_mask = (1 << blkbits) - 1;
+	unsigned long align = offset | iov_iter_alignment(iter);
+	struct block_device *bdev = inode->i_sb->s_bdev;
+
+	if (align & blocksize_mask) {
+		if (bdev)
+			blkbits = blksize_bits(bdev_logical_block_size(bdev));
+		blocksize_mask = (1 << blkbits) - 1;
+		if (align & blocksize_mask)
+			return -EINVAL;
+		return 1;
+	}
+	return 0;
+}
+
+static ssize_t erofs_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
+{
+	struct address_space *mapping = iocb->ki_filp->f_mapping;
+	struct inode *inode = mapping->host;
+	loff_t offset = iocb->ki_pos;
+	int err;
+
+	err = check_direct_IO(inode, iter, offset);
+	if (err)
+		return err < 0 ? err : 0;
+
+	return __blockdev_direct_IO(iocb, inode, inode->i_sb->s_bdev, iter,
+				    erofs_get_block, NULL, NULL,
+				    DIO_LOCKING | DIO_SKIP_HOLES);
+}
+
 static sector_t erofs_bmap(struct address_space *mapping, sector_t block)
 {
 	struct inode *inode = mapping->host;
@@ -336,6 +392,7 @@ static sector_t erofs_bmap(struct address_space *mapping, sector_t block)
 const struct address_space_operations erofs_raw_access_aops = {
 	.readpage = erofs_raw_access_readpage,
 	.readahead = erofs_raw_access_readahead,
+	.direct_IO = erofs_direct_IO,
 	.bmap = erofs_bmap,
 };
 
-- 
2.25.1

[PATCH] fsstress: support direct IO

[Huang Jianan]

From: huangjianan <huangjianan@oppo.com>

add direct IO test for the stress tool which was mentioned here:
https://lore.kernel.org/linux-erofs/20200206135631.1491-1-hsiangkao@aol.com/

Signed-off-by: Huang Jianan <huangjianan@oppo.com>
Signed-off-by: Guo Weichao <guoweichao@oppo.com>
---
 stress.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/stress.c b/stress.c
index f4bf874..7e7cc93 100644
--- a/stress.c
+++ b/stress.c
@@ -4,12 +4,14 @@
  *
  * Copyright (C) 2019-2020 Gao Xiang <hsiangkao@aol.com>
  */
+#define _GNU_SOURCE
 #define _LARGEFILE64_SOURCE
 #include <errno.h>
 #include <fcntl.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdint.h>
+#include <stdbool.h>
 #include <string.h>
 #include <sys/types.h>
 #include <sys/wait.h>
@@ -21,6 +23,7 @@
 #define MAX_CHUNKSIZE	(4 * 1024 * 1024)
 #define MAX_SCAN_CHUNKSIZE	(256 * 1024)
 
+bool direct_io = false;
 unsigned int nprocs = 512;
 sig_atomic_t should_stop = 0;
 
@@ -98,7 +101,7 @@ int drop_file_cache(int fd, int mode)
 
 int tryopen(char *filename)
 {
-	int fd = open(filename, O_RDONLY);
+	int fd = open(filename, direct_io ? O_RDONLY : O_RDONLY | O_DIRECT);
 
 	if (fd < 0)
 		return -errno;
@@ -166,6 +169,13 @@ int randread(int fd, int chkfd, uint64_t filesize)
 	if (start + length > filesize)
 		length = filesize - start;
 
+	if (direct_io) {
+		length = (((length - 1) >> PAGE_SHIFT) + 1)
+			<< PAGE_SHIFT;
+		if (!length || start + length > filesize)
+			return 0;
+	}
+
 	printf("randread(%u): %llu bytes @ %llu\n",
 	       getpid(), (unsigned long long)length,
 	       (unsigned long long)start);
@@ -212,7 +222,7 @@ int testfd(int fd, int chkfd, int mode)
 		err = doscan(fd, chkfd, filesize, chunksize);
 		if (err)
 			return err;
-	} else if (mode == RANDSCAN_UNALIGNED) {
+	} else if (mode == RANDSCAN_UNALIGNED && !direct_io) {
 		chunksize = (random() * random() % MAX_SCAN_CHUNKSIZE) + 1;
 		err = doscan(fd, chkfd, filesize, chunksize);
 		if (err)
@@ -252,8 +262,11 @@ static int parse_options(int argc, char *argv[])
 {
 	int opt;
 
-	while ((opt = getopt(argc, argv, "p:")) != -1) {
+	while ((opt = getopt(argc, argv, "dp:")) != -1) {
 		switch (opt) {
+		case 'd':
+			direct_io = true;
+			break;
 		case 'p':
 			nprocs = atoi(optarg);
 			if (nprocs < 0) {
@@ -281,6 +294,7 @@ void usage(void)
 {
 	fputs("usage: [options] TESTFILE [COMPRFILE]\n\n"
 	      "stress tester for read-only filesystems\n"
+	      " -d      use direct io\n"
 	      " -p#     set workers to #\n", stderr);
 }
 
-- 
2.7.4

Re: [PATCH] erofs: support direct IO for uncompressed file

[Gao Xiang]

Hi Jianan,

On Mon, Dec 14, 2020 at 10:04:27PM +0800, Huang Jianan wrote:
> direct IO is useful in certain scenarios for uncompressed files.
> For example, it can avoid double pagecache when use the uncompressed
> file to mount upper layer filesystem.
> 
> In addition, another patch adds direct IO test for the stress tool
> which was mentioned here:
> https://lore.kernel.org/linux-erofs/20200206135631.1491-1-hsiangkao@aol.com/
> 
> Signed-off-by: Huang Jianan <huangjianan@oppo.com>
> Signed-off-by: Guo Weichao <guoweichao@oppo.com>
> ---
>  fs/erofs/data.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
> 
> diff --git a/fs/erofs/data.c b/fs/erofs/data.c
> index ea4f693bee22..3067aa3defff 100644
> --- a/fs/erofs/data.c
> +++ b/fs/erofs/data.c
> @@ -6,6 +6,8 @@
>   */
>  #include "internal.h"
>  #include <linux/prefetch.h>
> +#include <linux/uio.h>
> +#include <linux/blkdev.h>
>  
>  #include <trace/events/erofs.h>
>  
> @@ -312,6 +314,60 @@ static void erofs_raw_access_readahead(struct readahead_control *rac)
>  		submit_bio(bio);
>  }
>  
> +static int erofs_get_block(struct inode *inode, sector_t iblock,
> +			   struct buffer_head *bh, int create)
> +{
> +	struct erofs_map_blocks map = {
> +		.m_la = blknr_to_addr(iblock),
> +	};
> +	int err;
> +
> +	err = erofs_map_blocks(inode, &map, EROFS_GET_BLOCKS_RAW);
> +	if (err)
> +		return err;
> +
> +	if (map.m_flags & EROFS_MAP_MAPPED)
> +		map_bh(bh, inode->i_sb, erofs_blknr(map.m_pa));
> +
> +	return err;
> +}
> +
> +static int check_direct_IO(struct inode *inode, struct iov_iter *iter,
> +			   loff_t offset)
> +{
> +	unsigned i_blkbits = READ_ONCE(inode->i_blkbits);

It would be better to fold in check_direct_IO, also the READ_ONCE above
is somewhat weird...

No rush here, since 5.11-rc1 haven't be out yet, we have >= 2 months to
work it out.

Thanks,
Gao Xiang

> +	unsigned blkbits = i_blkbits;
> +	unsigned blocksize_mask = (1 << blkbits) - 1;
> +	unsigned long align = offset | iov_iter_alignment(iter);
> +	struct block_device *bdev = inode->i_sb->s_bdev;
> +
> +	if (align & blocksize_mask) {
> +		if (bdev)
> +			blkbits = blksize_bits(bdev_logical_block_size(bdev));
> +		blocksize_mask = (1 << blkbits) - 1;
> +		if (align & blocksize_mask)
> +			return -EINVAL;
> +		return 1;
> +	}
> +	return 0;
> +}
> +
> +static ssize_t erofs_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
> +{
> +	struct address_space *mapping = iocb->ki_filp->f_mapping;
> +	struct inode *inode = mapping->host;
> +	loff_t offset = iocb->ki_pos;
> +	int err;
> +
> +	err = check_direct_IO(inode, iter, offset);
> +	if (err)
> +		return err < 0 ? err : 0;
> +
> +	return __blockdev_direct_IO(iocb, inode, inode->i_sb->s_bdev, iter,
> +				    erofs_get_block, NULL, NULL,
> +				    DIO_LOCKING | DIO_SKIP_HOLES);
> +}
> +
>  static sector_t erofs_bmap(struct address_space *mapping, sector_t block)
>  {
>  	struct inode *inode = mapping->host;
> @@ -336,6 +392,7 @@ static sector_t erofs_bmap(struct address_space *mapping, sector_t block)
>  const struct address_space_operations erofs_raw_access_aops = {
>  	.readpage = erofs_raw_access_readpage,
>  	.readahead = erofs_raw_access_readahead,
> +	.direct_IO = erofs_direct_IO,
>  	.bmap = erofs_bmap,
>  };
>  
> -- 
> 2.25.1
> 

Re: [PATCH] erofs: support direct IO for uncompressed file

[Christoph Hellwig]

Please do not add new callers of __blockdev_direct_IO and use the modern
iomap variant instead.

Re: [PATCH] erofs: support direct IO for uncompressed file

[Gao Xiang]

Hi Christoph,

On Tue, Dec 22, 2020 at 02:22:34PM +0000, Christoph Hellwig wrote:
> Please do not add new callers of __blockdev_direct_IO and use the modern
> iomap variant instead.

We've talked about this topic before. The current status is that iomap
doesn't support tail-packing inline data yet (Chao once sent out a version),
and erofs only cares about read intrastructure for now (So we don't think
more about how to deal with tail-packing inline write path). Plus, the
original patch was once lack of inline data regression test from gfs2 folks.

The main use case I know so far is to enable direct I/O and leave loop images
uncompressed for loop devices. And making the content of the loop images
compressed to avoid double caching.

Personally, I'd like to convert it to iomap infrastructure as well. So if it
has some concern for __blockdev_direct_IO as an interim solution, hope that
Jianan could pick this work up. That would be better.

Thanks,
Gao Xiang

> 

Re: [PATCH] erofs: support direct IO for uncompressed file

[Christoph Hellwig]

On Wed, Dec 23, 2020 at 03:39:01AM +0800, Gao Xiang wrote:
> Hi Christoph,
> 
> On Tue, Dec 22, 2020 at 02:22:34PM +0000, Christoph Hellwig wrote:
> > Please do not add new callers of __blockdev_direct_IO and use the modern
> > iomap variant instead.
> 
> We've talked about this topic before. The current status is that iomap
> doesn't support tail-packing inline data yet (Chao once sent out a version),
> and erofs only cares about read intrastructure for now (So we don't think
> more about how to deal with tail-packing inline write path). Plus, the
> original patch was once lack of inline data regression test from gfs2 folks.

So resend Chaos prep patch as part of the series switching parts of
erofs to iomap.  We need to move things off the old infrastructure instead
of adding more users and everyone needs to help a little.

Re: [PATCH] erofs: support direct IO for uncompressed file

[Huang Jianan]

Hi Christoph,

The reason we use dio is because we need to deploy the patch on some 
early kernel versions, and we don't pay much attention to the change of 
iomap. Anyway, I will study the problem mentioned by Gao Xiang and try 
to convert the current patch to iomap.

Thanks,

Jianan

> On Wed, Dec 23, 2020 at 03:39:01AM +0800, Gao Xiang wrote:
>> Hi Christoph,
>>
>> On Tue, Dec 22, 2020 at 02:22:34PM +0000, Christoph Hellwig wrote:
>>> Please do not add new callers of __blockdev_direct_IO and use the modern
>>> iomap variant instead.
>> We've talked about this topic before. The current status is that iomap
>> doesn't support tail-packing inline data yet (Chao once sent out a version),
>> and erofs only cares about read intrastructure for now (So we don't think
>> more about how to deal with tail-packing inline write path). Plus, the
>> original patch was once lack of inline data regression test from gfs2 folks.
> So resend Chaos prep patch as part of the series switching parts of
> erofs to iomap.  We need to move things off the old infrastructure instead
> of adding more users and everyone needs to help a little.

Re: [PATCH] erofs: support direct IO for uncompressed file

[Christoph Hellwig]

On Wed, Dec 23, 2020 at 04:48:20PM +0800, Huang Jianan wrote:
> Hi Christoph,
> 
> The reason we use dio is because we need to deploy the patch on some early
> kernel versions, and we don't pay much attention to the change of iomap.

No, that is never an excuse for upstream development.

Re: [PATCH] erofs: support direct IO for uncompressed file

[Gao Xiang]

On Wed, Dec 23, 2020 at 08:54:01AM +0000, Christoph Hellwig wrote:
> On Wed, Dec 23, 2020 at 04:48:20PM +0800, Huang Jianan wrote:
> > Hi Christoph,
> > 
> > The reason we use dio is because we need to deploy the patch on some early
> > kernel versions, and we don't pay much attention to the change of iomap.
> 
> No, that is never an excuse for upstream development.

Ok, personally I also agree this, let's go further in this way.

Thanks,
Gao Xiang

>