[Devel] [PATCH RHEL7 COMMIT] ploop: push_backup: merge pbd->cbt_map back to CBT

Maxim Patlasov mpatlasov at virtuozzo.com
Fri May 27 11:09:24 PDT 2016


On 05/27/2016 02:52 AM, Konstantin Khorenko wrote:
> Maxim, please verify the changed hunk.

Looks fine.

>
> + see comment below.

Good catch, thanks. I forgot to copy "ctl" with updated status back to 
userspace. Sending fix now.

Thanks,
Maxim

>
> On 05/27/2016 12:47 PM, Konstantin Khorenko wrote:
>> The commit is pushed to "branch-rh7-3.10.0-327.18.2.vz7.14.x-ovz" and 
>> will appear at https://src.openvz.org/scm/ovz/vzkernel.git
>> after rh7-3.10.0-327.18.2.vz7.14.8
>> ------>
>> commit 3bc16bb34d02489fd657d7be70bd77595f1015f9
>> Author: Maxim Patlasov <mpatlasov at virtuozzo.com>
>> Date:   Fri May 27 13:47:48 2016 +0400
>>
>>      ploop: push_backup: merge pbd->cbt_map back to CBT
>>
>>      The patch merges its copy of CBT mask back to CBT when
>>      push_backup internal state becomes invalid.
>>
>>      The only exception is when userspace explicitly asks
>>      kernel to release pbd->cbt_map without merge:
>>
>>      ioctl(PLOOP_IOC_PUSH_BACKUP_STOP) with ctl.status == 0.
>>
>>      https://jira.sw.ru/browse/PSBM-47429
>>
>>      Signed-off-by: Maxim Patlasov <mpatlasov at virtuozzo.com>
>>
>>      v2 changes:
>>      khorenko@: ploop_pb_destroy() hunk added instead of 
>> ploop_push_backup_stop(),
>>      caused by f43eec3 ("ploop: push_backup: factor out destroy").
>> ---
>>   drivers/block/ploop/dev.c         |  2 +-
>>   drivers/block/ploop/push_backup.c | 38 
>> ++++++++++++++++++++++++++++++++------
>>   drivers/block/ploop/push_backup.h |  2 +-
>>   3 files changed, 34 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c
>> index 8e3f97f..07764f9 100644
>> --- a/drivers/block/ploop/dev.c
>> +++ b/drivers/block/ploop/dev.c
>> @@ -3808,7 +3808,7 @@ static int ploop_stop(struct ploop_device * 
>> plo, struct block_device *bdev)
>>       }
>>
>>       clear_bit(PLOOP_S_PUSH_BACKUP, &plo->state);
>> -    ploop_pb_stop(plo->pbd);
>> +    ploop_pb_stop(plo->pbd, true);
>>
>>       for (p = plo->disk->minors - 1; p > 0; p--)
>>           invalidate_partition(plo->disk, p);
>> diff --git a/drivers/block/ploop/push_backup.c 
>> b/drivers/block/ploop/push_backup.c
>> index 16b1a5d..376052d 100644
>> --- a/drivers/block/ploop/push_backup.c
>> +++ b/drivers/block/ploop/push_backup.c
>> @@ -88,6 +88,29 @@ static void ploop_pb_map_free(struct page **map, 
>> unsigned long block_max)
>>       }
>>   }
>>
>> +int ploop_pb_cbt_map_release(struct ploop_pushbackup_desc *pbd, bool 
>> do_merge)
>> +{
>> +    int ret = 0;
>> +
>> +    if (pbd->cbt_map == NULL)
>> +        return 0;
>> +
>> +    if (do_merge) {
>> +        ret = blk_cbt_map_merge(pbd->plo->queue,
>> +                    pbd->cbt_uuid,
>> +                    pbd->cbt_map,
>> +                    pbd->cbt_block_max,
>> +                    pbd->cbt_block_bits);
>> +        if (ret)
>> +            printk("ploop(%d): blk_cbt_map_merge() failed with "
>> +                   "%d\n", pbd->plo->index, ret);
>> +    }
>> +
>> +    ploop_pb_map_free(pbd->cbt_map, pbd->cbt_block_max);
>> +    pbd->cbt_map = NULL;
>> +    return ret;
>> +}
>> +
>>   struct ploop_pushbackup_desc *ploop_pb_alloc(struct ploop_device *plo)
>>   {
>>       struct ploop_pushbackup_desc *pbd;
>> @@ -314,7 +337,7 @@ void ploop_pb_fini(struct ploop_pushbackup_desc 
>> *pbd)
>>           mutex_unlock(&plo->sysfs_mutex);
>>       }
>>
>> -    ploop_pb_map_free(pbd->cbt_map, pbd->cbt_block_max);
>> +    ploop_pb_cbt_map_release(pbd, true);
>>       ploop_pb_map_free(pbd->ppb_map, pbd->ppb_block_max);
>>       ploop_pb_map_free(pbd->reported_map, pbd->ppb_block_max);
>>
>> @@ -334,8 +357,6 @@ int ploop_pb_copy_cbt_to_user(struct 
>> ploop_pushbackup_desc *pbd, char *user_addr
>>           user_addr += PAGE_SIZE;
>>       }
>>
>> -    ploop_pb_map_free(pbd->cbt_map, pbd->cbt_block_max);
>> -    pbd->cbt_map = NULL;
>>       return 0;
>>   }
>>
>> @@ -496,14 +517,17 @@ int ploop_pb_preq_add_pending(struct 
>> ploop_pushbackup_desc *pbd,
>>       return 0;
>>   }
>>
>> -unsigned long ploop_pb_stop(struct ploop_pushbackup_desc *pbd)
>> +unsigned long ploop_pb_stop(struct ploop_pushbackup_desc *pbd, bool 
>> do_merge)
>>   {
>>       unsigned long ret = 0;
>> +    int merge_status = 0;
>>       LIST_HEAD(drop_list);
>>
>>       if (pbd == NULL)
>>           return 0;
>>
>> +    merge_status = ploop_pb_cbt_map_release(pbd, do_merge);
>> +
>>       spin_lock(&pbd->ppb_lock);
>>
>>       while (!RB_EMPTY_ROOT(&pbd->pending_tree)) {
>> @@ -535,7 +559,7 @@ unsigned long ploop_pb_stop(struct 
>> ploop_pushbackup_desc *pbd)
>>           spin_unlock_irq(&plo->lock);
>>       }
>>
>> -    return ret;
>> +    return merge_status ? : ret;
>>   }
>>
>>   int ploop_pb_get_pending(struct ploop_pushbackup_desc *pbd,
>> @@ -682,12 +706,14 @@ int ploop_pb_destroy(struct ploop_device *plo, 
>> __u32 *status)
>>   {
>>       struct ploop_pushbackup_desc *pbd = plo->pbd;
>>       unsigned long ret;
>> +    bool do_merge;
>>
>>       if (!test_and_clear_bit(PLOOP_S_PUSH_BACKUP, &plo->state))
>>           return -EINVAL;
>>
>>       BUG_ON (!pbd);
>> -    ret = ploop_pb_stop(pbd);
>> +    do_merge = status ? *status : true;
>> +    ret = ploop_pb_stop(pbd, do_merge);
>>
>>       if (status)
>>           *status = ret;
>
> Why do we need this *status = ret?
> May be we need to return not always 0 here, but "ret" in 
> ploop_pb_destroy()?
>
>> diff --git a/drivers/block/ploop/push_backup.h 
>> b/drivers/block/ploop/push_backup.h
>> index 1a8636a..1644785 100644
>> --- a/drivers/block/ploop/push_backup.h
>> +++ b/drivers/block/ploop/push_backup.h
>> @@ -4,7 +4,7 @@ struct ploop_pushbackup_desc *ploop_pb_alloc(struct 
>> ploop_device *plo);
>>   int ploop_pb_init(struct ploop_pushbackup_desc *pbd, __u8 *uuid, 
>> bool full);
>>   void ploop_pb_fini(struct ploop_pushbackup_desc *pbd);
>>   int ploop_pb_copy_cbt_to_user(struct ploop_pushbackup_desc *pbd, 
>> char *user_addr);
>> -unsigned long ploop_pb_stop(struct ploop_pushbackup_desc *pbd);
>> +unsigned long ploop_pb_stop(struct ploop_pushbackup_desc *pbd, bool 
>> do_merge);
>>   int ploop_pb_check_uuid(struct ploop_pushbackup_desc *pbd, __u8 
>> *uuid);
>>   int ploop_pb_get_uuid(struct ploop_pushbackup_desc *pbd, __u8 *uuid);
>>
>> .
>>



More information about the Devel mailing list