[DTrace-devel] [PATCH 03/04] rawtp: report lockmem issues when determining rawtp argument count

[Eugene Loh]

On 11/27/23 16:28, Kris Van Hees wrote:

> On Mon, Nov 27, 2023 at 04:07:51PM -0500, Eugene Loh wrote:
>> On 11/22/23 22:22, Kris Van Hees wrote:
>>
>>> On Wed, Nov 22, 2023 at 10:01:37PM -0500, Eugene Loh via DTrace-devel wrote:
>>>> On 11/22/23 10:49, Kris Van Hees via DTrace-devel wrote:
>>>>
>>>>> diff --git a/libdtrace/dt_prov_rawtp.c b/libdtrace/dt_prov_rawtp.c
>>>>> @@ -181,7 +181,9 @@ static int probe_info(dtrace_hdl_t *dtp, const dt_probe_t *prp,
>>>>>     		dif.dtdo_len = 2;
>>>>>     		bpf_fd = dt_bpf_prog_load(dt_rawtp.prog_type, &dif, 0, NULL, 0);
>>>>> -		if (bpf_fd == -1)
>>>>> +		if (bpf_fd == -EPERM)
>>>> Is that right?  I think the only negative value is -1.  Might it be that you
>>>> want to test errno==+EPERM?  And do we know that EPERM really means
>>>> lockmem?  Or are we simply guessing that that's the most likely explanation?
>>> It is actually correct, or as correct as we can get.  A result value of
>>> -EPERM is almost always a lockmem issue, based on the reading of the kernel
>>> code that relates to this.  And -EPERM *is* -1.
>>>
>>> But other error codes can be returned also, for other failures.
>> What I wrote might have been pretty confusing.  Let me try again. The return
>> value is either a legal value or else it is -1 (which coincidentally is
>> -EPERM).  In order to distinguish among different errors, we cannot test the
>> return value;  we have to test errno.
> Ah true, I meant to ensure that when dt_bpf_prog_load() fails, it would return
> -errno but never made that change (but thought I did).  Duh.
>
> Let me look at this - I probably will make that change anyway and then this
> will be correct, because it makes sense to return -errno on error and a valid
> fd on success.

Okay, I guess.  The dt_bpf_prog_load() does have other call sites. Some 
expect the return value to be -errno (which would be correct if you make 
the change you propose) while another checks errno explicitly (believing 
that EPERM means a helper function is unavailable rather than being a 
symptom of lockmem problems).

>>>> # dtrace -lv > out.default
>>>> # dtrace -xlockmem=1 -lv > out.small
>>>> # wc -l out.*
>>>>    1445230 out.default
>>>>    1380822 out.small
>>>>    2826052 total
>>>> # grep lockmem out.small
>>>> # grep Cannot out.small
>>>> #
>>>>
>>>> Are things working as expected?
>> I'm still curious about this point.  I thought the point of the patch was
>> that if there is a lockmem issue, that issue would be reported.  When I run
>> with -xlockmem=1, the output is truncated significantly, but no telltale
>> error message is printed.
> That is the behaviour you see with just this patch (and not having the later
> patch in the series applied)?  That is odd.  On what OL version and kernel?

OL7.9
5.4.17

FWIW, lockmem stuff is relevant only on these older kernels if one 
believes, e.g., test/unittest/misc/tst.lockmem-cmdline.x and the like:
     # Somehow, UEKR6 (5.4.17) has problems with the the locked-memory 
limit,
     # but UEKR7 (5.15.0) does not
     echo "no locked-memory limit on newer kernels?"

Anyhow, one question is whether one is detecting the lockmem issues 
correctly;  that is the EPERM stuff discussed above.  The other question 
is what happens when we pass the lockmem errmsg up the call stack.  I 
think this part of cmd/dtrace.c is pertinent:

         case DMODE_LIST:
                 for (i = 0; i < g_cmdc; i++)
                         list_prog(&g_cmdv[i]);
                 if (g_cmdc == 0)
                         dtrace_probe_iter(g_dtp, NULL, list_probe, NULL);

It looks like list_prog() -- specifically, its callee list_stmt() -- 
delivers the errmsg to the user while the g_cmdc==0 code path here does 
not.  So "dtrace -xlockmem=1 -lvP rawtp" should deliver the lockmem 
message, while "dtrace -xlockmem=1 -lv" will not deliver the errmsg in 
question.

Let me know if you'd like me to submit a patch or if you want to throw 
it into your lockmem/rawtp series.