On 09/14/2012 07:15 PM, David Sterba wrote:
> On Fri, Sep 14, 2012 at 04:58:04PM +0800, Liu Bo wrote:
>> --- a/fs/btrfs/transaction.c
>> +++ b/fs/btrfs/transaction.c
>> @@ -306,9 +306,17 @@ static struct btrfs_trans_handle *start_transaction(struct btrfs_root *root,
>> WARN_ON(type != TRANS_JOIN && type != TRANS_JOIN_NOLOCK &&
>> type != TRANS_JOIN_ONLY);
>> h = current->journal_info;
>> - h->use_count++;
>> - h->orig_rsv = h->block_rsv;
>> + if (h->block_rsv) {
>> + struct btrfs_trans_rsv_item *item;
>> + item = kmalloc(sizeof(*item), GFP_NOFS);
>
> I'd rather avoid the kmalloc here and add a list hook into
> btrfs_block_rsv itself (used only for this purpose).
>
> It also does not increase the failure surface and we don't have to
> handle error conditions from deep callchains.
>
Actually I placed a list hook at first, but I found the same block_rsv could be inserted into
the list chain twice, which will cause list_head's terrible warnings.
>> + if (!item)
>> + return ERR_PTR(-ENOMEM);
>> + item->rsv = h->block_rsv;
>> + INIT_LIST_HEAD(&item->list);
>> + list_add(&item->list, &h->blk_rsv_list);
>> + }
>> h->block_rsv = NULL;
>> + h->use_count++;
>> goto got_it;
>> } else if (type == TRANS_JOIN_ONLY) {
>> return ERR_PTR(-ENOENT);
>> --- a/fs/btrfs/transaction.h
>> +++ b/fs/btrfs/transaction.h
>> @@ -57,7 +57,6 @@ struct btrfs_trans_handle {
>> unsigned long delayed_ref_updates;
>> struct btrfs_transaction *transaction;
>> struct btrfs_block_rsv *block_rsv;
>> - struct btrfs_block_rsv *orig_rsv;
>> int aborted;
>> int adding_csums;
>> /*
>> @@ -68,6 +67,12 @@ struct btrfs_trans_handle {
>> struct btrfs_root *root;
>> struct seq_list delayed_ref_elem;
>> struct list_head qgroup_ref_list;
>> + struct list_head blk_rsv_list;
>
> Does it refer to chain of orig_rsv's ? Ie. naming it orig_blk_rsv_list
>
Make sense.
>> +};
>> +
>> +struct btrfs_trans_rsv_item {
>> + struct btrfs_block_rsv *rsv;
>> + struct list_head list;
>
> Generally, for such 'list of single pointers' structs I'd evaluate the
> possibility of embedding the hook inside the struct, the overhead
> (memory, processing) is not desirable.
>
See the above, plz.
thanks,
liubo
>> };
>>
>> struct btrfs_pending_snapshot {
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
