Christoph Hellwig
2023-Jan-10 08:22 UTC
[Ocfs2-devel] [PATCH 3/7] minix: don't flush page immediately for DIRSYNC directories
On Sun, Jan 08, 2023 at 09:17:26PM +0000, Matthew Wilcox wrote:> > + dir_commit_chunk(page, pos, sbi->s_dirsize); > > dir->i_mtime = dir->i_ctime = current_time(dir); > > mark_inode_dirty(dir); > > + minix_handle_dirsync(dir); > > Doesn't this need to be: > > err = minix_handle_dirsync(dir);Yes, fixed.> > > @@ -426,7 +436,7 @@ void minix_set_link(struct minix_dir_entry *de, struct page *page, > > ((minix3_dirent *) de)->inode = inode->i_ino; > > else > > de->inode = inode->i_ino; > > - err = dir_commit_chunk(page, pos, sbi->s_dirsize); > > + dir_commit_chunk(page, pos, sbi->s_dirsize); > > } else { > > unlock_page(page); > > } > > -- > > Aren't you missing a call to minix_handle_dirsync() in this function?Yes, fixed.
Al Viro
2023-Jan-11 02:20 UTC
[Ocfs2-devel] [PATCH 3/7] minix: don't flush page immediately for DIRSYNC directories
On Tue, Jan 10, 2023 at 09:22:25AM +0100, Christoph Hellwig wrote:> On Sun, Jan 08, 2023 at 09:17:26PM +0000, Matthew Wilcox wrote: > > > + dir_commit_chunk(page, pos, sbi->s_dirsize); > > > dir->i_mtime = dir->i_ctime = current_time(dir); > > > mark_inode_dirty(dir); > > > + minix_handle_dirsync(dir); > > > > Doesn't this need to be: > > > > err = minix_handle_dirsync(dir); > > Yes, fixed. > > > > > > @@ -426,7 +436,7 @@ void minix_set_link(struct minix_dir_entry *de, struct page *page, > > > ((minix3_dirent *) de)->inode = inode->i_ino; > > > else > > > de->inode = inode->i_ino; > > > - err = dir_commit_chunk(page, pos, sbi->s_dirsize); > > > + dir_commit_chunk(page, pos, sbi->s_dirsize); > > > } else { > > > unlock_page(page); > > > } > > > -- > > > > Aren't you missing a call to minix_handle_dirsync() in this function? > > Yes, fixed.More seriously, all those ..._set_link() need to return an error and their callers (..._rename()) need to deal with failures. That goes for ext2 as well, and that part is worth splitting off into a prereq - it's a -stable fodder.