* [PATCH 1/4] gfs2: fix bogus __mark_inode_dirty(I_DIRTY_SYNC | I_DIRTY_DATASYNC) calls
2018-02-21 15:54 a few I_DIRTY* related cleanups Christoph Hellwig
@ 2018-02-21 15:54 ` Christoph Hellwig
2018-02-21 15:54 ` [PATCH 2/4] ntfs: fix bogus __mark_inode_dirty(I_DIRTY_SYNC | I_DIRTY_DATASYNC) call Christoph Hellwig
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2018-02-21 15:54 UTC (permalink / raw)
To: viro; +Cc: linux-fsdevel, cluster-devel, linux-mtd, linux-ntfs-dev
I_DIRTY_DATASYNC is a strict superset of I_DIRTY_SYNC semantics, as
in mark dirty to be written out by fdatasync as well. So dirtying
for both flags makes no sense.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/gfs2/xattr.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c
index 05de20954659..f2bce1e0f6fb 100644
--- a/fs/gfs2/xattr.c
+++ b/fs/gfs2/xattr.c
@@ -308,7 +308,7 @@ static int ea_dealloc_unstuffed(struct gfs2_inode *ip, struct buffer_head *bh,
}
ip->i_inode.i_ctime = current_time(&ip->i_inode);
- __mark_inode_dirty(&ip->i_inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
+ __mark_inode_dirty(&ip->i_inode, I_DIRTY_DATASYNC);
gfs2_trans_end(sdp);
@@ -768,7 +768,7 @@ static int ea_alloc_skeleton(struct gfs2_inode *ip, struct gfs2_ea_request *er,
goto out_end_trans;
ip->i_inode.i_ctime = current_time(&ip->i_inode);
- __mark_inode_dirty(&ip->i_inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
+ __mark_inode_dirty(&ip->i_inode, I_DIRTY_DATASYNC);
out_end_trans:
gfs2_trans_end(GFS2_SB(&ip->i_inode));
@@ -896,7 +896,7 @@ static int ea_set_simple_noalloc(struct gfs2_inode *ip, struct buffer_head *bh,
ea_set_remove_stuffed(ip, es->es_el);
ip->i_inode.i_ctime = current_time(&ip->i_inode);
- __mark_inode_dirty(&ip->i_inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
+ __mark_inode_dirty(&ip->i_inode, I_DIRTY_DATASYNC);
gfs2_trans_end(GFS2_SB(&ip->i_inode));
return error;
@@ -1114,7 +1114,7 @@ static int ea_remove_stuffed(struct gfs2_inode *ip, struct gfs2_ea_location *el)
}
ip->i_inode.i_ctime = current_time(&ip->i_inode);
- __mark_inode_dirty(&ip->i_inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
+ __mark_inode_dirty(&ip->i_inode, I_DIRTY_DATASYNC);
gfs2_trans_end(GFS2_SB(&ip->i_inode));
--
2.14.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/4] ntfs: fix bogus __mark_inode_dirty(I_DIRTY_SYNC | I_DIRTY_DATASYNC) call
2018-02-21 15:54 a few I_DIRTY* related cleanups Christoph Hellwig
2018-02-21 15:54 ` [PATCH 1/4] gfs2: fix bogus __mark_inode_dirty(I_DIRTY_SYNC | I_DIRTY_DATASYNC) calls Christoph Hellwig
@ 2018-02-21 15:54 ` Christoph Hellwig
2018-02-21 15:54 ` [PATCH 3/4] ubifs: " Christoph Hellwig
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2018-02-21 15:54 UTC (permalink / raw)
To: viro; +Cc: linux-fsdevel, cluster-devel, linux-mtd, linux-ntfs-dev
I_DIRTY_DATASYNC is a strict superset of I_DIRTY_SYNC semantics, as
in mark dirty to be written out by fdatasync as well. So dirtying
for both flags makes no sense.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/ntfs/mft.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
index 2831f495a674..32c523cf5a2d 100644
--- a/fs/ntfs/mft.c
+++ b/fs/ntfs/mft.c
@@ -381,7 +381,7 @@ MFT_RECORD *map_extent_mft_record(ntfs_inode *base_ni, MFT_REF mref,
* vfs inode dirty. This ensures that any changes to the mft record are
* written out to disk.
*
- * NOTE: We only set I_DIRTY_SYNC and I_DIRTY_DATASYNC (and not I_DIRTY_PAGES)
+ * NOTE: We only set I_DIRTY_DATASYNC (and not I_DIRTY_PAGES)
* on the base vfs inode, because even though file data may have been modified,
* it is dirty in the inode meta data rather than the data page cache of the
* inode, and thus there are no data pages that need writing out. Therefore, a
@@ -407,7 +407,7 @@ void __mark_mft_record_dirty(ntfs_inode *ni)
else
base_ni = ni->ext.base_ntfs_ino;
mutex_unlock(&ni->extent_lock);
- __mark_inode_dirty(VFS_I(base_ni), I_DIRTY_SYNC | I_DIRTY_DATASYNC);
+ __mark_inode_dirty(VFS_I(base_ni), I_DIRTY_DATASYNC);
}
static const char *ntfs_please_email = "Please email "
--
2.14.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 3/4] ubifs: fix bogus __mark_inode_dirty(I_DIRTY_SYNC | I_DIRTY_DATASYNC) call
2018-02-21 15:54 a few I_DIRTY* related cleanups Christoph Hellwig
2018-02-21 15:54 ` [PATCH 1/4] gfs2: fix bogus __mark_inode_dirty(I_DIRTY_SYNC | I_DIRTY_DATASYNC) calls Christoph Hellwig
2018-02-21 15:54 ` [PATCH 2/4] ntfs: fix bogus __mark_inode_dirty(I_DIRTY_SYNC | I_DIRTY_DATASYNC) call Christoph Hellwig
@ 2018-02-21 15:54 ` Christoph Hellwig
2018-02-21 15:54 ` [PATCH 4/4] fs: move I_DIRTY_INODE to fs.h Christoph Hellwig
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2018-02-21 15:54 UTC (permalink / raw)
To: viro; +Cc: linux-fsdevel, cluster-devel, linux-mtd, linux-ntfs-dev
I_DIRTY_DATASYNC is a strict superset of I_DIRTY_SYNC semantics, as
in mark dirty to be written out by fdatasync as well. So dirtying
for both flags makes no sense.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/ubifs/file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index cf348ba99238..1acb2ff505e6 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1256,7 +1256,7 @@ static int do_setattr(struct ubifs_info *c, struct inode *inode,
* Inode length changed, so we have to make sure
* @I_DIRTY_DATASYNC is set.
*/
- __mark_inode_dirty(inode, I_DIRTY_SYNC | I_DIRTY_DATASYNC);
+ __mark_inode_dirty(inode, I_DIRTY_DATASYNC);
else
mark_inode_dirty_sync(inode);
mutex_unlock(&ui->ui_mutex);
--
2.14.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 4/4] fs: move I_DIRTY_INODE to fs.h
2018-02-21 15:54 a few I_DIRTY* related cleanups Christoph Hellwig
` (2 preceding siblings ...)
2018-02-21 15:54 ` [PATCH 3/4] ubifs: " Christoph Hellwig
@ 2018-02-21 15:54 ` Christoph Hellwig
2018-02-26 15:42 ` Jan Kara
2018-02-21 17:18 ` [Cluster-devel] a few I_DIRTY* related cleanups Steven Whitehouse
2018-03-19 11:08 ` Christoph Hellwig
5 siblings, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2018-02-21 15:54 UTC (permalink / raw)
To: viro; +Cc: linux-fsdevel, cluster-devel, linux-mtd, linux-ntfs-dev
And use it in a few more places rather than opencoding the values.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/ext4/inode.c | 4 ++--
fs/fs-writeback.c | 9 +++------
fs/gfs2/super.c | 2 +-
include/linux/fs.h | 3 ++-
4 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index c94780075b04..6d2a18991fcb 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5032,12 +5032,12 @@ static int other_inode_match(struct inode * inode, unsigned long ino,
if ((inode->i_ino != ino) ||
(inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW |
- I_DIRTY_SYNC | I_DIRTY_DATASYNC)) ||
+ I_DIRTY_INODE)) ||
((inode->i_state & I_DIRTY_TIME) == 0))
return 0;
spin_lock(&inode->i_lock);
if (((inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW |
- I_DIRTY_SYNC | I_DIRTY_DATASYNC)) == 0) &&
+ I_DIRTY_INODE)) == 0) &&
(inode->i_state & I_DIRTY_TIME)) {
struct ext4_inode_info *ei = EXT4_I(inode);
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index d4d04fee568a..1280f915079b 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -1343,7 +1343,7 @@ __writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
dirty = inode->i_state & I_DIRTY;
if (inode->i_state & I_DIRTY_TIME) {
- if ((dirty & (I_DIRTY_SYNC | I_DIRTY_DATASYNC)) ||
+ if ((dirty & I_DIRTY_INODE) ||
wbc->sync_mode == WB_SYNC_ALL ||
unlikely(inode->i_state & I_DIRTY_TIME_EXPIRED) ||
unlikely(time_after(jiffies,
@@ -2112,7 +2112,6 @@ static noinline void block_dump___mark_inode_dirty(struct inode *inode)
*/
void __mark_inode_dirty(struct inode *inode, int flags)
{
-#define I_DIRTY_INODE (I_DIRTY_SYNC | I_DIRTY_DATASYNC)
struct super_block *sb = inode->i_sb;
int dirtytime;
@@ -2122,7 +2121,7 @@ void __mark_inode_dirty(struct inode *inode, int flags)
* Don't do this for I_DIRTY_PAGES - that doesn't actually
* dirty the inode itself
*/
- if (flags & (I_DIRTY_SYNC | I_DIRTY_DATASYNC | I_DIRTY_TIME)) {
+ if (flags & (I_DIRTY_INODE | I_DIRTY_TIME)) {
trace_writeback_dirty_inode_start(inode, flags);
if (sb->s_op->dirty_inode)
@@ -2197,7 +2196,7 @@ void __mark_inode_dirty(struct inode *inode, int flags)
if (dirtytime)
inode->dirtied_time_when = jiffies;
- if (inode->i_state & (I_DIRTY_INODE | I_DIRTY_PAGES))
+ if (inode->i_state & I_DIRTY)
dirty_list = &wb->b_dirty;
else
dirty_list = &wb->b_dirty_time;
@@ -2221,8 +2220,6 @@ void __mark_inode_dirty(struct inode *inode, int flags)
}
out_unlock_inode:
spin_unlock(&inode->i_lock);
-
-#undef I_DIRTY_INODE
}
EXPORT_SYMBOL(__mark_inode_dirty);
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 620be0521866..cf5c7f3080d2 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -800,7 +800,7 @@ static void gfs2_dirty_inode(struct inode *inode, int flags)
int need_endtrans = 0;
int ret;
- if (!(flags & (I_DIRTY_DATASYNC|I_DIRTY_SYNC)))
+ if (!(flags & I_DIRTY_INODE))
return;
if (unlikely(test_bit(SDF_SHUTDOWN, &sdp->sd_flags)))
return;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 2a815560fda0..86112ab0a1c0 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2014,7 +2014,8 @@ static inline void init_sync_kiocb(struct kiocb *kiocb, struct file *filp)
#define I_WB_SWITCH (1 << 13)
#define I_OVL_INUSE (1 << 14)
-#define I_DIRTY (I_DIRTY_SYNC | I_DIRTY_DATASYNC | I_DIRTY_PAGES)
+#define I_DIRTY_INODE (I_DIRTY_SYNC | I_DIRTY_DATASYNC)
+#define I_DIRTY (I_DIRTY_INODE | I_DIRTY_PAGES)
#define I_DIRTY_ALL (I_DIRTY | I_DIRTY_TIME)
extern void __mark_inode_dirty(struct inode *, int);
--
2.14.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 4/4] fs: move I_DIRTY_INODE to fs.h
2018-02-21 15:54 ` [PATCH 4/4] fs: move I_DIRTY_INODE to fs.h Christoph Hellwig
@ 2018-02-26 15:42 ` Jan Kara
0 siblings, 0 replies; 8+ messages in thread
From: Jan Kara @ 2018-02-26 15:42 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, linux-fsdevel, cluster-devel, linux-mtd, linux-ntfs-dev
On Wed 21-02-18 07:54:49, Christoph Hellwig wrote:
> And use it in a few more places rather than opencoding the values.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks fine to me. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/inode.c | 4 ++--
> fs/fs-writeback.c | 9 +++------
> fs/gfs2/super.c | 2 +-
> include/linux/fs.h | 3 ++-
> 4 files changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index c94780075b04..6d2a18991fcb 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -5032,12 +5032,12 @@ static int other_inode_match(struct inode * inode, unsigned long ino,
>
> if ((inode->i_ino != ino) ||
> (inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW |
> - I_DIRTY_SYNC | I_DIRTY_DATASYNC)) ||
> + I_DIRTY_INODE)) ||
> ((inode->i_state & I_DIRTY_TIME) == 0))
> return 0;
> spin_lock(&inode->i_lock);
> if (((inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW |
> - I_DIRTY_SYNC | I_DIRTY_DATASYNC)) == 0) &&
> + I_DIRTY_INODE)) == 0) &&
> (inode->i_state & I_DIRTY_TIME)) {
> struct ext4_inode_info *ei = EXT4_I(inode);
>
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index d4d04fee568a..1280f915079b 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -1343,7 +1343,7 @@ __writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
>
> dirty = inode->i_state & I_DIRTY;
> if (inode->i_state & I_DIRTY_TIME) {
> - if ((dirty & (I_DIRTY_SYNC | I_DIRTY_DATASYNC)) ||
> + if ((dirty & I_DIRTY_INODE) ||
> wbc->sync_mode == WB_SYNC_ALL ||
> unlikely(inode->i_state & I_DIRTY_TIME_EXPIRED) ||
> unlikely(time_after(jiffies,
> @@ -2112,7 +2112,6 @@ static noinline void block_dump___mark_inode_dirty(struct inode *inode)
> */
> void __mark_inode_dirty(struct inode *inode, int flags)
> {
> -#define I_DIRTY_INODE (I_DIRTY_SYNC | I_DIRTY_DATASYNC)
> struct super_block *sb = inode->i_sb;
> int dirtytime;
>
> @@ -2122,7 +2121,7 @@ void __mark_inode_dirty(struct inode *inode, int flags)
> * Don't do this for I_DIRTY_PAGES - that doesn't actually
> * dirty the inode itself
> */
> - if (flags & (I_DIRTY_SYNC | I_DIRTY_DATASYNC | I_DIRTY_TIME)) {
> + if (flags & (I_DIRTY_INODE | I_DIRTY_TIME)) {
> trace_writeback_dirty_inode_start(inode, flags);
>
> if (sb->s_op->dirty_inode)
> @@ -2197,7 +2196,7 @@ void __mark_inode_dirty(struct inode *inode, int flags)
> if (dirtytime)
> inode->dirtied_time_when = jiffies;
>
> - if (inode->i_state & (I_DIRTY_INODE | I_DIRTY_PAGES))
> + if (inode->i_state & I_DIRTY)
> dirty_list = &wb->b_dirty;
> else
> dirty_list = &wb->b_dirty_time;
> @@ -2221,8 +2220,6 @@ void __mark_inode_dirty(struct inode *inode, int flags)
> }
> out_unlock_inode:
> spin_unlock(&inode->i_lock);
> -
> -#undef I_DIRTY_INODE
> }
> EXPORT_SYMBOL(__mark_inode_dirty);
>
> diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> index 620be0521866..cf5c7f3080d2 100644
> --- a/fs/gfs2/super.c
> +++ b/fs/gfs2/super.c
> @@ -800,7 +800,7 @@ static void gfs2_dirty_inode(struct inode *inode, int flags)
> int need_endtrans = 0;
> int ret;
>
> - if (!(flags & (I_DIRTY_DATASYNC|I_DIRTY_SYNC)))
> + if (!(flags & I_DIRTY_INODE))
> return;
> if (unlikely(test_bit(SDF_SHUTDOWN, &sdp->sd_flags)))
> return;
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 2a815560fda0..86112ab0a1c0 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2014,7 +2014,8 @@ static inline void init_sync_kiocb(struct kiocb *kiocb, struct file *filp)
> #define I_WB_SWITCH (1 << 13)
> #define I_OVL_INUSE (1 << 14)
>
> -#define I_DIRTY (I_DIRTY_SYNC | I_DIRTY_DATASYNC | I_DIRTY_PAGES)
> +#define I_DIRTY_INODE (I_DIRTY_SYNC | I_DIRTY_DATASYNC)
> +#define I_DIRTY (I_DIRTY_INODE | I_DIRTY_PAGES)
> #define I_DIRTY_ALL (I_DIRTY | I_DIRTY_TIME)
>
> extern void __mark_inode_dirty(struct inode *, int);
> --
> 2.14.2
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Cluster-devel] a few I_DIRTY* related cleanups
2018-02-21 15:54 a few I_DIRTY* related cleanups Christoph Hellwig
` (3 preceding siblings ...)
2018-02-21 15:54 ` [PATCH 4/4] fs: move I_DIRTY_INODE to fs.h Christoph Hellwig
@ 2018-02-21 17:18 ` Steven Whitehouse
2018-03-19 11:08 ` Christoph Hellwig
5 siblings, 0 replies; 8+ messages in thread
From: Steven Whitehouse @ 2018-02-21 17:18 UTC (permalink / raw)
To: Christoph Hellwig, viro
Cc: linux-fsdevel, cluster-devel, linux-mtd, linux-ntfs-dev
On 21/02/18 15:54, Christoph Hellwig wrote:
> Move I_DIRTY_INODE to fs.h, and use the I_DIRTY* defines more consistently.
>
Thanks - looks good to me:
Acked-by: Steven Whitehouse <swhiteho@redhat.com>
Steve.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: a few I_DIRTY* related cleanups
2018-02-21 15:54 a few I_DIRTY* related cleanups Christoph Hellwig
` (4 preceding siblings ...)
2018-02-21 17:18 ` [Cluster-devel] a few I_DIRTY* related cleanups Steven Whitehouse
@ 2018-03-19 11:08 ` Christoph Hellwig
5 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2018-03-19 11:08 UTC (permalink / raw)
To: viro; +Cc: linux-fsdevel, cluster-devel, linux-mtd, linux-ntfs-dev
On Wed, Feb 21, 2018 at 07:54:45AM -0800, Christoph Hellwig wrote:
> Move I_DIRTY_INODE to fs.h, and use the I_DIRTY* defines more consistently.
Al, are you going to pick this up?
^ permalink raw reply [flat|nested] 8+ messages in thread