Roland Dreier wrote:
> > +#include <linux/random.h>
>
> this isn't needed, is it?
Correct, I should have removed it
>
> > + queue_delayed_work(sense->sense_wq , &sense->sense_poll,
> > + round_jiffies(MLX4_SENSE_RANGE));
>
> should be round_jiffies_relative, right?
I did not find critical difference between the two:
unsigned long round_jiffies(unsigned long j)
{
return round_jiffies_common(j, raw_smp_processor_id(), false);
}
unsigned long __round_jiffies_relative(unsigned long j, int cpu)
{
unsigned long j0 = jiffies;
/* Use j0 because jiffies might change while we run */
return round_jiffies_common(j + j0, cpu, false) - j0;
}
>
> > + if (sense->resched)
>
> Do we need resched? Can't we just use cancel_delayed_work_sync() when
> we want to stop this from running?
>
> > +void mlx4_stop_sense(struct mlx4_dev *dev)
> > +{
> > + mlx4_priv(dev)->sense.resched = 0;
> > +}
>
> This doesn't stop anything... we need cancel_delayed_work_sync().
>
> > + sense->sense_wq = create_singlethread_workqueue("mlx4_sense");
>
You are right about this, and I am using cancel_delayed_work_sync() in
mlx4_sense_cleanup().
I was just using the resched flag as an indicator to rearm or not rearm the
task, I guess it is redundant
and just should use cancel_delayed_work_sync() in mlx4_stop_sense();
> Do we really another work queue, or can we share one queue for the
> catastrophic error and port sensing work?
There is no special reason for having another queue other then convenience.
The catastrophic work queue is defined in catas.c
Thanks,
Yevgeny
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
