On 3/22/18 6:36 AM, Steven Rostedt wrote:
On Wed, 21 Mar 2018 15:05:46 -0700 Alexei Starovoitov <a...@fb.com> wrote:Like the only reason my patch is counting till 17 is because of trace_iwlwifi_dev_ucode_error(). The next offenders are using 12 arguments: trace_mc_event() trace_mm_vmscan_lru_shrink_inactive() Clearly not every efficient usage of it: trace_mm_vmscan_lru_shrink_inactive(pgdat->node_id, nr_scanned, nr_reclaimed, stat.nr_dirty, stat.nr_writeback, stat.nr_congested, stat.nr_immediate, stat.nr_activate, stat.nr_ref_keep, stat.nr_unmap_fail, sc->priority, file); could have passed &stat instead.Yes they should have, and if I was on the Cc for that patch, I would have yelled at them and told them that's exactly what they needed to do. Perhaps I should add something to keep any tracepoint from having more than 6 arguments. That should force a clean up quickly.
I was hesitant to do anything about iwlwifi_dev_ucode_error's 17 args, because when the code is in such shape there are likely more skeletons in the closet. Turned out 'struct iwl_error_event_table' is defined twice with subtle different field names and layout. While the same trace_iwlwifi_dev_ucode_error() is used in what looks like two different cases. I think I managed to refactor it from 17 args to 4 while keeping all bugs in place, but it really should be a job of the author of the code to deal with such oddities. Will send the 'fix' in the next respin. So I definitely support the idea of build time warn for large number of args.
