Mark Fasheh wrote:
> On Thu, Oct 30, 2008 at 01:42:14PM +0800, Tao Ma wrote:
>
>> @@ -1548,6 +1528,11 @@ static int ocfs2_remove_value_outside(struct
>> inode*inode,
>> struct ocfs2_xattr_header *header)
>> {
>> int ret = 0, i;
>> + struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>> + struct ocfs2_xattr_set_ctxt ctxt;
>> +
>> + memset(&ctxt, 0, sizeof(ctxt));
>> + ocfs2_init_dealloc_ctxt(&ctxt.dealloc);
>
>
> You can lose the memset if you just do:
>
> struct ocfs2_xattr_set_ctxt ctxt = { NULL, };
Sure. Thanks.
>
>>
>> for (i = 0; i < le16_to_cpu(header->xh_count); i++) {
>> struct ocfs2_xattr_entry *entry = &header->xh_entries[i];
>
>
>> +static int ocfs2_init_xattr_set_ctxt(struct inode *inode,
>> + struct ocfs2_dinode *di,
>> + struct ocfs2_xattr_info *xi,
>> + struct ocfs2_xattr_search *xis,
>> + struct ocfs2_xattr_search *xbs,
>> + struct ocfs2_xattr_set_ctxt *ctxt)
>> +{
>
> This function looks suspiciously like ocfs2_lock_allocators()... That said,
> I think it's probably fine to leave it on it's own for now ;)
yeah, actually I refer to that function. ;) So maybe we can intergrate
them later.
>
>
>> + int clusters_add, meta_add, ret;
>> + struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>> +
>> + memset(ctxt, 0, sizeof(struct ocfs2_xattr_set_ctxt));
>> +
>> + ocfs2_init_dealloc_ctxt(&ctxt->dealloc);
>> +
>> + ret = ocfs2_calc_xattr_set_need(inode, di, xi, xis, xbs,
>> + &clusters_add, &meta_add);
>> + if (ret) {
>> + mlog_errno(ret);
>> + return ret;
>> + }
>> +
>> + mlog(0, "Set xattr %s, reserve meta blocks = %d, clusters = %d\n",
>> + xi->name, meta_add, clusters_add);
>> +
>> + if (meta_add) {
>> + ret = ocfs2_reserve_new_metadata_blocks(osb, meta_add,
>> + &ctxt->meta_ac);
>> + if (ret) {
>> + mlog_errno(ret);
>> + goto out;
>> + }
>> + }
>
>
>> @@ -2146,10 +2377,18 @@ int ocfs2_xattr_set(struct inode *inode,
>> */
>> xi.value = NULL;
>> xi.value_len = 0;
>> - ret = ocfs2_xattr_ibody_set(inode, &xi, &xis);
>> + ret = ocfs2_xattr_ibody_set(inode, &xi,
>> + &xis, &ctxt);
>> }
>> }
>> }
>> +free:
>> + if (ctxt.data_ac)
>> + ocfs2_free_alloc_context(ctxt.data_ac);
>> + if (ctxt.meta_ac)
>> + ocfs2_free_alloc_context(ctxt.meta_ac);
>> + ocfs2_schedule_truncate_log_flush(osb, 1);
>
> Since this is a set function, which only a small part of the time will be
> deleteing clusters, I think it would be better not to unconditionally start
> a truncate log flush. Maybe you can do that only in the case where we have
> clusters to free?
No problem, it is easy for us to detect whether ctxt.dealloc has some
clusters to free.
Regards,
Tao
_______________________________________________
Ocfs2-devel mailing list
[email protected]
http://oss.oracle.com/mailman/listinfo/ocfs2-devel
