Re: [PATCH 2/4] rbd: return earlier in rbd_header_from_disk()

[Josh Durgin <josh.durgin@inktank.com>]

On 08/06/2012 11:17 AM, Alex Elder wrote:
> The only caller of rbd_header_from_disk() is rbd_read_header().
> It passes as allocated_snaps the number of snapshots it will
> have received from the server for the snapshot context that
> rbd_header_from_disk() is to interpret.  The first time through
> it provides 0--mainly to extract the number of snapshots from
> the snapshot context header--so that it can allocate an
> appropriately-sized buffer to receive the entire snapshot
> context from the server in a second request.
>
> rbd_header_from_disk() will not fill in the array of snapshot ids
> unless the number in the snapshot matches the number the caller
> had allocated.
>
> This patch adjusts that logic a little further to be more efficient.
> rbd_read_header() doesn't even examine the snapshot context unless
> the snapshot count (stored in header->total_snaps) matches the
> number of snapshots allocated.  So rbd_header_from_disk() doesn't
> need to allocate or fill in the snapshot context field at all in
> that case.
>
> Signed-off-by: Alex Elder <elder@inktank.com>

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

> ---
>   drivers/block/rbd.c |   15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
>
> Index: b/drivers/block/rbd.c
> ===================================================================
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -541,7 +541,14 @@ static int rbd_header_from_disk(struct r
>       header->comp_type = ondisk->options.comp_type;
>       header->total_snaps = snap_count;
>
> -    /* Set up the snapshot context */
> +    /*
> +     * If the number of snapshot ids provided by the caller
> +     * doesn't match the number in the entire context there's
> +     * no point in going further.  Caller will try again after
> +     * getting an updated snapshot context from the server.
> +     */
> +    if (allocated_snaps != snap_count)
> +        return 0;
>
>       size = sizeof (struct ceph_snap_context);
>       size += snap_count * sizeof (header->snapc->snaps[0]);
> @@ -553,8 +560,10 @@ static int rbd_header_from_disk(struct r
>       header->snapc->seq = le64_to_cpu(ondisk->snap_seq);
>       header->snapc->num_snaps = snap_count;
>
> -    if (snap_count && allocated_snaps == snap_count) {
> -        int i;
> +    /* Fill in the snapshot information */
> +
> +    if (snap_count) {
> +        u32 i;
>
>           for (i = 0; i < snap_count; i++) {
>               header->snapc->snaps[i] =
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html