On Fri, 2011-03-25 at 12:00 +0100, Robert Richter wrote:
> On 21.01.11 05:06:52, Heinz Graalfs wrote:
> > From: Heinz Graalfs <graalfs@xxxxxxxxxxxxxxxxxx>
> >
> > System z's hardware sampling is described in detail in:
> >
> > SA23-2260-01 "The Load-Program-Parameter and CPU-Measurement Facilities"
> >
> > The patch introduces
> > - support for System z's hardware sampler in OProfile's kernel module
> > - it adds functions that control all hardware sampling related operations as
> > - checking if hardware sampling feature is available
> > - ie: on System z models z10 and up, in LPAR mode only, and authorised during LPAR activation
> > - allocating memory for the hardware sampling feature
> > - starting/stopping hardware sampling
> >
> > All functions required to start and stop hardware sampling have to be
> > invoked by the oprofile kernel module as provided by the other patches of this patch set.
> >
> > In case hardware based sampling cannot be setup standard timer based sampling is used by OProfile.
> >
> > Signed-off-by: Mahesh Salgaonkar <mahesh@xxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Maran Pakkirisamy <maranp@xxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Heinz Graalfs <graalfs@xxxxxxxxxxxxxxxxxx>
> > ---
> > arch/Kconfig | 3
> > arch/s390/Kconfig | 1
> > arch/s390/oprofile/hwsampler.c | 1256 +++++++++++++++++++++++++++++++++++++++++
> > arch/s390/oprofile/hwsampler.h | 113 +++
> > 4 files changed, 1373 insertions(+)
>
> > +int hwsampler_setup()
> > +{
> > + int rc;
> > + int cpu;
> > + struct hws_cpu_buffer *cb;
> > +
> > + mutex_lock(&hws_sem);
> > +
> > + rc = -EINVAL;
> > + if (hws_state)
> > + goto setup_exit;
> > +
> > + hws_state = HWS_INIT;
>
> hws_state is never set to zero again, so we will pass this point only
> one time, even after hwsampler_shutdown(). Is this intended? Only
> loading and unloading this as module would work.
>
> Maybe we clear hws_state in hwsampler_shutdown()?
>
> -Robert
>
yes, the intent is:
hwsampler_setup() is called on module load - oprofile_arch_init()
and
hwsampler_shutdown() is called on unload - oprofile_arch_exit()
hws_state is reset in hwsampler_shutdown()
hwsampler_shutdown processing only succeeds if we are in states
HWS_DEALLOCATED or HWS_STOPPED.
Usually this is ensured by the current approach the functions are
called, and in this case hws_state is also reset to HWS_INIT.
In case hwsampler_shutdown is called in other 'invalid states' it should
return -EINVAL.
However, in this case, the unregister_cpu_notifier() would be executed,
which is not good and should be avoided.
Although, current processing, never calls hwsampler_shutdown() in
invalid states, I suppose, it might better if we do the unregister call
as intended only if hwsampler_shutdown succeeds.
I will submit a separate patch.
Heinz
> > +
> > + init_all_cpu_buffers();
> > +
> > + rc = check_hardware_prerequisites();
> > + if (rc)
> > + goto setup_exit;
> > +
> > + rc = check_qsi_on_setup();
> > + if (rc)
> > + goto setup_exit;
> > +
> > + rc = -EINVAL;
> > + hws_wq = create_workqueue("hwsampler");
> > + if (!hws_wq)
> > + goto setup_exit;
> > +
> > + register_cpu_notifier(&hws_cpu_notifier);
> > +
> > + for_each_online_cpu(cpu) {
> > + cb = &per_cpu(sampler_cpu_buffer, cpu);
> > + INIT_WORK(&cb->worker, worker);
> > + rc = smp_ctl_qsi(cpu);
> > + WARN_ON(rc);
> > + if (min_sampler_rate != cb->qsi.min_sampl_rate) {
> > + if (min_sampler_rate) {
> > + printk(KERN_WARNING
> > + "hwsampler: different min sampler rate values.\n");
> > + if (min_sampler_rate < cb->qsi.min_sampl_rate)
> > + min_sampler_rate =
> > + cb->qsi.min_sampl_rate;
> > + } else
> > + min_sampler_rate = cb->qsi.min_sampl_rate;
> > + }
> > + if (max_sampler_rate != cb->qsi.max_sampl_rate) {
> > + if (max_sampler_rate) {
> > + printk(KERN_WARNING
> > + "hwsampler: different max sampler rate values.\n");
> > + if (max_sampler_rate > cb->qsi.max_sampl_rate)
> > + max_sampler_rate =
> > + cb->qsi.max_sampl_rate;
> > + } else
> > + max_sampler_rate = cb->qsi.max_sampl_rate;
> > + }
> > + }
> > + register_external_interrupt(0x1407, hws_ext_handler);
> > +
> > + hws_state = HWS_DEALLOCATED;
> > + rc = 0;
> > +
> > +setup_exit:
> > + mutex_unlock(&hws_sem);
> > + return rc;
> > +}
> > +
> > +int hwsampler_shutdown()
> > +{
> > + int rc;
> > +
> > + mutex_lock(&hws_sem);
> > +
> > + rc = -EINVAL;
> > + if (hws_state == HWS_DEALLOCATED || hws_state == HWS_STOPPED) {
> > + mutex_unlock(&hws_sem);
> > +
> > + if (hws_wq)
> > + flush_workqueue(hws_wq);
> > +
> > + mutex_lock(&hws_sem);
> > +
> > + if (hws_state == HWS_STOPPED) {
> > + smp_ctl_clear_bit(0, 5); /* set bit 58 CR0 off */
> > + deallocate_sdbt();
> > + }
> > + if (hws_wq) {
> > + destroy_workqueue(hws_wq);
> > + hws_wq = NULL;
> > + }
> > +
> > + unregister_external_interrupt(0x1407, hws_ext_handler);
> > + hws_state = HWS_INIT;
> > + rc = 0;
> > + }
> > + mutex_unlock(&hws_sem);
> > +
> > + unregister_cpu_notifier(&hws_cpu_notifier);
> > +
> > + return rc;
> > +}
>
--
To unsubscribe from this list: send the line "unsubscribe linux-s390" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
