From: Jan Kara <jack@suse.cz>
To: Dmitry Monakhov <dmonakhov@openvz.org>
Cc: linux-fsdevel@vger.kernel.org, jack@suse.cz, hch@infradead.org
Subject: Re: [PATCH 17/19] quota: Some stylistic cleanup for dquot interface
Date: Tue, 23 Nov 2010 12:37:51 +0100 [thread overview]
Message-ID: <20101123113751.GC6113@quack.suse.cz> (raw)
In-Reply-To: <1289477678-5669-18-git-send-email-dmonakhov@openvz.org>
On Thu 11-11-10 15:14:36, Dmitry Monakhov wrote:
> This patch performs only stylistic cleanup. No changes in logic at all.
> - Rename dqget() to find_get_dquot()
This seems as a pointless excercise to me...
> - Wrap direct dq_count increment to helper function
This makes sense so if you need a name for it, why not dqgrab() - along
the way how iget() vs igrab() work?
> Some places still access dq_count directly, but this is because
> reference counting algorithm. It will be changed in later patches.
Honza
>
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
> fs/ocfs2/file.c | 8 ++++----
> fs/ocfs2/quota_global.c | 2 +-
> fs/ocfs2/quota_local.c | 3 ++-
> fs/quota/dquot.c | 42 ++++++++++++++++++++++++++----------------
> include/linux/quotaops.h | 3 ++-
> 5 files changed, 35 insertions(+), 23 deletions(-)
>
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index 9a03c15..b7e7c9b 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -1205,8 +1205,8 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
> if (attr->ia_valid & ATTR_UID && attr->ia_uid != inode->i_uid
> && OCFS2_HAS_RO_COMPAT_FEATURE(sb,
> OCFS2_FEATURE_RO_COMPAT_USRQUOTA)) {
> - transfer_to[USRQUOTA] = dqget(sb, attr->ia_uid,
> - USRQUOTA);
> + transfer_to[USRQUOTA] =
> + find_get_dquot(sb, attr->ia_uid, USRQUOTA);
> if (!transfer_to[USRQUOTA]) {
> status = -ESRCH;
> goto bail_unlock;
> @@ -1215,8 +1215,8 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
> if (attr->ia_valid & ATTR_GID && attr->ia_gid != inode->i_gid
> && OCFS2_HAS_RO_COMPAT_FEATURE(sb,
> OCFS2_FEATURE_RO_COMPAT_GRPQUOTA)) {
> - transfer_to[GRPQUOTA] = dqget(sb, attr->ia_gid,
> - GRPQUOTA);
> + transfer_to[GRPQUOTA] =
> + find_get_dquot(sb, attr->ia_gid, GRPQUOTA);
> if (!transfer_to[GRPQUOTA]) {
> status = -ESRCH;
> goto bail_unlock;
> diff --git a/fs/ocfs2/quota_global.c b/fs/ocfs2/quota_global.c
> index 3d2841c..cdf2a23 100644
> --- a/fs/ocfs2/quota_global.c
> +++ b/fs/ocfs2/quota_global.c
> @@ -692,7 +692,7 @@ static int ocfs2_release_dquot(struct dquot *dquot)
> mlog_entry("id=%u, type=%d", dquot->dq_id, dquot->dq_type);
>
> mutex_lock(&dquot->dq_mutex);
> - /* Check whether we are not racing with some other dqget() */
> + /* Check whether we are not racing with some other find_get_dquot() */
> if (atomic_read(&dquot->dq_count) > 1)
> goto out;
> status = ocfs2_lock_global_qf(oinfo, 1);
> diff --git a/fs/ocfs2/quota_local.c b/fs/ocfs2/quota_local.c
> index 9e68ce5..6e5c7e9 100644
> --- a/fs/ocfs2/quota_local.c
> +++ b/fs/ocfs2/quota_local.c
> @@ -500,7 +500,8 @@ static int ocfs2_recover_local_quota_file(struct inode *lqinode,
> }
> dqblk = (struct ocfs2_local_disk_dqblk *)(qbh->b_data +
> ol_dqblk_block_off(sb, chunk, bit));
> - dquot = dqget(sb, le64_to_cpu(dqblk->dqb_id), type);
> + dquot = find_get_dquot(sb, le64_to_cpu(dqblk->dqb_id),
> + type);
> if (!dquot) {
> status = -EIO;
> mlog(ML_ERROR, "Failed to get quota structure "
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index 33dc32e..af3413e 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -16,7 +16,8 @@
> * Revised list management to avoid races
> * -- Bill Hawes, <whawes@star.net>, 9/98
> *
> - * Fixed races in dquot_transfer(), dqget() and dquot_alloc_...().
> + * Fixed races in dquot_transfer(), find_get_dquot() and
> + * dquot_alloc_...().
> * As the consequence the locking was moved from dquot_decr_...(),
> * dquot_incr_...() to calling functions.
> * invalidate_dquots() now writes modified dquots.
> @@ -109,8 +110,9 @@
> * Each dquot has its dq_mutex mutex. Locked dquots might not be referenced
> * from inodes (dquot_alloc_space() and such don't check the dq_mutex).
> * Currently dquot is locked only when it is being read to memory (or space for
> - * it is being allocated) on the first dqget() and when it is being released on
> - * the last dqput(). The allocation and release oparations are serialized by
> + * it is being allocated) on the first find_get_dquot() and when it is being
> + * released on the last dqput().
> + * The allocation and release oparations are serialized by
> * the dq_mutex and by checking the use count in dquot_release(). Write
> * operations on dquots don't hold dq_mutex as they copy data under dq_data_lock
> * spinlock to internal buffers before writing.
> @@ -522,7 +524,7 @@ int dquot_release(struct dquot *dquot)
> struct quota_info *dqopt = sb_dqopts(dquot);
>
> mutex_lock(&dquot->dq_mutex);
> - /* Check whether we are not racing with some other dqget() */
> + /* Check whether we are not racing with some other find_get_dquot() */
> if (atomic_read(&dquot->dq_count) > 1)
> goto out_dqlock;
> mutex_lock(&dqopt->dqio_mutex);
> @@ -577,7 +579,7 @@ restart:
> if (atomic_read(&dquot->dq_count)) {
> DEFINE_WAIT(wait);
>
> - atomic_inc(&dquot->dq_count);
> + dqget(dquot);
> prepare_to_wait(&dquot->dq_wait_unused, &wait,
> TASK_UNINTERRUPTIBLE);
> spin_unlock(&dqopt->dq_list_lock);
> @@ -627,7 +629,7 @@ int dquot_scan_active(struct super_block *sb,
> if (dquot->dq_sb != sb)
> continue;
> /* Now we have active dquot so we can just increase use count */
> - atomic_inc(&dquot->dq_count);
> + dqget(dquot);
> spin_unlock(&dqopt->dq_list_lock);
> dqstats_inc(DQST_LOOKUPS);
> dqput(old_dquot);
> @@ -674,7 +676,7 @@ int dquot_quota_sync(struct super_block *sb, int type, int wait)
> /* Now we have active dquot from which someone is
> * holding reference so we can safely just increase
> * use count */
> - atomic_inc(&dquot->dq_count);
> + dqget(dquot);
> spin_unlock(&dqopt->dq_list_lock);
> dqstats_inc(DQST_LOOKUPS);
> dqctl(sb)->dq_op->write_dquot(dquot);
> @@ -869,6 +871,11 @@ static struct dquot *get_empty_dquot(struct super_block *sb, int type)
> return dquot;
> }
>
> +inline void dqget(struct dquot *dquot)
> +{
> + atomic_inc(&dquot->dq_count);
> +}
> +
> /*
> * Get reference to dquot
> *
> @@ -877,7 +884,7 @@ static struct dquot *get_empty_dquot(struct super_block *sb, int type)
> * a) checking for quota flags under dq_list_lock and
> * b) getting a reference to dquot before we release dq_list_lock
> */
> -struct dquot *dqget(struct super_block *sb, unsigned int id, int type)
> +struct dquot *find_get_dquot(struct super_block *sb, unsigned int id, int type)
> {
> struct hlist_bl_head * blh = dquot_hash + hashfn(sb, id, type);
> struct dquot *dquot = NULL, *empty = NULL;
> @@ -922,7 +929,7 @@ we_slept:
> } else {
> if (!atomic_read(&dquot->dq_count))
> remove_free_dquot(dquot);
> - atomic_inc(&dquot->dq_count);
> + dqget(dquot);
> hlist_bl_unlock(blh);
> spin_unlock(&dqopt->dq_list_lock);
> dqstats_inc(DQST_CACHE_HITS);
> @@ -948,7 +955,7 @@ out:
>
> return dquot;
> }
> -EXPORT_SYMBOL(dqget);
> +EXPORT_SYMBOL(find_get_dquot);
>
> static int dqinit_needed(struct inode *inode, int type)
> {
> @@ -1427,7 +1434,7 @@ static int dquot_active(const struct inode *inode)
> * Initialize quota pointers in inode
> *
> * We do things in a bit complicated way but by that we avoid calling
> - * dqget() and thus filesystem callbacks under dqptr_sem.
> + * find_get_dquot() and thus filesystem callbacks under dqptr_sem.
> *
> * It is better to call this function outside of any transaction as it
> * might need a lot of space in journal for dquot structure allocation.
> @@ -1462,7 +1469,7 @@ static void __dquot_initialize(struct inode *inode, int type)
> id = inode->i_gid;
> break;
> }
> - got[cnt] = dqget(sb, id, cnt);
> + got[cnt] = find_get_dquot(sb, id, cnt);
> }
>
> down_write(&dqopts(sb)->dqptr_sem);
> @@ -1991,9 +1998,12 @@ int dquot_transfer(struct inode *inode, struct iattr *iattr)
> return 0;
>
> if (iattr->ia_valid & ATTR_UID && iattr->ia_uid != inode->i_uid)
> - transfer_to[USRQUOTA] = dqget(sb, iattr->ia_uid, USRQUOTA);
> + transfer_to[USRQUOTA] = find_get_dquot(sb, iattr->ia_uid,
> + USRQUOTA);
> +
> if (iattr->ia_valid & ATTR_GID && iattr->ia_gid != inode->i_gid)
> - transfer_to[GRPQUOTA] = dqget(sb, iattr->ia_gid, GRPQUOTA);
> + transfer_to[GRPQUOTA] = find_get_dquot(sb, iattr->ia_gid,
> + GRPQUOTA);
>
> ret = __dquot_transfer(inode, transfer_to);
> dqput_all(transfer_to);
> @@ -2547,7 +2557,7 @@ int dquot_get_dqblk(struct super_block *sb, int type, qid_t id,
> {
> struct dquot *dquot;
>
> - dquot = dqget(sb, id, type);
> + dquot = find_get_dquot(sb, id, type);
> if (!dquot)
> return -ESRCH;
> do_get_dqblk(dquot, di);
> @@ -2660,7 +2670,7 @@ int dquot_set_dqblk(struct super_block *sb, int type, qid_t id,
> struct dquot *dquot;
> int rc;
>
> - dquot = dqget(sb, id, type);
> + dquot = find_get_dquot(sb, id, type);
> if (!dquot) {
> rc = -ESRCH;
> goto out;
> diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h
> index 68ceef5..93e39c6 100644
> --- a/include/linux/quotaops.h
> +++ b/include/linux/quotaops.h
> @@ -52,7 +52,8 @@ void inode_sub_rsv_space(struct inode *inode, qsize_t number);
>
> void dquot_initialize(struct inode *inode);
> void dquot_drop(struct inode *inode);
> -struct dquot *dqget(struct super_block *sb, unsigned int id, int type);
> +struct dquot*find_get_dquot(struct super_block *sb, unsigned int id, int type);
> +void dqget(struct dquot *dquot);
> void dqput(struct dquot *dquot);
> int dquot_scan_active(struct super_block *sb,
> int (*fn)(struct dquot *dquot, unsigned long priv),
> --
> 1.6.5.2
>
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
next prev parent reply other threads:[~2010-11-23 11:37 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-11-11 12:14 [PATCH 00/19] quota: RFC SMP improvements for generic quota V3 Dmitry Monakhov
2010-11-11 12:14 ` [PATCH 01/19] quota: protect getfmt call with dqonoff_mutex lock Dmitry Monakhov
2010-11-11 13:36 ` Christoph Hellwig
2010-11-22 19:35 ` Jan Kara
2010-12-02 11:40 ` Dmitry
2010-11-11 12:14 ` [PATCH 02/19] kernel: add bl_list Dmitry Monakhov
2010-11-11 12:14 ` [PATCH 03/19] quota: Wrap common expression to helper function Dmitry Monakhov
2010-11-11 12:14 ` [PATCH 04/19] quota: protect dqget() from parallels quotaoff via SRCU Dmitry Monakhov
2010-11-22 21:21 ` Jan Kara
2010-11-22 21:53 ` Dmitry
2010-11-11 12:14 ` [PATCH 05/19] quota: mode quota internals from sb to quota_info Dmitry Monakhov
2010-11-11 12:14 ` [PATCH 06/19] quota: Remove state_lock Dmitry Monakhov
2010-11-22 21:12 ` Jan Kara
2010-11-22 21:31 ` Dmitry
2010-11-23 10:55 ` Jan Kara
2010-11-23 11:33 ` Jan Kara
2010-11-11 12:14 ` [PATCH 07/19] quota: add quota format lock Dmitry Monakhov
2010-11-11 12:14 ` [PATCH 08/19] quota: make dquot lists per-sb Dmitry Monakhov
2010-11-22 21:37 ` Jan Kara
2010-11-11 12:14 ` [PATCH 09/19] quota: optimize quota_initialize Dmitry Monakhov
2010-11-11 12:14 ` [PATCH 10/19] quota: user per-backet hlist lock for dquot_hash Dmitry Monakhov
2010-11-11 12:14 ` [PATCH 11/19] quota: remove global dq_list_lock Dmitry Monakhov
2010-11-11 12:14 ` [PATCH 12/19] quota: rename dq_lock Dmitry Monakhov
2010-11-11 12:14 ` [PATCH 13/19] quota: make per-sb dq_data_lock Dmitry Monakhov
2010-11-11 12:14 ` [PATCH 14/19] quota: protect dquot mem info with object's lock Dmitry Monakhov
2010-11-11 12:14 ` [PATCH 15/19] quota: drop dq_data_lock where possible Dmitry Monakhov
2010-11-11 12:14 ` [PATCH 16/19] quota: relax dq_data_lock dq_lock locking consistency Dmitry Monakhov
2010-11-11 12:14 ` [PATCH 17/19] quota: Some stylistic cleanup for dquot interface Dmitry Monakhov
2010-11-23 11:37 ` Jan Kara [this message]
2010-11-11 12:14 ` [PATCH 18/19] fs: add unlocked helpers Dmitry Monakhov
2010-11-11 12:14 ` [PATCH 19/19] quota: protect i_dquot with i_lock instead of dqptr_sem Dmitry Monakhov
2010-11-19 5:44 ` [PATCH 00/19] quota: RFC SMP improvements for generic quota V3 Dmitry
-- strict thread matches above, loose matches on Subject: below --
2010-10-22 17:34 [PATCH 00/19] quota: RFC SMP improvements for generic quota V2 Dmitry Monakhov
2010-10-22 17:35 ` [PATCH 17/19] quota: Some stylistic cleanup for dquot interface Dmitry Monakhov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20101123113751.GC6113@quack.suse.cz \
--to=jack@suse.cz \
--cc=dmonakhov@openvz.org \
--cc=hch@infradead.org \
--cc=linux-fsdevel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).