Re: [DTrace-devel] [PATCH v2 2/2] tests, actions: fix tst.symmod.sh

I do not remember the status of this patch, but the test passes for me 
on all my VMs rather consistently.

On 7/26/24 18:59, Kris Van Hees wrote:
> I am renewing my question from Feb 20...  It seems that a v3 of this patch is
> needed to address issues as discussed in the exchange below.  For now, I am
> deferring this patch pending such a v3.
>
> On Tue, Feb 20, 2024 at 02:31:42PM -0500, Eugene Loh via DTrace-devel wrote:
>> One question is what need there is for this test.  I think the "need" was to
>> wean the test suite off of /proc/kallmodsyms.  It's unclear to me what
>> urgency we feel on that.
>>
>> Generalizing to any symbol opens up various cans of worms.  E.g., DTrace
>> won't handle every kall[mod]syms symbol, and the test needs to be tweaked to
>> strip [] off mod names.
>>
>> Maybe for now, there is a simple solution.  I think the test name and
>> location make it clear that it's doing a sanity ("unit") test on the sym()
>> and mod() actions.  Nick points out that mod() is not robustly tested if we
>> only pick one symbol that we know is in vmlinux.  But still, maybe a "good
>> enough" solution for now is something like:
>>
>> - # pick a test symbol from /proc/kallmodsyms
>> + # pick a test symbol from /proc/kallsyms
>> - read ADD NAM MOD <<< `awk '/ksys_write/ {print $1, $4, $5}'
>> /proc/kallmodsyms`
>> + read ADD NAM <<< `awk '/ksys_write/ {print $1, $3}' /proc/kallsyms`
>> - # a blank module means the module is vmlinux
>> + # the module is vmlinux
>>   - if [ x$MOD == x ]; then
>> -     MOD=vmlinux
>> - fi
>> + MOD=vmlinux
>>
>> On 2/20/24 11:43, Kris Van Hees via DTrace-devel wrote:
>>> We still need a v3 posted, I think?
>>>
>>> On Tue, Feb 13, 2024 at 04:02:25PM +0000, Nick Alcock wrote:
>>>> On 13 Feb 2024, Eugene Loh via DTrace-devel told this:
>>>>
>>>>> The comment about never using shuf does not belong in the commit message.
>>>>>
>>>>> The indentation in the file is with tabs, so that style should be preserved.
>>>> ... it was? Whoops :)
>>>>
>>>>> Instead of inflooping (in flooping?), how about "infinitely looping".? (I do find some occurrences of "inflooping" on the internet,
>>>>> but might as well be less slangy?)
>>>> I find 'infinitely looping' clunky. "Looping infinitely" is even
>>>> clunkier. Inflooping is an established term of art, frankly.
>>>> (Irrelevant given the below anyway.)
>>>>
>>>>> Anyhow, instead of the shuffle approach, how about "awk 'NF == 4' /proc/kallsyms" to a temp file and use that?
>>>> Oh yeah that would work, though I don't see what's wrong with using
>>>> shuf...
>>>>
>>>>> But even that won't be enough.? Once you allow "any" symbol from
>>>>> /proc/kallsyms, you risk encountering symbols that DTrace doesn't know
>>>>> about.? Stuff like __ksymtab* and __kstrtab* and others.? I think we
>>>>> filter that stuff out in dt_modsym_addsym().? The same filtering is
>>>>> done in test/unittest/consumer/tst.symbols.c.? It's the headache one
>>>>> saves by going after a specific symbol like ksys_write.
>>>> ... but doing *that* seems like it would render the test nearly useless.
>>>>
>>>> the fact that tst.symmod.sh as I found it was broken for *all modules*
>>>> even though much of its code was meant to handle them suggests that
>>>> whatever it was intended to test, it wasn't actually testing it. (There
>>>> are no comments describing what it's for, which makes it hard to figure
>>>> out how to preserve that property when changing it.)
>>>>
>>>> (I mean among other things it was carefully selecting a guaranteed
>>>> non-modular symbol, so what all this code to figure out what module it
>>>> was in was for, even if it had worked, I have no idea...)
>>>>
>>> _______________________________________________
>>> DTrace-devel mailing list
>>> [email protected]
>>> https://oss.oracle.com/mailman/listinfo/dtrace-devel
>> _______________________________________________
>> DTrace-devel mailing list
>> [email protected]
>> https://oss.oracle.com/mailman/listinfo/dtrace-devel