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:   Fri, 9 Jun 2023 16:17:39 -0700
From:   Omar Sandoval <osandov@osandov.com>
To:     Josh Poimboeuf <jpoimboe@kernel.org>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        linux-kernel@vger.kernel.org, linux-debuggers@vger.kernel.org,
        kernel-team@fb.com
Subject: Re: [PATCH] x86/unwind/orc: add ELF section with ORC version number
Message-ID: <ZIOzE4f2xr9CCA45@telecaster>
References: <5b70bc58ef70aab0a821111cd3201eeced8bab95.1686263379.git.osandov@osandov.com>
 <20230609220430.agw2rtswmjbquzom@treble>
 <ZIOmpuqLTJROYQt8@telecaster>
 <20230609224803.275vqw345utsusah@treble>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230609224803.275vqw345utsusah@treble>
Precedence: bulk

On Fri, Jun 09, 2023 at 03:48:03PM -0700, Josh Poimboeuf wrote:
> On Fri, Jun 09, 2023 at 03:24:38PM -0700, Omar Sandoval wrote:
> > > Though, instead of
> > > using an incrementing version, Peter had the idea to hash the struct,
> > > like:
> > > 
> > >   awk '/^struct orc_entry {$/ { p=1 } p { print } /^}/ { p=0 }' arch/x86/include/asm/orc_types.h | sha1sum
> > > 
> > > That way we don't have to remember to bump the version number, and it
> > > would be more resilient to partial backports in distros.
> > > 
> > > Would something like that work for you?
> > 
> > Any sort of unique identifier works for me. One thing that the proposed
> > hash wouldn't catch is if ORC_REG_* or ORC_TYPE_* are ever renumbered
> > (i.e., the meanings of existing values change). It also wouldn't catch
> > if something about the .orc_unwind_ip section changed. But assuming
> > changes like that would be much rarer, it could be handled manually by
> > bumping a "salt" for the hash. E.g., by adding 'BEGIN { print <SALT> }'
> > to the awk script:
> > 
> > awk 'BEGIN { print 1 } /^struct orc_entry {$/ { p=1 } p { print } /^}/ { p=0 }' arch/x86/include/asm/orc_types.h | sha1sum
> > 
> > I'll defer to you guys whether it's easier to remember to bump a version
> > everytime or only in those rare cases.
> 
> I think I'd prefer only bumping it in the rare cases, because we're
> going to end up forgetting either way ;-)
> 
> To catch REG/TYPE changes, we could forego awk and just hash the whole
> file, the only downside being that it might introduce unnecessary
> changes if we change a comment or something.  But the file changes
> rarely enough.
> 
> Or we could tweak the awk to also print ORC_{REG,TYPE}_* pretty easily.

Printing ORC_{REG,TYPE}_* should cover almost everything, and if there's
anything not covered, it can be handled manually, so that sounds good to
me. I'll draft a patch doing that.

Thanks,
Omar