On Sun, 3 Jun 2007 07:41:35 -0700 (PDT)
Doug Thompson <[EMAIL PROTECTED]> wrote:
> From: Dave Jiang <[EMAIL PROTECTED]>
>
> Move the memory controller object to work queue based implementation
> from the
> kernel thread based.
>
> ...
>
> +#if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,20))
Please avoid doing this in the mainline kernel.
It'll diverge anyway. It is all-round better for people to carry (small)
backport patches outside the kernel.org tree if needed.
> +/*
> + * handler for EDAC to check if NMI type handler has asserted
> interrupt
> + */
> +static int edac_mc_assert_error_check_and_clear(void)
> +{
> + int vreg;
> +
> + if(edac_op_state == EDAC_OPSTATE_POLL)
> + return 1;
To quote Linus "`if' is not a function". Se we use "if (" (multiple
instances).
> + vreg = atomic_read(&edac_err_assert);
> + if(vreg) {
> + atomic_set(&edac_err_assert, 0);
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * edac_mc_workq_function
> + * performs the operation scheduled by a workq request
> + */
> +#if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,20))
> +static void edac_mc_workq_function(struct work_struct *work_req)
> +{
> + struct delayed_work *d_work = (struct delayed_work*) work_req;
> + struct mem_ctl_info *mci = to_edac_mem_ctl_work(d_work);
> +#else
> +static void edac_mc_workq_function(void *ptr)
> +{
> + struct mem_ctl_info *mci = (struct mem_ctl_info *) ptr;
Unneeded (and undesirable) cast.
> +#endif
> +
> + mutex_lock(&mem_ctls_mutex);
> +
> + /* Only poll controllers that are running polled and have a check */
> + if (edac_mc_assert_error_check_and_clear() && (mci->edac_check !=
> NULL))
> + mci->edac_check(mci);
> +
> + /*
> + * FIXME: temp place holder for PCI checks,
> + * goes away when we break out PCI
> + */
> + edac_pci_do_parity_check();
> +
> + mutex_unlock(&mem_ctls_mutex);
> +
> + /* Reschedule */
> + queue_delayed_work(edac_workqueue, &mci->work,
> edac_mc_get_poll_msec());
> +}
> + }
> +}
> +
> +/*
> + * edac_reset_delay_period
> + */
> +
> +void edac_reset_delay_period(struct mem_ctl_info *mci, unsigned long
> value)
> +{
> + mutex_lock(&mem_ctls_mutex);
> +
> + /* cancel the current workq request */
> + edac_mc_workq_teardown(mci);
> +
> + /* restart the workq request, with new delay value */
> + edac_mc_workq_setup(mci, value);
> +
> + mutex_unlock(&mem_ctls_mutex);
> +}
I suspect this is deadlocky, for the reasons described earlier.
> /* Return 0 on success, 1 on failure.
> * Before calling this function, caller must
> * assign a unique value to mci->mc_idx.
> @@ -351,6 +454,16 @@ int edac_mc_add_mc(struct mem_ctl_info *
> goto fail1;
> }
>
> + /* If there IS a check routine, then we are running POLLED */
> + if (mci->edac_check != NULL) {
> + /* This instance is NOW RUNNING */
> + mci->op_state = OP_RUNNING_POLL;
> +
> + edac_mc_workq_setup(mci, edac_mc_get_poll_msec());
> + } else {
> + mci->op_state = OP_RUNNING_INTERRUPT;
> + }
> +
-
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/
