Re: [PATCH 09/11] quota: protect dquot mem info with objects's lock

[Jan Kara <jack@suse.cz>]

On Wed 06-10-10 17:17:03, Dmitry wrote:
> Later i've plans to get rid of dqptr_sem by rearranging code like follows.
> But may be i've introduced copy logic too soon.
> 
> Variant (1)
>   1) Protect i_dquot dereference/modification with i_lock
>   So charge/free function will looks like follows
>   do_alloc_space (struct inode* inode, qsize_t num)
>   {
>       spin_lock(&inode->i_lock);
>       if (!check_bdq(inode->i_dquot, num))
>             return EQDUOT;
>       inode_add_bytes_unlocked(inode, num);
>       dquot_add_bytes(inode->i_dquot, num); 
>       /* We can sleep while we call quota dirty, to protect quota from
>        * being deleted we have to get ref for the quota.
>        */
>       atomic_inc(dquot->dq_count);
>       dquot = inode->i_dquot;
>       spin_unlock(&inode->i_lock);
>       /* i_lock was dropped, do not touch i_dquot any more */
>       mark_dquot_dirty(dquot);
>       dqput(dquot);
>   }
> This is beter than before, but we still have massive anomic operations
> So we can try second variant.
>  
> Variant (2) 
>   do_alloc_space(struct inode* inode, qsize_t num)
>   {
>       idx = srcu_read_lock(&dqptr)
>       spin_lock(&inode->i_lock);
>       if (!check_bdq(inode->i_dquot, num))
>             return EQDUOT;
>       inode_add_bytes_unlocked(inode, num);
>       dquot_add_bytes(inode->i_dquot, num); 
>       /* We can sleep while we call quota dirty,
>        * But this is ok since quota deletion is protected by SRCU
>        * The only thing we have do is to save old i_dquot array
>        */
>       dquot = inode->i_dquot;
>       spin_unlock(&inode->i_lock);
>       /* i_lock was dropped, do not touch i_dquot any more */
>       mark_dquot_dirty(dquot);
>       srcu_read_unlock(&dqptr, idx);
>   }
>    /* Destroy method may looks like whis, it will be called from
>       background work.
>     */
>    do_destroy_quotas_loop() {
>       /* Wait for all quotas in free list */ 
>       synchronize_srcu(dqptr&)
>       /* 
>          Ok now we can safely destroy all quotas in the list,
>          Some of them may be dirty, so we have to write it first.
>        */
>       list_for_each_entry(dquot, &free_list, dq_free)
>             dquot_release(dquot);
> 
>    }
  I see. Protecting dquots with RCU seems like a reasonable thing to do
and then I agree you'd need to copy the dquot pointers. But for now please
refrain from doing so. Thanks.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR