Re: [PATCH v5] modules: add heuristic when stripping unneeded symbols

From: Vimal Agrawal <avimalin@gmail.com>
To: Luis Chamberlain <mcgrof@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Michal Marek <michal.lkml@markovi.net>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Jan Beulich <JBeulich@suse.com>, Jeff Mahoney <jeffm@suse.com>,
	Sam Ravnborg <sam@ravnborg.org>,
	linux-kbuild@vger.kernel.org, jeyu@kernel.org,
	linux-kernel@vger.kernel.org, nishit.shah@sophos.com,
	Vimal Agrawal <vimal.agrawal@sophos.com>,
	Dirk VanDerMerwe <Dirk.VanDerMerwe@sophos.com>,
	gregkh@linuxfoundation.org
Subject: Re: [PATCH v5] modules: add heuristic when stripping unneeded symbols
Date: Tue, 8 Feb 2022 23:42:02 +0530	[thread overview]
Message-ID: <CALkUMdQBEWANnNM1UYKcKOmAK=d9Abt1TmgvqNKSKyW9Vqd1JQ@mail.gmail.com> (raw)
In-Reply-To: <YgKugFcF1kkWPsNa@bombadil.infradead.org>

> You are saying that sometimes having "best" evaluated to non zero
> yields incorrect results, where the symbol found is actualy not a .text
> symbol for a text address? If so, is this really true for cases where
> no stripping is used? If so this is bigger news and I'd like to address
> this separately in another commit but we need proof, not just
> speculation.
>
I have not seen this issue without stripping so far as in that case it will find
some better match with some .text address but I have seen this consistently
with stripping.

> And you seem to be suggesting that this seems to hold more true when
> "--strip-unneeded" is used given there are fewer symbols left after
> striping?
>
yes. This is seen with stripping only.

> > without stripping, it will definitely
> > resolve to some .text address closely matching in case of no stripping
>
> OK so there is no issue when stripping is used.
>
yes. I assume you meant when stripping is not used.

> > but it can go wrong with stripping. I have seen it a few times post
> > the first patch during testing.
>
> OK then we need to take care your added heuristics do not affect
> non-stripping.
>
yes. so I tested .init , .text and one data address (without being
stripped) to make
sure there is no affect. Attached the result in my previous mail.

> > best could be incorrect even for text address when --strip-unneeded is used.
> > e.g. in my case, it is resolving .init.text address to __this_module
>
> You should be explicit about this in your commit log.
>
Ok

> When you submit a v2 patch and you change something like that you must
> clarify changes which are not clear either in the commit log or below
> the --- lines after the diffstat and before the actual patch. Each new
> patch iteration should have a set of bullets with all the changes you
> have made so that the maintainer can track what you have done
> differently on each iteration.
>
> Right now you are not making any of this easy on me so I ask that you
> stop submitting new patches willy nilly until we have actualy discussed
> each item, and we decide what to do. I also ask that you keep track of
> each change you are making on each new patch iteration on the patch
> after the --- lines and before the patch, so I can easily tell all the
> changes you have made on each new iteration.
>
Sure. Thanks for educating me.

>   Luis