* remove the implicit .set_page_dirty default
@ 2021-06-14 6:15 Christoph Hellwig
2021-06-14 6:15 ` [PATCH 1/3] fs: unexport __set_page_dirty Christoph Hellwig
` (3 more replies)
0 siblings, 4 replies; 19+ messages in thread
From: Christoph Hellwig @ 2021-06-14 6:15 UTC (permalink / raw)
To: Andrew Morton
Cc: Jan Kara, Al Viro, Greg Kroah-Hartman, linux-mm, linux-fsdevel,
linux-kernel
Hi all,
this series cleans up a few lose ends around ->set_page_dirty, most
importantly removes the default to the buffer head based on if no
method is wired up.
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 1/3] fs: unexport __set_page_dirty 2021-06-14 6:15 remove the implicit .set_page_dirty default Christoph Hellwig @ 2021-06-14 6:15 ` Christoph Hellwig 2021-06-14 6:23 ` Greg Kroah-Hartman ` (2 more replies) 2021-06-14 6:15 ` [PATCH 2/3] fs: move ramfs_aops to libfs Christoph Hellwig ` (2 subsequent siblings) 3 siblings, 3 replies; 19+ messages in thread From: Christoph Hellwig @ 2021-06-14 6:15 UTC (permalink / raw) To: Andrew Morton Cc: Jan Kara, Al Viro, Greg Kroah-Hartman, linux-mm, linux-fsdevel, linux-kernel __set_page_dirty is only used by built-in code. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/buffer.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/buffer.c b/fs/buffer.c index ea48c01fb76b..3d18831c7ad8 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -611,7 +611,6 @@ void __set_page_dirty(struct page *page, struct address_space *mapping, } xa_unlock_irqrestore(&mapping->i_pages, flags); } -EXPORT_SYMBOL_GPL(__set_page_dirty); /* * Add a page to the dirty page list. -- 2.30.2 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] fs: unexport __set_page_dirty 2021-06-14 6:15 ` [PATCH 1/3] fs: unexport __set_page_dirty Christoph Hellwig @ 2021-06-14 6:23 ` Greg Kroah-Hartman 2021-06-14 9:42 ` Jan Kara 2021-06-14 12:32 ` Matthew Wilcox 2 siblings, 0 replies; 19+ messages in thread From: Greg Kroah-Hartman @ 2021-06-14 6:23 UTC (permalink / raw) To: Christoph Hellwig Cc: Andrew Morton, Jan Kara, Al Viro, linux-mm, linux-fsdevel, linux-kernel On Mon, Jun 14, 2021 at 08:15:10AM +0200, Christoph Hellwig wrote: > __set_page_dirty is only used by built-in code. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/buffer.c | 1 - > 1 file changed, 1 deletion(-) Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] fs: unexport __set_page_dirty 2021-06-14 6:15 ` [PATCH 1/3] fs: unexport __set_page_dirty Christoph Hellwig 2021-06-14 6:23 ` Greg Kroah-Hartman @ 2021-06-14 9:42 ` Jan Kara 2021-06-14 12:32 ` Matthew Wilcox 2 siblings, 0 replies; 19+ messages in thread From: Jan Kara @ 2021-06-14 9:42 UTC (permalink / raw) To: Christoph Hellwig Cc: Andrew Morton, Jan Kara, Al Viro, Greg Kroah-Hartman, linux-mm, linux-fsdevel, linux-kernel On Mon 14-06-21 08:15:10, Christoph Hellwig wrote: > __set_page_dirty is only used by built-in code. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/buffer.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/fs/buffer.c b/fs/buffer.c > index ea48c01fb76b..3d18831c7ad8 100644 > --- a/fs/buffer.c > +++ b/fs/buffer.c > @@ -611,7 +611,6 @@ void __set_page_dirty(struct page *page, struct address_space *mapping, > } > xa_unlock_irqrestore(&mapping->i_pages, flags); > } > -EXPORT_SYMBOL_GPL(__set_page_dirty); > > /* > * Add a page to the dirty page list. > -- > 2.30.2 > -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] fs: unexport __set_page_dirty 2021-06-14 6:15 ` [PATCH 1/3] fs: unexport __set_page_dirty Christoph Hellwig 2021-06-14 6:23 ` Greg Kroah-Hartman 2021-06-14 9:42 ` Jan Kara @ 2021-06-14 12:32 ` Matthew Wilcox 2021-06-14 15:52 ` Christoph Hellwig 2 siblings, 1 reply; 19+ messages in thread From: Matthew Wilcox @ 2021-06-14 12:32 UTC (permalink / raw) To: Christoph Hellwig Cc: Andrew Morton, Jan Kara, Al Viro, Greg Kroah-Hartman, linux-mm, linux-fsdevel, linux-kernel On Mon, Jun 14, 2021 at 08:15:10AM +0200, Christoph Hellwig wrote: > __set_page_dirty is only used by built-in code. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org> You might also want to do the equivalent of this: https://git.infradead.org/users/willy/pagecache.git/commitdiff/19b3bf0d1a51f41ce5450fdd863969c3d32dfe12 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] fs: unexport __set_page_dirty 2021-06-14 12:32 ` Matthew Wilcox @ 2021-06-14 15:52 ` Christoph Hellwig 0 siblings, 0 replies; 19+ messages in thread From: Christoph Hellwig @ 2021-06-14 15:52 UTC (permalink / raw) To: Matthew Wilcox Cc: Christoph Hellwig, Andrew Morton, Jan Kara, Al Viro, Greg Kroah-Hartman, linux-mm, linux-fsdevel, linux-kernel On Mon, Jun 14, 2021 at 01:32:42PM +0100, Matthew Wilcox wrote: > On Mon, Jun 14, 2021 at 08:15:10AM +0200, Christoph Hellwig wrote: > > __set_page_dirty is only used by built-in code. > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org> > > You might also want to do the equivalent of this: > > https://git.infradead.org/users/willy/pagecache.git/commitdiff/19b3bf0d1a51f41ce5450fdd863969c3d32dfe12 This looks nice, but not really directly related. I think you should just send it to Andrew. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/3] fs: move ramfs_aops to libfs 2021-06-14 6:15 remove the implicit .set_page_dirty default Christoph Hellwig 2021-06-14 6:15 ` [PATCH 1/3] fs: unexport __set_page_dirty Christoph Hellwig @ 2021-06-14 6:15 ` Christoph Hellwig 2021-06-14 6:24 ` Greg Kroah-Hartman 2021-06-14 9:45 ` Jan Kara 2021-06-14 6:15 ` [PATCH 3/3] mm: require ->set_page_dirty to be explicitly wire up Christoph Hellwig 2021-06-14 12:23 ` remove the implicit .set_page_dirty default Matthew Wilcox 3 siblings, 2 replies; 19+ messages in thread From: Christoph Hellwig @ 2021-06-14 6:15 UTC (permalink / raw) To: Andrew Morton Cc: Jan Kara, Al Viro, Greg Kroah-Hartman, linux-mm, linux-fsdevel, linux-kernel Move the ramfs aops to libfs and reuse them for kernfs and configfs. Thosw two did not wire up ->set_page_dirty before and now get __set_page_dirty_no_writeback, which is the right one for no-writeback address_space usage. Drop the now unused exports of the libfs helpers only used for ramfs-style pagecache usage. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/configfs/inode.c | 8 +------- fs/kernfs/inode.c | 8 +------- fs/libfs.c | 17 +++++++++++++---- fs/ramfs/inode.c | 9 +-------- include/linux/fs.h | 5 +---- 5 files changed, 17 insertions(+), 30 deletions(-) diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c index eb5ec3e46283..b601610e9907 100644 --- a/fs/configfs/inode.c +++ b/fs/configfs/inode.c @@ -28,12 +28,6 @@ static struct lock_class_key default_group_class[MAX_LOCK_DEPTH]; #endif -static const struct address_space_operations configfs_aops = { - .readpage = simple_readpage, - .write_begin = simple_write_begin, - .write_end = simple_write_end, -}; - static const struct inode_operations configfs_inode_operations ={ .setattr = configfs_setattr, }; @@ -114,7 +108,7 @@ struct inode *configfs_new_inode(umode_t mode, struct configfs_dirent *sd, struct inode * inode = new_inode(s); if (inode) { inode->i_ino = get_next_ino(); - inode->i_mapping->a_ops = &configfs_aops; + inode->i_mapping->a_ops = &ram_aops; inode->i_op = &configfs_inode_operations; if (sd->s_iattr) { diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c index d73950fc3d57..26f2aa3586f9 100644 --- a/fs/kernfs/inode.c +++ b/fs/kernfs/inode.c @@ -17,12 +17,6 @@ #include "kernfs-internal.h" -static const struct address_space_operations kernfs_aops = { - .readpage = simple_readpage, - .write_begin = simple_write_begin, - .write_end = simple_write_end, -}; - static const struct inode_operations kernfs_iops = { .permission = kernfs_iop_permission, .setattr = kernfs_iop_setattr, @@ -203,7 +197,7 @@ static void kernfs_init_inode(struct kernfs_node *kn, struct inode *inode) { kernfs_get(kn); inode->i_private = kn; - inode->i_mapping->a_ops = &kernfs_aops; + inode->i_mapping->a_ops = &ram_aops; inode->i_op = &kernfs_iops; inode->i_generation = kernfs_gen(kn); diff --git a/fs/libfs.c b/fs/libfs.c index e9b29c6ffccb..2d7f086b93d6 100644 --- a/fs/libfs.c +++ b/fs/libfs.c @@ -512,7 +512,7 @@ int simple_setattr(struct user_namespace *mnt_userns, struct dentry *dentry, } EXPORT_SYMBOL(simple_setattr); -int simple_readpage(struct file *file, struct page *page) +static int simple_readpage(struct file *file, struct page *page) { clear_highpage(page); flush_dcache_page(page); @@ -520,7 +520,6 @@ int simple_readpage(struct file *file, struct page *page) unlock_page(page); return 0; } -EXPORT_SYMBOL(simple_readpage); int simple_write_begin(struct file *file, struct address_space *mapping, loff_t pos, unsigned len, unsigned flags, @@ -568,7 +567,7 @@ EXPORT_SYMBOL(simple_write_begin); * * Use *ONLY* with simple_readpage() */ -int simple_write_end(struct file *file, struct address_space *mapping, +static int simple_write_end(struct file *file, struct address_space *mapping, loff_t pos, unsigned len, unsigned copied, struct page *page, void *fsdata) { @@ -597,7 +596,17 @@ int simple_write_end(struct file *file, struct address_space *mapping, return copied; } -EXPORT_SYMBOL(simple_write_end); + +/* + * Provides ramfs-style behavior: data in the pagecache, but no writeback. + */ +const struct address_space_operations ram_aops = { + .readpage = simple_readpage, + .write_begin = simple_write_begin, + .write_end = simple_write_end, + .set_page_dirty = __set_page_dirty_no_writeback, +}; +EXPORT_SYMBOL(ram_aops); /* * the inodes created here are not hashed. If you use iunique to generate diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c index 9ebd17d7befb..65e7e56005b8 100644 --- a/fs/ramfs/inode.c +++ b/fs/ramfs/inode.c @@ -53,13 +53,6 @@ struct ramfs_fs_info { static const struct super_operations ramfs_ops; static const struct inode_operations ramfs_dir_inode_operations; -static const struct address_space_operations ramfs_aops = { - .readpage = simple_readpage, - .write_begin = simple_write_begin, - .write_end = simple_write_end, - .set_page_dirty = __set_page_dirty_no_writeback, -}; - struct inode *ramfs_get_inode(struct super_block *sb, const struct inode *dir, umode_t mode, dev_t dev) { @@ -68,7 +61,7 @@ struct inode *ramfs_get_inode(struct super_block *sb, if (inode) { inode->i_ino = get_next_ino(); inode_init_owner(&init_user_ns, inode, dir, mode); - inode->i_mapping->a_ops = &ramfs_aops; + inode->i_mapping->a_ops = &ram_aops; mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER); mapping_set_unevictable(inode->i_mapping); inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode); diff --git a/include/linux/fs.h b/include/linux/fs.h index c3c88fdb9b2a..869909345420 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -3422,13 +3422,10 @@ extern void noop_invalidatepage(struct page *page, unsigned int offset, unsigned int length); extern ssize_t noop_direct_IO(struct kiocb *iocb, struct iov_iter *iter); extern int simple_empty(struct dentry *); -extern int simple_readpage(struct file *file, struct page *page); extern int simple_write_begin(struct file *file, struct address_space *mapping, loff_t pos, unsigned len, unsigned flags, struct page **pagep, void **fsdata); -extern int simple_write_end(struct file *file, struct address_space *mapping, - loff_t pos, unsigned len, unsigned copied, - struct page *page, void *fsdata); +extern const struct address_space_operations ram_aops; extern int always_delete_dentry(const struct dentry *); extern struct inode *alloc_anon_inode(struct super_block *); extern int simple_nosetlease(struct file *, long, struct file_lock **, void **); -- 2.30.2 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] fs: move ramfs_aops to libfs 2021-06-14 6:15 ` [PATCH 2/3] fs: move ramfs_aops to libfs Christoph Hellwig @ 2021-06-14 6:24 ` Greg Kroah-Hartman 2021-06-14 9:45 ` Jan Kara 1 sibling, 0 replies; 19+ messages in thread From: Greg Kroah-Hartman @ 2021-06-14 6:24 UTC (permalink / raw) To: Christoph Hellwig Cc: Andrew Morton, Jan Kara, Al Viro, linux-mm, linux-fsdevel, linux-kernel On Mon, Jun 14, 2021 at 08:15:11AM +0200, Christoph Hellwig wrote: > Move the ramfs aops to libfs and reuse them for kernfs and configfs. > Thosw two did not wire up ->set_page_dirty before and now get > __set_page_dirty_no_writeback, which is the right one for no-writeback > address_space usage. > > Drop the now unused exports of the libfs helpers only used for > ramfs-style pagecache usage. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/configfs/inode.c | 8 +------- > fs/kernfs/inode.c | 8 +------- > fs/libfs.c | 17 +++++++++++++---- > fs/ramfs/inode.c | 9 +-------- > include/linux/fs.h | 5 +---- > 5 files changed, 17 insertions(+), 30 deletions(-) > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] fs: move ramfs_aops to libfs 2021-06-14 6:15 ` [PATCH 2/3] fs: move ramfs_aops to libfs Christoph Hellwig 2021-06-14 6:24 ` Greg Kroah-Hartman @ 2021-06-14 9:45 ` Jan Kara 1 sibling, 0 replies; 19+ messages in thread From: Jan Kara @ 2021-06-14 9:45 UTC (permalink / raw) To: Christoph Hellwig Cc: Andrew Morton, Jan Kara, Al Viro, Greg Kroah-Hartman, linux-mm, linux-fsdevel, linux-kernel On Mon 14-06-21 08:15:11, Christoph Hellwig wrote: > Move the ramfs aops to libfs and reuse them for kernfs and configfs. > Thosw two did not wire up ->set_page_dirty before and now get > __set_page_dirty_no_writeback, which is the right one for no-writeback > address_space usage. > > Drop the now unused exports of the libfs helpers only used for > ramfs-style pagecache usage. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/configfs/inode.c | 8 +------- > fs/kernfs/inode.c | 8 +------- > fs/libfs.c | 17 +++++++++++++---- > fs/ramfs/inode.c | 9 +-------- > include/linux/fs.h | 5 +---- > 5 files changed, 17 insertions(+), 30 deletions(-) > > diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c > index eb5ec3e46283..b601610e9907 100644 > --- a/fs/configfs/inode.c > +++ b/fs/configfs/inode.c > @@ -28,12 +28,6 @@ > static struct lock_class_key default_group_class[MAX_LOCK_DEPTH]; > #endif > > -static const struct address_space_operations configfs_aops = { > - .readpage = simple_readpage, > - .write_begin = simple_write_begin, > - .write_end = simple_write_end, > -}; > - > static const struct inode_operations configfs_inode_operations ={ > .setattr = configfs_setattr, > }; > @@ -114,7 +108,7 @@ struct inode *configfs_new_inode(umode_t mode, struct configfs_dirent *sd, > struct inode * inode = new_inode(s); > if (inode) { > inode->i_ino = get_next_ino(); > - inode->i_mapping->a_ops = &configfs_aops; > + inode->i_mapping->a_ops = &ram_aops; > inode->i_op = &configfs_inode_operations; > > if (sd->s_iattr) { > diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c > index d73950fc3d57..26f2aa3586f9 100644 > --- a/fs/kernfs/inode.c > +++ b/fs/kernfs/inode.c > @@ -17,12 +17,6 @@ > > #include "kernfs-internal.h" > > -static const struct address_space_operations kernfs_aops = { > - .readpage = simple_readpage, > - .write_begin = simple_write_begin, > - .write_end = simple_write_end, > -}; > - > static const struct inode_operations kernfs_iops = { > .permission = kernfs_iop_permission, > .setattr = kernfs_iop_setattr, > @@ -203,7 +197,7 @@ static void kernfs_init_inode(struct kernfs_node *kn, struct inode *inode) > { > kernfs_get(kn); > inode->i_private = kn; > - inode->i_mapping->a_ops = &kernfs_aops; > + inode->i_mapping->a_ops = &ram_aops; > inode->i_op = &kernfs_iops; > inode->i_generation = kernfs_gen(kn); > > diff --git a/fs/libfs.c b/fs/libfs.c > index e9b29c6ffccb..2d7f086b93d6 100644 > --- a/fs/libfs.c > +++ b/fs/libfs.c > @@ -512,7 +512,7 @@ int simple_setattr(struct user_namespace *mnt_userns, struct dentry *dentry, > } > EXPORT_SYMBOL(simple_setattr); > > -int simple_readpage(struct file *file, struct page *page) > +static int simple_readpage(struct file *file, struct page *page) > { > clear_highpage(page); > flush_dcache_page(page); > @@ -520,7 +520,6 @@ int simple_readpage(struct file *file, struct page *page) > unlock_page(page); > return 0; > } > -EXPORT_SYMBOL(simple_readpage); > > int simple_write_begin(struct file *file, struct address_space *mapping, > loff_t pos, unsigned len, unsigned flags, > @@ -568,7 +567,7 @@ EXPORT_SYMBOL(simple_write_begin); > * > * Use *ONLY* with simple_readpage() > */ > -int simple_write_end(struct file *file, struct address_space *mapping, > +static int simple_write_end(struct file *file, struct address_space *mapping, > loff_t pos, unsigned len, unsigned copied, > struct page *page, void *fsdata) > { > @@ -597,7 +596,17 @@ int simple_write_end(struct file *file, struct address_space *mapping, > > return copied; > } > -EXPORT_SYMBOL(simple_write_end); > + > +/* > + * Provides ramfs-style behavior: data in the pagecache, but no writeback. > + */ > +const struct address_space_operations ram_aops = { > + .readpage = simple_readpage, > + .write_begin = simple_write_begin, > + .write_end = simple_write_end, > + .set_page_dirty = __set_page_dirty_no_writeback, > +}; > +EXPORT_SYMBOL(ram_aops); > > /* > * the inodes created here are not hashed. If you use iunique to generate > diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c > index 9ebd17d7befb..65e7e56005b8 100644 > --- a/fs/ramfs/inode.c > +++ b/fs/ramfs/inode.c > @@ -53,13 +53,6 @@ struct ramfs_fs_info { > static const struct super_operations ramfs_ops; > static const struct inode_operations ramfs_dir_inode_operations; > > -static const struct address_space_operations ramfs_aops = { > - .readpage = simple_readpage, > - .write_begin = simple_write_begin, > - .write_end = simple_write_end, > - .set_page_dirty = __set_page_dirty_no_writeback, > -}; > - > struct inode *ramfs_get_inode(struct super_block *sb, > const struct inode *dir, umode_t mode, dev_t dev) > { > @@ -68,7 +61,7 @@ struct inode *ramfs_get_inode(struct super_block *sb, > if (inode) { > inode->i_ino = get_next_ino(); > inode_init_owner(&init_user_ns, inode, dir, mode); > - inode->i_mapping->a_ops = &ramfs_aops; > + inode->i_mapping->a_ops = &ram_aops; > mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER); > mapping_set_unevictable(inode->i_mapping); > inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode); > diff --git a/include/linux/fs.h b/include/linux/fs.h > index c3c88fdb9b2a..869909345420 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -3422,13 +3422,10 @@ extern void noop_invalidatepage(struct page *page, unsigned int offset, > unsigned int length); > extern ssize_t noop_direct_IO(struct kiocb *iocb, struct iov_iter *iter); > extern int simple_empty(struct dentry *); > -extern int simple_readpage(struct file *file, struct page *page); > extern int simple_write_begin(struct file *file, struct address_space *mapping, > loff_t pos, unsigned len, unsigned flags, > struct page **pagep, void **fsdata); > -extern int simple_write_end(struct file *file, struct address_space *mapping, > - loff_t pos, unsigned len, unsigned copied, > - struct page *page, void *fsdata); > +extern const struct address_space_operations ram_aops; > extern int always_delete_dentry(const struct dentry *); > extern struct inode *alloc_anon_inode(struct super_block *); > extern int simple_nosetlease(struct file *, long, struct file_lock **, void **); > -- > 2.30.2 > -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/3] mm: require ->set_page_dirty to be explicitly wire up 2021-06-14 6:15 remove the implicit .set_page_dirty default Christoph Hellwig 2021-06-14 6:15 ` [PATCH 1/3] fs: unexport __set_page_dirty Christoph Hellwig 2021-06-14 6:15 ` [PATCH 2/3] fs: move ramfs_aops to libfs Christoph Hellwig @ 2021-06-14 6:15 ` Christoph Hellwig 2021-06-14 6:24 ` Greg Kroah-Hartman ` (2 more replies) 2021-06-14 12:23 ` remove the implicit .set_page_dirty default Matthew Wilcox 3 siblings, 3 replies; 19+ messages in thread From: Christoph Hellwig @ 2021-06-14 6:15 UTC (permalink / raw) To: Andrew Morton Cc: Jan Kara, Al Viro, Greg Kroah-Hartman, linux-mm, linux-fsdevel, linux-kernel Remove the CONFIG_BLOCK default to __set_page_dirty_buffers and just wire that method up for the missing instances. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/adfs/inode.c | 1 + fs/affs/file.c | 2 ++ fs/bfs/file.c | 1 + fs/block_dev.c | 1 + fs/exfat/inode.c | 1 + fs/ext2/inode.c | 2 ++ fs/fat/inode.c | 1 + fs/gfs2/meta_io.c | 2 ++ fs/hfs/inode.c | 2 ++ fs/hfsplus/inode.c | 2 ++ fs/hpfs/file.c | 1 + fs/jfs/inode.c | 1 + fs/minix/inode.c | 1 + fs/nilfs2/mdt.c | 1 + fs/ocfs2/aops.c | 1 + fs/omfs/file.c | 1 + fs/sysv/itree.c | 1 + fs/udf/file.c | 1 + fs/udf/inode.c | 1 + fs/ufs/inode.c | 1 + mm/page-writeback.c | 18 ++++-------------- 21 files changed, 29 insertions(+), 14 deletions(-) diff --git a/fs/adfs/inode.c b/fs/adfs/inode.c index fb7ee026d101..adbb3a1edcbf 100644 --- a/fs/adfs/inode.c +++ b/fs/adfs/inode.c @@ -73,6 +73,7 @@ static sector_t _adfs_bmap(struct address_space *mapping, sector_t block) } static const struct address_space_operations adfs_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = adfs_readpage, .writepage = adfs_writepage, .write_begin = adfs_write_begin, diff --git a/fs/affs/file.c b/fs/affs/file.c index d91b0133d95d..75ebd2b576ca 100644 --- a/fs/affs/file.c +++ b/fs/affs/file.c @@ -453,6 +453,7 @@ static sector_t _affs_bmap(struct address_space *mapping, sector_t block) } const struct address_space_operations affs_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = affs_readpage, .writepage = affs_writepage, .write_begin = affs_write_begin, @@ -833,6 +834,7 @@ static int affs_write_end_ofs(struct file *file, struct address_space *mapping, } const struct address_space_operations affs_aops_ofs = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = affs_readpage_ofs, //.writepage = affs_writepage_ofs, .write_begin = affs_write_begin_ofs, diff --git a/fs/bfs/file.c b/fs/bfs/file.c index 0dceefc54b48..7f8544abf636 100644 --- a/fs/bfs/file.c +++ b/fs/bfs/file.c @@ -188,6 +188,7 @@ static sector_t bfs_bmap(struct address_space *mapping, sector_t block) } const struct address_space_operations bfs_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = bfs_readpage, .writepage = bfs_writepage, .write_begin = bfs_write_begin, diff --git a/fs/block_dev.c b/fs/block_dev.c index 6cc4d4cfe0c2..eb34f5c357cf 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -1754,6 +1754,7 @@ static int blkdev_writepages(struct address_space *mapping, } static const struct address_space_operations def_blk_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = blkdev_readpage, .readahead = blkdev_readahead, .writepage = blkdev_writepage, diff --git a/fs/exfat/inode.c b/fs/exfat/inode.c index 1803ef3220fd..ca37d4344361 100644 --- a/fs/exfat/inode.c +++ b/fs/exfat/inode.c @@ -491,6 +491,7 @@ int exfat_block_truncate_page(struct inode *inode, loff_t from) } static const struct address_space_operations exfat_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = exfat_readpage, .readahead = exfat_readahead, .writepage = exfat_writepage, diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c index 68178b2234bd..bf41f579ed3e 100644 --- a/fs/ext2/inode.c +++ b/fs/ext2/inode.c @@ -961,6 +961,7 @@ ext2_dax_writepages(struct address_space *mapping, struct writeback_control *wbc } const struct address_space_operations ext2_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = ext2_readpage, .readahead = ext2_readahead, .writepage = ext2_writepage, @@ -975,6 +976,7 @@ const struct address_space_operations ext2_aops = { }; const struct address_space_operations ext2_nobh_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = ext2_readpage, .readahead = ext2_readahead, .writepage = ext2_nobh_writepage, diff --git a/fs/fat/inode.c b/fs/fat/inode.c index bab9b202b496..de0c9b013a85 100644 --- a/fs/fat/inode.c +++ b/fs/fat/inode.c @@ -342,6 +342,7 @@ int fat_block_truncate_page(struct inode *inode, loff_t from) } static const struct address_space_operations fat_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = fat_readpage, .readahead = fat_readahead, .writepage = fat_writepage, diff --git a/fs/gfs2/meta_io.c b/fs/gfs2/meta_io.c index d68184ebbfdd..7c9619997355 100644 --- a/fs/gfs2/meta_io.c +++ b/fs/gfs2/meta_io.c @@ -89,11 +89,13 @@ static int gfs2_aspace_writepage(struct page *page, struct writeback_control *wb } const struct address_space_operations gfs2_meta_aops = { + .set_page_dirty = __set_page_dirty_buffers, .writepage = gfs2_aspace_writepage, .releasepage = gfs2_releasepage, }; const struct address_space_operations gfs2_rgrp_aops = { + .set_page_dirty = __set_page_dirty_buffers, .writepage = gfs2_aspace_writepage, .releasepage = gfs2_releasepage, }; diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c index 3fc5cb346586..4a95a92546a0 100644 --- a/fs/hfs/inode.c +++ b/fs/hfs/inode.c @@ -159,6 +159,7 @@ static int hfs_writepages(struct address_space *mapping, } const struct address_space_operations hfs_btree_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = hfs_readpage, .writepage = hfs_writepage, .write_begin = hfs_write_begin, @@ -168,6 +169,7 @@ const struct address_space_operations hfs_btree_aops = { }; const struct address_space_operations hfs_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = hfs_readpage, .writepage = hfs_writepage, .write_begin = hfs_write_begin, diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c index 8ea447e5c470..70e8374ddac4 100644 --- a/fs/hfsplus/inode.c +++ b/fs/hfsplus/inode.c @@ -156,6 +156,7 @@ static int hfsplus_writepages(struct address_space *mapping, } const struct address_space_operations hfsplus_btree_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = hfsplus_readpage, .writepage = hfsplus_writepage, .write_begin = hfsplus_write_begin, @@ -165,6 +166,7 @@ const struct address_space_operations hfsplus_btree_aops = { }; const struct address_space_operations hfsplus_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = hfsplus_readpage, .writepage = hfsplus_writepage, .write_begin = hfsplus_write_begin, diff --git a/fs/hpfs/file.c b/fs/hpfs/file.c index 077c25128eb7..c3a49aacf20a 100644 --- a/fs/hpfs/file.c +++ b/fs/hpfs/file.c @@ -196,6 +196,7 @@ static int hpfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, } const struct address_space_operations hpfs_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = hpfs_readpage, .writepage = hpfs_writepage, .readahead = hpfs_readahead, diff --git a/fs/jfs/inode.c b/fs/jfs/inode.c index 6f65bfa9f18d..3663dd5a23bc 100644 --- a/fs/jfs/inode.c +++ b/fs/jfs/inode.c @@ -356,6 +356,7 @@ static ssize_t jfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter) } const struct address_space_operations jfs_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = jfs_readpage, .readahead = jfs_readahead, .writepage = jfs_writepage, diff --git a/fs/minix/inode.c b/fs/minix/inode.c index a532a99bbe81..a71f1cf894b9 100644 --- a/fs/minix/inode.c +++ b/fs/minix/inode.c @@ -442,6 +442,7 @@ static sector_t minix_bmap(struct address_space *mapping, sector_t block) } static const struct address_space_operations minix_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = minix_readpage, .writepage = minix_writepage, .write_begin = minix_write_begin, diff --git a/fs/nilfs2/mdt.c b/fs/nilfs2/mdt.c index c0361ce45f62..97769fe4d588 100644 --- a/fs/nilfs2/mdt.c +++ b/fs/nilfs2/mdt.c @@ -434,6 +434,7 @@ nilfs_mdt_write_page(struct page *page, struct writeback_control *wbc) static const struct address_space_operations def_mdt_aops = { + .set_page_dirty = __set_page_dirty_buffers, .writepage = nilfs_mdt_write_page, }; diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c index 1294925ac94a..b3517de178ff 100644 --- a/fs/ocfs2/aops.c +++ b/fs/ocfs2/aops.c @@ -2454,6 +2454,7 @@ static ssize_t ocfs2_direct_IO(struct kiocb *iocb, struct iov_iter *iter) } const struct address_space_operations ocfs2_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = ocfs2_readpage, .readahead = ocfs2_readahead, .writepage = ocfs2_writepage, diff --git a/fs/omfs/file.c b/fs/omfs/file.c index 11e733aab25d..89725b15a64b 100644 --- a/fs/omfs/file.c +++ b/fs/omfs/file.c @@ -372,6 +372,7 @@ const struct inode_operations omfs_file_inops = { }; const struct address_space_operations omfs_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = omfs_readpage, .readahead = omfs_readahead, .writepage = omfs_writepage, diff --git a/fs/sysv/itree.c b/fs/sysv/itree.c index 8b2e99b7bc9f..749385015a8d 100644 --- a/fs/sysv/itree.c +++ b/fs/sysv/itree.c @@ -495,6 +495,7 @@ static sector_t sysv_bmap(struct address_space *mapping, sector_t block) } const struct address_space_operations sysv_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = sysv_readpage, .writepage = sysv_writepage, .write_begin = sysv_write_begin, diff --git a/fs/udf/file.c b/fs/udf/file.c index 2846dcd92197..1baff8ddb754 100644 --- a/fs/udf/file.c +++ b/fs/udf/file.c @@ -125,6 +125,7 @@ static int udf_adinicb_write_end(struct file *file, struct address_space *mappin } const struct address_space_operations udf_adinicb_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = udf_adinicb_readpage, .writepage = udf_adinicb_writepage, .write_begin = udf_adinicb_write_begin, diff --git a/fs/udf/inode.c b/fs/udf/inode.c index 0dd2f93ac048..4917670860a0 100644 --- a/fs/udf/inode.c +++ b/fs/udf/inode.c @@ -235,6 +235,7 @@ static sector_t udf_bmap(struct address_space *mapping, sector_t block) } const struct address_space_operations udf_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = udf_readpage, .readahead = udf_readahead, .writepage = udf_writepage, diff --git a/fs/ufs/inode.c b/fs/ufs/inode.c index debc282c1bb4..ac628de69601 100644 --- a/fs/ufs/inode.c +++ b/fs/ufs/inode.c @@ -526,6 +526,7 @@ static sector_t ufs_bmap(struct address_space *mapping, sector_t block) } const struct address_space_operations ufs_aops = { + .set_page_dirty = __set_page_dirty_buffers, .readpage = ufs_readpage, .writepage = ufs_writepage, .write_begin = ufs_write_begin, diff --git a/mm/page-writeback.c b/mm/page-writeback.c index 0062d5c57d41..081fa02236c2 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -32,7 +32,6 @@ #include <linux/sysctl.h> #include <linux/cpu.h> #include <linux/syscalls.h> -#include <linux/buffer_head.h> /* __set_page_dirty_buffers */ #include <linux/pagevec.h> #include <linux/timer.h> #include <linux/sched/rt.h> @@ -2546,13 +2545,9 @@ EXPORT_SYMBOL(redirty_page_for_writepage); /* * Dirty a page. * - * For pages with a mapping this should be done under the page lock - * for the benefit of asynchronous memory errors who prefer a consistent - * dirty state. This rule can be broken in some special cases, - * but should be better not to. - * - * If the mapping doesn't provide a set_page_dirty a_op, then - * just fall through and assume that it wants buffer_heads. + * For pages with a mapping this should be done under the page lock for the + * benefit of asynchronous memory errors who prefer a consistent dirty state. + * This rule can be broken in some special cases, but should be better not to. */ int set_page_dirty(struct page *page) { @@ -2560,7 +2555,6 @@ int set_page_dirty(struct page *page) page = compound_head(page); if (likely(mapping)) { - int (*spd)(struct page *) = mapping->a_ops->set_page_dirty; /* * readahead/lru_deactivate_page could remain * PG_readahead/PG_reclaim due to race with end_page_writeback @@ -2573,11 +2567,7 @@ int set_page_dirty(struct page *page) */ if (PageReclaim(page)) ClearPageReclaim(page); -#ifdef CONFIG_BLOCK - if (!spd) - spd = __set_page_dirty_buffers; -#endif - return (*spd)(page); + return mapping->a_ops->set_page_dirty(page); } if (!PageDirty(page)) { if (!TestSetPageDirty(page)) -- 2.30.2 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] mm: require ->set_page_dirty to be explicitly wire up 2021-06-14 6:15 ` [PATCH 3/3] mm: require ->set_page_dirty to be explicitly wire up Christoph Hellwig @ 2021-06-14 6:24 ` Greg Kroah-Hartman 2021-06-14 9:51 ` Jan Kara 2021-06-18 19:23 ` Dmitry Osipenko 2 siblings, 0 replies; 19+ messages in thread From: Greg Kroah-Hartman @ 2021-06-14 6:24 UTC (permalink / raw) To: Christoph Hellwig Cc: Andrew Morton, Jan Kara, Al Viro, linux-mm, linux-fsdevel, linux-kernel On Mon, Jun 14, 2021 at 08:15:12AM +0200, Christoph Hellwig wrote: > Remove the CONFIG_BLOCK default to __set_page_dirty_buffers and just > wire that method up for the missing instances. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/adfs/inode.c | 1 + > fs/affs/file.c | 2 ++ > fs/bfs/file.c | 1 + > fs/block_dev.c | 1 + > fs/exfat/inode.c | 1 + > fs/ext2/inode.c | 2 ++ > fs/fat/inode.c | 1 + > fs/gfs2/meta_io.c | 2 ++ > fs/hfs/inode.c | 2 ++ > fs/hfsplus/inode.c | 2 ++ > fs/hpfs/file.c | 1 + > fs/jfs/inode.c | 1 + > fs/minix/inode.c | 1 + > fs/nilfs2/mdt.c | 1 + > fs/ocfs2/aops.c | 1 + > fs/omfs/file.c | 1 + > fs/sysv/itree.c | 1 + > fs/udf/file.c | 1 + > fs/udf/inode.c | 1 + > fs/ufs/inode.c | 1 + > mm/page-writeback.c | 18 ++++-------------- > 21 files changed, 29 insertions(+), 14 deletions(-) > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] mm: require ->set_page_dirty to be explicitly wire up 2021-06-14 6:15 ` [PATCH 3/3] mm: require ->set_page_dirty to be explicitly wire up Christoph Hellwig 2021-06-14 6:24 ` Greg Kroah-Hartman @ 2021-06-14 9:51 ` Jan Kara 2021-06-14 10:03 ` Christoph Hellwig 2021-06-18 19:23 ` Dmitry Osipenko 2 siblings, 1 reply; 19+ messages in thread From: Jan Kara @ 2021-06-14 9:51 UTC (permalink / raw) To: Christoph Hellwig Cc: Andrew Morton, Jan Kara, Al Viro, Greg Kroah-Hartman, linux-mm, linux-fsdevel, linux-kernel On Mon 14-06-21 08:15:12, Christoph Hellwig wrote: > Remove the CONFIG_BLOCK default to __set_page_dirty_buffers and just > wire that method up for the missing instances. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Make sense. Did you somehow autogenerate this? If this patch would race with addition of new aops struct, we'd get null-ptr-defer out of that so maybe providing the script would be better. But other than that the changes look good to me. You can add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/adfs/inode.c | 1 + > fs/affs/file.c | 2 ++ > fs/bfs/file.c | 1 + > fs/block_dev.c | 1 + > fs/exfat/inode.c | 1 + > fs/ext2/inode.c | 2 ++ > fs/fat/inode.c | 1 + > fs/gfs2/meta_io.c | 2 ++ > fs/hfs/inode.c | 2 ++ > fs/hfsplus/inode.c | 2 ++ > fs/hpfs/file.c | 1 + > fs/jfs/inode.c | 1 + > fs/minix/inode.c | 1 + > fs/nilfs2/mdt.c | 1 + > fs/ocfs2/aops.c | 1 + > fs/omfs/file.c | 1 + > fs/sysv/itree.c | 1 + > fs/udf/file.c | 1 + > fs/udf/inode.c | 1 + > fs/ufs/inode.c | 1 + > mm/page-writeback.c | 18 ++++-------------- > 21 files changed, 29 insertions(+), 14 deletions(-) > > diff --git a/fs/adfs/inode.c b/fs/adfs/inode.c > index fb7ee026d101..adbb3a1edcbf 100644 > --- a/fs/adfs/inode.c > +++ b/fs/adfs/inode.c > @@ -73,6 +73,7 @@ static sector_t _adfs_bmap(struct address_space *mapping, sector_t block) > } > > static const struct address_space_operations adfs_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = adfs_readpage, > .writepage = adfs_writepage, > .write_begin = adfs_write_begin, > diff --git a/fs/affs/file.c b/fs/affs/file.c > index d91b0133d95d..75ebd2b576ca 100644 > --- a/fs/affs/file.c > +++ b/fs/affs/file.c > @@ -453,6 +453,7 @@ static sector_t _affs_bmap(struct address_space *mapping, sector_t block) > } > > const struct address_space_operations affs_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = affs_readpage, > .writepage = affs_writepage, > .write_begin = affs_write_begin, > @@ -833,6 +834,7 @@ static int affs_write_end_ofs(struct file *file, struct address_space *mapping, > } > > const struct address_space_operations affs_aops_ofs = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = affs_readpage_ofs, > //.writepage = affs_writepage_ofs, > .write_begin = affs_write_begin_ofs, > diff --git a/fs/bfs/file.c b/fs/bfs/file.c > index 0dceefc54b48..7f8544abf636 100644 > --- a/fs/bfs/file.c > +++ b/fs/bfs/file.c > @@ -188,6 +188,7 @@ static sector_t bfs_bmap(struct address_space *mapping, sector_t block) > } > > const struct address_space_operations bfs_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = bfs_readpage, > .writepage = bfs_writepage, > .write_begin = bfs_write_begin, > diff --git a/fs/block_dev.c b/fs/block_dev.c > index 6cc4d4cfe0c2..eb34f5c357cf 100644 > --- a/fs/block_dev.c > +++ b/fs/block_dev.c > @@ -1754,6 +1754,7 @@ static int blkdev_writepages(struct address_space *mapping, > } > > static const struct address_space_operations def_blk_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = blkdev_readpage, > .readahead = blkdev_readahead, > .writepage = blkdev_writepage, > diff --git a/fs/exfat/inode.c b/fs/exfat/inode.c > index 1803ef3220fd..ca37d4344361 100644 > --- a/fs/exfat/inode.c > +++ b/fs/exfat/inode.c > @@ -491,6 +491,7 @@ int exfat_block_truncate_page(struct inode *inode, loff_t from) > } > > static const struct address_space_operations exfat_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = exfat_readpage, > .readahead = exfat_readahead, > .writepage = exfat_writepage, > diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c > index 68178b2234bd..bf41f579ed3e 100644 > --- a/fs/ext2/inode.c > +++ b/fs/ext2/inode.c > @@ -961,6 +961,7 @@ ext2_dax_writepages(struct address_space *mapping, struct writeback_control *wbc > } > > const struct address_space_operations ext2_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = ext2_readpage, > .readahead = ext2_readahead, > .writepage = ext2_writepage, > @@ -975,6 +976,7 @@ const struct address_space_operations ext2_aops = { > }; > > const struct address_space_operations ext2_nobh_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = ext2_readpage, > .readahead = ext2_readahead, > .writepage = ext2_nobh_writepage, > diff --git a/fs/fat/inode.c b/fs/fat/inode.c > index bab9b202b496..de0c9b013a85 100644 > --- a/fs/fat/inode.c > +++ b/fs/fat/inode.c > @@ -342,6 +342,7 @@ int fat_block_truncate_page(struct inode *inode, loff_t from) > } > > static const struct address_space_operations fat_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = fat_readpage, > .readahead = fat_readahead, > .writepage = fat_writepage, > diff --git a/fs/gfs2/meta_io.c b/fs/gfs2/meta_io.c > index d68184ebbfdd..7c9619997355 100644 > --- a/fs/gfs2/meta_io.c > +++ b/fs/gfs2/meta_io.c > @@ -89,11 +89,13 @@ static int gfs2_aspace_writepage(struct page *page, struct writeback_control *wb > } > > const struct address_space_operations gfs2_meta_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .writepage = gfs2_aspace_writepage, > .releasepage = gfs2_releasepage, > }; > > const struct address_space_operations gfs2_rgrp_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .writepage = gfs2_aspace_writepage, > .releasepage = gfs2_releasepage, > }; > diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c > index 3fc5cb346586..4a95a92546a0 100644 > --- a/fs/hfs/inode.c > +++ b/fs/hfs/inode.c > @@ -159,6 +159,7 @@ static int hfs_writepages(struct address_space *mapping, > } > > const struct address_space_operations hfs_btree_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = hfs_readpage, > .writepage = hfs_writepage, > .write_begin = hfs_write_begin, > @@ -168,6 +169,7 @@ const struct address_space_operations hfs_btree_aops = { > }; > > const struct address_space_operations hfs_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = hfs_readpage, > .writepage = hfs_writepage, > .write_begin = hfs_write_begin, > diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c > index 8ea447e5c470..70e8374ddac4 100644 > --- a/fs/hfsplus/inode.c > +++ b/fs/hfsplus/inode.c > @@ -156,6 +156,7 @@ static int hfsplus_writepages(struct address_space *mapping, > } > > const struct address_space_operations hfsplus_btree_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = hfsplus_readpage, > .writepage = hfsplus_writepage, > .write_begin = hfsplus_write_begin, > @@ -165,6 +166,7 @@ const struct address_space_operations hfsplus_btree_aops = { > }; > > const struct address_space_operations hfsplus_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = hfsplus_readpage, > .writepage = hfsplus_writepage, > .write_begin = hfsplus_write_begin, > diff --git a/fs/hpfs/file.c b/fs/hpfs/file.c > index 077c25128eb7..c3a49aacf20a 100644 > --- a/fs/hpfs/file.c > +++ b/fs/hpfs/file.c > @@ -196,6 +196,7 @@ static int hpfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, > } > > const struct address_space_operations hpfs_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = hpfs_readpage, > .writepage = hpfs_writepage, > .readahead = hpfs_readahead, > diff --git a/fs/jfs/inode.c b/fs/jfs/inode.c > index 6f65bfa9f18d..3663dd5a23bc 100644 > --- a/fs/jfs/inode.c > +++ b/fs/jfs/inode.c > @@ -356,6 +356,7 @@ static ssize_t jfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter) > } > > const struct address_space_operations jfs_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = jfs_readpage, > .readahead = jfs_readahead, > .writepage = jfs_writepage, > diff --git a/fs/minix/inode.c b/fs/minix/inode.c > index a532a99bbe81..a71f1cf894b9 100644 > --- a/fs/minix/inode.c > +++ b/fs/minix/inode.c > @@ -442,6 +442,7 @@ static sector_t minix_bmap(struct address_space *mapping, sector_t block) > } > > static const struct address_space_operations minix_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = minix_readpage, > .writepage = minix_writepage, > .write_begin = minix_write_begin, > diff --git a/fs/nilfs2/mdt.c b/fs/nilfs2/mdt.c > index c0361ce45f62..97769fe4d588 100644 > --- a/fs/nilfs2/mdt.c > +++ b/fs/nilfs2/mdt.c > @@ -434,6 +434,7 @@ nilfs_mdt_write_page(struct page *page, struct writeback_control *wbc) > > > static const struct address_space_operations def_mdt_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .writepage = nilfs_mdt_write_page, > }; > > diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c > index 1294925ac94a..b3517de178ff 100644 > --- a/fs/ocfs2/aops.c > +++ b/fs/ocfs2/aops.c > @@ -2454,6 +2454,7 @@ static ssize_t ocfs2_direct_IO(struct kiocb *iocb, struct iov_iter *iter) > } > > const struct address_space_operations ocfs2_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = ocfs2_readpage, > .readahead = ocfs2_readahead, > .writepage = ocfs2_writepage, > diff --git a/fs/omfs/file.c b/fs/omfs/file.c > index 11e733aab25d..89725b15a64b 100644 > --- a/fs/omfs/file.c > +++ b/fs/omfs/file.c > @@ -372,6 +372,7 @@ const struct inode_operations omfs_file_inops = { > }; > > const struct address_space_operations omfs_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = omfs_readpage, > .readahead = omfs_readahead, > .writepage = omfs_writepage, > diff --git a/fs/sysv/itree.c b/fs/sysv/itree.c > index 8b2e99b7bc9f..749385015a8d 100644 > --- a/fs/sysv/itree.c > +++ b/fs/sysv/itree.c > @@ -495,6 +495,7 @@ static sector_t sysv_bmap(struct address_space *mapping, sector_t block) > } > > const struct address_space_operations sysv_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = sysv_readpage, > .writepage = sysv_writepage, > .write_begin = sysv_write_begin, > diff --git a/fs/udf/file.c b/fs/udf/file.c > index 2846dcd92197..1baff8ddb754 100644 > --- a/fs/udf/file.c > +++ b/fs/udf/file.c > @@ -125,6 +125,7 @@ static int udf_adinicb_write_end(struct file *file, struct address_space *mappin > } > > const struct address_space_operations udf_adinicb_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = udf_adinicb_readpage, > .writepage = udf_adinicb_writepage, > .write_begin = udf_adinicb_write_begin, > diff --git a/fs/udf/inode.c b/fs/udf/inode.c > index 0dd2f93ac048..4917670860a0 100644 > --- a/fs/udf/inode.c > +++ b/fs/udf/inode.c > @@ -235,6 +235,7 @@ static sector_t udf_bmap(struct address_space *mapping, sector_t block) > } > > const struct address_space_operations udf_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = udf_readpage, > .readahead = udf_readahead, > .writepage = udf_writepage, > diff --git a/fs/ufs/inode.c b/fs/ufs/inode.c > index debc282c1bb4..ac628de69601 100644 > --- a/fs/ufs/inode.c > +++ b/fs/ufs/inode.c > @@ -526,6 +526,7 @@ static sector_t ufs_bmap(struct address_space *mapping, sector_t block) > } > > const struct address_space_operations ufs_aops = { > + .set_page_dirty = __set_page_dirty_buffers, > .readpage = ufs_readpage, > .writepage = ufs_writepage, > .write_begin = ufs_write_begin, > diff --git a/mm/page-writeback.c b/mm/page-writeback.c > index 0062d5c57d41..081fa02236c2 100644 > --- a/mm/page-writeback.c > +++ b/mm/page-writeback.c > @@ -32,7 +32,6 @@ > #include <linux/sysctl.h> > #include <linux/cpu.h> > #include <linux/syscalls.h> > -#include <linux/buffer_head.h> /* __set_page_dirty_buffers */ > #include <linux/pagevec.h> > #include <linux/timer.h> > #include <linux/sched/rt.h> > @@ -2546,13 +2545,9 @@ EXPORT_SYMBOL(redirty_page_for_writepage); > /* > * Dirty a page. > * > - * For pages with a mapping this should be done under the page lock > - * for the benefit of asynchronous memory errors who prefer a consistent > - * dirty state. This rule can be broken in some special cases, > - * but should be better not to. > - * > - * If the mapping doesn't provide a set_page_dirty a_op, then > - * just fall through and assume that it wants buffer_heads. > + * For pages with a mapping this should be done under the page lock for the > + * benefit of asynchronous memory errors who prefer a consistent dirty state. > + * This rule can be broken in some special cases, but should be better not to. > */ > int set_page_dirty(struct page *page) > { > @@ -2560,7 +2555,6 @@ int set_page_dirty(struct page *page) > > page = compound_head(page); > if (likely(mapping)) { > - int (*spd)(struct page *) = mapping->a_ops->set_page_dirty; > /* > * readahead/lru_deactivate_page could remain > * PG_readahead/PG_reclaim due to race with end_page_writeback > @@ -2573,11 +2567,7 @@ int set_page_dirty(struct page *page) > */ > if (PageReclaim(page)) > ClearPageReclaim(page); > -#ifdef CONFIG_BLOCK > - if (!spd) > - spd = __set_page_dirty_buffers; > -#endif > - return (*spd)(page); > + return mapping->a_ops->set_page_dirty(page); > } > if (!PageDirty(page)) { > if (!TestSetPageDirty(page)) > -- > 2.30.2 > -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] mm: require ->set_page_dirty to be explicitly wire up 2021-06-14 9:51 ` Jan Kara @ 2021-06-14 10:03 ` Christoph Hellwig 0 siblings, 0 replies; 19+ messages in thread From: Christoph Hellwig @ 2021-06-14 10:03 UTC (permalink / raw) To: Jan Kara Cc: Christoph Hellwig, Andrew Morton, Al Viro, Greg Kroah-Hartman, linux-mm, linux-fsdevel, linux-kernel On Mon, Jun 14, 2021 at 11:51:07AM +0200, Jan Kara wrote: > On Mon 14-06-21 08:15:12, Christoph Hellwig wrote: > > Remove the CONFIG_BLOCK default to __set_page_dirty_buffers and just > > wire that method up for the missing instances. > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Make sense. Did you somehow autogenerate this? If this patch would race > with addition of new aops struct, we'd get null-ptr-defer out of that so > maybe providing the script would be better. But other than that the changes > look good to me. You can add: No, this was done manually as I audited all instances for actually being able to dirty pages. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] mm: require ->set_page_dirty to be explicitly wire up 2021-06-14 6:15 ` [PATCH 3/3] mm: require ->set_page_dirty to be explicitly wire up Christoph Hellwig 2021-06-14 6:24 ` Greg Kroah-Hartman 2021-06-14 9:51 ` Jan Kara @ 2021-06-18 19:23 ` Dmitry Osipenko 2021-06-21 5:26 ` Christoph Hellwig 2 siblings, 1 reply; 19+ messages in thread From: Dmitry Osipenko @ 2021-06-18 19:23 UTC (permalink / raw) To: Christoph Hellwig, Andrew Morton Cc: Jan Kara, Al Viro, Greg Kroah-Hartman, linux-mm, linux-fsdevel, linux-kernel 14.06.2021 09:15, Christoph Hellwig пишет: > Remove the CONFIG_BLOCK default to __set_page_dirty_buffers and just > wire that method up for the missing instances. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/adfs/inode.c | 1 + > fs/affs/file.c | 2 ++ > fs/bfs/file.c | 1 + > fs/block_dev.c | 1 + > fs/exfat/inode.c | 1 + > fs/ext2/inode.c | 2 ++ > fs/fat/inode.c | 1 + > fs/gfs2/meta_io.c | 2 ++ > fs/hfs/inode.c | 2 ++ > fs/hfsplus/inode.c | 2 ++ > fs/hpfs/file.c | 1 + > fs/jfs/inode.c | 1 + > fs/minix/inode.c | 1 + > fs/nilfs2/mdt.c | 1 + > fs/ocfs2/aops.c | 1 + > fs/omfs/file.c | 1 + > fs/sysv/itree.c | 1 + > fs/udf/file.c | 1 + > fs/udf/inode.c | 1 + > fs/ufs/inode.c | 1 + > mm/page-writeback.c | 18 ++++-------------- > 21 files changed, 29 insertions(+), 14 deletions(-) The ecryptfs is now crashing with NULL deference, please fix. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] mm: require ->set_page_dirty to be explicitly wire up 2021-06-18 19:23 ` Dmitry Osipenko @ 2021-06-21 5:26 ` Christoph Hellwig 0 siblings, 0 replies; 19+ messages in thread From: Christoph Hellwig @ 2021-06-21 5:26 UTC (permalink / raw) To: Dmitry Osipenko, Tyler Hicks Cc: Christoph Hellwig, Andrew Morton, Jan Kara, Al Viro, Greg Kroah-Hartman, linux-mm, linux-fsdevel, linux-kernel, ecryptfs On Fri, Jun 18, 2021 at 10:23:55PM +0300, Dmitry Osipenko wrote: > > 21 files changed, 29 insertions(+), 14 deletions(-) > > The ecryptfs is now crashing with NULL deference, please fix. Which means it crashed the same before on configs without CONFIG_BLOCK. Tyler, can you look at what ecryptfs should do for ->set_page_dirty? Currently it implicitly gets __set_page_dirty_buffers for kernels with COFIG_BLOCK set, but given that this function looks at buffer_heads that ecryptfs doesn't use it can't really be the right choice. __set_page_dirty_nobuffers will probably work, but I'd love to see an audit of the page dirtying and writeback for ecryptfs while we're at it. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: remove the implicit .set_page_dirty default 2021-06-14 6:15 remove the implicit .set_page_dirty default Christoph Hellwig ` (2 preceding siblings ...) 2021-06-14 6:15 ` [PATCH 3/3] mm: require ->set_page_dirty to be explicitly wire up Christoph Hellwig @ 2021-06-14 12:23 ` Matthew Wilcox 2021-06-14 15:53 ` Christoph Hellwig 3 siblings, 1 reply; 19+ messages in thread From: Matthew Wilcox @ 2021-06-14 12:23 UTC (permalink / raw) To: Christoph Hellwig Cc: Andrew Morton, Jan Kara, Al Viro, Greg Kroah-Hartman, linux-mm, linux-fsdevel, linux-kernel On Mon, Jun 14, 2021 at 08:15:09AM +0200, Christoph Hellwig wrote: > Hi all, > > this series cleans up a few lose ends around ->set_page_dirty, most > importantly removes the default to the buffer head based on if no > method is wired up. i have a somewhat similar series in the works ... https://git.infradead.org/users/willy/pagecache.git/commitdiff/1e7e8c2d82666b55690705d5bbe908e31d437edb https://git.infradead.org/users/willy/pagecache.git/commitdiff/bf767a4969c0bc6735275ff7457a8082eef4c3fd ... the other patches rather depend on the folio work. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: remove the implicit .set_page_dirty default 2021-06-14 12:23 ` remove the implicit .set_page_dirty default Matthew Wilcox @ 2021-06-14 15:53 ` Christoph Hellwig 2021-06-14 15:55 ` Christoph Hellwig 0 siblings, 1 reply; 19+ messages in thread From: Christoph Hellwig @ 2021-06-14 15:53 UTC (permalink / raw) To: Matthew Wilcox Cc: Christoph Hellwig, Andrew Morton, Jan Kara, Al Viro, Greg Kroah-Hartman, linux-mm, linux-fsdevel, linux-kernel On Mon, Jun 14, 2021 at 01:23:22PM +0100, Matthew Wilcox wrote: > i have a somewhat similar series in the works ... > > https://git.infradead.org/users/willy/pagecache.git/commitdiff/1e7e8c2d82666b55690705d5bbe908e31d437edb > https://git.infradead.org/users/willy/pagecache.git/commitdiff/bf767a4969c0bc6735275ff7457a8082eef4c3fd > > ... the other patches rather depend on the folio work. Yes, these looks useful to me as well. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: remove the implicit .set_page_dirty default 2021-06-14 15:53 ` Christoph Hellwig @ 2021-06-14 15:55 ` Christoph Hellwig 2021-06-14 16:17 ` Matthew Wilcox 0 siblings, 1 reply; 19+ messages in thread From: Christoph Hellwig @ 2021-06-14 15:55 UTC (permalink / raw) To: Matthew Wilcox Cc: Christoph Hellwig, Andrew Morton, Jan Kara, Al Viro, Greg Kroah-Hartman, linux-mm, linux-fsdevel, linux-kernel On Mon, Jun 14, 2021 at 05:53:33PM +0200, Christoph Hellwig wrote: > On Mon, Jun 14, 2021 at 01:23:22PM +0100, Matthew Wilcox wrote: > > i have a somewhat similar series in the works ... > > > > https://git.infradead.org/users/willy/pagecache.git/commitdiff/1e7e8c2d82666b55690705d5bbe908e31d437edb > > https://git.infradead.org/users/willy/pagecache.git/commitdiff/bf767a4969c0bc6735275ff7457a8082eef4c3fd > > > > ... the other patches rather depend on the folio work. > > Yes, these looks useful to me as well. And in fact I suspect the code in __set_page_dirty_no_writeback should really be the default if no ->set_page_dirty is set up. It is the same code as the no-mapping case and really makes sense as the default case.. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: remove the implicit .set_page_dirty default 2021-06-14 15:55 ` Christoph Hellwig @ 2021-06-14 16:17 ` Matthew Wilcox 0 siblings, 0 replies; 19+ messages in thread From: Matthew Wilcox @ 2021-06-14 16:17 UTC (permalink / raw) To: Christoph Hellwig Cc: Andrew Morton, Jan Kara, Al Viro, Greg Kroah-Hartman, linux-mm, linux-fsdevel, linux-kernel On Mon, Jun 14, 2021 at 05:55:30PM +0200, Christoph Hellwig wrote: > On Mon, Jun 14, 2021 at 05:53:33PM +0200, Christoph Hellwig wrote: > > On Mon, Jun 14, 2021 at 01:23:22PM +0100, Matthew Wilcox wrote: > > > i have a somewhat similar series in the works ... > > > > > > https://git.infradead.org/users/willy/pagecache.git/commitdiff/1e7e8c2d82666b55690705d5bbe908e31d437edb > > > https://git.infradead.org/users/willy/pagecache.git/commitdiff/bf767a4969c0bc6735275ff7457a8082eef4c3fd > > > > > > ... the other patches rather depend on the folio work. > > > > Yes, these looks useful to me as well. > > > And in fact I suspect the code in __set_page_dirty_no_writeback should > really be the default if no ->set_page_dirty is set up. It is the > same code as the no-mapping case and really makes sense as the default > case.. yes; i was even wondering if that should be conditional on one of the "no writeback" bits in mapping ... ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2021-06-21 5:26 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-14 6:15 remove the implicit .set_page_dirty default Christoph Hellwig 2021-06-14 6:15 ` [PATCH 1/3] fs: unexport __set_page_dirty Christoph Hellwig 2021-06-14 6:23 ` Greg Kroah-Hartman 2021-06-14 9:42 ` Jan Kara 2021-06-14 12:32 ` Matthew Wilcox 2021-06-14 15:52 ` Christoph Hellwig 2021-06-14 6:15 ` [PATCH 2/3] fs: move ramfs_aops to libfs Christoph Hellwig 2021-06-14 6:24 ` Greg Kroah-Hartman 2021-06-14 9:45 ` Jan Kara 2021-06-14 6:15 ` [PATCH 3/3] mm: require ->set_page_dirty to be explicitly wire up Christoph Hellwig 2021-06-14 6:24 ` Greg Kroah-Hartman 2021-06-14 9:51 ` Jan Kara 2021-06-14 10:03 ` Christoph Hellwig 2021-06-18 19:23 ` Dmitry Osipenko 2021-06-21 5:26 ` Christoph Hellwig 2021-06-14 12:23 ` remove the implicit .set_page_dirty default Matthew Wilcox 2021-06-14 15:53 ` Christoph Hellwig 2021-06-14 15:55 ` Christoph Hellwig 2021-06-14 16:17 ` Matthew Wilcox
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).