breaking thread because it grew too big.
On Fri, May 04, 2012 at 07:48:42AM -0300, Mauro Carvalho Chehab wrote:
[ … ]
> + memset(&pos, 0, sizeof(pos));
> + row = 0;
> + chn = 0;
> + debugf4("%s: initializing %d %s\n", __func__, tot_dimms,
> + per_rank ? "ranks" : "dimms");
> + for (i = 0; i < tot_dimms; i++) {
> + chan = &csi[row].channels[chn];
> + dimm = EDAC_DIMM_PTR(layer, mci->dimms, n_layers,
> + pos[0], pos[1], pos[2]);
> + dimm->mci = mci;
> +
> + debugf2("%s: %d: %s%zd (%d:%d:%d): row %d, chan %d\n", __func__,
> + i, per_rank ? "rank" : "dimm", (dimm - mci->dimms),
> + pos[0], pos[1], pos[2], row, chn);
> +
> + /* Copy DIMM location */
> + for (j = 0; j < n_layers; j++)
> + dimm->location[j] = pos[j];
> +
> + /* Link it to the csrows old API data */
> + chan->dimm = dimm;
> + dimm->csrow = row;
> + dimm->cschannel = chn;
> +
> + /* Increment csrow location */
> + for (j = n_layers - 1; j >= 0; j--)
> + if (layers[j].is_virt_csrow)
> + break;
This looks fishy: the for-loop above iterates over layers[j] to break on
the first ->is_virt_csrow.
> + row++;
And row here gets incremented unconditionally, independent from the loop
above. And you're not using any results from the loop: j gets reset
below in the next loop.
What's going on?
> + if (row == tot_csrows) {
> + row = 0;
> + chn++;
> + }
> +
> + /* Increment dimm location */
> + for (j = n_layers - 1; j >= 0; j--) {
> + pos[j]++;
> + if (pos[j] < layers[j].size)
> + break;
> + pos[j] = 0;
> }
> }
>
> @@ -263,6 +373,46 @@ struct mem_ctl_info *edac_mc_alloc(unsigned sz_pvt,
> unsigned nr_csrows,
> */
> return mci;
> }
> +EXPORT_SYMBOL_GPL(new_edac_mc_alloc);
> +
> +/**
> + * edac_mc_alloc: Allocate and partially fill a struct mem_ctl_info structure
> + * @mc_num: Memory controller number
> + * @n_layers: Number of layers at the MC hierarchy
> + * layers: Describes each layer as seen by the Memory Controller
> + * @size_pvt: Size of private storage needed
> + *
> + *
> + * FIXME: drivers handle multi-rank memories in different ways: some
> + * drivers map multi-ranked DIMMs as one DIMM while others
> + * as several DIMMs.
> + *
> + * Everything is kmalloc'ed as one big chunk - more efficient.
> + * It can only be used if all structures have the same lifetime - otherwise
> + * you have to allocate and initialize your own structures.
> + *
> + * Use edac_mc_free() to free mc structures allocated by this function.
> + *
> + * Returns:
> + * On failure: NULL
> + * On success: struct mem_ctl_info pointer
> + */
> +
[ … ]
> +void edac_mc_handle_error(const enum hw_event_mc_err_type type,
> + struct mem_ctl_info *mci,
> + const unsigned long page_frame_number,
> + const unsigned long offset_in_page,
> + const unsigned long syndrome,
> + const int layer0,
> + const int layer1,
> + const int layer2,
> + const char *msg,
> + const char *other_detail,
> + const void *mcelog)
> {
> - int len = EDAC_MC_LABEL_LEN * 4;
> - char labels[len + 1];
> - char *pos = labels;
> - int chan;
> - int chars;
> - char *label = NULL;
> + /* FIXME: too much for stack: move it to some pre-alocated area */
I'm assuming all those new FIXMEs are going to be addressed soonish :)
Rest looks ok,
thanks.
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
