svn commit: r217748 - head/sys/netinet/cc

[mdf at FreeBSD.org]

For sbuf use for a sysctl you can use sbuf_init_for_sysctl() which
will, instead of growing, push the current data out using SYSCTL_OUT
to a wired user buffer.  There's a few examples in the vm/ code.  This
can sometimes significantly simplify the code since there's no need to
worry about held mutex/rwlock anymore.

Thanks,
matthew

On Sun, Jan 23, 2011 at 5:00 AM, Lawrence Stewart <lstewart at freebsd.org> wrote:
> Author: lstewart
> Date: Sun Jan 23 13:00:25 2011
> New Revision: 217748
> URL: http://svn.freebsd.org/changeset/base/217748
>
> Log:
>  An sbuf configured with SBUF_AUTOEXTEND will call malloc with M_WAITOK when a
>  write to the buffer causes it to overflow. We therefore can't hold the CC list
>  rwlock over a call to sbuf_printf() for an sbuf configured with SBUF_AUTOEXTEND.
>
>  Switch to a fixed length sbuf which should be of sufficient size except in the
>  very unlikely event that the sysctl is being processed as one or more new
>  algorithms are loaded. If that happens, we accept the race and may fail the
>  sysctl gracefully if there is insufficient room to print the names of all the
>  algorithms.
>
>  This should address a WITNESS warning and the potential panic that would occur
>  if the sbuf call to malloc did sleep whilst holding the CC list rwlock.
>
>  Sponsored by: FreeBSD Foundation
>  Reported by:  Nick Hibma
>  Reviewed by:  bz
>  MFC after:    3 weeks
>  X-MFC with:   r215166
>
> Modified:
>  head/sys/netinet/cc/cc.c
>
> Modified: head/sys/netinet/cc/cc.c
> ==============================================================================
> --- head/sys/netinet/cc/cc.c    Sun Jan 23 12:44:17 2011        (r217747)
> +++ head/sys/netinet/cc/cc.c    Sun Jan 23 13:00:25 2011        (r217748)
> @@ -128,20 +128,37 @@ cc_list_available(SYSCTL_HANDLER_ARGS)
>  {
>        struct cc_algo *algo;
>        struct sbuf *s;
> -       int err, first;
> +       int err, first, nalgos;
>
> -       err = 0;
> +       err = nalgos = 0;
>        first = 1;
> -       s = sbuf_new(NULL, NULL, TCP_CA_NAME_MAX, SBUF_AUTOEXTEND);
> +
> +       CC_LIST_RLOCK();
> +       STAILQ_FOREACH(algo, &cc_list, entries) {
> +               nalgos++;
> +       }
> +       CC_LIST_RUNLOCK();
> +
> +       s = sbuf_new(NULL, NULL, nalgos * TCP_CA_NAME_MAX, SBUF_FIXEDLEN);
>
>        if (s == NULL)
>                return (ENOMEM);
>
> +       /*
> +        * It is theoretically possible for the CC list to have grown in size
> +        * since the call to sbuf_new() and therefore for the sbuf to be too
> +        * small. If this were to happen (incredibly unlikely), the sbuf will
> +        * reach an overflow condition, sbuf_printf() will return an error and
> +        * the sysctl will fail gracefully.
> +        */
>        CC_LIST_RLOCK();
>        STAILQ_FOREACH(algo, &cc_list, entries) {
>                err = sbuf_printf(s, first ? "%s" : ", %s", algo->name);
> -               if (err)
> +               if (err) {
> +                       /* Sbuf overflow condition. */
> +                       err = EOVERFLOW;
>                        break;
> +               }
>                first = 0;
>        }
>        CC_LIST_RUNLOCK();
>