| Message ID | 20230307143125.27778-2-hch@lst.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] ufs: don't flush page immediately for DIRSYNC directories | expand |
On Tue, Mar 07, 2023 at 03:31:23PM +0100, Christoph Hellwig wrote: > We do not need to writeout modified directory blocks immediately when > modifying them while the page is locked. It is enough to do the flush > somewhat later which has the added benefit that inode times can be > flushed as well. It also allows us to stop depending on > write_one_page() function. > > Ported from an ext2 patch by Jan Kara. Umm... I'll throw it in ufs pile, I guess (tomorrow - I'll need to sort out Fabio's patches in the area as well; IIRC, the latest had been in late December).
On Fri, Mar 10, 2023 at 03:53:53AM +0000, Al Viro wrote: > On Tue, Mar 07, 2023 at 03:31:23PM +0100, Christoph Hellwig wrote: > > We do not need to writeout modified directory blocks immediately when > > modifying them while the page is locked. It is enough to do the flush > > somewhat later which has the added benefit that inode times can be > > flushed as well. It also allows us to stop depending on > > write_one_page() function. > > > > Ported from an ext2 patch by Jan Kara. > > Umm... I'll throw it in ufs pile, I guess (tomorrow - I'll need to > sort out Fabio's patches in the area as well; IIRC, the latest > had been in late December). Well, the three patches really should go together, otherwise we miss yet another merge window.
On Fri, Mar 10, 2023 at 07:37:56AM +0100, Christoph Hellwig wrote: > On Fri, Mar 10, 2023 at 03:53:53AM +0000, Al Viro wrote: > > On Tue, Mar 07, 2023 at 03:31:23PM +0100, Christoph Hellwig wrote: > > > We do not need to writeout modified directory blocks immediately when > > > modifying them while the page is locked. It is enough to do the flush > > > somewhat later which has the added benefit that inode times can be > > > flushed as well. It also allows us to stop depending on > > > write_one_page() function. > > > > > > Ported from an ext2 patch by Jan Kara. > > > > Umm... I'll throw it in ufs pile, I guess (tomorrow - I'll need to > > sort out Fabio's patches in the area as well; IIRC, the latest > > had been in late December). > > Well, the three patches really should go together, otherwise we miss > yet another merge window. Umm... Do you need them in the same (never-rebased?) branch, or would it suffice to have all of them reach mainline by 6.4-rc1?
On Fri, Mar 10, 2023 at 06:52:35AM +0000, Al Viro wrote: > > > Umm... I'll throw it in ufs pile, I guess (tomorrow - I'll need to > > > sort out Fabio's patches in the area as well; IIRC, the latest > > > had been in late December). > > > > Well, the three patches really should go together, otherwise we miss > > yet another merge window. > > Umm... Do you need them in the same (never-rebased?) branch, or would > it suffice to have all of them reach mainline by 6.4-rc1? The latter. But patch 3 depends on 1 and 2.
On Fri, Mar 10, 2023 at 08:00:47AM +0100, Christoph Hellwig wrote: > On Fri, Mar 10, 2023 at 06:52:35AM +0000, Al Viro wrote: > > > > Umm... I'll throw it in ufs pile, I guess (tomorrow - I'll need to > > > > sort out Fabio's patches in the area as well; IIRC, the latest > > > > had been in late December). > > > > > > Well, the three patches really should go together, otherwise we miss > > > yet another merge window. > > > > Umm... Do you need them in the same (never-rebased?) branch, or would > > it suffice to have all of them reach mainline by 6.4-rc1? > > The latter. But patch 3 depends on 1 and 2. Obviously, but that's not hard to arrange. OK, will do tomorrow; remind me if all three are not in vfs.git by Monday...
diff --git a/fs/ufs/dir.c b/fs/ufs/dir.c index 391efaf1d52897..379d75796a5ce3 100644 --- a/fs/ufs/dir.c +++ b/fs/ufs/dir.c @@ -42,11 +42,10 @@ static inline int ufs_match(struct super_block *sb, int len, return !memcmp(name, de->d_name, len); } -static int ufs_commit_chunk(struct page *page, loff_t pos, unsigned len) +static void ufs_commit_chunk(struct page *page, loff_t pos, unsigned len) { struct address_space *mapping = page->mapping; struct inode *dir = mapping->host; - int err = 0; inode_inc_iversion(dir); block_write_end(NULL, mapping, pos, len, len, page, NULL); @@ -54,10 +53,16 @@ static int ufs_commit_chunk(struct page *page, loff_t pos, unsigned len) i_size_write(dir, pos+len); mark_inode_dirty(dir); } - if (IS_DIRSYNC(dir)) - err = write_one_page(page); - else - unlock_page(page); + unlock_page(page); +} + +static int ufs_handle_dirsync(struct inode *dir) +{ + int err; + + err = filemap_write_and_wait(dir->i_mapping); + if (!err) + err = sync_inode_metadata(dir, 1); return err; } @@ -99,11 +104,12 @@ void ufs_set_link(struct inode *dir, struct ufs_dir_entry *de, de->d_ino = cpu_to_fs32(dir->i_sb, inode->i_ino); ufs_set_de_type(dir->i_sb, de, inode->i_mode); - err = ufs_commit_chunk(page, pos, len); + ufs_commit_chunk(page, pos, len); ufs_put_page(page); if (update_times) dir->i_mtime = dir->i_ctime = current_time(dir); mark_inode_dirty(dir); + ufs_handle_dirsync(dir); } @@ -390,10 +396,11 @@ int ufs_add_link(struct dentry *dentry, struct inode *inode) de->d_ino = cpu_to_fs32(sb, inode->i_ino); ufs_set_de_type(sb, de, inode->i_mode); - err = ufs_commit_chunk(page, pos, rec_len); + ufs_commit_chunk(page, pos, rec_len); dir->i_mtime = dir->i_ctime = current_time(dir); mark_inode_dirty(dir); + err = ufs_handle_dirsync(dir); /* OFFSET_CACHE */ out_put: ufs_put_page(page); @@ -531,9 +538,10 @@ int ufs_delete_entry(struct inode *inode, struct ufs_dir_entry *dir, if (pde) pde->d_reclen = cpu_to_fs16(sb, to - from); dir->d_ino = 0; - err = ufs_commit_chunk(page, pos, to - from); + ufs_commit_chunk(page, pos, to - from); inode->i_ctime = inode->i_mtime = current_time(inode); mark_inode_dirty(inode); + err = ufs_handle_dirsync(inode); out: ufs_put_page(page); UFSD("EXIT\n"); @@ -579,7 +587,8 @@ int ufs_make_empty(struct inode * inode, struct inode *dir) strcpy (de->d_name, ".."); kunmap(page); - err = ufs_commit_chunk(page, 0, chunk_size); + ufs_commit_chunk(page, 0, chunk_size); + err = ufs_handle_dirsync(inode); fail: put_page(page); return err;
We do not need to writeout modified directory blocks immediately when modifying them while the page is locked. It is enough to do the flush somewhat later which has the added benefit that inode times can be flushed as well. It also allows us to stop depending on write_one_page() function. Ported from an ext2 patch by Jan Kara. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/ufs/dir.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-)