On 04/12/2017 05:45 PM, Andrey Ryabinin wrote:
> On 04/11/2017 08:08 PM, Alexey Kuznetsov wrote:
>> Hello!
>>
>> Good job!
>>
>> Before submitting this to mainstream look
>> at truncate_inode_pages.
>>
>> It has condition:
>>
>> if (mapping->nrpages == 0 && mapping->nrexceptional == 0)
>> return;
>>
>> I have no idea what are those exceptions are, but it definitely
>> looks illegal to check only for nrpages in invalidate_inode_pages2_range,
>> it clears exceptional entries as well.
>>
>
> AFAIU exceptional entries are either dax entires, they store sector number and
> entry size (PMD, PTE,...), or they used by workingset code to store some
> information
> about page eviction.
>
> Given that, invalidate_inode_pages2_range() supposed to invalidate stale data
> in page
> cache/cleancache (per my understanding at least) I would say that
> invalidate_inode_pages2_range()
> shouldn't remove exceptional entries. But not sure that my understanding is
> correct,
> so I'm going to add ->nrexceptional check in v2, but will ask about this
> ambiguousity
> upstream folks.
>
Just for easier review, this is diff between v1 and v2:
---
fs/xfs/xfs_file.c | 14 ++++++--------
mm/truncate.c | 2 +-
2 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index d758443..0b7a35b 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -346,7 +346,7 @@ xfs_file_aio_read(
* serialisation.
*/
xfs_rw_ilock(ip, XFS_IOLOCK_SHARED);
- if ((ioflags & XFS_IO_ISDIRECT) && inode->i_mapping->nrpages) {
+ if ((ioflags & XFS_IO_ISDIRECT)) {
xfs_rw_iunlock(ip, XFS_IOLOCK_SHARED);
xfs_rw_ilock(ip, XFS_IOLOCK_EXCL);
@@ -361,14 +361,12 @@ xfs_file_aio_read(
* flush and reduce the chances of repeated iolock cycles going
* forward.
*/
- if (inode->i_mapping->nrpages) {
- ret = filemap_write_and_wait(VFS_I(ip)->i_mapping);
- if (ret) {
- xfs_rw_iunlock(ip, XFS_IOLOCK_EXCL);
- return ret;
- }
-
+ ret = filemap_write_and_wait(VFS_I(ip)->i_mapping);
+ if (ret) {
+ xfs_rw_iunlock(ip, XFS_IOLOCK_EXCL);
+ return ret;
}
+
/*
* Invalidate whole pages. This can return an error if
* we fail to invalidate a page, but this should never
diff --git a/mm/truncate.c b/mm/truncate.c
index f015a86..ce4b1d8 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -629,7 +629,7 @@ int invalidate_inode_pages2_range(struct address_space
*mapping,
cleancache_invalidate_inode(mapping);
- if (mapping->nrpages == 0)
+ if (mapping->nrpages == 0 && mapping->nrexceptional == 0)
return 0;
pagevec_init(&pvec, 0);
--
2.10.2
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
