diff mbox

[v7,05/12] fs: allow to always dirty inode in __generic_write_end

Message ID 20180604123729.23414-6-agruenba@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andreas Gruenbacher June 4, 2018, 12:37 p.m. UTC
In __generic_write_end, we usually dirty the inode only when the file
size changes.  Allow to dirty the inode even when the file size stays
the same; this will be used when writing to inline data stored in the
inode.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/buffer.c   | 9 ++++-----
 fs/internal.h | 2 +-
 2 files changed, 5 insertions(+), 6 deletions(-)

Comments

Christoph Hellwig June 4, 2018, 12:48 p.m. UTC | #1
On Mon, Jun 04, 2018 at 02:37:22PM +0200, Andreas Gruenbacher wrote:
> @@ -2106,7 +2105,7 @@ int __generic_write_end(struct inode *inode, loff_t pos, unsigned copied,
>  	 * ordering of page lock and transaction start for journaling
>  	 * filesystems.
>  	 */
> -	if (i_size_changed)
> +	if (dirty_inode)
>  		mark_inode_dirty(inode);
>  	return copied;

Calling mark_inode_dirty on an already dirty inode is cheap, so how
about just calling it directly in your caller that always wants to
set the inode dirty?
Andreas Grünbacher June 4, 2018, 4:24 p.m. UTC | #2
2018-06-04 14:48 GMT+02:00 Christoph Hellwig <hch@lst.de>:
> On Mon, Jun 04, 2018 at 02:37:22PM +0200, Andreas Gruenbacher wrote:
>> @@ -2106,7 +2105,7 @@ int __generic_write_end(struct inode *inode, loff_t pos, unsigned copied,
>>        * ordering of page lock and transaction start for journaling
>>        * filesystems.
>>        */
>> -     if (i_size_changed)
>> +     if (dirty_inode)
>>               mark_inode_dirty(inode);
>>       return copied;
>
> Calling mark_inode_dirty on an already dirty inode is cheap, so how
> about just calling it directly in your caller that always wants to
> set the inode dirty?

The dirty_inode hook is where gfs2 converts the in-core inode into the
on-disk format (the equivalent of xfs_inode_to_disk), so it's not
quite that cheap. I realize this could be done differently in gfs2,
but that's not where we stand today, and I really can't fix ten things
all at the same time.

Thanks,
Andreas
diff mbox

Patch

diff --git a/fs/buffer.c b/fs/buffer.c
index cb5bbf22b1ce..a59d281ca51e 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2078,10 +2078,9 @@  int block_write_begin(struct address_space *mapping, loff_t pos, unsigned len,
 EXPORT_SYMBOL(block_write_begin);
 
 int __generic_write_end(struct inode *inode, loff_t pos, unsigned copied,
-		struct page *page)
+		struct page *page, bool dirty_inode)
 {
 	loff_t old_size = inode->i_size;
-	bool i_size_changed = false;
 
 	/*
 	 * No need to use i_size_read() here, the i_size cannot change under us
@@ -2092,7 +2091,7 @@  int __generic_write_end(struct inode *inode, loff_t pos, unsigned copied,
 	 */
 	if (pos + copied > inode->i_size) {
 		i_size_write(inode, pos + copied);
-		i_size_changed = true;
+		dirty_inode = true;
 	}
 
 	unlock_page(page);
@@ -2106,7 +2105,7 @@  int __generic_write_end(struct inode *inode, loff_t pos, unsigned copied,
 	 * ordering of page lock and transaction start for journaling
 	 * filesystems.
 	 */
-	if (i_size_changed)
+	if (dirty_inode)
 		mark_inode_dirty(inode);
 	return copied;
 }
@@ -2152,7 +2151,7 @@  int generic_write_end(struct file *file, struct address_space *mapping,
 			struct page *page, void *fsdata)
 {
 	copied = block_write_end(file, mapping, pos, len, copied, page, fsdata);
-	return __generic_write_end(mapping->host, pos, copied, page);
+	return __generic_write_end(mapping->host, pos, copied, page, false);
 }
 EXPORT_SYMBOL(generic_write_end);
 
diff --git a/fs/internal.h b/fs/internal.h
index 4a18bdbd2214..8572af552285 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -44,7 +44,7 @@  extern void guard_bio_eod(int rw, struct bio *bio);
 extern int __block_write_begin_int(struct page *page, loff_t pos, unsigned len,
 		get_block_t *get_block, struct iomap *iomap);
 int __generic_write_end(struct inode *inode, loff_t pos, unsigned copied,
-		struct page *page);
+		struct page *page, bool dirty_inode);
 
 /*
  * char_dev.c