Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 20 Sep 2022 21:20:00 +0100
From:   Arnaldo Carvalho de Melo <acme@kernel.org>
To:     Namhyung Kim <namhyung@kernel.org>
Cc:     Adrian Hunter <adrian.hunter@intel.com>,
        Jiri Olsa <jolsa@redhat.com>, Ian Rogers <irogers@google.com>,
        Daniel Dao <dqminh@cloudflare.com>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        linux-perf-users <linux-perf-users@vger.kernel.org>
Subject: Re: [PATCH] perf kcore_copy: Do not check /proc/modules is unchanged
Message-ID: <YyogcHTpJRKjevIn@kernel.org>
References: <20220914122429.8770-1-adrian.hunter@intel.com>
 <CAM9d7chgWF_iCFeSN0ovoiq=c+JCmD16VVopKzFjjr5En9Eqaw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAM9d7chgWF_iCFeSN0ovoiq=c+JCmD16VVopKzFjjr5En9Eqaw@mail.gmail.com>
Precedence: bulk

Em Fri, Sep 16, 2022 at 11:02:25AM -0700, Namhyung Kim escreveu:
> On Wed, Sep 14, 2022 at 5:24 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
> >
> > /proc/kallsyms and /proc/modules are compared before and after the copy
> > in order to ensure no changes during the copy. However /proc/modules
> > also might change due to reference counts changing even though that
> > does not make any difference. Any modules loaded or unloaded should be
> > visible in changes to kallsyms, so it is not necessary to check
> > /proc/modules also anyway.
> >
> > Remove the comparison checking that /proc/modules is unchanged.
> >
> > Reported-by: Daniel Dao <dqminh@cloudflare.com>
> > Fixes: fc1b691d7651 ("perf buildid-cache: Add ability to add kcore to the cache")
> > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> 
> Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks, applied.

- Arnaldo

 
> Thanks,
> Namhyung
> 
> 
> > ---
> >  tools/perf/util/symbol-elf.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> > index 75bec32d4f57..647b7dff8ef3 100644
> > --- a/tools/perf/util/symbol-elf.c
> > +++ b/tools/perf/util/symbol-elf.c
> > @@ -2102,8 +2102,8 @@ static int kcore_copy__compare_file(const char *from_dir, const char *to_dir,
> >   * unusual.  One significant peculiarity is that the mapping (start -> pgoff)
> >   * is not the same for the kernel map and the modules map.  That happens because
> >   * the data is copied adjacently whereas the original kcore has gaps.  Finally,
> > - * kallsyms and modules files are compared with their copies to check that
> > - * modules have not been loaded or unloaded while the copies were taking place.
> > + * kallsyms file is compared with its copy to check that modules have not been
> > + * loaded or unloaded while the copies were taking place.
> >   *
> >   * Return: %0 on success, %-1 on failure.
> >   */
> > @@ -2166,9 +2166,6 @@ int kcore_copy(const char *from_dir, const char *to_dir)
> >                         goto out_extract_close;
> >         }
> >
> > -       if (kcore_copy__compare_file(from_dir, to_dir, "modules"))
> > -               goto out_extract_close;
> > -
> >         if (kcore_copy__compare_file(from_dir, to_dir, "kallsyms"))
> >                 goto out_extract_close;
> >
> > --
> > 2.25.1
> >

-- 

- Arnaldo