On Thu 11-11-10 15:14:23, Dmitry Monakhov wrote:
> In order to hide quota internals inside didicated structure pointer.
> We have to serialize that object lifetime with dqget(), and change/uncharge
> functions.
> Quota_info construction/destruction will be protected via ->dq_srcu.
> SRCU counter temproraly placed inside sb, but will be moved inside
> quota object pointer in next patch.
The changelog seems rather insufficient to me. Could you please write
here the new locking rules more in detail? What structures are exactly
protected by RCU? Which lock are you able to relax after these changes (is
it only dq_state_lock?)? The rules should be also placed in dquot.c where
locking is described.
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index 748d744..7e937b0 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -805,7 +805,7 @@ static struct dquot *get_empty_dquot(struct super_block *sb, int type)
> /*
> * Get reference to dquot
> *
> - * Locking is slightly tricky here. We are guarded from parallel quotaoff()
> + * We are guarded from parallel quotaoff() by holding srcu_read_lock
The comment does not make sense after your change anymore.
> * destroying our dquot by:
> * a) checking for quota flags under dq_list_lock and
> * b) getting a reference to dquot before we release dq_list_lock
> @@ -814,9 +814,15 @@ struct dquot *dqget(struct super_block *sb, unsigned int id, int type)
> {
> unsigned int hashent = hashfn(sb, id, type);
> struct dquot *dquot = NULL, *empty = NULL;
> + int idx;
>
> - if (!sb_has_quota_active(sb, type))
> + rcu_read_lock();
> + if (!sb_has_quota_active(sb, type)) {
> + rcu_read_unlock();
> return NULL;
> + }
> + idx = srcu_read_lock(&dqopts(sb)->dq_srcu);
> + rcu_read_unlock();
Ugh, I'm kind of puzzled by your combination of RCU and SRCU. What's the
point?
Honza
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
