On 01/20/2015 06:41 AM, Ilya Dryomov wrote:
> This effectively reverts the last hunk of 392a9dad7e77 ("rbd: detect
> when clone image is flattened").
>
> The problem with parent_overlap != 0 condition is that it's possible
> and completely valid to have an image with parent_overlap == 0 whose
> parent state needs to be cleaned up on unmap. The next commit, which
> drops the "clone image now standalone" logic, opens up another window
> of opportunity to hit this, but even without it
>
> # cat parent-ref.sh
> #!/bin/bash
> rbd create --image-format 2 --size 1 foo
> rbd snap create foo@snap
> rbd snap protect foo@snap
> rbd clone foo@snap bar
> rbd resize --allow-shrink --size 0 bar
> rbd resize --size 1 bar
> DEV=$(rbd map bar)
> rbd unmap $DEV
>
> leaves rbd_device/rbd_spec/etc and rbd_client along with ceph_client
> hanging around.
I'm not sure why the last reference to the parent
doesn't get dropped (and state cleaned up) as soon
as the overlap becomes 0. I suspect it's the original
reference taken when there's a parent, we don't get
rid of it until it's torn down. (I think we should.)
It seems to me the test here should be for a non-null
parent_spec pointer rather than non-zero parent_overlap.
And that's done inside rbd_dev_parent_put(), so your
change looks reasonable to me.
Reviewed-by: Alex Elder <elder@xxxxxxxxxx>
>
> My thinking behind calling rbd_dev_parent_put() unconditionally is that
> there shouldn't be any requests in flight at that point in time as we
> are deep into unmap sequence. Hence, even if rbd_dev_unparent() caused
> by flatten is delayed by in-flight requests, it will have finished by
> the time we reach rbd_dev_unprobe() caused by unmap, thus turning
> unconditional rbd_dev_parent_put() into a no-op.
>
> Fixes: http://tracker.ceph.com/issues/10352
>
> Cc: stable@xxxxxxxxxxxxxxx # 3.11+
> Signed-off-by: Ilya Dryomov <idryomov@xxxxxxxxxx>
> ---
> drivers/block/rbd.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 2990a1c75159..b85d52005a21 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -5075,10 +5075,7 @@ static void rbd_dev_unprobe(struct rbd_device *rbd_dev)
> {
> struct rbd_image_header *header;
>
> - /* Drop parent reference unless it's already been done (or none) */
> -
> - if (rbd_dev->parent_overlap)
> - rbd_dev_parent_put(rbd_dev);
> + rbd_dev_parent_put(rbd_dev);
>
> /* Free dynamic fields from the header, then zero it out */
>
>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
