From: Eric Biggers <ebiggers@kernel.org> To: linux-fsdevel@vger.kernel.org Cc: linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-xfs@vger.kernel.org, Theodore Ts'o <tytso@mit.edu>, Christoph Hellwig <hch@lst.de> Subject: [PATCH 10/13] fs: clean up __mark_inode_dirty() a bit Date: Mon, 4 Jan 2021 16:54:49 -0800 [thread overview] Message-ID: <20210105005452.92521-11-ebiggers@kernel.org> (raw) In-Reply-To: <20210105005452.92521-1-ebiggers@kernel.org> From: Eric Biggers <ebiggers@google.com> Improve some comments, and don't bother checking for the I_DIRTY_TIME flag in the case where we just cleared it. Also, warn if I_DIRTY_TIME and I_DIRTY_PAGES are passed to __mark_inode_dirty() at the same time, as this case isn't handled. Signed-off-by: Eric Biggers <ebiggers@google.com> --- fs/fs-writeback.c | 49 +++++++++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c index 3f5a589399afe..ed76112bd067b 100644 --- a/fs/fs-writeback.c +++ b/fs/fs-writeback.c @@ -2227,23 +2227,24 @@ static noinline void block_dump___mark_inode_dirty(struct inode *inode) } /** - * __mark_inode_dirty - internal function + * __mark_inode_dirty - internal function to mark an inode dirty * * @inode: inode to mark - * @flags: what kind of dirty (i.e. I_DIRTY_SYNC) + * @flags: what kind of dirty, e.g. I_DIRTY_SYNC. This can be a combination of + * multiple I_DIRTY_* flags, except that I_DIRTY_TIME can't be combined + * with I_DIRTY_PAGES. * - * Mark an inode as dirty. Callers should use mark_inode_dirty or - * mark_inode_dirty_sync. + * Mark an inode as dirty. We notify the filesystem, then update the inode's + * dirty flags. Then, if needed we add the inode to the appropriate dirty list. * - * Put the inode on the super block's dirty list. + * Most callers should use mark_inode_dirty() or mark_inode_dirty_sync() + * instead of calling this directly. * - * CAREFUL! We mark it dirty unconditionally, but move it onto the - * dirty list only if it is hashed or if it refers to a blockdev. - * If it was not hashed, it will never be added to the dirty list - * even if it is later hashed, as it will have been marked dirty already. + * CAREFUL! We only add the inode to the dirty list if it is hashed or if it + * refers to a blockdev. Unhashed inodes will never be added to the dirty list + * even if they are later hashed, as they will have been marked dirty already. * - * In short, make sure you hash any inodes _before_ you start marking - * them dirty. + * In short, ensure you hash any inodes _before_ you start marking them dirty. * * Note that for blockdevs, inode->dirtied_when represents the dirtying time of * the block-special inode (/dev/hda1) itself. And the ->dirtied_when field of @@ -2255,25 +2256,34 @@ static noinline void block_dump___mark_inode_dirty(struct inode *inode) void __mark_inode_dirty(struct inode *inode, int flags) { struct super_block *sb = inode->i_sb; - int dirtytime; + int dirtytime = 0; trace_writeback_mark_inode_dirty(inode, flags); - /* - * Don't do this for I_DIRTY_PAGES - that doesn't actually - * dirty the inode itself - */ if (flags & I_DIRTY_INODE) { + /* + * Notify the filesystem about the inode being dirtied, so that + * (if needed) it can update on-disk fields and journal the + * inode. This is only needed when the inode itself is being + * dirtied now. I.e. it's only needed for I_DIRTY_INODE, not + * for just I_DIRTY_PAGES or I_DIRTY_TIME. + */ trace_writeback_dirty_inode_start(inode, flags); - if (sb->s_op->dirty_inode) sb->s_op->dirty_inode(inode, flags & I_DIRTY_INODE); - trace_writeback_dirty_inode(inode, flags); + /* I_DIRTY_INODE supersedes I_DIRTY_TIME. */ flags &= ~I_DIRTY_TIME; + } else { + /* + * Else it's either I_DIRTY_PAGES, I_DIRTY_TIME, or nothing. + * (We don't support setting both I_DIRTY_PAGES and I_DIRTY_TIME + * in one call to __mark_inode_dirty().) + */ + dirtytime = flags & I_DIRTY_TIME; + WARN_ON_ONCE(dirtytime && (flags != I_DIRTY_TIME)); } - dirtytime = flags & I_DIRTY_TIME; /* * Paired with smp_mb() in __writeback_single_inode() for the @@ -2296,6 +2306,7 @@ void __mark_inode_dirty(struct inode *inode, int flags) inode_attach_wb(inode, NULL); + /* I_DIRTY_INODE supersedes I_DIRTY_TIME. */ if (flags & I_DIRTY_INODE) inode->i_state &= ~I_DIRTY_TIME; inode->i_state |= flags; -- 2.30.0
WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers@kernel.org> To: linux-fsdevel@vger.kernel.org Cc: linux-xfs@vger.kernel.org, linux-ext4@vger.kernel.org, Theodore Ts'o <tytso@mit.edu>, Christoph Hellwig <hch@lst.de>, linux-f2fs-devel@lists.sourceforge.net Subject: [f2fs-dev] [PATCH 10/13] fs: clean up __mark_inode_dirty() a bit Date: Mon, 4 Jan 2021 16:54:49 -0800 [thread overview] Message-ID: <20210105005452.92521-11-ebiggers@kernel.org> (raw) In-Reply-To: <20210105005452.92521-1-ebiggers@kernel.org> From: Eric Biggers <ebiggers@google.com> Improve some comments, and don't bother checking for the I_DIRTY_TIME flag in the case where we just cleared it. Also, warn if I_DIRTY_TIME and I_DIRTY_PAGES are passed to __mark_inode_dirty() at the same time, as this case isn't handled. Signed-off-by: Eric Biggers <ebiggers@google.com> --- fs/fs-writeback.c | 49 +++++++++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c index 3f5a589399afe..ed76112bd067b 100644 --- a/fs/fs-writeback.c +++ b/fs/fs-writeback.c @@ -2227,23 +2227,24 @@ static noinline void block_dump___mark_inode_dirty(struct inode *inode) } /** - * __mark_inode_dirty - internal function + * __mark_inode_dirty - internal function to mark an inode dirty * * @inode: inode to mark - * @flags: what kind of dirty (i.e. I_DIRTY_SYNC) + * @flags: what kind of dirty, e.g. I_DIRTY_SYNC. This can be a combination of + * multiple I_DIRTY_* flags, except that I_DIRTY_TIME can't be combined + * with I_DIRTY_PAGES. * - * Mark an inode as dirty. Callers should use mark_inode_dirty or - * mark_inode_dirty_sync. + * Mark an inode as dirty. We notify the filesystem, then update the inode's + * dirty flags. Then, if needed we add the inode to the appropriate dirty list. * - * Put the inode on the super block's dirty list. + * Most callers should use mark_inode_dirty() or mark_inode_dirty_sync() + * instead of calling this directly. * - * CAREFUL! We mark it dirty unconditionally, but move it onto the - * dirty list only if it is hashed or if it refers to a blockdev. - * If it was not hashed, it will never be added to the dirty list - * even if it is later hashed, as it will have been marked dirty already. + * CAREFUL! We only add the inode to the dirty list if it is hashed or if it + * refers to a blockdev. Unhashed inodes will never be added to the dirty list + * even if they are later hashed, as they will have been marked dirty already. * - * In short, make sure you hash any inodes _before_ you start marking - * them dirty. + * In short, ensure you hash any inodes _before_ you start marking them dirty. * * Note that for blockdevs, inode->dirtied_when represents the dirtying time of * the block-special inode (/dev/hda1) itself. And the ->dirtied_when field of @@ -2255,25 +2256,34 @@ static noinline void block_dump___mark_inode_dirty(struct inode *inode) void __mark_inode_dirty(struct inode *inode, int flags) { struct super_block *sb = inode->i_sb; - int dirtytime; + int dirtytime = 0; trace_writeback_mark_inode_dirty(inode, flags); - /* - * Don't do this for I_DIRTY_PAGES - that doesn't actually - * dirty the inode itself - */ if (flags & I_DIRTY_INODE) { + /* + * Notify the filesystem about the inode being dirtied, so that + * (if needed) it can update on-disk fields and journal the + * inode. This is only needed when the inode itself is being + * dirtied now. I.e. it's only needed for I_DIRTY_INODE, not + * for just I_DIRTY_PAGES or I_DIRTY_TIME. + */ trace_writeback_dirty_inode_start(inode, flags); - if (sb->s_op->dirty_inode) sb->s_op->dirty_inode(inode, flags & I_DIRTY_INODE); - trace_writeback_dirty_inode(inode, flags); + /* I_DIRTY_INODE supersedes I_DIRTY_TIME. */ flags &= ~I_DIRTY_TIME; + } else { + /* + * Else it's either I_DIRTY_PAGES, I_DIRTY_TIME, or nothing. + * (We don't support setting both I_DIRTY_PAGES and I_DIRTY_TIME + * in one call to __mark_inode_dirty().) + */ + dirtytime = flags & I_DIRTY_TIME; + WARN_ON_ONCE(dirtytime && (flags != I_DIRTY_TIME)); } - dirtytime = flags & I_DIRTY_TIME; /* * Paired with smp_mb() in __writeback_single_inode() for the @@ -2296,6 +2306,7 @@ void __mark_inode_dirty(struct inode *inode, int flags) inode_attach_wb(inode, NULL); + /* I_DIRTY_INODE supersedes I_DIRTY_TIME. */ if (flags & I_DIRTY_INODE) inode->i_state &= ~I_DIRTY_TIME; inode->i_state |= flags; -- 2.30.0 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next prev parent reply other threads:[~2021-01-05 0:56 UTC|newest] Thread overview: 73+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-01-05 0:54 [PATCH 00/13] lazytime fixes and cleanups Eric Biggers 2021-01-05 0:54 ` [f2fs-dev] " Eric Biggers 2021-01-05 0:54 ` [PATCH 01/13] fs: avoid double-writing inodes on lazytime expiration Eric Biggers 2021-01-05 0:54 ` [f2fs-dev] " Eric Biggers 2021-01-07 14:47 ` Jan Kara 2021-01-07 14:58 ` Matthew Wilcox 2021-01-07 14:58 ` [f2fs-dev] " Matthew Wilcox 2021-01-07 21:46 ` Eric Biggers 2021-01-07 21:46 ` [f2fs-dev] " Eric Biggers 2021-01-08 8:54 ` Christoph Hellwig 2021-01-08 8:54 ` [f2fs-dev] " Christoph Hellwig 2021-01-08 9:01 ` Christoph Hellwig 2021-01-08 9:01 ` [f2fs-dev] " Christoph Hellwig 2021-01-09 17:11 ` Eric Biggers 2021-01-09 17:11 ` [f2fs-dev] " Eric Biggers 2021-01-05 0:54 ` [PATCH 02/13] gfs2: don't worry about I_DIRTY_TIME in gfs2_fsync() Eric Biggers 2021-01-05 0:54 ` [f2fs-dev] " Eric Biggers 2021-01-08 8:54 ` Christoph Hellwig 2021-01-08 8:54 ` [f2fs-dev] " Christoph Hellwig 2021-01-05 0:54 ` [PATCH 03/13] fs: only specify I_DIRTY_TIME when needed in generic_update_time() Eric Biggers 2021-01-05 0:54 ` [f2fs-dev] " Eric Biggers 2021-01-08 8:57 ` Christoph Hellwig 2021-01-08 8:57 ` [f2fs-dev] " Christoph Hellwig 2021-01-05 0:54 ` [PATCH 04/13] fat: only specify I_DIRTY_TIME when needed in fat_update_time() Eric Biggers 2021-01-05 0:54 ` [f2fs-dev] " Eric Biggers 2021-01-07 13:13 ` Jan Kara 2021-01-07 13:13 ` [f2fs-dev] " Jan Kara 2021-01-07 19:10 ` Eric Biggers 2021-01-07 19:10 ` [f2fs-dev] " Eric Biggers 2021-01-05 0:54 ` [PATCH 05/13] fs: don't call ->dirty_inode for lazytime timestamp updates Eric Biggers 2021-01-05 0:54 ` [f2fs-dev] " Eric Biggers 2021-01-07 13:17 ` Jan Kara 2021-01-07 13:17 ` [f2fs-dev] " Jan Kara 2021-01-07 13:18 ` Jan Kara 2021-01-07 13:18 ` [f2fs-dev] " Jan Kara 2021-01-08 9:02 ` Christoph Hellwig 2021-01-08 9:02 ` [f2fs-dev] " Christoph Hellwig 2021-01-05 0:54 ` [PATCH 06/13] fs: pass only I_DIRTY_INODE flags to ->dirty_inode Eric Biggers 2021-01-05 0:54 ` [f2fs-dev] " Eric Biggers 2021-01-08 9:02 ` Christoph Hellwig 2021-01-08 9:02 ` [f2fs-dev] " Christoph Hellwig 2021-01-05 0:54 ` [PATCH 07/13] fs: correctly document the inode dirty flags Eric Biggers 2021-01-05 0:54 ` [f2fs-dev] " Eric Biggers 2021-01-08 9:03 ` Christoph Hellwig 2021-01-08 9:03 ` [f2fs-dev] " Christoph Hellwig 2021-01-05 0:54 ` [PATCH 08/13] ext4: simplify i_state checks in __ext4_update_other_inode_time() Eric Biggers 2021-01-05 0:54 ` [f2fs-dev] " Eric Biggers 2021-01-07 13:24 ` Jan Kara 2021-01-07 13:24 ` [f2fs-dev] " Jan Kara 2021-01-07 19:06 ` Eric Biggers 2021-01-07 19:06 ` [f2fs-dev] " Eric Biggers 2021-01-05 0:54 ` [PATCH 09/13] fs: drop redundant checks from __writeback_single_inode() Eric Biggers 2021-01-05 0:54 ` [f2fs-dev] " Eric Biggers 2021-01-08 9:12 ` Christoph Hellwig 2021-01-08 9:12 ` [f2fs-dev] " Christoph Hellwig 2021-01-05 0:54 ` Eric Biggers [this message] 2021-01-05 0:54 ` [f2fs-dev] [PATCH 10/13] fs: clean up __mark_inode_dirty() a bit Eric Biggers 2021-01-08 9:10 ` Christoph Hellwig 2021-01-08 9:10 ` [f2fs-dev] " Christoph Hellwig 2021-01-05 0:54 ` [PATCH 11/13] fs: add a lazytime_expired method Eric Biggers 2021-01-05 0:54 ` [f2fs-dev] " Eric Biggers 2021-01-07 14:02 ` Jan Kara 2021-01-07 14:02 ` [f2fs-dev] " Jan Kara 2021-01-07 22:05 ` Eric Biggers 2021-01-07 22:05 ` [f2fs-dev] " Eric Biggers 2021-01-08 9:14 ` Christoph Hellwig 2021-01-08 9:14 ` [f2fs-dev] " Christoph Hellwig 2021-01-05 0:54 ` [PATCH 12/13] xfs: remove a stale comment from xfs_file_aio_write_checks() Eric Biggers 2021-01-05 0:54 ` [f2fs-dev] " Eric Biggers 2021-01-08 9:15 ` Christoph Hellwig 2021-01-08 9:15 ` [f2fs-dev] " Christoph Hellwig 2021-01-05 0:54 ` [PATCH 13/13] xfs: implement lazytime_expired Eric Biggers 2021-01-05 0:54 ` [f2fs-dev] " Eric Biggers
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210105005452.92521-11-ebiggers@kernel.org \ --to=ebiggers@kernel.org \ --cc=hch@lst.de \ --cc=linux-ext4@vger.kernel.org \ --cc=linux-f2fs-devel@lists.sourceforge.net \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-xfs@vger.kernel.org \ --cc=tytso@mit.edu \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.