Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 19 Mar 2012 21:06:36 +0100
From:      Davide Italiano <davide.italiano@gmail.com>
To:        Gleb Smirnoff <glebius@freebsd.org>, George Neville-Neil <gnn@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r233045 - in head/sys: conf kern
Message-ID:  <CACYV=-HnKmkcw-p2i9o_VyKGkp6V5yzO0B61uCBOAebg=VHd-A@mail.gmail.com>
In-Reply-To: <CACYV=-EmbDNep1rPwF6jfHB2M8JuhLDFvrsvZoBMuVB7kOC1bA@mail.gmail.com>
References:  <201203162032.q2GKWBRV033715@svn.freebsd.org> <20120319194431.GD30704@FreeBSD.org> <CACYV=-EmbDNep1rPwF6jfHB2M8JuhLDFvrsvZoBMuVB7kOC1bA@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help

On Mon, Mar 19, 2012 at 8:54 PM, Davide Italiano
<davide.italiano@gmail.com> wrote:
> 2012/3/19 Gleb Smirnoff <glebius@freebsd.org>:
>>  Davide,
>>
>> On Fri, Mar 16, 2012 at 08:32:11PM +0000, Davide Italiano wrote:
>> D> Author: davide
>> D> Date: Fri Mar 16 20:32:11 2012
>> D> New Revision: 233045
>> D> URL: http://svn.freebsd.org/changeset/base/233045
>> D>
>> D> Log:
>> D>   Add rudimentary profiling of the hash table used in the in the umtx code to
>> D>   hold active lock queues.
>> D>
>> D>   Reviewed by:       attilio
>> D>   Approved by:       davidxu, gnn (mentor)
>> D>   MFC after: 3 weeks
>> D>
>> D> Modified:
>> D>   head/sys/conf/NOTES
>> D>   head/sys/conf/options
>> D>   head/sys/kern/kern_umtx.c
>>
>> ...
>>
>> D>  static void
>> D>  umtxq_sysinit(void *arg __unused)
>> D>  {
>> D> @@ -232,8 +265,15 @@ umtxq_sysinit(void *arg __unused)
>> D>                      TAILQ_INIT(&umtxq_chains[i][j].uc_pi_list);
>> D>                      umtxq_chains[i][j].uc_busy = 0;
>> D>                      umtxq_chains[i][j].uc_waiters = 0;
>> D> +                    #ifdef UMTX_PROFILING
>> D> +                    umtxq_chains[i][j].length = 0;
>> D> +                    umtxq_chains[i][j].max_length = 0;
>> D> +                    #endif
>> D>              }
>> D>      }
>> D> +    #ifdef UMTX_PROFILING
>> D> +    umtx_init_profiling();
>> D> +    #endif
>> D>      mtx_init(&umtx_lock, "umtx lock", NULL, MTX_SPIN);
>> D>      EVENTHANDLER_REGISTER(process_exec, umtx_exec_hook, NULL,
>> D>          EVENTHANDLER_PRI_ANY);
>> D> @@ -384,6 +424,14 @@ umtxq_insert_queue(struct umtx_q *uq, in
>> D>
>> D>      TAILQ_INSERT_TAIL(&uh->head, uq, uq_link);
>> D>      uh->length++;
>> D> +    #ifdef UMTX_PROFILING
>> D> +    uc->length++;
>> D> +    if (uc->length > uc->max_length) {
>> D> +            uc->max_length = uc->length;
>> D> +            if (uc->max_length > max_length)
>> D> +                    max_length = uc->max_length;
>> D> +    }
>> D> +    #endif
>> D>      uq->uq_flags |= UQF_UMTXQ;
>> D>      uq->uq_cur_queue = uh;
>> D>      return;
>> D> @@ -401,6 +449,9 @@ umtxq_remove_queue(struct umtx_q *uq, in
>> D>              uh = uq->uq_cur_queue;
>> D>              TAILQ_REMOVE(&uh->head, uq, uq_link);
>> D>              uh->length--;
>> D> +            #ifdef UMTX_PROFILING
>> D> +            uc->length--;
>> D> +            #endif
>> D>              uq->uq_flags &= ~UQF_UMTXQ;
>> D>              if (TAILQ_EMPTY(&uh->head)) {
>> D>                      KASSERT(uh->length == 0,
>>
>> These indented ifdefs look like a major violation of style used throughout
>> the FreeBSD kernel code. Can you please keep with common style?
>>
>> --
>> Totus tuus, Glebius.
>
> Heh,
> sorry, also Juli Mallet noticed this, I'm writing a fix for this and
> after I'll have approval from my mentor I'll commit.
>
> Davide

This should fix:
http://people.freebsd.org/~davide/umtx_stylefix.diff
Can you plase give it a closer look?
George, if everythin' is ok, can I have also your approval?



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACYV=-HnKmkcw-p2i9o_VyKGkp6V5yzO0B61uCBOAebg=VHd-A>