* [PATCH v2 1/3] fs: remove redundant cache invalidation after async direct-io write
@ 2019-11-02 13:12 Konstantin Khlebnikov
2019-11-02 13:13 ` [PATCH v2 2/3] fs: keep dio_warn_stale_pagecache() when CONFIG_BLOCK=n Konstantin Khlebnikov
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Konstantin Khlebnikov @ 2019-11-02 13:12 UTC (permalink / raw)
To: linux-mm, Andrew Morton, linux-kernel
Cc: linux-fsdevel, Jens Axboe, Jan Kara, Alexander Viro
Function generic_file_direct_write() invalidates cache at entry. Second
time this should be done when request completes. But this function calls
second invalidation at exit unconditionally even for async requests.
This patch skips second invalidation for async requests (-EIOCBQUEUED).
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
mm/filemap.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/mm/filemap.c b/mm/filemap.c
index 85b7d087eb45..288e38199068 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -3218,9 +3218,11 @@ generic_file_direct_write(struct kiocb *iocb, struct iov_iter *from)
* Most of the time we do not need this since dio_complete() will do
* the invalidation for us. However there are some file systems that
* do not end up with dio_complete() being called, so let's not break
- * them by removing it completely
+ * them by removing it completely.
+ *
+ * Skip invalidation for async writes or if mapping has no pages.
*/
- if (mapping->nrpages)
+ if (written > 0 && mapping->nrpages)
invalidate_inode_pages2_range(mapping,
pos >> PAGE_SHIFT, end);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/3] fs: keep dio_warn_stale_pagecache() when CONFIG_BLOCK=n
2019-11-02 13:12 [PATCH v2 1/3] fs: remove redundant cache invalidation after async direct-io write Konstantin Khlebnikov
@ 2019-11-02 13:13 ` Konstantin Khlebnikov
2019-11-21 16:22 ` Jan Kara
2019-11-02 13:13 ` [PATCH v2 3/3] fs: warn if stale pagecache is left after direct write Konstantin Khlebnikov
2019-11-21 16:21 ` [PATCH v2 1/3] fs: remove redundant cache invalidation after async direct-io write Jan Kara
2 siblings, 1 reply; 6+ messages in thread
From: Konstantin Khlebnikov @ 2019-11-02 13:13 UTC (permalink / raw)
To: linux-mm, Andrew Morton, linux-kernel
Cc: linux-fsdevel, Jens Axboe, Jan Kara, Alexander Viro
This helper prints warning if direct I/O write failed to invalidate cache,
and set EIO at inode to warn usersapce about possible data corruption.
See also commit 5a9d929d6e13 ("iomap: report collisions between directio and
buffered writes to userspace").
Direct I/O is supported by non-disk filesystems, for example NFS.
Thus generic code needs this even in kernel without CONFIG_BLOCK.
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
fs/direct-io.c | 21 ---------------------
include/linux/fs.h | 6 +++++-
mm/filemap.c | 21 +++++++++++++++++++++
3 files changed, 26 insertions(+), 22 deletions(-)
diff --git a/fs/direct-io.c b/fs/direct-io.c
index 9329ced91f1d..0ec4f270139f 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -220,27 +220,6 @@ static inline struct page *dio_get_page(struct dio *dio,
return dio->pages[sdio->head];
}
-/*
- * Warn about a page cache invalidation failure during a direct io write.
- */
-void dio_warn_stale_pagecache(struct file *filp)
-{
- static DEFINE_RATELIMIT_STATE(_rs, 86400 * HZ, DEFAULT_RATELIMIT_BURST);
- char pathname[128];
- struct inode *inode = file_inode(filp);
- char *path;
-
- errseq_set(&inode->i_mapping->wb_err, -EIO);
- if (__ratelimit(&_rs)) {
- path = file_path(filp, pathname, sizeof(pathname));
- if (IS_ERR(path))
- path = "(unknown)";
- pr_crit("Page cache invalidation failure on direct I/O. Possible data corruption due to collision with buffered I/O!\n");
- pr_crit("File: %s PID: %d Comm: %.20s\n", path, current->pid,
- current->comm);
- }
-}
-
/*
* dio_complete() - called when all DIO BIO I/O has been completed
*
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e0d909d35763..b4e4560d1c38 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3153,7 +3153,6 @@ enum {
};
void dio_end_io(struct bio *bio);
-void dio_warn_stale_pagecache(struct file *filp);
ssize_t __blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
struct block_device *bdev, struct iov_iter *iter,
@@ -3198,6 +3197,11 @@ static inline void inode_dio_end(struct inode *inode)
wake_up_bit(&inode->i_state, __I_DIO_WAKEUP);
}
+/*
+ * Warn about a page cache invalidation failure diring a direct I/O write.
+ */
+void dio_warn_stale_pagecache(struct file *filp);
+
extern void inode_set_flags(struct inode *inode, unsigned int flags,
unsigned int mask);
diff --git a/mm/filemap.c b/mm/filemap.c
index 288e38199068..189b8f318da2 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -3161,6 +3161,27 @@ int pagecache_write_end(struct file *file, struct address_space *mapping,
}
EXPORT_SYMBOL(pagecache_write_end);
+/*
+ * Warn about a page cache invalidation failure during a direct I/O write.
+ */
+void dio_warn_stale_pagecache(struct file *filp)
+{
+ static DEFINE_RATELIMIT_STATE(_rs, 86400 * HZ, DEFAULT_RATELIMIT_BURST);
+ char pathname[128];
+ struct inode *inode = file_inode(filp);
+ char *path;
+
+ errseq_set(&inode->i_mapping->wb_err, -EIO);
+ if (__ratelimit(&_rs)) {
+ path = file_path(filp, pathname, sizeof(pathname));
+ if (IS_ERR(path))
+ path = "(unknown)";
+ pr_crit("Page cache invalidation failure on direct I/O. Possible data corruption due to collision with buffered I/O!\n");
+ pr_crit("File: %s PID: %d Comm: %.20s\n", path, current->pid,
+ current->comm);
+ }
+}
+
ssize_t
generic_file_direct_write(struct kiocb *iocb, struct iov_iter *from)
{
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] fs: warn if stale pagecache is left after direct write
2019-11-02 13:12 [PATCH v2 1/3] fs: remove redundant cache invalidation after async direct-io write Konstantin Khlebnikov
2019-11-02 13:13 ` [PATCH v2 2/3] fs: keep dio_warn_stale_pagecache() when CONFIG_BLOCK=n Konstantin Khlebnikov
@ 2019-11-02 13:13 ` Konstantin Khlebnikov
2019-11-21 16:23 ` Jan Kara
2019-11-21 16:21 ` [PATCH v2 1/3] fs: remove redundant cache invalidation after async direct-io write Jan Kara
2 siblings, 1 reply; 6+ messages in thread
From: Konstantin Khlebnikov @ 2019-11-02 13:13 UTC (permalink / raw)
To: linux-mm, Andrew Morton, linux-kernel
Cc: linux-fsdevel, Jens Axboe, Jan Kara, Alexander Viro
Function generic_file_direct_write() tries to invalidate pagecache after
O_DIRECT write. Unlike to similar code in dio_complete() this silently
ignores error returned from invalidate_inode_pages2_range().
According to comment this code here because not all filesystems call
dio_complete() to do proper invalidation after O_DIRECT write.
Noticeable example is a blkdev_direct_IO().
This patch calls dio_warn_stale_pagecache() if invalidation fails.
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
mm/filemap.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/mm/filemap.c b/mm/filemap.c
index 189b8f318da2..dc3b78db079b 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -3241,11 +3241,13 @@ generic_file_direct_write(struct kiocb *iocb, struct iov_iter *from)
* do not end up with dio_complete() being called, so let's not break
* them by removing it completely.
*
+ * Noticeable example is a blkdev_direct_IO().
+ *
* Skip invalidation for async writes or if mapping has no pages.
*/
- if (written > 0 && mapping->nrpages)
- invalidate_inode_pages2_range(mapping,
- pos >> PAGE_SHIFT, end);
+ if (written > 0 && mapping->nrpages &&
+ invalidate_inode_pages2_range(mapping, pos >> PAGE_SHIFT, end))
+ dio_warn_stale_pagecache(file);
if (written > 0) {
pos += written;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/3] fs: remove redundant cache invalidation after async direct-io write
2019-11-02 13:12 [PATCH v2 1/3] fs: remove redundant cache invalidation after async direct-io write Konstantin Khlebnikov
2019-11-02 13:13 ` [PATCH v2 2/3] fs: keep dio_warn_stale_pagecache() when CONFIG_BLOCK=n Konstantin Khlebnikov
2019-11-02 13:13 ` [PATCH v2 3/3] fs: warn if stale pagecache is left after direct write Konstantin Khlebnikov
@ 2019-11-21 16:21 ` Jan Kara
2 siblings, 0 replies; 6+ messages in thread
From: Jan Kara @ 2019-11-21 16:21 UTC (permalink / raw)
To: Konstantin Khlebnikov
Cc: linux-mm, Andrew Morton, linux-kernel, linux-fsdevel, Jens Axboe,
Jan Kara, Alexander Viro
On Sat 02-11-19 16:12:58, Konstantin Khlebnikov wrote:
> Function generic_file_direct_write() invalidates cache at entry. Second
> time this should be done when request completes. But this function calls
> second invalidation at exit unconditionally even for async requests.
>
> This patch skips second invalidation for async requests (-EIOCBQUEUED).
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Looks good to me. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> mm/filemap.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 85b7d087eb45..288e38199068 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -3218,9 +3218,11 @@ generic_file_direct_write(struct kiocb *iocb, struct iov_iter *from)
> * Most of the time we do not need this since dio_complete() will do
> * the invalidation for us. However there are some file systems that
> * do not end up with dio_complete() being called, so let's not break
> - * them by removing it completely
> + * them by removing it completely.
> + *
> + * Skip invalidation for async writes or if mapping has no pages.
> */
> - if (mapping->nrpages)
> + if (written > 0 && mapping->nrpages)
> invalidate_inode_pages2_range(mapping,
> pos >> PAGE_SHIFT, end);
>
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/3] fs: keep dio_warn_stale_pagecache() when CONFIG_BLOCK=n
2019-11-02 13:13 ` [PATCH v2 2/3] fs: keep dio_warn_stale_pagecache() when CONFIG_BLOCK=n Konstantin Khlebnikov
@ 2019-11-21 16:22 ` Jan Kara
0 siblings, 0 replies; 6+ messages in thread
From: Jan Kara @ 2019-11-21 16:22 UTC (permalink / raw)
To: Konstantin Khlebnikov
Cc: linux-mm, Andrew Morton, linux-kernel, linux-fsdevel, Jens Axboe,
Jan Kara, Alexander Viro
On Sat 02-11-19 16:13:00, Konstantin Khlebnikov wrote:
> This helper prints warning if direct I/O write failed to invalidate cache,
> and set EIO at inode to warn usersapce about possible data corruption.
> See also commit 5a9d929d6e13 ("iomap: report collisions between directio and
> buffered writes to userspace").
>
> Direct I/O is supported by non-disk filesystems, for example NFS.
> Thus generic code needs this even in kernel without CONFIG_BLOCK.
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Looks good to me. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/direct-io.c | 21 ---------------------
> include/linux/fs.h | 6 +++++-
> mm/filemap.c | 21 +++++++++++++++++++++
> 3 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/fs/direct-io.c b/fs/direct-io.c
> index 9329ced91f1d..0ec4f270139f 100644
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -220,27 +220,6 @@ static inline struct page *dio_get_page(struct dio *dio,
> return dio->pages[sdio->head];
> }
>
> -/*
> - * Warn about a page cache invalidation failure during a direct io write.
> - */
> -void dio_warn_stale_pagecache(struct file *filp)
> -{
> - static DEFINE_RATELIMIT_STATE(_rs, 86400 * HZ, DEFAULT_RATELIMIT_BURST);
> - char pathname[128];
> - struct inode *inode = file_inode(filp);
> - char *path;
> -
> - errseq_set(&inode->i_mapping->wb_err, -EIO);
> - if (__ratelimit(&_rs)) {
> - path = file_path(filp, pathname, sizeof(pathname));
> - if (IS_ERR(path))
> - path = "(unknown)";
> - pr_crit("Page cache invalidation failure on direct I/O. Possible data corruption due to collision with buffered I/O!\n");
> - pr_crit("File: %s PID: %d Comm: %.20s\n", path, current->pid,
> - current->comm);
> - }
> -}
> -
> /*
> * dio_complete() - called when all DIO BIO I/O has been completed
> *
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index e0d909d35763..b4e4560d1c38 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -3153,7 +3153,6 @@ enum {
> };
>
> void dio_end_io(struct bio *bio);
> -void dio_warn_stale_pagecache(struct file *filp);
>
> ssize_t __blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
> struct block_device *bdev, struct iov_iter *iter,
> @@ -3198,6 +3197,11 @@ static inline void inode_dio_end(struct inode *inode)
> wake_up_bit(&inode->i_state, __I_DIO_WAKEUP);
> }
>
> +/*
> + * Warn about a page cache invalidation failure diring a direct I/O write.
> + */
> +void dio_warn_stale_pagecache(struct file *filp);
> +
> extern void inode_set_flags(struct inode *inode, unsigned int flags,
> unsigned int mask);
>
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 288e38199068..189b8f318da2 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -3161,6 +3161,27 @@ int pagecache_write_end(struct file *file, struct address_space *mapping,
> }
> EXPORT_SYMBOL(pagecache_write_end);
>
> +/*
> + * Warn about a page cache invalidation failure during a direct I/O write.
> + */
> +void dio_warn_stale_pagecache(struct file *filp)
> +{
> + static DEFINE_RATELIMIT_STATE(_rs, 86400 * HZ, DEFAULT_RATELIMIT_BURST);
> + char pathname[128];
> + struct inode *inode = file_inode(filp);
> + char *path;
> +
> + errseq_set(&inode->i_mapping->wb_err, -EIO);
> + if (__ratelimit(&_rs)) {
> + path = file_path(filp, pathname, sizeof(pathname));
> + if (IS_ERR(path))
> + path = "(unknown)";
> + pr_crit("Page cache invalidation failure on direct I/O. Possible data corruption due to collision with buffered I/O!\n");
> + pr_crit("File: %s PID: %d Comm: %.20s\n", path, current->pid,
> + current->comm);
> + }
> +}
> +
> ssize_t
> generic_file_direct_write(struct kiocb *iocb, struct iov_iter *from)
> {
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 3/3] fs: warn if stale pagecache is left after direct write
2019-11-02 13:13 ` [PATCH v2 3/3] fs: warn if stale pagecache is left after direct write Konstantin Khlebnikov
@ 2019-11-21 16:23 ` Jan Kara
0 siblings, 0 replies; 6+ messages in thread
From: Jan Kara @ 2019-11-21 16:23 UTC (permalink / raw)
To: Konstantin Khlebnikov
Cc: linux-mm, Andrew Morton, linux-kernel, linux-fsdevel, Jens Axboe,
Jan Kara, Alexander Viro
On Sat 02-11-19 16:13:03, Konstantin Khlebnikov wrote:
> Function generic_file_direct_write() tries to invalidate pagecache after
> O_DIRECT write. Unlike to similar code in dio_complete() this silently
> ignores error returned from invalidate_inode_pages2_range().
>
> According to comment this code here because not all filesystems call
> dio_complete() to do proper invalidation after O_DIRECT write.
> Noticeable example is a blkdev_direct_IO().
>
> This patch calls dio_warn_stale_pagecache() if invalidation fails.
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Looks good to me. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> mm/filemap.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 189b8f318da2..dc3b78db079b 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -3241,11 +3241,13 @@ generic_file_direct_write(struct kiocb *iocb, struct iov_iter *from)
> * do not end up with dio_complete() being called, so let's not break
> * them by removing it completely.
> *
> + * Noticeable example is a blkdev_direct_IO().
> + *
> * Skip invalidation for async writes or if mapping has no pages.
> */
> - if (written > 0 && mapping->nrpages)
> - invalidate_inode_pages2_range(mapping,
> - pos >> PAGE_SHIFT, end);
> + if (written > 0 && mapping->nrpages &&
> + invalidate_inode_pages2_range(mapping, pos >> PAGE_SHIFT, end))
> + dio_warn_stale_pagecache(file);
>
> if (written > 0) {
> pos += written;
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-11-21 16:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-02 13:12 [PATCH v2 1/3] fs: remove redundant cache invalidation after async direct-io write Konstantin Khlebnikov
2019-11-02 13:13 ` [PATCH v2 2/3] fs: keep dio_warn_stale_pagecache() when CONFIG_BLOCK=n Konstantin Khlebnikov
2019-11-21 16:22 ` Jan Kara
2019-11-02 13:13 ` [PATCH v2 3/3] fs: warn if stale pagecache is left after direct write Konstantin Khlebnikov
2019-11-21 16:23 ` Jan Kara
2019-11-21 16:21 ` [PATCH v2 1/3] fs: remove redundant cache invalidation after async direct-io write Jan Kara
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).