[PATCH v2] Documentation/dax: Update description of DAX policy changing
After commit 77573fa310d9 ("fs: Kill DCACHE_DONTCACHE dentry even if
DCACHE_REFERENCED is set"), changes to DAX policy will take effect
as soon as all references to this file are gone.
Update the documentation accordingly.
Signed-off-by: Hao Li
---
Changes in v2:
* simplify sentences and fix style problems.
Documentation/filesystems/dax.txt | 17 +++--
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/Documentation/filesystems/dax.txt
b/Documentation/filesystems/dax.txt
index 8fdb78f3c6c9..e03c20564f3a 100644
--- a/Documentation/filesystems/dax.txt
+++ b/Documentation/filesystems/dax.txt
@@ -83,20 +83,9 @@ Summary
directories. This has runtime constraints and limitations that are
described in 6) below.
- 6. When changing the S_DAX policy via toggling the persistent FS_XFLAG_DAX
flag,
-the change in behaviour for existing regular files may not occur
-immediately. If the change must take effect immediately, the administrator
-needs to:
-
-a) stop the application so there are no active references to the data set
- the policy change will affect
-
-b) evict the data set from kernel caches so it will be re-instantiated when
- the application is restarted. This can be achieved by:
-
- i. drop-caches
- ii. a filesystem unmount and mount cycle
- iii. a system reboot
+ 6. When changing the S_DAX policy via toggling the persistent FS_XFLAG_DAX
+flag, the change to existing regular files won't take effect until the
+files are closed by all processes.
Details
--
2.29.2
[PATCH] Documentation/dax: Update description of DAX policy changing
After commit 77573fa310d9 ("fs: Kill DCACHE_DONTCACHE dentry even if
DCACHE_REFERENCED is set"), changes to DAX policy will take effect
as soon as all references to this file are gone.
Update the documentation accordingly.
Signed-off-by: Hao Li
---
Documentation/filesystems/dax.txt | 15 ++-
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/Documentation/filesystems/dax.txt
b/Documentation/filesystems/dax.txt
index 8fdb78f3c6c9..a5af22831087 100644
--- a/Documentation/filesystems/dax.txt
+++ b/Documentation/filesystems/dax.txt
@@ -84,19 +84,8 @@ Summary
described in 6) below.
6. When changing the S_DAX policy via toggling the persistent FS_XFLAG_DAX
flag,
-the change in behaviour for existing regular files may not occur
-immediately. If the change must take effect immediately, the administrator
-needs to:
-
-a) stop the application so there are no active references to the data set
- the policy change will affect
-
-b) evict the data set from kernel caches so it will be re-instantiated when
- the application is restarted. This can be achieved by:
-
- i. drop-caches
- ii. a filesystem unmount and mount cycle
- iii. a system reboot
+the change to existing regular file won't take effect until the file is
closed
+by all processes or all processes referencing the file are stopped.
Details
--
2.29.2
[PATCH] fs: Kill DCACHE_DONTCACHE dentry even if DCACHE_REFERENCED is set
If DCACHE_REFERENCED is set, fast_dput() will return true, and then retain_dentry() have no chance to check DCACHE_DONTCACHE. As a result, the dentry won't be killed and the corresponding inode can't be evicted. In the following example, the DAX policy can't take effects unless we do a drop_caches manually. # DCACHE_LRU_LIST will be set echo abcdefg > test.txt # DCACHE_REFERENCED will be set and DCACHE_DONTCACHE can't do anything xfs_io -c 'chattr +x' test.txt # Drop caches to make DAX changing take effects echo 2 > /proc/sys/vm/drop_caches What this patch does is preventing fast_dput() from returning true if DCACHE_DONTCACHE is set. Then retain_dentry() will detect the DCACHE_DONTCACHE and will return false. As a result, the dentry will be killed and the inode will be evicted. In this way, if we change per-file DAX policy, it will take effects automatically after this file is closed by all processes. I also add some comments to make the code more clear. Signed-off-by: Hao Li Reviewed-by: Jan Kara Reviewed-by: Ira Weiny --- This patch may have been forgotten. Original patch: https://lore.kernel.org/linux-fsdevel/20200924055958.825515-1-lihao2018.f...@cn.fujitsu.com/ fs/dcache.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/fs/dcache.c b/fs/dcache.c index ea0485861d93..97e81a844a96 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -793,10 +793,17 @@ static inline bool fast_dput(struct dentry *dentry) * a reference to the dentry and change that, but * our work is done - we can leave the dentry * around with a zero refcount. +* +* Nevertheless, there are two cases that we should kill +* the dentry anyway. +* 1. free disconnected dentries as soon as their refcount +*reached zero. +* 2. free dentries if they should not be cached. */ smp_rmb(); d_flags = READ_ONCE(dentry->d_flags); - d_flags &= DCACHE_REFERENCED | DCACHE_LRU_LIST | DCACHE_DISCONNECTED; + d_flags &= DCACHE_REFERENCED | DCACHE_LRU_LIST | + DCACHE_DISCONNECTED | DCACHE_DONTCACHE; /* Nothing to do? Dropping the reference was all we needed? */ if (d_flags == (DCACHE_REFERENCED | DCACHE_LRU_LIST) && !d_unhashed(dentry)) -- 2.28.0
[PATCH] fs: Handle I_DONTCACHE in iput_final() instead of generic_drop_inode()
If generic_drop_inode() returns true, it means iput_final() can evict
this inode regardless of whether it is dirty or not. If we check
I_DONTCACHE in generic_drop_inode(), any inode with this bit set will be
evicted unconditionally. This is not the desired behavior because
I_DONTCACHE only means the inode shouldn't be cached on the LRU list.
As for whether we need to evict this inode, this is what
generic_drop_inode() should do. This patch corrects the usage of
I_DONTCACHE.
This patch was proposed in [1].
[1]:
https://lore.kernel.org/linux-fsdevel/20200831003407.ge12...@dread.disaster.area/
Fixes: dae2f8ed7992 ("fs: Lift XFS_IDONTCACHE to the VFS layer")
Signed-off-by: Hao Li
Reviewed-by: Dave Chinner
Reviewed-by: Ira Weiny
---
This patch may have been forgotten.
Original patch:
https://lore.kernel.org/linux-fsdevel/20200904075939.176366-1-lihao2018.f...@cn.fujitsu.com/
fs/inode.c | 4 +++-
include/linux/fs.h | 3 +--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index 9d78c37b00b8..5eea9912a0b9 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1627,7 +1627,9 @@ static void iput_final(struct inode *inode)
else
drop = generic_drop_inode(inode);
- if (!drop && (sb->s_flags & SB_ACTIVE)) {
+ if (!drop &&
+ !(inode->i_state & I_DONTCACHE) &&
+ (sb->s_flags & SB_ACTIVE)) {
inode_add_lru(inode);
spin_unlock(&inode->i_lock);
return;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8667d0cdc71e..8bde32cf9711 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2878,8 +2878,7 @@ extern int inode_needs_sync(struct inode *inode);
extern int generic_delete_inode(struct inode *inode);
static inline int generic_drop_inode(struct inode *inode)
{
- return !inode->i_nlink || inode_unhashed(inode) ||
- (inode->i_state & I_DONTCACHE);
+ return !inode->i_nlink || inode_unhashed(inode);
}
extern void d_mark_dontcache(struct inode *inode);
--
2.28.0
[PATCH] fs: Kill DCACHE_DONTCACHE dentry even if DCACHE_REFERENCED is set
If DCACHE_REFERENCED is set, fast_dput() will return true, and then retain_dentry() have no chance to check DCACHE_DONTCACHE. As a result, the dentry won't be killed and the corresponding inode can't be evicted. In the following example, the DAX policy can't take effects unless we do a drop_caches manually. # DCACHE_LRU_LIST will be set echo abcdefg > test.txt # DCACHE_REFERENCED will be set and DCACHE_DONTCACHE can't do anything xfs_io -c 'chattr +x' test.txt # Drop caches to make DAX changing take effects echo 2 > /proc/sys/vm/drop_caches What this patch does is preventing fast_dput() from returning true if DCACHE_DONTCACHE is set. Then retain_dentry() will detect the DCACHE_DONTCACHE and will return false. As a result, the dentry will be killed and the inode will be evicted. In this way, if we change per-file DAX policy, it will take effects automatically after this file is closed by all processes. I also add some comments to make the code more clear. Signed-off-by: Hao Li Reviewed-by: Jan Kara Reviewed-by: Ira Weiny --- This patch may have been forgotten. Original patch: https://lore.kernel.org/linux-fsdevel/20200924055958.825515-1-lihao2018.f...@cn.fujitsu.com/ fs/dcache.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/fs/dcache.c b/fs/dcache.c index ea0485861d93..97e81a844a96 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -793,10 +793,17 @@ static inline bool fast_dput(struct dentry *dentry) * a reference to the dentry and change that, but * our work is done - we can leave the dentry * around with a zero refcount. +* +* Nevertheless, there are two cases that we should kill +* the dentry anyway. +* 1. free disconnected dentries as soon as their refcount +*reached zero. +* 2. free dentries if they should not be cached. */ smp_rmb(); d_flags = READ_ONCE(dentry->d_flags); - d_flags &= DCACHE_REFERENCED | DCACHE_LRU_LIST | DCACHE_DISCONNECTED; + d_flags &= DCACHE_REFERENCED | DCACHE_LRU_LIST | + DCACHE_DISCONNECTED | DCACHE_DONTCACHE; /* Nothing to do? Dropping the reference was all we needed? */ if (d_flags == (DCACHE_REFERENCED | DCACHE_LRU_LIST) && !d_unhashed(dentry)) -- 2.28.0
[PATCH] fs: Handle I_DONTCACHE in iput_final() instead of generic_drop_inode()
If generic_drop_inode() returns true, it means iput_final() can evict
this inode regardless of whether it is dirty or not. If we check
I_DONTCACHE in generic_drop_inode(), any inode with this bit set will be
evicted unconditionally. This is not the desired behavior because
I_DONTCACHE only means the inode shouldn't be cached on the LRU list.
As for whether we need to evict this inode, this is what
generic_drop_inode() should do. This patch corrects the usage of
I_DONTCACHE.
This patch was proposed in [1].
[1]:
https://lore.kernel.org/linux-fsdevel/20200831003407.ge12...@dread.disaster.area/
Fixes: dae2f8ed7992 ("fs: Lift XFS_IDONTCACHE to the VFS layer")
Signed-off-by: Hao Li
Reviewed-by: Dave Chinner
Reviewed-by: Ira Weiny
---
This patch may have been forgotten.
Original patch:
https://lore.kernel.org/linux-fsdevel/20200904075939.176366-1-lihao2018.f...@cn.fujitsu.com/
fs/inode.c | 4 +++-
include/linux/fs.h | 3 +--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index 9d78c37b00b8..5eea9912a0b9 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1627,7 +1627,9 @@ static void iput_final(struct inode *inode)
else
drop = generic_drop_inode(inode);
- if (!drop && (sb->s_flags & SB_ACTIVE)) {
+ if (!drop &&
+ !(inode->i_state & I_DONTCACHE) &&
+ (sb->s_flags & SB_ACTIVE)) {
inode_add_lru(inode);
spin_unlock(&inode->i_lock);
return;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 21cc971fd960..fce52d09fc9c 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2862,8 +2862,7 @@ extern int inode_needs_sync(struct inode *inode);
extern int generic_delete_inode(struct inode *inode);
static inline int generic_drop_inode(struct inode *inode)
{
- return !inode->i_nlink || inode_unhashed(inode) ||
- (inode->i_state & I_DONTCACHE);
+ return !inode->i_nlink || inode_unhashed(inode);
}
extern void d_mark_dontcache(struct inode *inode);
--
2.28.0
[PATCH v2] fs: Kill DCACHE_DONTCACHE dentry even if DCACHE_REFERENCED is set
If DCACHE_REFERENCED is set, fast_dput() will return true, and then retain_dentry() have no chance to check DCACHE_DONTCACHE. As a result, the dentry won't be killed and the corresponding inode can't be evicted. In the following example, the DAX policy can't take effects unless we do a drop_caches manually. # DCACHE_LRU_LIST will be set echo abcdefg > test.txt # DCACHE_REFERENCED will be set and DCACHE_DONTCACHE can't do anything xfs_io -c 'chattr +x' test.txt # Drop caches to make DAX changing take effects echo 2 > /proc/sys/vm/drop_caches What this patch does is preventing fast_dput() from returning true if DCACHE_DONTCACHE is set. Then retain_dentry() will detect the DCACHE_DONTCACHE and will return false. As a result, the dentry will be killed and the inode will be evicted. In this way, if we change per-file DAX policy, it will take effects automatically after this file is closed by all processes. I also add some comments to make the code more clear. Signed-off-by: Hao Li --- v1 is split into two standalone patch as discussed in [1], and the first patch has been reviewed in [2]. This is the second patch. [1]: https://lore.kernel.org/linux-fsdevel/20200831003407.ge12...@dread.disaster.area/ [2]: https://lore.kernel.org/linux-fsdevel/20200906214002.gi12...@dread.disaster.area/ fs/dcache.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/fs/dcache.c b/fs/dcache.c index ea0485861d93..97e81a844a96 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -793,10 +793,17 @@ static inline bool fast_dput(struct dentry *dentry) * a reference to the dentry and change that, but * our work is done - we can leave the dentry * around with a zero refcount. +* +* Nevertheless, there are two cases that we should kill +* the dentry anyway. +* 1. free disconnected dentries as soon as their refcount +*reached zero. +* 2. free dentries if they should not be cached. */ smp_rmb(); d_flags = READ_ONCE(dentry->d_flags); - d_flags &= DCACHE_REFERENCED | DCACHE_LRU_LIST | DCACHE_DISCONNECTED; + d_flags &= DCACHE_REFERENCED | DCACHE_LRU_LIST | + DCACHE_DISCONNECTED | DCACHE_DONTCACHE; /* Nothing to do? Dropping the reference was all we needed? */ if (d_flags == (DCACHE_REFERENCED | DCACHE_LRU_LIST) && !d_unhashed(dentry)) -- 2.28.0
Re: [PATCH v2] fs: Handle I_DONTCACHE in iput_final() instead of generic_drop_inode()
On Mon, Sep 07, 2020 at 07:40:02AM +1000, Dave Chinner wrote:
> On Fri, Sep 04, 2020 at 03:59:39PM +0800, Hao Li wrote:
> > If generic_drop_inode() returns true, it means iput_final() can evict
> > this inode regardless of whether it is dirty or not. If we check
> > I_DONTCACHE in generic_drop_inode(), any inode with this bit set will be
> > evicted unconditionally. This is not the desired behavior because
> > I_DONTCACHE only means the inode shouldn't be cached on the LRU list.
> > As for whether we need to evict this inode, this is what
> > generic_drop_inode() should do. This patch corrects the usage of
> > I_DONTCACHE.
> >
> > This patch was proposed in [1].
> >
> > [1]:
> > https://lore.kernel.org/linux-fsdevel/20200831003407.ge12...@dread.disaster.area/
> >
> > Fixes: dae2f8ed7992 ("fs: Lift XFS_IDONTCACHE to the VFS layer")
> > Signed-off-by: Hao Li
> > ---
> > Changes in v2:
> > - Adjust code format
> > - Add Fixes tag in commit message
> >
> > fs/inode.c | 4 +++-
> > include/linux/fs.h | 3 +--
> > 2 files changed, 4 insertions(+), 3 deletions(-)
>
> Looks good.
>
> Reviewed-by: Dave Chinner
>
Hi,
As discussed in [1], this patch is the basis of another one. Could I
submit the second patch now to change the DCACHE_DONTCACHE behavior or I
have to wait for this patch to be merged.
[1]: https://lkml.org/lkml/2020/8/30/360
Thanks,
Hao Li
> --
> Dave Chinner
> da...@fromorbit.com
>
>
[PATCH v2] fs: Handle I_DONTCACHE in iput_final() instead of generic_drop_inode()
If generic_drop_inode() returns true, it means iput_final() can evict
this inode regardless of whether it is dirty or not. If we check
I_DONTCACHE in generic_drop_inode(), any inode with this bit set will be
evicted unconditionally. This is not the desired behavior because
I_DONTCACHE only means the inode shouldn't be cached on the LRU list.
As for whether we need to evict this inode, this is what
generic_drop_inode() should do. This patch corrects the usage of
I_DONTCACHE.
This patch was proposed in [1].
[1]:
https://lore.kernel.org/linux-fsdevel/20200831003407.ge12...@dread.disaster.area/
Fixes: dae2f8ed7992 ("fs: Lift XFS_IDONTCACHE to the VFS layer")
Signed-off-by: Hao Li
---
Changes in v2:
- Adjust code format
- Add Fixes tag in commit message
fs/inode.c | 4 +++-
include/linux/fs.h | 3 +--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index 72c4c347afb7..19ad823f781c 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1625,7 +1625,9 @@ static void iput_final(struct inode *inode)
else
drop = generic_drop_inode(inode);
- if (!drop && (sb->s_flags & SB_ACTIVE)) {
+ if (!drop &&
+ !(inode->i_state & I_DONTCACHE) &&
+ (sb->s_flags & SB_ACTIVE)) {
inode_add_lru(inode);
spin_unlock(&inode->i_lock);
return;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e019ea2f1347..93caee80ce47 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2922,8 +2922,7 @@ extern int inode_needs_sync(struct inode *inode);
extern int generic_delete_inode(struct inode *inode);
static inline int generic_drop_inode(struct inode *inode)
{
- return !inode->i_nlink || inode_unhashed(inode) ||
- (inode->i_state & I_DONTCACHE);
+ return !inode->i_nlink || inode_unhashed(inode);
}
extern void d_mark_dontcache(struct inode *inode);
--
2.28.0
[PATCH] fs: Handle I_DONTCACHE in iput_final() instead of generic_drop_inode()
If generic_drop_inode() returns true, it means iput_final() can evict
this inode regardless of whether it is dirty or not. If we check
I_DONTCACHE in generic_drop_inode(), any inode with this bit set will be
evicted unconditionally. This is not the desired behavior because
I_DONTCACHE only means the inode shouldn't be cached on the LRU list.
As for whether we need to evict this inode, this is what
generic_drop_inode() should do. This patch corrects the usage of
I_DONTCACHE.
This patch was proposed in [1].
[1]:
https://lore.kernel.org/linux-fsdevel/20200831003407.ge12...@dread.disaster.area/
Signed-off-by: Hao Li
---
fs/inode.c | 3 ++-
include/linux/fs.h | 3 +--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index 72c4c347afb7..4e45d5ea3d0f 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1625,7 +1625,8 @@ static void iput_final(struct inode *inode)
else
drop = generic_drop_inode(inode);
- if (!drop && (sb->s_flags & SB_ACTIVE)) {
+ if (!drop && !(inode->i_state & I_DONTCACHE) &&
+ (sb->s_flags & SB_ACTIVE)) {
inode_add_lru(inode);
spin_unlock(&inode->i_lock);
return;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e019ea2f1347..93caee80ce47 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2922,8 +2922,7 @@ extern int inode_needs_sync(struct inode *inode);
extern int generic_delete_inode(struct inode *inode);
static inline int generic_drop_inode(struct inode *inode)
{
- return !inode->i_nlink || inode_unhashed(inode) ||
- (inode->i_state & I_DONTCACHE);
+ return !inode->i_nlink || inode_unhashed(inode);
}
extern void d_mark_dontcache(struct inode *inode);
--
2.28.0
[PATCH] fs: Kill DCACHE_DONTCACHE dentry even if DCACHE_REFERENCED is set
Currently, DCACHE_REFERENCED prevents the dentry with DCACHE_DONTCACHE
set from being killed, so the corresponding inode can't be evicted. If
the DAX policy of an inode is changed, we can't make policy changing
take effects unless dropping caches manually.
This patch fixes this problem and flushes the inode to disk to prepare
for evicting it.
Signed-off-by: Hao Li
---
fs/dcache.c | 3 ++-
fs/inode.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index ea0485861d93..486c7409dc82 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -796,7 +796,8 @@ static inline bool fast_dput(struct dentry *dentry)
*/
smp_rmb();
d_flags = READ_ONCE(dentry->d_flags);
- d_flags &= DCACHE_REFERENCED | DCACHE_LRU_LIST | DCACHE_DISCONNECTED;
+ d_flags &= DCACHE_REFERENCED | DCACHE_LRU_LIST | DCACHE_DISCONNECTED
+ | DCACHE_DONTCACHE;
/* Nothing to do? Dropping the reference was all we needed? */
if (d_flags == (DCACHE_REFERENCED | DCACHE_LRU_LIST) &&
!d_unhashed(dentry))
diff --git a/fs/inode.c b/fs/inode.c
index 72c4c347afb7..5218a8aebd7f 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1632,7 +1632,7 @@ static void iput_final(struct inode *inode)
}
state = inode->i_state;
- if (!drop) {
+ if (!drop || (drop && (inode->i_state & I_DONTCACHE))) {
WRITE_ONCE(inode->i_state, state | I_WILL_FREE);
spin_unlock(&inode->i_lock);
--
2.28.0
[PATCH] dax: Fix wrong error-number passed into xas_set_err()
The error-number passed into xas_set_err() should be negative. Otherwise,
the xas_error() will return 0, and grab_mapping_entry() will return the
found entry instead of a SIGBUS error when the entry is not a value.
And then, the subsequent code path would be wrong.
Signed-off-by: Hao Li
---
fs/dax.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/dax.c b/fs/dax.c
index 11b16729b86f..acac675fe7a6 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -488,7 +488,7 @@ static void *grab_mapping_entry(struct xa_state *xas,
if (dax_is_conflict(entry))
goto fallback;
if (!xa_is_value(entry)) {
- xas_set_err(xas, EIO);
+ xas_set_err(xas, -EIO);
goto out_unlock;
}
--
2.28.0
