From 05848db2083d4f232e84e385845dcd98d5c511b2 Mon Sep 17 00:00:00 2001 From: Chengguang Xu Date: Thu, 26 Sep 2019 16:34:08 +0800 Subject: [PATCH 01/28] quota: avoid increasing DQST_LOOKUPS when iterating over dirty/inuse list It is meaningless to increase DQST_LOOKUPS number while iterating over dirty/inuse list, so just avoid it. Link: https://lore.kernel.org/r/20190926083408.4269-1-cgxu519@zoho.com.cn Signed-off-by: Chengguang Xu Signed-off-by: Jan Kara --- fs/quota/dquot.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index 6e826b454082..00a3c6df2ea3 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -595,7 +595,6 @@ int dquot_scan_active(struct super_block *sb, /* Now we have active dquot so we can just increase use count */ atomic_inc(&dquot->dq_count); spin_unlock(&dq_list_lock); - dqstats_inc(DQST_LOOKUPS); dqput(old_dquot); old_dquot = dquot; /* @@ -649,7 +648,6 @@ int dquot_writeback_dquots(struct super_block *sb, int type) * use count */ dqgrab(dquot); spin_unlock(&dq_list_lock); - dqstats_inc(DQST_LOOKUPS); err = sb->dq_op->write_dquot(dquot); if (err) { /* From 24fc755f56b15bf3215c0d0b8b62d7204f369bab Mon Sep 17 00:00:00 2001 From: Chengguang Xu Date: Mon, 23 Sep 2019 21:52:23 +0800 Subject: [PATCH 02/28] quota: code cleanup for hash bits calculation Code cleanup for hash bits calculation by calling ilog2(). Link: https://lore.kernel.org/r/20190923135223.27674-1-cgxu519@zoho.com.cn Signed-off-by: Chengguang Xu Signed-off-by: Jan Kara --- fs/quota/dquot.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index 00a3c6df2ea3..26812a67ed77 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -2981,11 +2981,7 @@ static int __init dquot_init(void) /* Find power-of-two hlist_heads which can fit into allocation */ nr_hash = (1UL << order) * PAGE_SIZE / sizeof(struct hlist_head); - dq_hash_bits = 0; - do { - dq_hash_bits++; - } while (nr_hash >> dq_hash_bits); - dq_hash_bits--; + dq_hash_bits = ilog2(nr_hash); nr_hash = 1UL << dq_hash_bits; dq_hash_mask = nr_hash - 1; From a6810312b835ed927edaef0e96217c02084c7f4f Mon Sep 17 00:00:00 2001 From: Chengguang Xu Date: Tue, 8 Oct 2019 22:50:59 +0800 Subject: [PATCH 03/28] quota: check quota type in early stage Check quota type in early stage so we can avoid many unncessary operations when the type is wrong. Link: https://lore.kernel.org/r/20191008145059.21402-1-cgxu519@mykernel.net Signed-off-by: Chengguang Xu Signed-off-by: Jan Kara --- fs/quota/quota.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/fs/quota/quota.c b/fs/quota/quota.c index cb13fb76dbee..5444d3c4d93f 100644 --- a/fs/quota/quota.c +++ b/fs/quota/quota.c @@ -60,8 +60,6 @@ static int quota_sync_all(int type) { int ret; - if (type >= MAXQUOTAS) - return -EINVAL; ret = security_quotactl(Q_SYNC, type, 0, NULL); if (!ret) iterate_supers(quota_sync_one, &type); @@ -686,8 +684,6 @@ static int do_quotactl(struct super_block *sb, int type, int cmd, qid_t id, { int ret; - if (type >= MAXQUOTAS) - return -EINVAL; type = array_index_nospec(type, MAXQUOTAS); /* * Quota not supported on this fs? Check this before s_quota_types @@ -831,6 +827,9 @@ int kernel_quotactl(unsigned int cmd, const char __user *special, cmds = cmd >> SUBCMDSHIFT; type = cmd & SUBCMDMASK; + if (type >= MAXQUOTAS) + return -EINVAL; + /* * As a special case Q_SYNC can be called without a specific device. * It will iterate all superblocks that have quota enabled and call From d02601c398522de953a44c11b282e89b01782959 Mon Sep 17 00:00:00 2001 From: Chengguang Xu Date: Thu, 10 Oct 2019 21:09:24 +0800 Subject: [PATCH 04/28] quota: minor code cleanup for v1_format_ops It's not a functinal change, it's just for keeping consistent coding style. Link: https://lore.kernel.org/r/20191010130924.17697-1-cgxu519@mykernel.net Signed-off-by: Chengguang Xu Signed-off-by: Jan Kara --- fs/quota/quota_v1.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/quota/quota_v1.c b/fs/quota/quota_v1.c index c740e5572eb8..cd92e5fa0062 100644 --- a/fs/quota/quota_v1.c +++ b/fs/quota/quota_v1.c @@ -217,7 +217,6 @@ static const struct quota_format_ops v1_format_ops = { .check_quota_file = v1_check_quota_file, .read_file_info = v1_read_file_info, .write_file_info = v1_write_file_info, - .free_file_info = NULL, .read_dqblk = v1_read_dqblk, .commit_dqblk = v1_commit_dqblk, }; From 158be76c01172f4ffa3f10e2b4b433b3e496c75d Mon Sep 17 00:00:00 2001 From: Chengguang Xu Date: Mon, 21 Oct 2019 07:23:26 +0800 Subject: [PATCH 05/28] ext2: adjust block num when retry allocation Set block num to original *count in a case of retrying allocation in case num < *count Link: https://lore.kernel.org/r/20191020232326.84881-1-cgxu519@mykernel.net Signed-off-by: Chengguang Xu Signed-off-by: Jan Kara --- fs/ext2/balloc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c index e0cc55164505..924c1c765306 100644 --- a/fs/ext2/balloc.c +++ b/fs/ext2/balloc.c @@ -1404,6 +1404,7 @@ allocated: * use. So we may want to selectively mark some of the blocks * as free */ + num = *count; goto retry_alloc; } From ba1af2e47446ac353395b4c274408944284ceeaa Mon Sep 17 00:00:00 2001 From: Chengguang Xu Date: Tue, 22 Oct 2019 17:17:38 +0800 Subject: [PATCH 06/28] ext2: add missing brelse in ext2_new_blocks() Fix missing brelse of bitmap_bh in the case of retry. Link: https://lore.kernel.org/r/20191022091738.9160-1-cgxu519@mykernel.net Signed-off-by: Chengguang Xu Signed-off-by: Jan Kara --- fs/ext2/balloc.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c index 924c1c765306..4664d324567c 100644 --- a/fs/ext2/balloc.c +++ b/fs/ext2/balloc.c @@ -1313,6 +1313,13 @@ retry_alloc: if (free_blocks > 0) { grp_target_blk = ((goal - le32_to_cpu(es->s_first_data_block)) % EXT2_BLOCKS_PER_GROUP(sb)); + /* + * In case we retry allocation (due to fs reservation not + * working out or fs corruption), the bitmap_bh is non-null + * pointer and we have to release it before calling + * read_block_bitmap(). + */ + brelse(bitmap_bh); bitmap_bh = read_block_bitmap(sb, group_no); if (!bitmap_bh) goto io_error; From acd1f04679803af84e7ab76f37df30d8598b07b9 Mon Sep 17 00:00:00 2001 From: Chengguang Xu Date: Wed, 23 Oct 2019 21:56:43 +0800 Subject: [PATCH 07/28] ext2: return error when fail to allocating memory in ioctl Currently, we do not check memory allocation result for ei->i_block_alloc_info in ioctl, this patch checks it and returns error in failure case. Link: https://lore.kernel.org/r/20191023135643.28837-1-cgxu519@mykernel.net Signed-off-by: Chengguang Xu Signed-off-by: Jan Kara --- fs/ext2/ioctl.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/fs/ext2/ioctl.c b/fs/ext2/ioctl.c index 1b853fb0b163..32a8d10b579d 100644 --- a/fs/ext2/ioctl.c +++ b/fs/ext2/ioctl.c @@ -145,10 +145,13 @@ setversion_out: if (ei->i_block_alloc_info){ struct ext2_reserve_window_node *rsv = &ei->i_block_alloc_info->rsv_window_node; rsv->rsv_goal_size = rsv_window_size; + } else { + ret = -ENOMEM; } + mutex_unlock(&ei->truncate_mutex); mnt_drop_write_file(filp); - return 0; + return ret; } default: return -ENOTTY; From 60e4cf67a582d64f07713eda5fcc8ccdaf7833e6 Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Thu, 24 Oct 2019 10:31:27 -0400 Subject: [PATCH 08/28] reiserfs: fix extended attributes on the root directory Since commit d0a5b995a308 (vfs: Add IOP_XATTR inode operations flag) extended attributes haven't worked on the root directory in reiserfs. This is due to reiserfs conditionally setting the sb->s_xattrs handler array depending on whether it located or create the internal privroot directory. It necessarily does this after the root inode is already read in. The IOP_XATTR flag is set during inode initialization, so it never gets set on the root directory. This commit unconditionally assigns sb->s_xattrs and clears IOP_XATTR on internal inodes. The old return values due to the conditional assignment are handled via open_xa_root, which now returns EOPNOTSUPP as the VFS would have done. Link: https://lore.kernel.org/r/20191024143127.17509-1-jeffm@suse.com CC: stable@vger.kernel.org Fixes: d0a5b995a308 ("vfs: Add IOP_XATTR inode operations flag") Signed-off-by: Jeff Mahoney Signed-off-by: Jan Kara --- fs/reiserfs/inode.c | 12 ++++++++++-- fs/reiserfs/namei.c | 7 +++++-- fs/reiserfs/reiserfs.h | 2 ++ fs/reiserfs/super.c | 2 ++ fs/reiserfs/xattr.c | 19 ++++++++++++------- fs/reiserfs/xattr_acl.c | 4 +--- 6 files changed, 32 insertions(+), 14 deletions(-) diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c index 132ec4406ed0..6419e6dacc39 100644 --- a/fs/reiserfs/inode.c +++ b/fs/reiserfs/inode.c @@ -2097,6 +2097,15 @@ int reiserfs_new_inode(struct reiserfs_transaction_handle *th, goto out_inserted_sd; } + /* + * Mark it private if we're creating the privroot + * or something under it. + */ + if (IS_PRIVATE(dir) || dentry == REISERFS_SB(sb)->priv_root) { + inode->i_flags |= S_PRIVATE; + inode->i_opflags &= ~IOP_XATTR; + } + if (reiserfs_posixacl(inode->i_sb)) { reiserfs_write_unlock(inode->i_sb); retval = reiserfs_inherit_default_acl(th, dir, dentry, inode); @@ -2111,8 +2120,7 @@ int reiserfs_new_inode(struct reiserfs_transaction_handle *th, reiserfs_warning(inode->i_sb, "jdm-13090", "ACLs aren't enabled in the fs, " "but vfs thinks they are!"); - } else if (IS_PRIVATE(dir)) - inode->i_flags |= S_PRIVATE; + } if (security->name) { reiserfs_write_unlock(inode->i_sb); diff --git a/fs/reiserfs/namei.c b/fs/reiserfs/namei.c index 97f3fc4fdd79..959a066b7bb0 100644 --- a/fs/reiserfs/namei.c +++ b/fs/reiserfs/namei.c @@ -377,10 +377,13 @@ static struct dentry *reiserfs_lookup(struct inode *dir, struct dentry *dentry, /* * Propagate the private flag so we know we're - * in the priv tree + * in the priv tree. Also clear IOP_XATTR + * since we don't have xattrs on xattr files. */ - if (IS_PRIVATE(dir)) + if (IS_PRIVATE(dir)) { inode->i_flags |= S_PRIVATE; + inode->i_opflags &= ~IOP_XATTR; + } } reiserfs_write_unlock(dir->i_sb); if (retval == IO_ERROR) { diff --git a/fs/reiserfs/reiserfs.h b/fs/reiserfs/reiserfs.h index e5ca9ed79e54..726580114d55 100644 --- a/fs/reiserfs/reiserfs.h +++ b/fs/reiserfs/reiserfs.h @@ -1168,6 +1168,8 @@ static inline int bmap_would_wrap(unsigned bmap_nr) return bmap_nr > ((1LL << 16) - 1); } +extern const struct xattr_handler *reiserfs_xattr_handlers[]; + /* * this says about version of key of all items (but stat data) the * object consists of diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c index d69b4ac0ae2f..3244037b1286 100644 --- a/fs/reiserfs/super.c +++ b/fs/reiserfs/super.c @@ -2049,6 +2049,8 @@ static int reiserfs_fill_super(struct super_block *s, void *data, int silent) if (replay_only(s)) goto error_unlocked; + s->s_xattr = reiserfs_xattr_handlers; + if (bdev_read_only(s->s_bdev) && !sb_rdonly(s)) { SWARN(silent, s, "clm-7000", "Detected readonly device, marking FS readonly"); diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c index b5b26d8a192c..62b40df36c98 100644 --- a/fs/reiserfs/xattr.c +++ b/fs/reiserfs/xattr.c @@ -122,13 +122,13 @@ static struct dentry *open_xa_root(struct super_block *sb, int flags) struct dentry *xaroot; if (d_really_is_negative(privroot)) - return ERR_PTR(-ENODATA); + return ERR_PTR(-EOPNOTSUPP); inode_lock_nested(d_inode(privroot), I_MUTEX_XATTR); xaroot = dget(REISERFS_SB(sb)->xattr_root); if (!xaroot) - xaroot = ERR_PTR(-ENODATA); + xaroot = ERR_PTR(-EOPNOTSUPP); else if (d_really_is_negative(xaroot)) { int err = -ENODATA; @@ -619,6 +619,10 @@ int reiserfs_xattr_set(struct inode *inode, const char *name, int error, error2; size_t jbegin_count = reiserfs_xattr_nblocks(inode, buffer_size); + /* Check before we start a transaction and then do nothing. */ + if (!d_really_is_positive(REISERFS_SB(inode->i_sb)->priv_root)) + return -EOPNOTSUPP; + if (!(flags & XATTR_REPLACE)) jbegin_count += reiserfs_xattr_jcreate_nblocks(inode); @@ -841,8 +845,7 @@ ssize_t reiserfs_listxattr(struct dentry * dentry, char *buffer, size_t size) if (d_really_is_negative(dentry)) return -EINVAL; - if (!dentry->d_sb->s_xattr || - get_inode_sd_version(d_inode(dentry)) == STAT_DATA_V1) + if (get_inode_sd_version(d_inode(dentry)) == STAT_DATA_V1) return -EOPNOTSUPP; dir = open_xa_dir(d_inode(dentry), XATTR_REPLACE); @@ -882,6 +885,7 @@ static int create_privroot(struct dentry *dentry) } d_inode(dentry)->i_flags |= S_PRIVATE; + d_inode(dentry)->i_opflags &= ~IOP_XATTR; reiserfs_info(dentry->d_sb, "Created %s - reserved for xattr " "storage.\n", PRIVROOT_NAME); @@ -895,7 +899,7 @@ static int create_privroot(struct dentry *dentry) { return 0; } #endif /* Actual operations that are exported to VFS-land */ -static const struct xattr_handler *reiserfs_xattr_handlers[] = { +const struct xattr_handler *reiserfs_xattr_handlers[] = { #ifdef CONFIG_REISERFS_FS_XATTR &reiserfs_xattr_user_handler, &reiserfs_xattr_trusted_handler, @@ -966,8 +970,10 @@ int reiserfs_lookup_privroot(struct super_block *s) if (!IS_ERR(dentry)) { REISERFS_SB(s)->priv_root = dentry; d_set_d_op(dentry, &xattr_lookup_poison_ops); - if (d_really_is_positive(dentry)) + if (d_really_is_positive(dentry)) { d_inode(dentry)->i_flags |= S_PRIVATE; + d_inode(dentry)->i_opflags &= ~IOP_XATTR; + } } else err = PTR_ERR(dentry); inode_unlock(d_inode(s->s_root)); @@ -996,7 +1002,6 @@ int reiserfs_xattr_init(struct super_block *s, int mount_flags) } if (d_really_is_positive(privroot)) { - s->s_xattr = reiserfs_xattr_handlers; inode_lock(d_inode(privroot)); if (!REISERFS_SB(s)->xattr_root) { struct dentry *dentry; diff --git a/fs/reiserfs/xattr_acl.c b/fs/reiserfs/xattr_acl.c index aa9380bac196..05f666794561 100644 --- a/fs/reiserfs/xattr_acl.c +++ b/fs/reiserfs/xattr_acl.c @@ -320,10 +320,8 @@ reiserfs_inherit_default_acl(struct reiserfs_transaction_handle *th, * would be useless since permissions are ignored, and a pain because * it introduces locking cycles */ - if (IS_PRIVATE(dir)) { - inode->i_flags |= S_PRIVATE; + if (IS_PRIVATE(inode)) goto apply_umask; - } err = posix_acl_create(dir, &inode->i_mode, &default_acl, &acl); if (err) From 30ef0e4085070312101be26c015f36277f02e12b Mon Sep 17 00:00:00 2001 From: Chengguang Xu Date: Sat, 26 Oct 2019 17:07:21 +0800 Subject: [PATCH 09/28] ext2: don't set *count in the case of failure in ext2_try_to_allocate() Currently we set *count to num(value 0) in the failure of block allocation in ext2_try_to_allocate(). Without reservation, we reuse *count(value 0) to retry block allocation and wrong *count will cause only allocating maximum 1 block even though having sufficent free blocks in that block group. Finally, it probably cause significant fragmentation. Link: https://lore.kernel.org/r/20191026090721.23794-1-cgxu519@mykernel.net Signed-off-by: Chengguang Xu Signed-off-by: Jan Kara --- fs/ext2/balloc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c index 4664d324567c..19bce75d207b 100644 --- a/fs/ext2/balloc.c +++ b/fs/ext2/balloc.c @@ -736,7 +736,6 @@ repeat: *count = num; return grp_goal - num; fail_access: - *count = num; return -1; } From 6ff33d99fc5c96797103b48b7b0902c296f09c05 Mon Sep 17 00:00:00 2001 From: Dmitry Monakhov Date: Thu, 31 Oct 2019 10:39:19 +0000 Subject: [PATCH 10/28] quota: fix livelock in dquot_writeback_dquots Write only quotas which are dirty at entry. XFSTEST: https://github.com/dmonakhov/xfstests/commit/b10ad23566a5bf75832a6f500e1236084083cddc Link: https://lore.kernel.org/r/20191031103920.3919-1-dmonakhov@openvz.org CC: stable@vger.kernel.org Signed-off-by: Konstantin Khlebnikov Signed-off-by: Dmitry Monakhov Signed-off-by: Jan Kara --- fs/quota/dquot.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index 26812a67ed77..b492b9ea829a 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -622,7 +622,7 @@ EXPORT_SYMBOL(dquot_scan_active); /* Write all dquot structures to quota files */ int dquot_writeback_dquots(struct super_block *sb, int type) { - struct list_head *dirty; + struct list_head dirty; struct dquot *dquot; struct quota_info *dqopt = sb_dqopt(sb); int cnt; @@ -636,9 +636,10 @@ int dquot_writeback_dquots(struct super_block *sb, int type) if (!sb_has_quota_active(sb, cnt)) continue; spin_lock(&dq_list_lock); - dirty = &dqopt->info[cnt].dqi_dirty_list; - while (!list_empty(dirty)) { - dquot = list_first_entry(dirty, struct dquot, + /* Move list away to avoid livelock. */ + list_replace_init(&dqopt->info[cnt].dqi_dirty_list, &dirty); + while (!list_empty(&dirty)) { + dquot = list_first_entry(&dirty, struct dquot, dq_dirty); WARN_ON(!test_bit(DQ_ACTIVE_B, &dquot->dq_flags)); From df4bb5d128e2c44848aeb36b7ceceba3ac85080d Mon Sep 17 00:00:00 2001 From: Dmitry Monakhov Date: Thu, 31 Oct 2019 10:39:20 +0000 Subject: [PATCH 11/28] quota: Check that quota is not dirty before release There is a race window where quota was redirted once we drop dq_list_lock inside dqput(), but before we grab dquot->dq_lock inside dquot_release() TASK1 TASK2 (chowner) ->dqput() we_slept: spin_lock(&dq_list_lock) if (dquot_dirty(dquot)) { spin_unlock(&dq_list_lock); dquot->dq_sb->dq_op->write_dquot(dquot); goto we_slept if (test_bit(DQ_ACTIVE_B, &dquot->dq_flags)) { spin_unlock(&dq_list_lock); dquot->dq_sb->dq_op->release_dquot(dquot); dqget() mark_dquot_dirty() dqput() goto we_slept; } So dquot dirty quota will be released by TASK1, but on next we_sleept loop we detect this and call ->write_dquot() for it. XFSTEST: https://github.com/dmonakhov/xfstests/commit/440a80d4cbb39e9234df4d7240aee1d551c36107 Link: https://lore.kernel.org/r/20191031103920.3919-2-dmonakhov@openvz.org CC: stable@vger.kernel.org Signed-off-by: Dmitry Monakhov Signed-off-by: Jan Kara --- fs/ocfs2/quota_global.c | 2 +- fs/quota/dquot.c | 2 +- include/linux/quotaops.h | 10 ++++++++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/fs/ocfs2/quota_global.c b/fs/ocfs2/quota_global.c index 7a922190a8c7..eda83487c9ec 100644 --- a/fs/ocfs2/quota_global.c +++ b/fs/ocfs2/quota_global.c @@ -728,7 +728,7 @@ static int ocfs2_release_dquot(struct dquot *dquot) mutex_lock(&dquot->dq_lock); /* Check whether we are not racing with some other dqget() */ - if (atomic_read(&dquot->dq_count) > 1) + if (dquot_is_busy(dquot)) goto out; /* Running from downconvert thread? Postpone quota processing to wq */ if (current == osb->dc_task) { diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index b492b9ea829a..72d24a5451bb 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -497,7 +497,7 @@ int dquot_release(struct dquot *dquot) mutex_lock(&dquot->dq_lock); /* Check whether we are not racing with some other dqget() */ - if (atomic_read(&dquot->dq_count) > 1) + if (dquot_is_busy(dquot)) goto out_dqlock; if (dqopt->ops[dquot->dq_id.type]->release_dqblk) { ret = dqopt->ops[dquot->dq_id.type]->release_dqblk(dquot); diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h index 185d94829701..91e0b7624053 100644 --- a/include/linux/quotaops.h +++ b/include/linux/quotaops.h @@ -54,6 +54,16 @@ static inline struct dquot *dqgrab(struct dquot *dquot) atomic_inc(&dquot->dq_count); return dquot; } + +static inline bool dquot_is_busy(struct dquot *dquot) +{ + if (test_bit(DQ_MOD_B, &dquot->dq_flags)) + return true; + if (atomic_read(&dquot->dq_count) > 1) + return true; + return false; +} + void dqput(struct dquot *dquot); int dquot_scan_active(struct super_block *sb, int (*fn)(struct dquot *dquot, unsigned long priv), From c7d3d28360fdb3ed3a5aa0bab19315e0fdc994a1 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Fri, 1 Nov 2019 17:45:31 +0100 Subject: [PATCH 12/28] quota: Factor out setup of quota inode Factor out setting up of quota inode and eventual error cleanup from vfs_load_quota_inode(). This will simplify situation for filesystems that don't have any quota inodes. Signed-off-by: Jan Kara --- fs/quota/dquot.c | 108 ++++++++++++++++++++++++--------------- include/linux/quotaops.h | 2 + 2 files changed, 69 insertions(+), 41 deletions(-) diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index 6e826b454082..9e8eb6e71675 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -2299,28 +2299,60 @@ EXPORT_SYMBOL(dquot_quota_off); * Turn quotas on on a device */ -/* - * Helper function to turn quotas on when we already have the inode of - * quota file and no quota information is loaded. - */ -static int vfs_load_quota_inode(struct inode *inode, int type, int format_id, +static int vfs_setup_quota_inode(struct inode *inode, int type) +{ + struct super_block *sb = inode->i_sb; + struct quota_info *dqopt = sb_dqopt(sb); + + if (!S_ISREG(inode->i_mode)) + return -EACCES; + if (IS_RDONLY(inode)) + return -EROFS; + if (sb_has_quota_loaded(sb, type)) + return -EBUSY; + + dqopt->files[type] = igrab(inode); + if (!dqopt->files[type]) + return -EIO; + if (!(dqopt->flags & DQUOT_QUOTA_SYS_FILE)) { + /* We don't want quota and atime on quota files (deadlocks + * possible) Also nobody should write to the file - we use + * special IO operations which ignore the immutable bit. */ + inode_lock(inode); + inode->i_flags |= S_NOQUOTA; + inode_unlock(inode); + /* + * When S_NOQUOTA is set, remove dquot references as no more + * references can be added + */ + __dquot_drop(inode); + } + return 0; +} + +static void vfs_cleanup_quota_inode(struct super_block *sb, int type) +{ + struct quota_info *dqopt = sb_dqopt(sb); + struct inode *inode = dqopt->files[type]; + + if (!(dqopt->flags & DQUOT_QUOTA_SYS_FILE)) { + inode_lock(inode); + inode->i_flags &= ~S_NOQUOTA; + inode_unlock(inode); + } + dqopt->files[type] = NULL; + iput(inode); +} + +int dquot_load_quota_sb(struct super_block *sb, int type, int format_id, unsigned int flags) { struct quota_format_type *fmt = find_quota_format(format_id); - struct super_block *sb = inode->i_sb; struct quota_info *dqopt = sb_dqopt(sb); int error; if (!fmt) return -ESRCH; - if (!S_ISREG(inode->i_mode)) { - error = -EACCES; - goto out_fmt; - } - if (IS_RDONLY(inode)) { - error = -EROFS; - goto out_fmt; - } if (!sb->s_op->quota_write || !sb->s_op->quota_read || (type == PRJQUOTA && sb->dq_op->get_projid == NULL)) { error = -EINVAL; @@ -2352,27 +2384,9 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id, invalidate_bdev(sb->s_bdev); } - if (!(dqopt->flags & DQUOT_QUOTA_SYS_FILE)) { - /* We don't want quota and atime on quota files (deadlocks - * possible) Also nobody should write to the file - we use - * special IO operations which ignore the immutable bit. */ - inode_lock(inode); - inode->i_flags |= S_NOQUOTA; - inode_unlock(inode); - /* - * When S_NOQUOTA is set, remove dquot references as no more - * references can be added - */ - __dquot_drop(inode); - } - - error = -EIO; - dqopt->files[type] = igrab(inode); - if (!dqopt->files[type]) - goto out_file_flags; error = -EINVAL; if (!fmt->qf_ops->check_quota_file(sb, type)) - goto out_file_init; + goto out_fmt; dqopt->ops[type] = fmt->qf_ops; dqopt->info[type].dqi_format = fmt; @@ -2380,7 +2394,7 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id, INIT_LIST_HEAD(&dqopt->info[type].dqi_dirty_list); error = dqopt->ops[type]->read_file_info(sb, type); if (error < 0) - goto out_file_init; + goto out_fmt; if (dqopt->flags & DQUOT_QUOTA_SYS_FILE) { spin_lock(&dq_data_lock); dqopt->info[type].dqi_flags |= DQF_SYS_FILE; @@ -2395,18 +2409,30 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id, dquot_disable(sb, type, flags); return error; -out_file_init: - dqopt->files[type] = NULL; - iput(inode); -out_file_flags: - inode_lock(inode); - inode->i_flags &= ~S_NOQUOTA; - inode_unlock(inode); out_fmt: put_quota_format(fmt); return error; } +EXPORT_SYMBOL(dquot_load_quota_sb); + +/* + * Helper function to turn quotas on when we already have the inode of + * quota file and no quota information is loaded. + */ +static int vfs_load_quota_inode(struct inode *inode, int type, int format_id, + unsigned int flags) +{ + int err; + + err = vfs_setup_quota_inode(inode, type); + if (err < 0) + return err; + err = dquot_load_quota_sb(inode->i_sb, type, format_id, flags); + if (err < 0) + vfs_cleanup_quota_inode(inode->i_sb, type); + return err; +} /* Reenable quotas on remount RW */ int dquot_resume(struct super_block *sb, int type) diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h index 185d94829701..2625766bcfe7 100644 --- a/include/linux/quotaops.h +++ b/include/linux/quotaops.h @@ -89,6 +89,8 @@ int dquot_file_open(struct inode *inode, struct file *file); int dquot_enable(struct inode *inode, int type, int format_id, unsigned int flags); +int dquot_load_quota_sb(struct super_block *sb, int type, int format_id, + unsigned int flags); int dquot_quota_on(struct super_block *sb, int type, int format_id, const struct path *path); int dquot_quota_on_mount(struct super_block *sb, char *qf_name, From ae45f07d47cc30e9170488a4e5fe91ba4fe5ed4e Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Fri, 1 Nov 2019 17:51:05 +0100 Subject: [PATCH 13/28] quota: Simplify dquot_resume() We already have quota inode loaded when resuming quotas. Use vfs_load_quota() to avoid some pointless churn with the quota inode. Signed-off-by: Jan Kara --- fs/quota/dquot.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index 9e8eb6e71675..ecdae91029ed 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -2438,7 +2438,6 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id, int dquot_resume(struct super_block *sb, int type) { struct quota_info *dqopt = sb_dqopt(sb); - struct inode *inode; int ret = 0, cnt; unsigned int flags; @@ -2452,8 +2451,6 @@ int dquot_resume(struct super_block *sb, int type) if (!sb_has_quota_suspended(sb, cnt)) continue; - inode = dqopt->files[cnt]; - dqopt->files[cnt] = NULL; spin_lock(&dq_state_lock); flags = dqopt->flags & dquot_state_flag(DQUOT_USAGE_ENABLED | DQUOT_LIMITS_ENABLED, @@ -2462,9 +2459,10 @@ int dquot_resume(struct super_block *sb, int type) spin_unlock(&dq_state_lock); flags = dquot_generic_flag(flags, cnt); - ret = vfs_load_quota_inode(inode, cnt, - dqopt->info[cnt].dqi_fmt_id, flags); - iput(inode); + ret = dquot_load_quota_sb(sb, cnt, dqopt->info[cnt].dqi_fmt_id, + flags); + if (ret < 0) + vfs_cleanup_quota_inode(sb, type); } return ret; From dc19432ae1c22d696f91edea11ae06c348b4e88a Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Fri, 1 Nov 2019 18:37:44 +0100 Subject: [PATCH 14/28] quota: Rename vfs_load_quota_inode() to dquot_load_quota_inode() Rename vfs_load_quota_inode() to dquot_load_quota_inode() to be consistent with naming of other functions used for enabling quota accounting from filesystems. Also export the function and add some sanity checks to assure filesystems are calling the function properly. Signed-off-by: Jan Kara --- fs/quota/dquot.c | 19 +++++++++++++------ include/linux/quotaops.h | 2 ++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index ecdae91029ed..0ddcbce596f8 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -2351,6 +2351,12 @@ int dquot_load_quota_sb(struct super_block *sb, int type, int format_id, struct quota_info *dqopt = sb_dqopt(sb); int error; + /* Just unsuspend quotas? */ + BUG_ON(flags & DQUOT_SUSPENDED); + /* s_umount should be held in exclusive mode */ + if (WARN_ON_ONCE(down_read_trylock(&sb->s_umount))) + up_read(&sb->s_umount); + if (!fmt) return -ESRCH; if (!sb->s_op->quota_write || !sb->s_op->quota_read || @@ -2417,10 +2423,10 @@ out_fmt: EXPORT_SYMBOL(dquot_load_quota_sb); /* - * Helper function to turn quotas on when we already have the inode of - * quota file and no quota information is loaded. + * More powerful function for turning on quotas on given quota inode allowing + * setting of individual quota flags */ -static int vfs_load_quota_inode(struct inode *inode, int type, int format_id, +int dquot_load_quota_inode(struct inode *inode, int type, int format_id, unsigned int flags) { int err; @@ -2433,6 +2439,7 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id, vfs_cleanup_quota_inode(inode->i_sb, type); return err; } +EXPORT_SYMBOL(dquot_load_quota_inode); /* Reenable quotas on remount RW */ int dquot_resume(struct super_block *sb, int type) @@ -2479,7 +2486,7 @@ int dquot_quota_on(struct super_block *sb, int type, int format_id, if (path->dentry->d_sb != sb) error = -EXDEV; else - error = vfs_load_quota_inode(d_inode(path->dentry), type, + error = dquot_load_quota_inode(d_inode(path->dentry), type, format_id, DQUOT_USAGE_ENABLED | DQUOT_LIMITS_ENABLED); return error; @@ -2517,7 +2524,7 @@ int dquot_enable(struct inode *inode, int type, int format_id, return 0; } - return vfs_load_quota_inode(inode, type, format_id, flags); + return dquot_load_quota_inode(inode, type, format_id, flags); } EXPORT_SYMBOL(dquot_enable); @@ -2542,7 +2549,7 @@ int dquot_quota_on_mount(struct super_block *sb, char *qf_name, error = security_quota_on(dentry); if (!error) - error = vfs_load_quota_inode(d_inode(dentry), type, format_id, + error = dquot_load_quota_inode(d_inode(dentry), type, format_id, DQUOT_USAGE_ENABLED | DQUOT_LIMITS_ENABLED); out: diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h index 2625766bcfe7..0ce9da5a1a93 100644 --- a/include/linux/quotaops.h +++ b/include/linux/quotaops.h @@ -91,6 +91,8 @@ int dquot_enable(struct inode *inode, int type, int format_id, unsigned int flags); int dquot_load_quota_sb(struct super_block *sb, int type, int format_id, unsigned int flags); +int dquot_load_quota_inode(struct inode *inode, int type, int format_id, + unsigned int flags); int dquot_quota_on(struct super_block *sb, int type, int format_id, const struct path *path); int dquot_quota_on_mount(struct super_block *sb, char *qf_name, From 7212b95e61516671672380c6c9d6d80f4f306198 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Fri, 1 Nov 2019 18:55:38 +0100 Subject: [PATCH 15/28] fs: Use dquot_load_quota_inode() from filesystems Use dquot_load_quota_inode from filesystems instead of dquot_enable(). In all three cases we want to load quota inode and never use the function to update quota flags. Signed-off-by: Jan Kara --- fs/ext4/super.c | 2 +- fs/f2fs/super.c | 2 +- fs/ocfs2/super.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/fs/ext4/super.c b/fs/ext4/super.c index dd654e53ba3d..1b947c95eff2 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -5835,7 +5835,7 @@ static int ext4_quota_enable(struct super_block *sb, int type, int format_id, /* Don't account quota for quota files to avoid recursion */ qf_inode->i_flags |= S_NOQUOTA; lockdep_set_quota_inode(qf_inode, I_DATA_SEM_QUOTA); - err = dquot_enable(qf_inode, type, format_id, flags); + err = dquot_load_quota_inode(qf_inode, type, format_id, flags); if (err) lockdep_set_quota_inode(qf_inode, I_DATA_SEM_NORMAL); iput(qf_inode); diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 1443cee15863..91745d5b718d 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -1932,7 +1932,7 @@ static int f2fs_quota_enable(struct super_block *sb, int type, int format_id, /* Don't account quota for quota files to avoid recursion */ qf_inode->i_flags |= S_NOQUOTA; - err = dquot_enable(qf_inode, type, format_id, flags); + err = dquot_load_quota_inode(qf_inode, type, format_id, flags); iput(qf_inode); return err; } diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c index c81e86c62380..05dd68ade293 100644 --- a/fs/ocfs2/super.c +++ b/fs/ocfs2/super.c @@ -926,8 +926,8 @@ static int ocfs2_enable_quotas(struct ocfs2_super *osb) status = -ENOENT; goto out_quota_off; } - status = dquot_enable(inode[type], type, QFMT_OCFS2, - DQUOT_USAGE_ENABLED); + status = dquot_load_quota_inode(inode[type], type, QFMT_OCFS2, + DQUOT_USAGE_ENABLED); if (status < 0) goto out_quota_off; } From 069a9166369773627e51c5249cd7f9169aecd7fa Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Fri, 1 Nov 2019 18:57:56 +0100 Subject: [PATCH 16/28] quota: Drop dquot_enable() Now dquot_enable() has only two internal callers and both of them just need to update quota flags and don't need most of checks. Just drop dquot_enable() and fold necessary functionality into the two calling places. Signed-off-by: Jan Kara --- fs/quota/dquot.c | 61 +++++++++++----------------------------- include/linux/quotaops.h | 2 -- 2 files changed, 16 insertions(+), 47 deletions(-) diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index 0ddcbce596f8..3e4cf0d10955 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -2493,41 +2493,6 @@ int dquot_quota_on(struct super_block *sb, int type, int format_id, } EXPORT_SYMBOL(dquot_quota_on); -/* - * More powerful function for turning on quotas allowing setting - * of individual quota flags - */ -int dquot_enable(struct inode *inode, int type, int format_id, - unsigned int flags) -{ - struct super_block *sb = inode->i_sb; - - /* Just unsuspend quotas? */ - BUG_ON(flags & DQUOT_SUSPENDED); - /* s_umount should be held in exclusive mode */ - if (WARN_ON_ONCE(down_read_trylock(&sb->s_umount))) - up_read(&sb->s_umount); - - if (!flags) - return 0; - /* Just updating flags needed? */ - if (sb_has_quota_loaded(sb, type)) { - if (flags & DQUOT_USAGE_ENABLED && - sb_has_quota_usage_enabled(sb, type)) - return -EBUSY; - if (flags & DQUOT_LIMITS_ENABLED && - sb_has_quota_limits_enabled(sb, type)) - return -EBUSY; - spin_lock(&dq_state_lock); - sb_dqopt(sb)->flags |= dquot_state_flag(flags, type); - spin_unlock(&dq_state_lock); - return 0; - } - - return dquot_load_quota_inode(inode, type, format_id, flags); -} -EXPORT_SYMBOL(dquot_enable); - /* * This function is used when filesystem needs to initialize quotas * during mount time. @@ -2574,13 +2539,17 @@ static int dquot_quota_enable(struct super_block *sb, unsigned int flags) if (!(flags & qtype_enforce_flag(type))) continue; /* Can't enforce without accounting */ - if (!sb_has_quota_usage_enabled(sb, type)) - return -EINVAL; - ret = dquot_enable(dqopt->files[type], type, - dqopt->info[type].dqi_fmt_id, - DQUOT_LIMITS_ENABLED); - if (ret < 0) + if (!sb_has_quota_usage_enabled(sb, type)) { + ret = -EINVAL; goto out_err; + } + if (sb_has_quota_limits_enabled(sb, type)) { + ret = -EBUSY; + goto out_err; + } + spin_lock(&dq_state_lock); + dqopt->flags |= dquot_state_flag(DQUOT_LIMITS_ENABLED, type); + spin_unlock(&dq_state_lock); } return 0; out_err: @@ -2630,10 +2599,12 @@ static int dquot_quota_disable(struct super_block *sb, unsigned int flags) out_err: /* Backout enforcement disabling we already did */ for (type--; type >= 0; type--) { - if (flags & qtype_enforce_flag(type)) - dquot_enable(dqopt->files[type], type, - dqopt->info[type].dqi_fmt_id, - DQUOT_LIMITS_ENABLED); + if (flags & qtype_enforce_flag(type)) { + spin_lock(&dq_state_lock); + dqopt->flags |= + dquot_state_flag(DQUOT_LIMITS_ENABLED, type); + spin_unlock(&dq_state_lock); + } } return ret; } diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h index 0ce9da5a1a93..6b8ebc8d715e 100644 --- a/include/linux/quotaops.h +++ b/include/linux/quotaops.h @@ -87,8 +87,6 @@ int dquot_mark_dquot_dirty(struct dquot *dquot); int dquot_file_open(struct inode *inode, struct file *file); -int dquot_enable(struct inode *inode, int type, int format_id, - unsigned int flags); int dquot_load_quota_sb(struct super_block *sb, int type, int format_id, unsigned int flags); int dquot_load_quota_inode(struct inode *inode, int type, int format_id, From 2ec1f3011f3fdcbafcaad23aafa5acb6861a2646 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Mon, 4 Nov 2019 11:12:44 +0100 Subject: [PATCH 17/28] quota: Make dquot_disable() work without quota inodes Quota on and quota off are protected by s_umount semaphore held in exclusive mode since commit 7d6cd73d33b6 "quota: Hold s_umount in exclusive mode when enabling / disabling quotas". This makes it impossible for dquot_disable() to race with other enabling or disabling of quotas. Simplify the cleanup done by dquot_disable() based on this fact and also remove some stale comments. As a bonus this cleanup makes dquot_disable() properly handle a case when there are no quota inodes. Signed-off-by: Jan Kara --- fs/quota/dquot.c | 73 +++++++++++++++++++----------------------------- 1 file changed, 29 insertions(+), 44 deletions(-) diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index 3e4cf0d10955..4c3da4ea31bc 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -2162,14 +2162,29 @@ int dquot_file_open(struct inode *inode, struct file *file) } EXPORT_SYMBOL(dquot_file_open); +static void vfs_cleanup_quota_inode(struct super_block *sb, int type) +{ + struct quota_info *dqopt = sb_dqopt(sb); + struct inode *inode = dqopt->files[type]; + + if (!inode) + return; + if (!(dqopt->flags & DQUOT_QUOTA_SYS_FILE)) { + inode_lock(inode); + inode->i_flags &= ~S_NOQUOTA; + inode_unlock(inode); + } + dqopt->files[type] = NULL; + iput(inode); +} + /* * Turn quota off on a device. type == -1 ==> quotaoff for all types (umount) */ int dquot_disable(struct super_block *sb, int type, unsigned int flags) { - int cnt, ret = 0; + int cnt; struct quota_info *dqopt = sb_dqopt(sb); - struct inode *toputinode[MAXQUOTAS]; /* s_umount should be held in exclusive mode */ if (WARN_ON_ONCE(down_read_trylock(&sb->s_umount))) @@ -2191,7 +2206,6 @@ int dquot_disable(struct super_block *sb, int type, unsigned int flags) return 0; for (cnt = 0; cnt < MAXQUOTAS; cnt++) { - toputinode[cnt] = NULL; if (type != -1 && cnt != type) continue; if (!sb_has_quota_loaded(sb, cnt)) @@ -2211,8 +2225,7 @@ int dquot_disable(struct super_block *sb, int type, unsigned int flags) dqopt->flags &= ~dquot_state_flag( DQUOT_SUSPENDED, cnt); spin_unlock(&dq_state_lock); - iput(dqopt->files[cnt]); - dqopt->files[cnt] = NULL; + vfs_cleanup_quota_inode(sb, cnt); continue; } spin_unlock(&dq_state_lock); @@ -2234,10 +2247,6 @@ int dquot_disable(struct super_block *sb, int type, unsigned int flags) if (dqopt->ops[cnt]->free_file_info) dqopt->ops[cnt]->free_file_info(sb, cnt); put_quota_format(dqopt->info[cnt].dqi_format); - - toputinode[cnt] = dqopt->files[cnt]; - if (!sb_has_quota_loaded(sb, cnt)) - dqopt->files[cnt] = NULL; dqopt->info[cnt].dqi_flags = 0; dqopt->info[cnt].dqi_igrace = 0; dqopt->info[cnt].dqi_bgrace = 0; @@ -2259,32 +2268,22 @@ int dquot_disable(struct super_block *sb, int type, unsigned int flags) * must also discard the blockdev buffers so that we see the * changes done by userspace on the next quotaon() */ for (cnt = 0; cnt < MAXQUOTAS; cnt++) - /* This can happen when suspending quotas on remount-ro... */ - if (toputinode[cnt] && !sb_has_quota_loaded(sb, cnt)) { - inode_lock(toputinode[cnt]); - toputinode[cnt]->i_flags &= ~S_NOQUOTA; - truncate_inode_pages(&toputinode[cnt]->i_data, 0); - inode_unlock(toputinode[cnt]); - mark_inode_dirty_sync(toputinode[cnt]); + if (!sb_has_quota_loaded(sb, cnt) && dqopt->files[cnt]) { + inode_lock(dqopt->files[cnt]); + truncate_inode_pages(&dqopt->files[cnt]->i_data, 0); + inode_unlock(dqopt->files[cnt]); } if (sb->s_bdev) invalidate_bdev(sb->s_bdev); put_inodes: + /* We are done when suspending quotas */ + if (flags & DQUOT_SUSPENDED) + return 0; + for (cnt = 0; cnt < MAXQUOTAS; cnt++) - if (toputinode[cnt]) { - /* On remount RO, we keep the inode pointer so that we - * can reenable quota on the subsequent remount RW. We - * have to check 'flags' variable and not use sb_has_ - * function because another quotaon / quotaoff could - * change global state before we got here. We refuse - * to suspend quotas when there is pending delete on - * the quota file... */ - if (!(flags & DQUOT_SUSPENDED)) - iput(toputinode[cnt]); - else if (!toputinode[cnt]->i_nlink) - ret = -EBUSY; - } - return ret; + if (!sb_has_quota_loaded(sb, cnt)) + vfs_cleanup_quota_inode(sb, cnt); + return 0; } EXPORT_SYMBOL(dquot_disable); @@ -2330,20 +2329,6 @@ static int vfs_setup_quota_inode(struct inode *inode, int type) return 0; } -static void vfs_cleanup_quota_inode(struct super_block *sb, int type) -{ - struct quota_info *dqopt = sb_dqopt(sb); - struct inode *inode = dqopt->files[type]; - - if (!(dqopt->flags & DQUOT_QUOTA_SYS_FILE)) { - inode_lock(inode); - inode->i_flags &= ~S_NOQUOTA; - inode_unlock(inode); - } - dqopt->files[type] = NULL; - iput(inode); -} - int dquot_load_quota_sb(struct super_block *sb, int type, int format_id, unsigned int flags) { From a0828b6ccbdfd46afbbaa9f28df359081c29109b Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Mon, 4 Nov 2019 11:18:17 +0100 Subject: [PATCH 18/28] quota: Handle quotas without quota inodes in dquot_get_state() Make dquot_get_state() gracefully handle a situation when there are no quota files present even though quotas are enabled. Signed-off-by: Jan Kara --- fs/quota/dquot.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index 4c3da4ea31bc..a69a657209a6 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -2787,8 +2787,10 @@ int dquot_get_state(struct super_block *sb, struct qc_state *state) tstate->flags |= QCI_LIMITS_ENFORCED; tstate->spc_timelimit = mi->dqi_bgrace; tstate->ino_timelimit = mi->dqi_igrace; - tstate->ino = dqopt->files[type]->i_ino; - tstate->blocks = dqopt->files[type]->i_blocks; + if (dqopt->files[type]) { + tstate->ino = dqopt->files[type]->i_ino; + tstate->blocks = dqopt->files[type]->i_blocks; + } tstate->nextents = 1; /* We don't know... */ spin_unlock(&dq_data_lock); } From e705f4b8aa27a59f8933e8f384e9752f052c469c Mon Sep 17 00:00:00 2001 From: Chengguang Xu Date: Tue, 5 Nov 2019 12:51:00 +0800 Subject: [PATCH 19/28] ext2: check err when partial != NULL Check err when partial == NULL is meaningless because partial == NULL means getting branch successfully without error. CC: stable@vger.kernel.org Link: https://lore.kernel.org/r/20191105045100.7104-1-cgxu519@mykernel.net Signed-off-by: Chengguang Xu Signed-off-by: Jan Kara --- fs/ext2/inode.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c index 7004ce581a32..a16c53655e77 100644 --- a/fs/ext2/inode.c +++ b/fs/ext2/inode.c @@ -701,10 +701,13 @@ static int ext2_get_blocks(struct inode *inode, if (!partial) { count++; mutex_unlock(&ei->truncate_mutex); - if (err) - goto cleanup; goto got_it; } + + if (err) { + mutex_unlock(&ei->truncate_mutex); + goto cleanup; + } } /* From a9913d7eafa74eb2e34e26aa31fe80449b999f8e Mon Sep 17 00:00:00 2001 From: Nikitas Angelinas Date: Sun, 3 Nov 2019 01:44:54 -0800 Subject: [PATCH 20/28] reiserfs: replace open-coded atomic_dec_and_mutex_lock() Replace the open-coded logic of atomic_dec_and_mutex_lock() in reiserfs_file_release(). Link: https://lore.kernel.org/r/20191103094431.GA18576-nikitas.angelinas@gmail.com Signed-off-by: Nikitas Angelinas Signed-off-by: Jan Kara --- fs/reiserfs/file.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/fs/reiserfs/file.c b/fs/reiserfs/file.c index 843aadcc123c..84cf8bdbec9c 100644 --- a/fs/reiserfs/file.c +++ b/fs/reiserfs/file.c @@ -38,16 +38,10 @@ static int reiserfs_file_release(struct inode *inode, struct file *filp) BUG_ON(!S_ISREG(inode->i_mode)); - if (atomic_add_unless(&REISERFS_I(inode)->openers, -1, 1)) + if (!atomic_dec_and_mutex_lock(&REISERFS_I(inode)->openers, + &REISERFS_I(inode)->tailpack)) return 0; - mutex_lock(&REISERFS_I(inode)->tailpack); - - if (!atomic_dec_and_test(&REISERFS_I(inode)->openers)) { - mutex_unlock(&REISERFS_I(inode)->tailpack); - return 0; - } - /* fast out for when nothing needs to be done */ if ((!(REISERFS_I(inode)->i_flags & i_pack_on_close_mask) || !tail_has_to_be_packed(inode)) && From eb9e47fc230aafc3ac1e5eeae6d0d8d5e108bbae Mon Sep 17 00:00:00 2001 From: Chengguang Xu Date: Mon, 4 Nov 2019 19:40:32 +0800 Subject: [PATCH 21/28] ext2: introduce new helper ext2_group_last_block_no() Introduce new helper ext2_group_last_block_no() to calculate last block num for specific block group, we can replace open coded logic by calling this common helper. Link: https://lore.kernel.org/r/20191104114036.9893-1-cgxu519@mykernel.net Signed-off-by: Chengguang Xu Signed-off-by: Jan Kara --- fs/ext2/ext2.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h index 10ab238de9a6..8178bd38a9d6 100644 --- a/fs/ext2/ext2.h +++ b/fs/ext2/ext2.h @@ -813,6 +813,18 @@ ext2_group_first_block_no(struct super_block *sb, unsigned long group_no) le32_to_cpu(EXT2_SB(sb)->s_es->s_first_data_block); } +static inline ext2_fsblk_t +ext2_group_last_block_no(struct super_block *sb, unsigned long group_no) +{ + struct ext2_sb_info *sbi = EXT2_SB(sb); + + if (group_no == sbi->s_groups_count - 1) + return le32_to_cpu(sbi->s_es->s_blocks_count) - 1; + else + return ext2_group_first_block_no(sb, group_no) + + EXT2_BLOCKS_PER_GROUP(sb) - 1; +} + #define ext2_set_bit __test_and_set_bit_le #define ext2_clear_bit __test_and_clear_bit_le #define ext2_test_bit test_bit_le From 90f3741c2b567d3f16c13d574dbd5d42ed076b2e Mon Sep 17 00:00:00 2001 From: Chengguang Xu Date: Mon, 4 Nov 2019 19:40:33 +0800 Subject: [PATCH 22/28] ext2: code cleanup by calling ext2_group_last_block_no() Call common helper ext2_group_last_block_no() to calculate group last block number. Link: https://lore.kernel.org/r/20191104114036.9893-2-cgxu519@mykernel.net Signed-off-by: Chengguang Xu Signed-off-by: Jan Kara --- fs/ext2/balloc.c | 16 ++++++++-------- fs/ext2/super.c | 8 +------- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c index 19bce75d207b..994a1fd18e93 100644 --- a/fs/ext2/balloc.c +++ b/fs/ext2/balloc.c @@ -269,7 +269,7 @@ goal_in_my_reservation(struct ext2_reserve_window *rsv, ext2_grpblk_t grp_goal, ext2_fsblk_t group_first_block, group_last_block; group_first_block = ext2_group_first_block_no(sb, group); - group_last_block = group_first_block + EXT2_BLOCKS_PER_GROUP(sb) - 1; + group_last_block = ext2_group_last_block_no(sb, group); if ((rsv->_rsv_start > group_last_block) || (rsv->_rsv_end < group_first_block)) @@ -666,22 +666,22 @@ ext2_try_to_allocate(struct super_block *sb, int group, unsigned long *count, struct ext2_reserve_window *my_rsv) { - ext2_fsblk_t group_first_block; + ext2_fsblk_t group_first_block = ext2_group_first_block_no(sb, group); + ext2_fsblk_t group_last_block = ext2_group_last_block_no(sb, group); ext2_grpblk_t start, end; unsigned long num = 0; /* we do allocation within the reservation window if we have a window */ if (my_rsv) { - group_first_block = ext2_group_first_block_no(sb, group); if (my_rsv->_rsv_start >= group_first_block) start = my_rsv->_rsv_start - group_first_block; else /* reservation window cross group boundary */ start = 0; end = my_rsv->_rsv_end - group_first_block + 1; - if (end > EXT2_BLOCKS_PER_GROUP(sb)) + if (end > group_last_block - group_first_block + 1) /* reservation window crosses group boundary */ - end = EXT2_BLOCKS_PER_GROUP(sb); + end = group_last_block - group_first_block + 1; if ((start <= grp_goal) && (grp_goal < end)) start = grp_goal; else @@ -691,7 +691,7 @@ ext2_try_to_allocate(struct super_block *sb, int group, start = grp_goal; else start = 0; - end = EXT2_BLOCKS_PER_GROUP(sb); + end = group_last_block - group_first_block + 1; } BUG_ON(start > EXT2_BLOCKS_PER_GROUP(sb)); @@ -907,7 +907,7 @@ static int alloc_new_reservation(struct ext2_reserve_window_node *my_rsv, spinlock_t *rsv_lock = &EXT2_SB(sb)->s_rsv_window_lock; group_first_block = ext2_group_first_block_no(sb, group); - group_end_block = group_first_block + (EXT2_BLOCKS_PER_GROUP(sb) - 1); + group_end_block = ext2_group_last_block_no(sb, group); if (grp_goal < 0) start_block = group_first_block; @@ -1114,7 +1114,7 @@ ext2_try_to_allocate_with_rsv(struct super_block *sb, unsigned int group, * first block is the block number of the first block in this group */ group_first_block = ext2_group_first_block_no(sb, group); - group_last_block = group_first_block + (EXT2_BLOCKS_PER_GROUP(sb) - 1); + group_last_block = ext2_group_last_block_no(sb, group); /* * Basically we will allocate a new block from inode's reservation diff --git a/fs/ext2/super.c b/fs/ext2/super.c index 30c630d73f0f..4cd401a2f207 100644 --- a/fs/ext2/super.c +++ b/fs/ext2/super.c @@ -702,13 +702,7 @@ static int ext2_check_descriptors(struct super_block *sb) for (i = 0; i < sbi->s_groups_count; i++) { struct ext2_group_desc *gdp = ext2_get_group_desc(sb, i, NULL); ext2_fsblk_t first_block = ext2_group_first_block_no(sb, i); - ext2_fsblk_t last_block; - - if (i == sbi->s_groups_count - 1) - last_block = le32_to_cpu(sbi->s_es->s_blocks_count) - 1; - else - last_block = first_block + - (EXT2_BLOCKS_PER_GROUP(sb) - 1); + ext2_fsblk_t last_block = ext2_group_last_block_no(sb, i); if (le32_to_cpu(gdp->bg_block_bitmap) < first_block || le32_to_cpu(gdp->bg_block_bitmap) > last_block) From cf4eb321b3ad63248069a8038df126fcf9ed9100 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Wed, 6 Nov 2019 16:39:26 +0100 Subject: [PATCH 23/28] ext2: Simplify initialization in ext2_try_to_allocate() Somewhat simplify the logic initializing search start and end in ext2_try_to_allocate(). No functional change. Signed-off-by: Jan Kara --- fs/ext2/balloc.c | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c index 994a1fd18e93..b7a4d74dfde7 100644 --- a/fs/ext2/balloc.c +++ b/fs/ext2/balloc.c @@ -671,29 +671,17 @@ ext2_try_to_allocate(struct super_block *sb, int group, ext2_grpblk_t start, end; unsigned long num = 0; + start = 0; + end = group_last_block - group_first_block + 1; /* we do allocation within the reservation window if we have a window */ if (my_rsv) { if (my_rsv->_rsv_start >= group_first_block) start = my_rsv->_rsv_start - group_first_block; - else - /* reservation window cross group boundary */ - start = 0; - end = my_rsv->_rsv_end - group_first_block + 1; - if (end > group_last_block - group_first_block + 1) - /* reservation window crosses group boundary */ - end = group_last_block - group_first_block + 1; - if ((start <= grp_goal) && (grp_goal < end)) - start = grp_goal; - else + if (my_rsv->_rsv_end < group_last_block) + end = my_rsv->_rsv_end - group_first_block + 1; + if (grp_goal < start || grp_goal >= end) grp_goal = -1; - } else { - if (grp_goal > 0) - start = grp_goal; - else - start = 0; - end = group_last_block - group_first_block + 1; } - BUG_ON(start > EXT2_BLOCKS_PER_GROUP(sb)); repeat: From ac3820f8222b6563580605f427ff03474a2f003a Mon Sep 17 00:00:00 2001 From: Chengguang Xu Date: Mon, 4 Nov 2019 19:40:34 +0800 Subject: [PATCH 24/28] ext2: skip unnecessary operations in ext2_try_to_allocate() Move 'repeat' tag to proper place so that we can skip unnecessary operations in ext2_try_to_allocate(). Link: https://lore.kernel.org/r/20191104114036.9893-3-cgxu519@mykernel.net Signed-off-by: Chengguang Xu Signed-off-by: Jan Kara --- fs/ext2/balloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c index b7a4d74dfde7..f3ff05edaf8c 100644 --- a/fs/ext2/balloc.c +++ b/fs/ext2/balloc.c @@ -684,7 +684,6 @@ ext2_try_to_allocate(struct super_block *sb, int group, } BUG_ON(start > EXT2_BLOCKS_PER_GROUP(sb)); -repeat: if (grp_goal < 0) { grp_goal = find_next_usable_block(start, bitmap_bh, end); if (grp_goal < 0) @@ -701,6 +700,7 @@ repeat: } start = grp_goal; +repeat: if (ext2_set_bit_atomic(sb_bgl_lock(EXT2_SB(sb), group), grp_goal, bitmap_bh->b_data)) { /* From 44dd6161338ef528b5b011184639beadcf038b14 Mon Sep 17 00:00:00 2001 From: Chengguang Xu Date: Mon, 4 Nov 2019 19:40:35 +0800 Subject: [PATCH 25/28] ext2: code cleanup for ext2_try_to_allocate() Code cleanup by removing duplicated code. Link: https://lore.kernel.org/r/20191104114036.9893-4-cgxu519@mykernel.net Signed-off-by: Chengguang Xu Signed-off-by: Jan Kara --- fs/ext2/balloc.c | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c index f3ff05edaf8c..4beb416be9cf 100644 --- a/fs/ext2/balloc.c +++ b/fs/ext2/balloc.c @@ -698,29 +698,20 @@ ext2_try_to_allocate(struct super_block *sb, int group, ; } } - start = grp_goal; -repeat: - if (ext2_set_bit_atomic(sb_bgl_lock(EXT2_SB(sb), group), grp_goal, - bitmap_bh->b_data)) { - /* - * The block was allocated by another thread, or it was - * allocated and then freed by another thread - */ - start++; - grp_goal++; - if (start >= end) - goto fail_access; - goto repeat; - } - num++; - grp_goal++; - while (num < *count && grp_goal < end - && !ext2_set_bit_atomic(sb_bgl_lock(EXT2_SB(sb), group), + for (; num < *count && grp_goal < end; grp_goal++) { + if (ext2_set_bit_atomic(sb_bgl_lock(EXT2_SB(sb), group), grp_goal, bitmap_bh->b_data)) { + if (num == 0) + continue; + break; + } num++; - grp_goal++; } + + if (num == 0) + goto fail_access; + *count = num; return grp_goal - num; fail_access: From 355b9aae868512747c0ff90231e8ae8ea0278a60 Mon Sep 17 00:00:00 2001 From: Chengguang Xu Date: Mon, 4 Nov 2019 19:40:36 +0800 Subject: [PATCH 26/28] ext2: fix improper function comment Just fix a improper function comment. Link: https://lore.kernel.org/r/20191104114036.9893-5-cgxu519@mykernel.net Signed-off-by: Chengguang Xu Signed-off-by: Jan Kara --- fs/ext2/balloc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c index 4beb416be9cf..fa9c951d3471 100644 --- a/fs/ext2/balloc.c +++ b/fs/ext2/balloc.c @@ -732,10 +732,9 @@ fail_access: * but we will shift to the place where start_block is, * then start from there, when looking for a reservable space. * - * @size: the target new reservation window size + * @sb: the super block. * - * @group_first_block: the first block we consider to start - * the real search from + * @start_block: the first block we consider to start the real search from * * @last_block: * the maximum block number that our goal reservable space From 6fcbcec9cfc7b3c6a2c1f1a23ebacedff7073e0a Mon Sep 17 00:00:00 2001 From: Konstantin Khlebnikov Date: Sun, 10 Nov 2019 12:49:06 +0300 Subject: [PATCH 27/28] fs/quota: handle overflows of sysctl fs.quota.* and report as unsigned long Quota statistics counted as 64-bit per-cpu counter. Reading sums per-cpu fractions as signed 64-bit int, filters negative values and then reports lower half as signed 32-bit int. Result may looks like: fs.quota.allocated_dquots = 22327 fs.quota.cache_hits = -489852115 fs.quota.drops = -487288718 fs.quota.free_dquots = 22083 fs.quota.lookups = -486883485 fs.quota.reads = 22327 fs.quota.syncs = 335064 fs.quota.writes = 3088689 Values bigger than 2^31-1 reported as negative. All counters except "allocated_dquots" and "free_dquots" are monotonic, thus they should be reported as is without filtering negative values. Kernel doesn't have generic helper for 64-bit sysctl yet, let's use at least unsigned long. Link: https://lore.kernel.org/r/157337934693.2078.9842146413181153727.stgit@buzz Signed-off-by: Konstantin Khlebnikov Signed-off-by: Jan Kara --- fs/quota/dquot.c | 29 +++++++++++++++++------------ include/linux/quota.h | 2 +- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index 54a6626c2e9b..4639d53e96a3 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -2848,68 +2848,73 @@ EXPORT_SYMBOL(dquot_quotactl_sysfile_ops); static int do_proc_dqstats(struct ctl_table *table, int write, void __user *buffer, size_t *lenp, loff_t *ppos) { - unsigned int type = (int *)table->data - dqstats.stat; + unsigned int type = (unsigned long *)table->data - dqstats.stat; + s64 value = percpu_counter_sum(&dqstats.counter[type]); + + /* Filter negative values for non-monotonic counters */ + if (value < 0 && (type == DQST_ALLOC_DQUOTS || + type == DQST_FREE_DQUOTS)) + value = 0; /* Update global table */ - dqstats.stat[type] = - percpu_counter_sum_positive(&dqstats.counter[type]); - return proc_dointvec(table, write, buffer, lenp, ppos); + dqstats.stat[type] = value; + return proc_doulongvec_minmax(table, write, buffer, lenp, ppos); } static struct ctl_table fs_dqstats_table[] = { { .procname = "lookups", .data = &dqstats.stat[DQST_LOOKUPS], - .maxlen = sizeof(int), + .maxlen = sizeof(unsigned long), .mode = 0444, .proc_handler = do_proc_dqstats, }, { .procname = "drops", .data = &dqstats.stat[DQST_DROPS], - .maxlen = sizeof(int), + .maxlen = sizeof(unsigned long), .mode = 0444, .proc_handler = do_proc_dqstats, }, { .procname = "reads", .data = &dqstats.stat[DQST_READS], - .maxlen = sizeof(int), + .maxlen = sizeof(unsigned long), .mode = 0444, .proc_handler = do_proc_dqstats, }, { .procname = "writes", .data = &dqstats.stat[DQST_WRITES], - .maxlen = sizeof(int), + .maxlen = sizeof(unsigned long), .mode = 0444, .proc_handler = do_proc_dqstats, }, { .procname = "cache_hits", .data = &dqstats.stat[DQST_CACHE_HITS], - .maxlen = sizeof(int), + .maxlen = sizeof(unsigned long), .mode = 0444, .proc_handler = do_proc_dqstats, }, { .procname = "allocated_dquots", .data = &dqstats.stat[DQST_ALLOC_DQUOTS], - .maxlen = sizeof(int), + .maxlen = sizeof(unsigned long), .mode = 0444, .proc_handler = do_proc_dqstats, }, { .procname = "free_dquots", .data = &dqstats.stat[DQST_FREE_DQUOTS], - .maxlen = sizeof(int), + .maxlen = sizeof(unsigned long), .mode = 0444, .proc_handler = do_proc_dqstats, }, { .procname = "syncs", .data = &dqstats.stat[DQST_SYNCS], - .maxlen = sizeof(int), + .maxlen = sizeof(unsigned long), .mode = 0444, .proc_handler = do_proc_dqstats, }, diff --git a/include/linux/quota.h b/include/linux/quota.h index f32dd270b8e3..27aab84fcbaa 100644 --- a/include/linux/quota.h +++ b/include/linux/quota.h @@ -263,7 +263,7 @@ enum { }; struct dqstats { - int stat[_DQST_DQSTAT_LAST]; + unsigned long stat[_DQST_DQSTAT_LAST]; struct percpu_counter counter[_DQST_DQSTAT_LAST]; }; From 545886fead7abfdbeb46d3ac62256e1db72739a3 Mon Sep 17 00:00:00 2001 From: Chengguang Xu Date: Sat, 16 Nov 2019 06:49:00 +0800 Subject: [PATCH 28/28] ext2: code cleanup for descriptor_loc() Code cleanup by removing unnecessary variable in descriptor_loc(). Link: https://lore.kernel.org/r/20191115224900.2613-1-cgxu519@mykernel.net Signed-off-by: Chengguang Xu Signed-off-by: Jan Kara --- fs/ext2/super.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/fs/ext2/super.c b/fs/ext2/super.c index 4cd401a2f207..bcffe25da2f0 100644 --- a/fs/ext2/super.c +++ b/fs/ext2/super.c @@ -800,7 +800,6 @@ static unsigned long descriptor_loc(struct super_block *sb, { struct ext2_sb_info *sbi = EXT2_SB(sb); unsigned long bg, first_meta_bg; - int has_super = 0; first_meta_bg = le32_to_cpu(sbi->s_es->s_first_meta_bg); @@ -808,10 +807,8 @@ static unsigned long descriptor_loc(struct super_block *sb, nr < first_meta_bg) return (logic_sb_block + nr + 1); bg = sbi->s_desc_per_block * nr; - if (ext2_bg_has_super(sb, bg)) - has_super = 1; - return ext2_group_first_block_no(sb, bg) + has_super; + return ext2_group_first_block_no(sb, bg) + ext2_bg_has_super(sb, bg); } static int ext2_fill_super(struct super_block *sb, void *data, int silent)