Re: [PATCH 4/6] shmem: prepare shmem quota infrastructure

[Carlos Maiolino <cem@kernel.org>]

Hi Honza

> > > +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.
> > 

I've been looking into this today, and I'm not sure if there is any error we
should be reporting here, as there isn't anything to really go wrong here. I was
comparing it with other filesystems, and most of them uses dquot_release()
return value, as a return value for .release_dquot. And on such cases, the error
could be other than zero, if something failed while writing the dquot to disk.
In the case here, we just write to the RB tree in memory, and it has already
been allocated, so, I don't think there is any error we could be returning here.
Does it sound right to you?

Cheers.


-- 
Carlos Maiolino