Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scheduler and performance counters operate on negative return values from general_work() #995

Open
mbr0wn opened this issue Sep 17, 2016 · 2 comments

Comments

@mbr0wn
Copy link
Member

mbr0wn commented Sep 17, 2016

The scheduler and performance counters operate on the special, negative return values from general_work(), which is a mathematically incorrect thing to do.

In the case of gr::block::WORK_DONE, this probably doesn't matter.

In the case of gr::block::WORK_CALLED_PRODUCE, values of the performance counters and the block's relative rate will be wrong.

Only 3 blocks return WORK_CALLED_PRODUCE.

Offending lines:

https://github.com/gnuradio/gnuradio/blob/master/gnuradio-runtime/lib/block_executor.cc#L455 :

  int n = m->general_work(noutput_items, d_ninput_items,
                          d_input_items, d_output_items);

ifdef GR_PERFORMANCE_COUNTERS

  if(d_use_pc)
    d->stop_perf_counters(noutput_items, n);

endif /* GR_PERFORMANCE_COUNTERS */

and https://github.com/gnuradio/gnuradio/blob/master/gnuradio-runtime/lib/block_executor.cc#L472

  if(m->update_rate()) {
    rrate = ((double)(m->nitems_written(0)+n)) / ((double)m->nitems_read(0));
    if(rrate > 0)
      m->set_relative_rate(rrate);
  }

and https://github.com/gnuradio/gnuradio/blob/master/gnuradio-runtime/lib/single_threaded_scheduler.cc#L338 :

    d->produce_each(n);     // advance write pointers
    if(n > 0)
      making_progress = true;

The bad case in this, is someone using the single threaded scheduler and the blocks that return WORK_CALLED_PRODUCE, as it will cause buffer::index_add() to do a very wrong write pointer index update.

@marcusmueller
Copy link
Member

I hope this is fixed, but I'm not sure.

@marcusmueller
Copy link
Member

would be high impact if people were using perf counters. Might be time to supersede them with instrumentation through uprobes, at least on Linux and FreeBSD

@marcusmueller marcusmueller added the Low Issues of low priority. label Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants