svn commit: r283104 - head/sys/geom/uncompress

[Adrian Chadd]

Hi,

How big are the images you were using?

Did you try on the smaller footprint boards (eg things with < 128MB
RAM) to see if it changed behaviour?


-a


On 19 May 2015 at 02:28, Andrey V. Elsukov <ae at freebsd.org> wrote:
> Author: ae
> Date: Tue May 19 09:28:52 2015
> New Revision: 283104
> URL: https://svnweb.freebsd.org/changeset/base/283104
>
> Log:
>   Read GEOM_UNCOMPRESS metadata using several requests that fit into
>   MAXPHYS. For large compressed images the metadata size can be bigger
>   than MAXPHYS and this triggers KASSERT in g_read_data().
>   Also use g_free() to free memory allocated by g_read_data().
>
>   PR:           199476
>   MFC after:    2 weeks
>
> Modified:
>   head/sys/geom/uncompress/g_uncompress.c
>
> Modified: head/sys/geom/uncompress/g_uncompress.c
> ==============================================================================
> --- head/sys/geom/uncompress/g_uncompress.c     Tue May 19 09:22:06 2015        (r283103)
> +++ head/sys/geom/uncompress/g_uncompress.c     Tue May 19 09:28:52 2015        (r283104)
> @@ -464,7 +464,8 @@ g_uncompress_taste(struct g_class *mp, s
>         struct g_provider *pp2;
>         struct g_consumer *cp;
>         struct g_geom *gp;
> -       uint32_t i, total_offsets, type;
> +       uint64_t *offsets;
> +       uint32_t i, r, total, total_offsets, type;
>         uint8_t *buf;
>         int error;
>
> @@ -499,8 +500,8 @@ g_uncompress_taste(struct g_class *mp, s
>          */
>         DPRINTF(("%s: media sectorsize %u, mediasize %jd\n",
>             gp->name, pp->sectorsize, (intmax_t)pp->mediasize));
> -       i = roundup(sizeof(struct cloop_header), pp->sectorsize);
> -       buf = g_read_data(cp, 0, i, NULL);
> +       total = roundup(sizeof(struct cloop_header), pp->sectorsize);
> +       buf = g_read_data(cp, 0, total, NULL);
>         if (buf == NULL)
>                 goto err;
>         header = (struct cloop_header *) buf;
> @@ -557,20 +558,29 @@ g_uncompress_taste(struct g_class *mp, s
>                     gp->name, sc->nblocks);
>                 goto err;
>         }
> -       free(buf, M_GEOM);
> +       g_free(buf);
>
> -       i = roundup((sizeof(struct cloop_header) +
> -           total_offsets * sizeof(uint64_t)), pp->sectorsize);
> -       buf = g_read_data(cp, 0, i, NULL);
> -       if (buf == NULL)
> -               goto err;
>         sc->offsets = malloc(total_offsets * sizeof(uint64_t),
> -           M_GEOM_UNCOMPRESS, M_WAITOK);
> -       for (i = 0; i <= total_offsets; i++) {
> -               sc->offsets[i] = be64toh(((uint64_t *)
> -                   (buf+sizeof(struct cloop_header)))[i]);
> +           M_GEOM_UNCOMPRESS, M_WAITOK | M_ZERO);
> +       total = roundup((sizeof(struct cloop_header) +
> +           total_offsets * sizeof(uint64_t)), pp->sectorsize);
> +#define        RSZ     ((total - r) > MAXPHYS ? MAXPHYS: (total - r))
> +       for (r = 0, i = 0; r < total; r += MAXPHYS) {
> +               buf = g_read_data(cp, r, RSZ, &error);
> +               if (buf == NULL) {
> +                       free(sc->offsets, M_GEOM_UNCOMPRESS);
> +                       goto err;
> +               }
> +               offsets = (uint64_t *)buf;
> +               if (r == 0)
> +                       offsets +=
> +                           sizeof(struct cloop_header) / sizeof(uint64_t);
> +               for (; i < total_offsets && offsets < (uint64_t *)(buf + RSZ);
> +                   i++, offsets++)
> +                       sc->offsets[i] = be64toh(*offsets);
> +               g_free(buf);
>         }
> -       free(buf, M_GEOM);
> +#undef RSZ
>         buf = NULL;
>         DPRINTF(("%s: done reading offsets\n", gp->name));
>         mtx_init(&sc->last_mtx, "geom_uncompress cache", NULL, MTX_DEF);
> @@ -619,7 +629,7 @@ err:
>         g_topology_lock();
>         g_access(cp, -1, 0, 0);
>         if (buf != NULL)
> -               free(buf, M_GEOM);
> +               g_free(buf);
>         if (gp->softc != NULL) {
>                 g_uncompress_softc_free(gp->softc, NULL);
>                 gp->softc = NULL;
>