From 4a29a1262a47acd8e81a3824492a5aa8f8c5fc95 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Wed, 14 Dec 2022 12:57:27 -0500 Subject: minix: make minix_new_inode() return error as ERR_PTR(-E...) adjust the callers Signed-off-by: Al Viro --- fs/minix/bitmap.c | 16 +++++-------- fs/minix/minix.h | 2 +- fs/minix/namei.c | 68 ++++++++++++++++++++++++------------------------------- 3 files changed, 36 insertions(+), 50 deletions(-) (limited to 'fs/minix') diff --git a/fs/minix/bitmap.c b/fs/minix/bitmap.c index 9115948c624e..af50d8e9cd89 100644 --- a/fs/minix/bitmap.c +++ b/fs/minix/bitmap.c @@ -210,7 +210,7 @@ void minix_free_inode(struct inode * inode) mark_buffer_dirty(bh); } -struct inode *minix_new_inode(const struct inode *dir, umode_t mode, int *error) +struct inode *minix_new_inode(const struct inode *dir, umode_t mode) { struct super_block *sb = dir->i_sb; struct minix_sb_info *sbi = minix_sb(sb); @@ -220,13 +220,10 @@ struct inode *minix_new_inode(const struct inode *dir, umode_t mode, int *error) unsigned long j; int i; - if (!inode) { - *error = -ENOMEM; - return NULL; - } + if (!inode) + return ERR_PTR(-ENOMEM); j = bits_per_zone; bh = NULL; - *error = -ENOSPC; spin_lock(&bitmap_lock); for (i = 0; i < sbi->s_imap_blocks; i++) { bh = sbi->s_imap[i]; @@ -237,20 +234,20 @@ struct inode *minix_new_inode(const struct inode *dir, umode_t mode, int *error) if (!bh || j >= bits_per_zone) { spin_unlock(&bitmap_lock); iput(inode); - return NULL; + return ERR_PTR(-ENOSPC); } if (minix_test_and_set_bit(j, bh->b_data)) { /* shouldn't happen */ spin_unlock(&bitmap_lock); printk("minix_new_inode: bit already set\n"); iput(inode); - return NULL; + return ERR_PTR(-ENOSPC); } spin_unlock(&bitmap_lock); mark_buffer_dirty(bh); j += i * bits_per_zone; if (!j || j > sbi->s_ninodes) { iput(inode); - return NULL; + return ERR_PTR(-ENOSPC); } inode_init_owner(&init_user_ns, inode, dir, mode); inode->i_ino = j; @@ -260,7 +257,6 @@ struct inode *minix_new_inode(const struct inode *dir, umode_t mode, int *error) insert_inode_hash(inode); mark_inode_dirty(inode); - *error = 0; return inode; } diff --git a/fs/minix/minix.h b/fs/minix/minix.h index 202173368025..7158a68bdc63 100644 --- a/fs/minix/minix.h +++ b/fs/minix/minix.h @@ -45,7 +45,7 @@ struct minix_sb_info { extern struct inode *minix_iget(struct super_block *, unsigned long); extern struct minix_inode * minix_V1_raw_inode(struct super_block *, ino_t, struct buffer_head **); extern struct minix2_inode * minix_V2_raw_inode(struct super_block *, ino_t, struct buffer_head **); -extern struct inode * minix_new_inode(const struct inode *, umode_t, int *); +extern struct inode * minix_new_inode(const struct inode *, umode_t); extern void minix_free_inode(struct inode * inode); extern unsigned long minix_count_free_inodes(struct super_block *sb); extern int minix_new_block(struct inode * inode); diff --git a/fs/minix/namei.c b/fs/minix/namei.c index 8afdc408ca4f..a4f5e6caf8fe 100644 --- a/fs/minix/namei.c +++ b/fs/minix/namei.c @@ -36,33 +36,31 @@ static struct dentry *minix_lookup(struct inode * dir, struct dentry *dentry, un static int minix_mknod(struct user_namespace *mnt_userns, struct inode *dir, struct dentry *dentry, umode_t mode, dev_t rdev) { - int error; struct inode *inode; if (!old_valid_dev(rdev)) return -EINVAL; - inode = minix_new_inode(dir, mode, &error); + inode = minix_new_inode(dir, mode); + if (IS_ERR(inode)) + return PTR_ERR(inode); - if (inode) { - minix_set_inode(inode, rdev); - mark_inode_dirty(inode); - error = add_nondir(dentry, inode); - } - return error; + minix_set_inode(inode, rdev); + mark_inode_dirty(inode); + return add_nondir(dentry, inode); } static int minix_tmpfile(struct user_namespace *mnt_userns, struct inode *dir, struct file *file, umode_t mode) { - int error; - struct inode *inode = minix_new_inode(dir, mode, &error); - if (inode) { - minix_set_inode(inode, 0); - mark_inode_dirty(inode); - d_tmpfile(file, inode); - } - return finish_open_simple(file, error); + struct inode *inode = minix_new_inode(dir, mode); + + if (IS_ERR(inode)) + return finish_open_simple(file, PTR_ERR(inode)); + minix_set_inode(inode, 0); + mark_inode_dirty(inode); + d_tmpfile(file, inode); + return finish_open_simple(file, 0); } static int minix_create(struct user_namespace *mnt_userns, struct inode *dir, @@ -74,30 +72,25 @@ static int minix_create(struct user_namespace *mnt_userns, struct inode *dir, static int minix_symlink(struct user_namespace *mnt_userns, struct inode *dir, struct dentry *dentry, const char *symname) { - int err = -ENAMETOOLONG; int i = strlen(symname)+1; struct inode * inode; + int err; if (i > dir->i_sb->s_blocksize) - goto out; + return -ENAMETOOLONG; - inode = minix_new_inode(dir, S_IFLNK | 0777, &err); - if (!inode) - goto out; + inode = minix_new_inode(dir, S_IFLNK | 0777); + if (IS_ERR(inode)) + return PTR_ERR(inode); minix_set_inode(inode, 0); err = page_symlink(inode, symname, i); - if (err) - goto out_fail; - - err = add_nondir(dentry, inode); -out: - return err; - -out_fail: - inode_dec_link_count(inode); - iput(inode); - goto out; + if (unlikely(err)) { + inode_dec_link_count(inode); + iput(inode); + return err; + } + return add_nondir(dentry, inode); } static int minix_link(struct dentry * old_dentry, struct inode * dir, @@ -117,14 +110,12 @@ static int minix_mkdir(struct user_namespace *mnt_userns, struct inode *dir, struct inode * inode; int err; - inode_inc_link_count(dir); - - inode = minix_new_inode(dir, S_IFDIR | mode, &err); - if (!inode) - goto out_dir; + inode = minix_new_inode(dir, S_IFDIR | mode); + if (IS_ERR(inode)) + return PTR_ERR(inode); + inode_inc_link_count(dir); minix_set_inode(inode, 0); - inode_inc_link_count(inode); err = minix_make_empty(inode, dir); @@ -143,7 +134,6 @@ out_fail: inode_dec_link_count(inode); inode_dec_link_count(inode); iput(inode); -out_dir: inode_dec_link_count(dir); goto out; } -- cgit v1.2.3 From 35bb6a092cfcab895ea7e9c31a2cc6b80b25c134 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 18 Jan 2023 18:30:21 +0100 Subject: minix: move releasing pages into unlink and rename Instead of consuming the page reference and kmap in the low-level minix_delete_entry and minix_set_link helpers, do it in the callers where that code can be shared with the error cleanup path. Signed-off-by: Christoph Hellwig Signed-off-by: Al Viro --- fs/minix/dir.c | 2 -- fs/minix/namei.c | 19 ++++++++++--------- 2 files changed, 10 insertions(+), 11 deletions(-) (limited to 'fs/minix') diff --git a/fs/minix/dir.c b/fs/minix/dir.c index dcfe5b25378b..ec462330e749 100644 --- a/fs/minix/dir.c +++ b/fs/minix/dir.c @@ -306,7 +306,6 @@ int minix_delete_entry(struct minix_dir_entry *de, struct page *page) } else { unlock_page(page); } - dir_put_page(page); inode->i_ctime = inode->i_mtime = current_time(inode); mark_inode_dirty(inode); return err; @@ -430,7 +429,6 @@ void minix_set_link(struct minix_dir_entry *de, struct page *page, } else { unlock_page(page); } - dir_put_page(page); dir->i_mtime = dir->i_ctime = current_time(dir); mark_inode_dirty(dir); } diff --git a/fs/minix/namei.c b/fs/minix/namei.c index a4f5e6caf8fe..7dc481418871 100644 --- a/fs/minix/namei.c +++ b/fs/minix/namei.c @@ -140,23 +140,23 @@ out_fail: static int minix_unlink(struct inode * dir, struct dentry *dentry) { - int err = -ENOENT; struct inode * inode = d_inode(dentry); struct page * page; struct minix_dir_entry * de; + int err; de = minix_find_entry(dentry, &page); if (!de) - goto end_unlink; - + return -ENOENT; err = minix_delete_entry(de, page); - if (err) - goto end_unlink; + kunmap(page); + put_page(page); + if (err) + return err; inode->i_ctime = dir->i_ctime; inode_dec_link_count(inode); -end_unlink: - return err; + return 0; } static int minix_rmdir(struct inode * dir, struct dentry *dentry) @@ -213,7 +213,10 @@ static int minix_rename(struct user_namespace *mnt_userns, new_de = minix_find_entry(new_dentry, &new_page); if (!new_de) goto out_dir; + err = 0; minix_set_link(new_de, new_page, old_inode); + kunmap(new_page); + put_page(new_page); new_inode->i_ctime = current_time(new_inode); if (dir_de) drop_nlink(new_inode); @@ -233,8 +236,6 @@ static int minix_rename(struct user_namespace *mnt_userns, minix_set_link(dir_de, dir_page, new_dir); inode_dec_link_count(old_dir); } - return 0; - out_dir: if (dir_de) { kunmap(dir_page); -- cgit v1.2.3 From b61d15d5ff67c07907c194d4354ec0c35bc657d1 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 18 Jan 2023 18:30:22 +0100 Subject: minix: fix error handling in minix_delete_entry If minix_prepare_chunk fails, updating c/mtime and marking the dir inode dirty is wrong, as the inode hasn't been modified. Signed-off-by: Christoph Hellwig Signed-off-by: Al Viro --- fs/minix/dir.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) (limited to 'fs/minix') diff --git a/fs/minix/dir.c b/fs/minix/dir.c index ec462330e749..242e179aa1fb 100644 --- a/fs/minix/dir.c +++ b/fs/minix/dir.c @@ -297,18 +297,20 @@ int minix_delete_entry(struct minix_dir_entry *de, struct page *page) lock_page(page); err = minix_prepare_chunk(page, pos, len); - if (err == 0) { - if (sbi->s_version == MINIX_V3) - ((minix3_dirent *) de)->inode = 0; - else - de->inode = 0; - err = dir_commit_chunk(page, pos, len); - } else { + if (err) { unlock_page(page); + return err; } + if (sbi->s_version == MINIX_V3) + ((minix3_dirent *)de)->inode = 0; + else + de->inode = 0; + err = dir_commit_chunk(page, pos, len); + if (err) + return err; inode->i_ctime = inode->i_mtime = current_time(inode); mark_inode_dirty(inode); - return err; + return 0; } int minix_make_empty(struct inode *inode, struct inode *dir) -- cgit v1.2.3 From 2d1a9d599b3ed9b9c58f64b8b71b5b9d770ceacf Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 18 Jan 2023 18:30:23 +0100 Subject: minix: fix error handling in minix_set_link If minix_prepare_chunk fails, updating c/mtime and marking the dir inode dirty is wrong, as the inode hasn't been modified. Also propagate the error to the caller. Note that this moves the dir_put_page call later, but that matches other uses of this helper in the directory code. Signed-off-by: Christoph Hellwig Signed-off-by: Al Viro --- fs/minix/dir.c | 22 ++++++++++++---------- fs/minix/minix.h | 3 ++- fs/minix/namei.c | 10 ++++++---- 3 files changed, 20 insertions(+), 15 deletions(-) (limited to 'fs/minix') diff --git a/fs/minix/dir.c b/fs/minix/dir.c index 242e179aa1fb..34c1cdb5dc7d 100644 --- a/fs/minix/dir.c +++ b/fs/minix/dir.c @@ -410,8 +410,8 @@ not_empty: } /* Releases the page */ -void minix_set_link(struct minix_dir_entry *de, struct page *page, - struct inode *inode) +int minix_set_link(struct minix_dir_entry *de, struct page *page, + struct inode *inode) { struct inode *dir = page->mapping->host; struct minix_sb_info *sbi = minix_sb(dir->i_sb); @@ -420,19 +420,21 @@ void minix_set_link(struct minix_dir_entry *de, struct page *page, int err; lock_page(page); - err = minix_prepare_chunk(page, pos, sbi->s_dirsize); - if (err == 0) { - if (sbi->s_version == MINIX_V3) - ((minix3_dirent *) de)->inode = inode->i_ino; - else - de->inode = inode->i_ino; - err = dir_commit_chunk(page, pos, sbi->s_dirsize); - } else { + if (err) { unlock_page(page); + return err; } + if (sbi->s_version == MINIX_V3) + ((minix3_dirent *)de)->inode = inode->i_ino; + else + de->inode = inode->i_ino; + err = dir_commit_chunk(page, pos, sbi->s_dirsize); + if (err) + return err; dir->i_mtime = dir->i_ctime = current_time(dir); mark_inode_dirty(dir); + return 0; } struct minix_dir_entry * minix_dotdot (struct inode *dir, struct page **p) diff --git a/fs/minix/minix.h b/fs/minix/minix.h index 7158a68bdc63..65c19e078ec4 100644 --- a/fs/minix/minix.h +++ b/fs/minix/minix.h @@ -69,7 +69,8 @@ extern int minix_add_link(struct dentry*, struct inode*); extern int minix_delete_entry(struct minix_dir_entry*, struct page*); extern int minix_make_empty(struct inode*, struct inode*); extern int minix_empty_dir(struct inode*); -extern void minix_set_link(struct minix_dir_entry*, struct page*, struct inode*); +int minix_set_link(struct minix_dir_entry *de, struct page *page, + struct inode *inode); extern struct minix_dir_entry *minix_dotdot(struct inode*, struct page**); extern ino_t minix_inode_by_name(struct dentry*); diff --git a/fs/minix/namei.c b/fs/minix/namei.c index 7dc481418871..382ead96bf54 100644 --- a/fs/minix/namei.c +++ b/fs/minix/namei.c @@ -213,10 +213,11 @@ static int minix_rename(struct user_namespace *mnt_userns, new_de = minix_find_entry(new_dentry, &new_page); if (!new_de) goto out_dir; - err = 0; - minix_set_link(new_de, new_page, old_inode); + err = minix_set_link(new_de, new_page, old_inode); kunmap(new_page); put_page(new_page); + if (err) + goto out_dir; new_inode->i_ctime = current_time(new_inode); if (dir_de) drop_nlink(new_inode); @@ -233,8 +234,9 @@ static int minix_rename(struct user_namespace *mnt_userns, mark_inode_dirty(old_inode); if (dir_de) { - minix_set_link(dir_de, dir_page, new_dir); - inode_dec_link_count(old_dir); + err = minix_set_link(dir_de, dir_page, new_dir); + if (!err) + inode_dec_link_count(old_dir); } out_dir: if (dir_de) { -- cgit v1.2.3 From f556e776b14ae4f853147705603d263bfa20ecd1 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 18 Jan 2023 18:30:24 +0100 Subject: minix: don't flush page immediately for DIRSYNC directories 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 Signed-off-by: Al Viro --- fs/minix/dir.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) (limited to 'fs/minix') diff --git a/fs/minix/dir.c b/fs/minix/dir.c index 34c1cdb5dc7d..bf9858f76b6a 100644 --- a/fs/minix/dir.c +++ b/fs/minix/dir.c @@ -46,21 +46,27 @@ minix_last_byte(struct inode *inode, unsigned long page_nr) return last_byte; } -static int dir_commit_chunk(struct page *page, loff_t pos, unsigned len) +static void dir_commit_chunk(struct page *page, loff_t pos, unsigned len) { struct address_space *mapping = page->mapping; struct inode *dir = mapping->host; - int err = 0; + block_write_end(NULL, mapping, pos, len, len, page, NULL); if (pos+len > dir->i_size) { 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 minix_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; } @@ -274,9 +280,10 @@ got_it: memset (namx + namelen, 0, sbi->s_dirsize - namelen - 2); de->inode = inode->i_ino; } - err = dir_commit_chunk(page, pos, sbi->s_dirsize); + dir_commit_chunk(page, pos, sbi->s_dirsize); dir->i_mtime = dir->i_ctime = current_time(dir); mark_inode_dirty(dir); + err = minix_handle_dirsync(dir); out_put: dir_put_page(page); out: @@ -305,12 +312,10 @@ int minix_delete_entry(struct minix_dir_entry *de, struct page *page) ((minix3_dirent *)de)->inode = 0; else de->inode = 0; - err = dir_commit_chunk(page, pos, len); - if (err) - return err; + dir_commit_chunk(page, pos, len); inode->i_ctime = inode->i_mtime = current_time(inode); mark_inode_dirty(inode); - return 0; + return minix_handle_dirsync(inode); } int minix_make_empty(struct inode *inode, struct inode *dir) @@ -350,7 +355,8 @@ int minix_make_empty(struct inode *inode, struct inode *dir) } kunmap_atomic(kaddr); - err = dir_commit_chunk(page, 0, 2 * sbi->s_dirsize); + dir_commit_chunk(page, 0, 2 * sbi->s_dirsize); + err = minix_handle_dirsync(inode); fail: put_page(page); return err; @@ -429,12 +435,10 @@ int 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); - if (err) - return err; + dir_commit_chunk(page, pos, sbi->s_dirsize); dir->i_mtime = dir->i_ctime = current_time(dir); mark_inode_dirty(dir); - return 0; + return minix_handle_dirsync(dir); } struct minix_dir_entry * minix_dotdot (struct inode *dir, struct page **p) -- cgit v1.2.3 From 2cb6a44220b974a7832d1a09630b4cee870b023a Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 19 Jan 2023 19:29:26 -0500 Subject: minix_rename(): minix_delete_entry() might fail Signed-off-by: Al Viro --- fs/minix/namei.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'fs/minix') diff --git a/fs/minix/namei.c b/fs/minix/namei.c index 382ead96bf54..b99696b90b42 100644 --- a/fs/minix/namei.c +++ b/fs/minix/namei.c @@ -230,7 +230,10 @@ static int minix_rename(struct user_namespace *mnt_userns, inode_inc_link_count(new_dir); } - minix_delete_entry(old_de, old_page); + err = minix_delete_entry(old_de, old_page); + if (err) + goto out_dir; + mark_inode_dirty(old_inode); if (dir_de) { -- cgit v1.2.3