David,
On Mon, Jun 04, 2007 at 07:50:27AM -0700, David Rientjes wrote:
> > + * 02111-1307 USA
> > + */
> > +#include <linux/kernel.h>
> > +#include <linux/perfmon.h>
> > +
> > +/*
> > + * used only in UP mode
> > + */
> > +void pfm_save_prev_context(struct pfm_context *ctxp)
> > +{
>
> Is this comment still valid even though there's a call to this function in
> the self-monitoring case in __pfm_load_context()?
>
Yes. I have added the #ifndef CONFIG_SMP if __pfm_load_context(). The reason
for this is that in UP mode we use lazy save/restore on context switch. Thus
when you come in to load a context on the PMU, you may need to push the previous
context out.
> > +
> > +void pfm_save_pmds(struct pfm_context *ctx, struct pfm_event_set *set)
> > +{
> > + u64 val, hw_val, *pmds, ovfl_mask;
> > + u64 *used_mask, *cnt_mask;
> > + u16 i, num;
> > +
> > + ovfl_mask = pfm_pmu_conf->ovfl_mask;
> > + num = set->nused_pmds;
> > + cnt_mask = pfm_pmu_conf->cnt_pmds;
> > + used_mask = set->used_pmds;
> > + pmds = set->view->set_pmds;
> > +
> > + for (i = 0; num; i++) {
>
> This should probably be "i++, num--".
>
No. Num represents the number of PMD registers we need to read. We only
read the PMD registers we use. Thus num needs to be decremented only when
i is the index of a *used* PM register.
> > + if (test_bit(i, ulp(used_mask))) {
> > + hw_val = val = pfm_read_pmd(ctx, i);
> > + if (likely(test_bit(i, ulp(cnt_mask))))
> > + val = (pmds[i] & ~ovfl_mask) |
> > + (hw_val & ovfl_mask);
> > + pmds[i] = val;
> > + num--;
> > + }
> > + }
> > +}
>
> You can elimiate the hw_val automatic by eliminating its assignment and
> replacing its use with val.
>
Done.
Thanks for your feedback.
--
-Stephane
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
