svn commit: r255439 - head/sys/dev/cpuctl

[Matthew Fleming]

On Mon, Sep 9, 2013 at 10:17 PM, Konstantin Belousov <kib at freebsd.org>wrote:

> Author: kib
> Date: Tue Sep 10 05:17:53 2013
> New Revision: 255439
> URL: http://svnweb.freebsd.org/changeset/base/255439
>
> Log:
>   Call free() on the pointer returned from malloc().
>
>   Reported and tested by:       Oliver Pinter <oliver.pntr at gmail.com>
>   Sponsored by: The FreeBSD Foundation
>   MFC after:    3 days
>   Approved by:  re (delphij)
>
> Modified:
>   head/sys/dev/cpuctl/cpuctl.c
>
> Modified: head/sys/dev/cpuctl/cpuctl.c
>
> ==============================================================================
> --- head/sys/dev/cpuctl/cpuctl.c        Tue Sep 10 03:48:18 2013
>  (r255438)
> +++ head/sys/dev/cpuctl/cpuctl.c        Tue Sep 10 05:17:53 2013
>  (r255439)
> @@ -295,10 +295,10 @@ cpuctl_do_update(int cpu, cpuctl_update_
>  static int
>  update_intel(int cpu, cpuctl_update_args_t *args, struct thread *td)
>  {
> -       void *ptr = NULL;
> +       void *ptr;
>         uint64_t rev0, rev1;
>         uint32_t tmp[4];
> -       int is_bound = 0;
> +       int is_bound;
>         int oldcpu;
>         int ret;
>
> @@ -312,10 +312,11 @@ update_intel(int cpu, cpuctl_update_args
>         }
>
>         /*
> -        * 16 byte alignment required.
> +        * 16 byte alignment required.  Rely on the fact that
> +        * malloc(9) always returns the pointer aligned at least on
> +        * the size of the allocation.
>          */
>         ptr = malloc(args->size + 16, M_CPUCTL, M_WAITOK);
> -       ptr = (void *)(16 + ((intptr_t)ptr & ~0xf));
>         if (copyin(args->data, ptr, args->size) != 0) {
>                 DPRINTF("[cpuctl,%d]: copyin %p->%p of %zd bytes failed",
>                     __LINE__, args->data, ptr, args->size);
> @@ -408,10 +409,10 @@ fail:
>  static int
>  update_via(int cpu, cpuctl_update_args_t *args, struct thread *td)
>  {
> -       void *ptr = NULL;
> +       void *ptr;
>         uint64_t rev0, rev1, res;
>         uint32_t tmp[4];
> -       int is_bound = 0;
> +       int is_bound;
>         int oldcpu;
>         int ret;
>
> @@ -427,8 +428,7 @@ update_via(int cpu, cpuctl_update_args_t
>         /*
>          * 4 byte alignment required.
>          */
> -       ptr = malloc(args->size + 16, M_CPUCTL, M_WAITOK);
> -       ptr = (void *)(16 + ((intptr_t)ptr & ~0xf));
> +       ptr = malloc(args->size, M_CPUCTL, M_WAITOK);
>         if (copyin(args->data, ptr, args->size) != 0) {
>                 DPRINTF("[cpuctl,%d]: copyin %p->%p of %zd bytes failed",
>                     __LINE__, args->data, ptr, args->size);
>

I don't know exactly what the stock malloc(9) will return, but memguard(9),
under its default mode with vm.memguard.options having MG_GUARD_AROUND set
will align the returned pointer to only 16 bytes.  When I added that
feature I almost made it 8 bytes, but I think I saw that uma(9) had a
16-byte alignment so I preserved that.  I.e., this code does still work
with malloc(9) and memguard(9).

But why does this need 16 byte alignment?  Especially when one of the
comments says 4-byte alignment?

Thanks,
matthew