* Re: [cocci] [BUG] exponentially slow spatch
2023-11-21 20:45 ` [cocci] [BUG] exponentially slow spatch Jesse Brandeburg
@ 2023-11-21 20:58 ` Julia Lawall
2023-11-21 22:34 ` Julia Lawall
2023-11-22 8:20 ` Markus Elfring
2 siblings, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2023-11-21 20:58 UTC (permalink / raw)
To: Jesse Brandeburg; +Cc: Przemek Kitszel, cocci, Jacob Keller
On Tue, 21 Nov 2023, Jesse Brandeburg wrote:
> On 11/9/2023 8:18 AM, Przemek Kitszel wrote:
> > Hi Julia,
> >
> > we were bothered by, what we have called a "flaky spatch" issues
> > in our CI servers, which have manifested as spatch "just exiting",
> > without any proper exit codes.
> > Jake & Jesse found out that processing large files (like 100kloc
> > amalgamate builds...) or parallel processing makes it more common,
> > so we have disabled those.
> > Now I see that those builds were likely killed (or spatch "crashes"?).
> >
> > The issue hit me again, so I dig deeper, and the case is that processing
> > time grows exponentially for seemingly easy case, attached.
> >
> > I'm on github provided sources, as of commit d235ef51f6be ("in modern versions
> > of Linux, PAGE_ALIGNED looks like an ordinary macro call"), running
> > Rhel 9.3 Beta.
> >
> > Przemek
>
> Adding the cocci list since it's been a couple of weeks, if you want me
> to bounce the original mail with the patches to the list please let me know.
Sorry, I think I missed the original email.
I see nothing very alarming about this code
>
> > ---
> > small.cocci | 8 ++++++++
> > y.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 61 insertions(+)
> > create mode 100644 small.cocci
> > create mode 100644 y.c
> >
> > diff --git a/small.cocci b/small.cocci
> > new file mode 100644
> > index 000000000000..6a6cee97e21f
> > --- /dev/null
> > +++ b/small.cocci
> > @@ -0,0 +1,8 @@
> > +@@
> > +expression list E;
> > +expression hw;
> > +@@
> > +(
> > +- ice_info(hw,E);
> > ++ dev_info(ice_hw_to_dev(hw),E);
> > +)
The outer parentheses are not needed. I don't think they should cause bad
behavior, but still no need to have them.
I'll see how this runs on my side.
julia
> > diff --git a/y.c b/y.c
> > new file mode 100644
> > index 000000000000..560ddc26430b
> > --- /dev/null
> > +++ b/y.c
> > @@ -0,0 +1,53 @@
> > +/**
> > + * ice_dump_common_caps - print struct ice_hw_common_caps fields
> > + * @hw: pointer to the ice_hw instance
> > + * @caps: pointer to common caps instance
> > + * @prefix: string to prefix when printing
> > + */
> > +static void
> > +ice_dump_common_caps(struct ice_hw *hw, struct ice_hw_common_caps *caps,
> > + char const *prefix)
> > +{
> > + ice_info(hw, "%s: switching_mode = %d\n", prefix, caps->switching_mode);
> > + ice_info(hw, "%s: mgmt_mode = %d\n", prefix, caps->mgmt_mode);
> > + ice_info(hw, "%s: mgmt_protocols_mctp = %d\n", prefix,
> > + caps->mgmt_protocols_mctp);
> > + ice_info(hw, "%s: os2bmc = %d\n", prefix, caps->os2bmc);
> > + ice_info(hw, "%s: valid_functions (bitmap) = %d\n", prefix,
> > + caps->valid_functions);
> > + ice_info(hw, "%s: sr_iov_1_1 = %d\n", prefix, caps->sr_iov_1_1);
> > + ice_info(hw, "%s: vmdq = %d\n", prefix, caps->vmdq);
> > + ice_info(hw, "%s: evb_802_1_qbg = %d\n", prefix, caps->evb_802_1_qbg);
> > + ice_info(hw, "%s: evb_802_1_qbh = %d\n", prefix, caps->evb_802_1_qbh);
> > + ice_info(hw, "%s: dcb = %d\n", prefix, caps->dcb);
> > + ice_info(hw, "%s: active_tc_bitmap = %d\n", prefix,
> > + caps->active_tc_bitmap);
> > + ice_info(hw, "%s: maxtc = %d\n", prefix, caps->maxtc);
> > + ice_info(hw, "%s: iscsi = %d\n", prefix, caps->iscsi);
> > + ice_info(hw, "%s: rss_table_size = %d\n", prefix, caps->rss_table_size);
> > + ice_info(hw, "%s: rss_table_entry_width = %d\n", prefix,
> > + caps->rss_table_entry_width);
> > + ice_info(hw, "%s: num_rxq = %d\n", prefix, caps->num_rxq);
> > + ice_info(hw, "%s: rxq_first_id = %d\n", prefix, caps->rxq_first_id);
> > + ice_info(hw, "%s: num_txq = %d\n", prefix, caps->num_txq);
> > + ice_info(hw, "%s: txq_first_id = %d\n", prefix, caps->txq_first_id);
> > + ice_info(hw, "%s: num_msix_vectors = %d\n", prefix,
> > + caps->num_msix_vectors);
> > + ice_info(hw, "%s: msix_vector_first_id = %d\n", prefix,
> > + caps->msix_vector_first_id);
> > + ice_info(hw, "%s: ieee_1588 = %d\n", prefix, caps->ieee_1588);
> > + ice_info(hw, "%s: mgmt_cem = %d\n", prefix, caps->mgmt_cem);
> > + ice_info(hw, "%s: iwarp = %d\n", prefix, caps->iwarp);
> > + ice_info(hw, "%s: roce_lag = %d\n", prefix, caps->roce_lag);
> > +} /* This takes 27 seconds as of now, doubles with each ice_info() call added
> > +
> > + ice_info(hw, "%s: wr_csr_prot = 0x%llX\n", prefix,
> > + (unsigned long long)caps->wr_csr_prot);
> > + ice_info(hw, "%s: num_wol_proxy_fltr = %d\n", prefix,
> > + caps->num_wol_proxy_fltr);
> > + ice_info(hw, "%s: wol_proxy_vsi_seid = %d\n", prefix,
> > + caps->wol_proxy_vsi_seid);
> > + ice_info(hw, "%s: max_mtu = %d\n", prefix, caps->max_mtu);
> > + ice_print_led_caps(hw, caps, prefix, false);
> > + ice_print_sdp_caps(hw, caps, prefix, false);
> > +*/
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [cocci] [BUG] exponentially slow spatch
2023-11-21 20:45 ` [cocci] [BUG] exponentially slow spatch Jesse Brandeburg
2023-11-21 20:58 ` Julia Lawall
@ 2023-11-21 22:34 ` Julia Lawall
2023-11-21 22:57 ` Julia Lawall
2023-11-22 8:20 ` Markus Elfring
2 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2023-11-21 22:34 UTC (permalink / raw)
To: Jesse Brandeburg; +Cc: Przemek Kitszel, cocci, Jacob Keller
On Tue, 21 Nov 2023, Jesse Brandeburg wrote:
> On 11/9/2023 8:18 AM, Przemek Kitszel wrote:
> > Hi Julia,
> >
> > we were bothered by, what we have called a "flaky spatch" issues
> > in our CI servers, which have manifested as spatch "just exiting",
> > without any proper exit codes.
> > Jake & Jesse found out that processing large files (like 100kloc
> > amalgamate builds...) or parallel processing makes it more common,
> > so we have disabled those.
> > Now I see that those builds were likely killed (or spatch "crashes"?).
> >
> > The issue hit me again, so I dig deeper, and the case is that processing
> > time grows exponentially for seemingly easy case, attached.
> >
> > I'm on github provided sources, as of commit d235ef51f6be ("in modern versions
> > of Linux, PAGE_ALIGNED looks like an ordinary macro call"), running
> > Rhel 9.3 Beta.
> >
> > Przemek
>
> Adding the cocci list since it's been a couple of weeks, if you want me
> to bounce the original mail with the patches to the list please let me know.
Wow... The characterization of exponential was completely accurate. A
function was redundantly called on its output in the final pretty
printing. The fix will be pushed to github shortly.
Thanks very much for the report!
julia
>
> > ---
> > small.cocci | 8 ++++++++
> > y.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 61 insertions(+)
> > create mode 100644 small.cocci
> > create mode 100644 y.c
> >
> > diff --git a/small.cocci b/small.cocci
> > new file mode 100644
> > index 000000000000..6a6cee97e21f
> > --- /dev/null
> > +++ b/small.cocci
> > @@ -0,0 +1,8 @@
> > +@@
> > +expression list E;
> > +expression hw;
> > +@@
> > +(
> > +- ice_info(hw,E);
> > ++ dev_info(ice_hw_to_dev(hw),E);
> > +)
> > diff --git a/y.c b/y.c
> > new file mode 100644
> > index 000000000000..560ddc26430b
> > --- /dev/null
> > +++ b/y.c
> > @@ -0,0 +1,53 @@
> > +/**
> > + * ice_dump_common_caps - print struct ice_hw_common_caps fields
> > + * @hw: pointer to the ice_hw instance
> > + * @caps: pointer to common caps instance
> > + * @prefix: string to prefix when printing
> > + */
> > +static void
> > +ice_dump_common_caps(struct ice_hw *hw, struct ice_hw_common_caps *caps,
> > + char const *prefix)
> > +{
> > + ice_info(hw, "%s: switching_mode = %d\n", prefix, caps->switching_mode);
> > + ice_info(hw, "%s: mgmt_mode = %d\n", prefix, caps->mgmt_mode);
> > + ice_info(hw, "%s: mgmt_protocols_mctp = %d\n", prefix,
> > + caps->mgmt_protocols_mctp);
> > + ice_info(hw, "%s: os2bmc = %d\n", prefix, caps->os2bmc);
> > + ice_info(hw, "%s: valid_functions (bitmap) = %d\n", prefix,
> > + caps->valid_functions);
> > + ice_info(hw, "%s: sr_iov_1_1 = %d\n", prefix, caps->sr_iov_1_1);
> > + ice_info(hw, "%s: vmdq = %d\n", prefix, caps->vmdq);
> > + ice_info(hw, "%s: evb_802_1_qbg = %d\n", prefix, caps->evb_802_1_qbg);
> > + ice_info(hw, "%s: evb_802_1_qbh = %d\n", prefix, caps->evb_802_1_qbh);
> > + ice_info(hw, "%s: dcb = %d\n", prefix, caps->dcb);
> > + ice_info(hw, "%s: active_tc_bitmap = %d\n", prefix,
> > + caps->active_tc_bitmap);
> > + ice_info(hw, "%s: maxtc = %d\n", prefix, caps->maxtc);
> > + ice_info(hw, "%s: iscsi = %d\n", prefix, caps->iscsi);
> > + ice_info(hw, "%s: rss_table_size = %d\n", prefix, caps->rss_table_size);
> > + ice_info(hw, "%s: rss_table_entry_width = %d\n", prefix,
> > + caps->rss_table_entry_width);
> > + ice_info(hw, "%s: num_rxq = %d\n", prefix, caps->num_rxq);
> > + ice_info(hw, "%s: rxq_first_id = %d\n", prefix, caps->rxq_first_id);
> > + ice_info(hw, "%s: num_txq = %d\n", prefix, caps->num_txq);
> > + ice_info(hw, "%s: txq_first_id = %d\n", prefix, caps->txq_first_id);
> > + ice_info(hw, "%s: num_msix_vectors = %d\n", prefix,
> > + caps->num_msix_vectors);
> > + ice_info(hw, "%s: msix_vector_first_id = %d\n", prefix,
> > + caps->msix_vector_first_id);
> > + ice_info(hw, "%s: ieee_1588 = %d\n", prefix, caps->ieee_1588);
> > + ice_info(hw, "%s: mgmt_cem = %d\n", prefix, caps->mgmt_cem);
> > + ice_info(hw, "%s: iwarp = %d\n", prefix, caps->iwarp);
> > + ice_info(hw, "%s: roce_lag = %d\n", prefix, caps->roce_lag);
> > +} /* This takes 27 seconds as of now, doubles with each ice_info() call added
> > +
> > + ice_info(hw, "%s: wr_csr_prot = 0x%llX\n", prefix,
> > + (unsigned long long)caps->wr_csr_prot);
> > + ice_info(hw, "%s: num_wol_proxy_fltr = %d\n", prefix,
> > + caps->num_wol_proxy_fltr);
> > + ice_info(hw, "%s: wol_proxy_vsi_seid = %d\n", prefix,
> > + caps->wol_proxy_vsi_seid);
> > + ice_info(hw, "%s: max_mtu = %d\n", prefix, caps->max_mtu);
> > + ice_print_led_caps(hw, caps, prefix, false);
> > + ice_print_sdp_caps(hw, caps, prefix, false);
> > +*/
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [cocci] [BUG] exponentially slow spatch
2023-11-21 20:45 ` [cocci] [BUG] exponentially slow spatch Jesse Brandeburg
2023-11-21 20:58 ` Julia Lawall
2023-11-21 22:34 ` Julia Lawall
@ 2023-11-22 8:20 ` Markus Elfring
2023-11-27 19:48 ` Keller, Jacob E
2 siblings, 1 reply; 6+ messages in thread
From: Markus Elfring @ 2023-11-22 8:20 UTC (permalink / raw)
To: Jesse Brandeburg, Przemek Kitszel, Jacob Keller, Julia Lawall,
cocci
>> we were bothered by, what we have called a "flaky spatch" issues
>> in our CI servers, which have manifested as spatch "just exiting",
>> without any proper exit codes.
>> Jake & Jesse found out that processing large files (like 100kloc
>> amalgamate builds...) or parallel processing makes it more common,
>> so we have disabled those.
>> Now I see that those builds were likely killed (or spatch "crashes"?).
>>
>> The issue hit me again, so I dig deeper, and the case is that processing
>> time grows exponentially for seemingly easy case, attached.
…
>> +++ b/small.cocci
>> @@ -0,0 +1,8 @@
>> +@@
>> +expression list E;
>> +expression hw;
>> +@@
>> +(
>> +- ice_info(hw,E);
>> ++ dev_info(ice_hw_to_dev(hw),E);
>> +)
How do you think about to use a script variant (for the semantic patch language)
like the following?
@adjustment@
expression hw;
@@
-ice_info
+dev_info
(
+ ice_hw_to_dev(
hw
+ )
,
...
)
;
Do you observe more desirable run time characteristics for the Coccinelle software
also according to another update?
avoid double call to line_removed
2023-11-21
https://gitlab.inria.fr/coccinelle/coccinelle/-/commit/162a9149cfa92702fa4c26e513cb69e310ab0e10
Regards,
Markus
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [cocci] [BUG] exponentially slow spatch
2023-11-22 8:20 ` Markus Elfring
@ 2023-11-27 19:48 ` Keller, Jacob E
0 siblings, 0 replies; 6+ messages in thread
From: Keller, Jacob E @ 2023-11-27 19:48 UTC (permalink / raw)
To: Markus Elfring, Brandeburg, Jesse, Kitszel, Przemyslaw,
Julia Lawall, cocci@inria.fr
> -----Original Message-----
> From: Markus Elfring <Markus.Elfring@web.de>
> Sent: Wednesday, November 22, 2023 12:20 AM
> To: Brandeburg, Jesse <jesse.brandeburg@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>; Julia
> Lawall <Julia.Lawall@lip6.fr>; cocci@inria.fr
> Subject: Re: [cocci] [BUG] exponentially slow spatch
>
> >> we were bothered by, what we have called a "flaky spatch" issues
> >> in our CI servers, which have manifested as spatch "just exiting",
> >> without any proper exit codes.
> >> Jake & Jesse found out that processing large files (like 100kloc
> >> amalgamate builds...) or parallel processing makes it more common,
> >> so we have disabled those.
> >> Now I see that those builds were likely killed (or spatch "crashes"?).
> >>
> >> The issue hit me again, so I dig deeper, and the case is that processing
> >> time grows exponentially for seemingly easy case, attached.
> …
> >> +++ b/small.cocci
> >> @@ -0,0 +1,8 @@
> >> +@@
> >> +expression list E;
> >> +expression hw;
> >> +@@
> >> +(
> >> +- ice_info(hw,E);
> >> ++ dev_info(ice_hw_to_dev(hw),E);
> >> +)
>
> How do you think about to use a script variant (for the semantic patch language)
> like the following?
>
> @adjustment@
> expression hw;
> @@
> -ice_info
> +dev_info
> (
> + ice_hw_to_dev(
> hw
> + )
> ,
> ...
> )
> ;
>
>
>
> Do you observe more desirable run time characteristics for the Coccinelle
> software
> also according to another update?
>
> avoid double call to line_removed
> 2023-11-21
> https://gitlab.inria.fr/coccinelle/coccinelle/-
> /commit/162a9149cfa92702fa4c26e513cb69e310ab0e10
>
>
> Regards,
> Markus
Yea, Julia reported this change to us. I don't think we've had a chance to test it fully yet, but it looks promising.
^ permalink raw reply [flat|nested] 6+ messages in thread