Hi Honza.
> > + if (!dquot->dq_dqb.dqb_bhardlimit &&
> > + !dquot->dq_dqb.dqb_bsoftlimit &&
> > + !dquot->dq_dqb.dqb_ihardlimit &&
> > + !dquot->dq_dqb.dqb_isoftlimit)
> > + set_bit(DQ_FAKE_B, &dquot->dq_flags);
> > + spin_unlock(&dquot->dq_dqb_lock);
> > +
> > + /* Make sure flags update is visible after dquot has been filled */
> > + smp_mb__before_atomic();
> > + set_bit(DQ_ACTIVE_B, &dquot->dq_flags);
>
> I'm slightly wondering whether we shouldn't have a dquot_mark_active()
> helper for this to hide the barrier details...
This sounds good to me, would be ok for you if I simply add this to my todo
list, and do it once this series is merged? I'd rather avoid to add more patches
to the series now adding more review overhead.
I can send a new patch later creating a new helper and replacing all
set_bit(DQ_ACTIVE_B, ...) calls with the new helper.
>
> > +out_unlock:
> > + up_write(&dqopt->dqio_sem);
> > + mutex_unlock(&dquot->dq_lock);
> > + return ret;
> > +}
> > +
> > +/*
> > + * Store limits from dquot in the tree unless it's fake. If it is fake
> > + * remove the id from the tree since there is no useful information in
> > + * there.
> > + */
> > +static int shmem_release_dquot(struct dquot *dquot)
> > +{
> > + struct mem_dqinfo *info = sb_dqinfo(dquot->dq_sb, dquot->dq_id.type);
> > + struct rb_node *node = ((struct rb_root *)info->dqi_priv)->rb_node;
> > + qid_t id = from_kqid(&init_user_ns, dquot->dq_id);
> > + struct quota_info *dqopt = sb_dqopt(dquot->dq_sb);
> > + struct quota_id *entry = NULL;
> > +
> > + mutex_lock(&dquot->dq_lock);
> > + /* Check whether we are not racing with some other dqget() */
> > + if (dquot_is_busy(dquot))
> > + goto out_dqlock;
> > +
> > + down_write(&dqopt->dqio_sem);
> > + while (node) {
> > + entry = rb_entry(node, struct quota_id, node);
> > +
> > + if (id < entry->id)
> > + node = node->rb_left;
> > + else if (id > entry->id)
> > + node = node->rb_right;
> > + else
> > + goto found;
> > + }
> > +
> > + up_write(&dqopt->dqio_sem);
> > + mutex_unlock(&dquot->dq_lock);
>
> We should report some kind of error here, shouldn't we? We do expect to
> have the quota_id allocated from shmem_acquire_dquot() and we will be
> possibly loosing set limits here.
>
Sounds correct, I'll update it once we agree on how to proceed with your above
suggestion of dquot_mark_active().
> Otherwise the patch looks good to me.
>
> Honza
> --
> Jan Kara <jack@xxxxxxxx>
> SUSE Labs, CR
--
Carlos Maiolino
