Re: [PATCH 05/10] Invalidate or shrink dcache when setting is changed.

[Doug Evans <dje@google.com>]

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 ();
  ...