linux-kernel - Re: [PATCH v10 07/21] Replace XIP read and write with DAX I/O

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5415A22F.2010900@gmail.com>
Date:	Sun, 14 Sep 2014 17:11:59 +0300
From:	Boaz Harrosh <openosd@...il.com>
To:	Matthew Wilcox <matthew.r.wilcox@...el.com>,
	linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, Dave Chinner <david@...morbit.com>
CC:	willy@...ux.intel.com
Subject: Re: [PATCH v10 07/21] Replace XIP read and write with DAX I/O

On 08/27/2014 06:45 AM, Matthew Wilcox wrote:
> Use the generic AIO infrastructure instead of custom read and write
> methods.  In addition to giving us support for AIO, this adds the missing
> locking between read() and truncate().
> 
> Signed-off-by: Matthew Wilcox <matthew.r.wilcox@...el.com>
> Reviewed-by: Ross Zwisler <ross.zwisler@...ux.intel.com>
> Reviewed-by: Jan Kara <jack@...e.cz>
> ---
>  MAINTAINERS        |   6 ++
>  fs/Makefile        |   1 +
>  fs/dax.c           | 195 ++++++++++++++++++++++++++++++++++++++++++++
>  fs/ext2/file.c     |   6 +-
>  fs/ext2/inode.c    |   8 +-
>  include/linux/fs.h |  18 ++++-
>  mm/filemap.c       |   6 +-
>  mm/filemap_xip.c   | 234 -----------------------------------------------------
>  8 files changed, 229 insertions(+), 245 deletions(-)
>  create mode 100644 fs/dax.c
> 
<>
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 90effcd..19bdb68 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1690,8 +1690,7 @@ generic_file_read_iter(struct kiocb *iocb, struct iov_iter *iter)
>  	loff_t *ppos = &iocb->ki_pos;
>  	loff_t pos = *ppos;
>  
> -	/* coalesce the iovecs and go direct-to-BIO for O_DIRECT */
> -	if (file->f_flags & O_DIRECT) {
> +	if (io_is_direct(file)) {
>  		struct address_space *mapping = file->f_mapping;
>  		struct inode *inode = mapping->host;
>  		size_t count = iov_iter_count(iter);
> @@ -2579,8 +2578,7 @@ ssize_t __generic_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>  	if (err)
>  		goto out;
>  
> -	/* coalesce the iovecs and go direct-to-BIO for O_DIRECT */
> -	if (unlikely(file->f_flags & O_DIRECT)) {
> +	if (io_is_direct(file)) {
>  		loff_t endbyte;
>  
>  		written = generic_file_direct_write(iocb, from, pos);

Hi Matthew

As pointed out by Dave Chinner, I think we must add the below hunks to this patch.
I do not see a case where it is allowed with current DAX code for any FS to
enable both DAX access/mmap in parallel to any buffered read/write.

Do we want to also put a
	WARN_ON(IS_DAX(inode));

In generic_perform_write and/or in extX->write_begin() ?

----
diff --git a/mm/filemap.c b/mm/filemap.c
index 19bdb68..22210c9 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1719,7 +1719,8 @@ generic_file_read_iter(struct kiocb *iocb, struct iov_iter *iter)
 		 * and return.  Otherwise fallthrough to buffered io for
 		 * the rest of the read.
 		 */
-		if (retval < 0 || !iov_iter_count(iter) || *ppos >= size) {
+		if (retval < 0 || !iov_iter_count(iter) || *ppos >= size ||
+		    IS_DAX(inode)) {
 			file_accessed(file);
 			goto out;
 		}
@@ -2582,7 +2583,7 @@ ssize_t __generic_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 		loff_t endbyte;
 
 		written = generic_file_direct_write(iocb, from, pos);
-		if (written < 0 || written == count)
+		if (written < 0 || written == count || IS_DAX(inode))
 			goto out;
 
 		/*
----

Thanks
Boaz

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/