From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gdb-patches-return-107282-listarch-gdb-patches=sources.redhat.com@sourceware.org>
Received: (qmail 8632 invoked by alias); 17 Nov 2013 20:34:44 -0000
Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <gdb-patches.sourceware.org>
List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/gdb-patches/>
List-Post: <mailto:gdb-patches@sourceware.org>
List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs>
Sender: gdb-patches-owner@sourceware.org
Received: (qmail 8622 invoked by uid 89); 17 Nov 2013 20:34:43 -0000
Authentication-Results: sourceware.org; auth=none
X-Virus-Found: No
X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=AWL,BAYES_50,RDNS_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no version=3.3.2
X-HELO: mail-ve0-f169.google.com
Received: from Unknown (HELO mail-ve0-f169.google.com) (209.85.128.169) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Sun, 17 Nov 2013 20:34:42 +0000
Received: by mail-ve0-f169.google.com with SMTP id pa12so4510943veb.0        for <gdb-patches@sourceware.org>; Sun, 17 Nov 2013 12:34:34 -0800 (PST)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;        d=1e100.net; s=20130820;        h=x-gm-message-state:mime-version:in-reply-to:references:date         :message-id:subject:from:to:cc:content-type;        bh=k3dvLKGOCx54kYaXB0xew3n/pZ4tgKpF8Jh2TJx4r4Y=;        b=mS2nKu9IXLFEQEoFGBhKX0GqvgJQupAI7UGsYnbW8ylNzkBm5dR4llzs2ugg+o/EQb         DTKB8wTSDUbcwh1NYce4IAei4IvazRCOj+4C1uTFRlGbgdskD1mxqzqm6CRFZoQfoW2L         8Z5QIfjvYWeUoU8Xt3AHLtzAoMrIZl3miBxPSZIphzCo7+UyoeFx8s524+reTsjr/zNC         62LEFFpjShuXx+Xn9qG3MP83r2NTUCymkAYdnleg05tWaVeRQmxCyAdpjqOBdbQtEbiS         pWvZbFNcUPXjjfWTcZSOFTD9ZUkclLCo4NBtS/CvAHTIMVaO6Gis8KVVTh3D3rS4lwCs         H64A==
X-Gm-Message-State: ALoCoQkX9qxS8LKz+hUuIR8tJNrdqe3kKQLi9/mpNleHEIZcOiDp3ooPw7ZnhezsqjrXuEAaGVPLsJVLlR3vOO6gu+/osbrtSvQbrcJ3RhQlWKUHFqCFtMiybTzPGzAwM3h7cyaEezyr4zuddoazBH8TI4sFIn2OOPG8x4rkRlsvrR1oGBPEaEdOXDHIwwcAJB97G836P4wGSXKc2Xt9ITZEzuoZFaK7qg==
MIME-Version: 1.0
X-Received: by 10.58.178.239 with SMTP id db15mr12113456vec.9.1384720474089; Sun, 17 Nov 2013 12:34:34 -0800 (PST)
Received: by 10.52.163.52 with HTTP; Sun, 17 Nov 2013 12:34:34 -0800 (PST)
In-Reply-To: <1383458049-20893-6-git-send-email-yao@codesourcery.com>
References: <1383458049-20893-1-git-send-email-yao@codesourcery.com>	<1383458049-20893-6-git-send-email-yao@codesourcery.com>
Date: Sun, 17 Nov 2013 20:55:00 -0000
Message-ID: <CADPb22R2Di9SVVPeftvXrYjrt+sCkemTeoyJj3r=2Nm-QWRoHA@mail.gmail.com>
Subject: Re: [PATCH 05/10] Invalidate or shrink dcache when setting is changed.
From: Doug Evans <dje@google.com>
To: Yao Qi <yao@codesourcery.com>
Cc: gdb-patches <gdb-patches@sourceware.org>
Content-Type: text/plain; charset=ISO-8859-1
X-IsSubscribed: yes
X-SW-Source: 2013-11/txt/msg00449.txt.bz2

On Sat, Nov 2, 2013 at 10:54 PM, Yao Qi <yao@codesourcery.com> wrote:
> Nowadays, when cache size or line size is changed by command,
> 'target_dcache' is invalidated.  It is too conservative.  We can
> optimize in the following ways,
>
>  - Don't have to invalidate dcache immediately after cache size or
>    line size is changed.  We can postpone the invalidation to the moment
>    using 'target_dcache'.
>  - Don't have to invalidate dcache if the cache size is changed.  If
>    cache size is changed to the value which is still greater than
>    dcache's size, nothing should be done.  If change to the value
>    which is less than dcache's size, just evict cache lines.
>
> This is what this patch does.
>
> gdb:
>
> 2013-11-02  Yao Qi  <yao@codesourcery.com>
>
>         * dcache.c (dcache_evict): New function.
>         (dcache_shrink): New function.
>         (dcache_invalidate_p): New function.
>         (dcache_alloc): Call dcache_evict.
>         (set_dcache_size): Remove call target_dcache_invalidate.
>         (set_dcache_line_size): Likewise.
>         * dcache.h (dcache_shrink): Declare.
>         (dcache_invalidate_p): Declare.
>         * target-dcache.c (target_dcache_get): Invalidate and shrink
>         cache if necessary.
>         (target_dcache_get_or_init): Likewise.
> [...]
> diff --git a/gdb/target-dcache.c b/gdb/target-dcache.c
> index bf04679..28f1aa6 100644
> --- a/gdb/target-dcache.c
> +++ b/gdb/target-dcache.c
> @@ -45,6 +45,14 @@ target_dcache_invalidate (void)
>  DCACHE *
>  target_dcache_get (void)
>  {
> +  if (target_dcache_init_p ())
> +    {
> +      if (dcache_invalidate_p (target_dcache))
> +       dcache_invalidate (target_dcache);
> +
> +      dcache_shrink (target_dcache);
> +    }
> +
>    return target_dcache;
>  }

It feels simpler if this were:

  if (target_dcache_init_p ())
    dcache_check_resize (target_dcache);
  return target_dcache;

There may be a better name than dcache_check_resize, one may want to
call it for situations other than resizings, but here target-dcache.c
is using 3 API routines from dcache.c to achieve what is essentially
one operation: check if the cache needs to be reconfigured and update
if so.

> @@ -54,7 +62,14 @@ target_dcache_get (void)
>  DCACHE *
>  target_dcache_get_or_init (void)
>  {
> -  if (!target_dcache_init_p ())
> +  if (target_dcache_init_p ())
> +    {
> +      if (dcache_invalidate_p (target_dcache))
> +       dcache_invalidate (target_dcache);
> +
> +      dcache_shrink (target_dcache);
> +    }
> +  else
>      target_dcache = dcache_init ();
>
>    return target_dcache;

I think it would be better to remove the duplication and have:

  if (target_dcache_init_p ())
    return target_dcache_get ();
  ...