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

Boaz Harrosh
2014-09-14T14:12:06Z

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@intel.com>
> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Reviewed-by: Jan Kara <jack@suse.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