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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.