Commit b2a4ac0c2860b27670bce99e8c9c281bf431c272

Authored by Doug Thompson
Committed by Linus Torvalds
1 parent ba9a5918c8

drivers/edac: fix edac_device sysfs corner case bug

Some simple fixes to properly reference counter values from the block
attribute level of edac_device objects.  Properly sequencing the array pointer
was added, resulting in correct identification of block level attributes from
their base class functions.

Added more verbose debug statement for event tracking.

Also during some corner testing, found a bug in the store/show sequence
of operations for the block attribute/controls management.

An old intermediate structure for 'blocks' was still in the processing
pipeline.  This patch removes that old structure and correctly utilizes the
new struct edac_dev_sysfs_block_attribute for passing control from the sysfs
to the low level store/show function of the edac driver.

Now the proper kobj pointer to passed downward to the store/show
functions.

Signed-off-by: Doug Thompson <dougthompson@xmission.com>
Cc: Greg KH <greg@kroah.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Showing 3 changed files with 85 additions and 51 deletions Side-by-side Diff

drivers/edac/edac_core.h
... ... @@ -485,7 +485,16 @@
485 485 };
486 486  
487 487 /* edac_dev_sysfs_block_attribute structure
  488 + *
488 489 * used in leaf 'block' nodes for adding controls/attributes
  490 + *
  491 + * each block in each instance of the containing control structure
  492 + * can have an array of the following. The show and store functions
  493 + * will be filled in with the show/store function in the
  494 + * low level driver.
  495 + *
  496 + * The 'value' field will be the actual value field used for
  497 + * counting
489 498 */
490 499 struct edac_dev_sysfs_block_attribute {
491 500 struct attribute attr;
... ... @@ -494,8 +503,6 @@
494 503 const char *, size_t);
495 504 struct edac_device_block *block;
496 505  
497   - /* low driver use */
498   - void *arg;
499 506 unsigned int value;
500 507 };
501 508  
drivers/edac/edac_device.c
... ... @@ -83,7 +83,7 @@
83 83 void *pvt;
84 84 int err;
85 85  
86   - debugf1("%s() instances=%d blocks=%d\n",
  86 + debugf4("%s() instances=%d blocks=%d\n",
87 87 __func__, nr_instances, nr_blocks);
88 88  
89 89 /* Calculate the size of memory we need to allocate AND
... ... @@ -158,6 +158,9 @@
158 158 /* Name of this edac device */
159 159 snprintf(dev_ctl->name,sizeof(dev_ctl->name),"%s",edac_device_name);
160 160  
  161 + debugf4("%s() edac_dev=%p next after end=%p\n",
  162 + __func__, dev_ctl, pvt + sz_private );
  163 +
161 164 /* Initialize every Instance */
162 165 for (instance = 0; instance < nr_instances; instance++) {
163 166 inst = &dev_inst[instance];
... ... @@ -177,8 +180,10 @@
177 180 snprintf(blk->name, sizeof(blk->name),
178 181 "%s%d", edac_block_name, block+offset_value);
179 182  
180   - debugf1("%s() instance=%d block=%d name=%s\n",
181   - __func__, instance, block, blk->name);
  183 + debugf4("%s() instance=%d inst_p=%p block=#%d "
  184 + "block_p=%p name='%s'\n",
  185 + __func__, instance, inst, block,
  186 + blk, blk->name);
182 187  
183 188 /* if there are NO attributes OR no attribute pointer
184 189 * then continue on to next block iteration
185 190  
186 191  
187 192  
188 193  
... ... @@ -191,20 +196,32 @@
191 196 attrib_p = &dev_attrib[block*nr_instances*nr_attrib];
192 197 blk->block_attributes = attrib_p;
193 198  
  199 + debugf4("%s() THIS BLOCK_ATTRIB=%p\n",
  200 + __func__, blk->block_attributes);
  201 +
194 202 /* Initialize every user specified attribute in this
195 203 * block with the data the caller passed in
  204 + * Each block gets its own copy of pointers,
  205 + * and its unique 'value'
196 206 */
197 207 for (attr = 0; attr < nr_attrib; attr++) {
198 208 attrib = &attrib_p[attr];
199   - attrib->attr = attrib_spec->attr;
200   - attrib->show = attrib_spec->show;
201   - attrib->store = attrib_spec->store;
202 209  
203   - /* up reference this block */
204   - attrib->block = blk;
  210 + /* populate the unique per attrib
  211 + * with the code pointers and info
  212 + */
  213 + attrib->attr = attrib_spec[attr].attr;
  214 + attrib->show = attrib_spec[attr].show;
  215 + attrib->store = attrib_spec[attr].store;
205 216  
206   - /* bump the attrib_spec */
207   - attrib_spec++;
  217 + attrib->block = blk; /* up link */
  218 +
  219 + debugf4("%s() alloc-attrib=%p attrib_name='%s' "
  220 + "attrib-spec=%p spec-name=%s\n",
  221 + __func__, attrib, attrib->attr.name,
  222 + &attrib_spec[attr],
  223 + attrib_spec[attr].attr.name
  224 + );
208 225 }
209 226 }
210 227 }
... ... @@ -258,7 +275,7 @@
258 275 struct edac_device_ctl_info *edac_dev;
259 276 struct list_head *item;
260 277  
261   - debugf3("%s()\n", __func__);
  278 + debugf0("%s()\n", __func__);
262 279  
263 280 list_for_each(item, &edac_device_list) {
264 281 edac_dev = list_entry(item, struct edac_device_ctl_info, link);
... ... @@ -402,7 +419,6 @@
402 419 struct delayed_work *d_work = (struct delayed_work *)work_req;
403 420 struct edac_device_ctl_info *edac_dev = to_edac_device_ctl_work(d_work);
404 421  
405   - //debugf0("%s() here and running\n", __func__);
406 422 mutex_lock(&device_ctls_mutex);
407 423  
408 424 /* Only poll controllers that are running polled and have a check */
... ... @@ -582,7 +598,7 @@
582 598 {
583 599 struct edac_device_ctl_info *edac_dev;
584 600  
585   - debugf0("MC: %s()\n", __func__);
  601 + debugf0("%s()\n", __func__);
586 602  
587 603 mutex_lock(&device_ctls_mutex);
588 604  
drivers/edac/edac_device_sysfs.c
... ... @@ -200,7 +200,7 @@
200 200 {
201 201 struct edac_device_ctl_info *edac_dev = to_edacdev(kobj);
202 202  
203   - debugf1("%s() control index=%d\n", __func__, edac_dev->dev_idx);
  203 + debugf4("%s() control index=%d\n", __func__, edac_dev->dev_idx);
204 204  
205 205 /* decrement the EDAC CORE module ref count */
206 206 module_put(edac_dev->owner);
... ... @@ -252,7 +252,7 @@
252 252 edac_dev->kobj.parent = &edac_class->kset.kobj;
253 253  
254 254 /* generate sysfs "..../edac/<name>" */
255   - debugf1("%s() set name of kobject to: %s\n", __func__, edac_dev->name);
  255 + debugf4("%s() set name of kobject to: %s\n", __func__, edac_dev->name);
256 256 err = kobject_set_name(&edac_dev->kobj, "%s", edac_dev->name);
257 257 if (err)
258 258 goto err_out;
... ... @@ -279,7 +279,7 @@
279 279 * edac_device_unregister_sysfs_main_kobj() must be used
280 280 */
281 281  
282   - debugf1("%s() Registered '.../edac/%s' kobject\n",
  282 + debugf4("%s() Registered '.../edac/%s' kobject\n",
283 283 __func__, edac_dev->name);
284 284  
285 285 return 0;
... ... @@ -300,7 +300,7 @@
300 300 struct edac_device_ctl_info *edac_dev)
301 301 {
302 302 debugf0("%s()\n", __func__);
303   - debugf1("%s() name of kobject is: %s\n",
  303 + debugf4("%s() name of kobject is: %s\n",
304 304 __func__, kobject_name(&edac_dev->kobj));
305 305  
306 306 /*
307 307  
308 308  
309 309  
310 310  
311 311  
... ... @@ -416,22 +416,29 @@
416 416  
417 417 /* edac_dev -> instance -> block information */
418 418  
  419 +#define to_block(k) container_of(k, struct edac_device_block, kobj)
  420 +#define to_block_attr(a) \
  421 + container_of(a, struct edac_dev_sysfs_block_attribute, attr)
  422 +
419 423 /*
420 424 * Set of low-level block attribute show functions
421 425 */
422   -static ssize_t block_ue_count_show(struct edac_device_block *block, char *data)
  426 +static ssize_t block_ue_count_show(struct kobject *kobj,
  427 + struct attribute *attr, char *data)
423 428 {
  429 + struct edac_device_block *block = to_block(kobj);
  430 +
424 431 return sprintf(data, "%u\n", block->counters.ue_count);
425 432 }
426 433  
427   -static ssize_t block_ce_count_show(struct edac_device_block *block, char *data)
  434 +static ssize_t block_ce_count_show(struct kobject *kobj,
  435 + struct attribute *attr, char *data)
428 436 {
  437 + struct edac_device_block *block = to_block(kobj);
  438 +
429 439 return sprintf(data, "%u\n", block->counters.ce_count);
430 440 }
431 441  
432   -#define to_block(k) container_of(k, struct edac_device_block, kobj)
433   -#define to_block_attr(a) container_of(a,struct block_attribute,attr)
434   -
435 442 /* DEVICE block kobject release() function */
436 443 static void edac_device_ctrl_block_release(struct kobject *kobj)
437 444 {
438 445  
439 446  
... ... @@ -448,22 +455,16 @@
448 455 kobject_put(&block->instance->ctl->kobj);
449 456 }
450 457  
451   -/* block specific attribute structure */
452   -struct block_attribute {
453   - struct attribute attr;
454   - ssize_t(*show) (struct edac_device_block *, char *);
455   - ssize_t(*store) (struct edac_device_block *, const char *, size_t);
456   -};
457 458  
458 459 /* Function to 'show' fields from the edac_dev 'block' structure */
459 460 static ssize_t edac_dev_block_show(struct kobject *kobj,
460 461 struct attribute *attr, char *buffer)
461 462 {
462   - struct edac_device_block *block = to_block(kobj);
463   - struct block_attribute *block_attr = to_block_attr(attr);
  463 + struct edac_dev_sysfs_block_attribute *block_attr =
  464 + to_block_attr(attr);
464 465  
465 466 if (block_attr->show)
466   - return block_attr->show(block, buffer);
  467 + return block_attr->show(kobj, attr, buffer);
467 468 return -EIO;
468 469 }
469 470  
470 471  
471 472  
... ... @@ -472,11 +473,12 @@
472 473 struct attribute *attr,
473 474 const char *buffer, size_t count)
474 475 {
475   - struct edac_device_block *block = to_block(kobj);
476   - struct block_attribute *block_attr = to_block_attr(attr);
  476 + struct edac_dev_sysfs_block_attribute *block_attr;
477 477  
  478 + block_attr = to_block_attr(attr);
  479 +
478 480 if (block_attr->store)
479   - return block_attr->store(block, buffer, count);
  481 + return block_attr->store(kobj, attr, buffer, count);
480 482 return -EIO;
481 483 }
482 484  
... ... @@ -487,7 +489,7 @@
487 489 };
488 490  
489 491 #define BLOCK_ATTR(_name,_mode,_show,_store) \
490   -static struct block_attribute attr_block_##_name = { \
  492 +static struct edac_dev_sysfs_block_attribute attr_block_##_name = { \
491 493 .attr = {.name = __stringify(_name), .mode = _mode }, \
492 494 .show = _show, \
493 495 .store = _store, \
... ... @@ -497,7 +499,7 @@
497 499 BLOCK_ATTR(ue_count, S_IRUGO, block_ue_count_show, NULL);
498 500  
499 501 /* list of edac_dev 'block' attributes */
500   -static struct block_attribute *device_block_attr[] = {
  502 +static struct edac_dev_sysfs_block_attribute *device_block_attr[] = {
501 503 &attr_block_ce_count,
502 504 &attr_block_ue_count,
503 505 NULL,
504 506  
... ... @@ -524,14 +526,15 @@
524 526 struct edac_dev_sysfs_block_attribute *sysfs_attrib;
525 527 struct kobject *main_kobj;
526 528  
527   - debugf1("%s() Instance '%s' block '%s'\n",
528   - __func__, instance->name, block->name);
  529 + debugf4("%s() Instance '%s' inst_p=%p block '%s' block_p=%p\n",
  530 + __func__, instance->name, instance, block->name, block);
  531 + debugf4("%s() block kobj=%p block kobj->parent=%p\n",
  532 + __func__, &block->kobj, &block->kobj.parent);
529 533  
530 534 /* init this block's kobject */
531 535 memset(&block->kobj, 0, sizeof(struct kobject));
532 536 block->kobj.parent = &instance->kobj;
533 537 block->kobj.ktype = &ktype_block_ctrl;
534   - block->instance = instance;
535 538  
536 539 err = kobject_set_name(&block->kobj, "%s", block->name);
537 540 if (err)
538 541  
539 542  
... ... @@ -560,14 +563,20 @@
560 563 * to the block kobject
561 564 */
562 565 sysfs_attrib = block->block_attributes;
563   - if (sysfs_attrib) {
564   - for (i = 0; i < block->nr_attribs; i++) {
  566 + if (sysfs_attrib && block->nr_attribs) {
  567 + for (i = 0; i < block->nr_attribs; i++, sysfs_attrib++) {
  568 +
  569 + debugf4("%s() creating block attrib='%s' "
  570 + "attrib->%p to kobj=%p\n",
  571 + __func__,
  572 + sysfs_attrib->attr.name,
  573 + sysfs_attrib, &block->kobj);
  574 +
  575 + /* Create each block_attribute file */
565 576 err = sysfs_create_file(&block->kobj,
566   - (struct attribute *) sysfs_attrib);
  577 + &sysfs_attrib->attr);
567 578 if (err)
568 579 goto err_on_attrib;
569   -
570   - sysfs_attrib++;
571 580 }
572 581 }
573 582  
... ... @@ -595,7 +604,9 @@
595 604 */
596 605 sysfs_attrib = block->block_attributes;
597 606 if (sysfs_attrib && block->nr_attribs) {
598   - for (i = 0; i < block->nr_attribs; i++) {
  607 + for (i = 0; i < block->nr_attribs; i++, sysfs_attrib++) {
  608 +
  609 + /* remove each block_attrib file */
599 610 sysfs_remove_file(&block->kobj,
600 611 (struct attribute *) sysfs_attrib);
601 612 }
... ... @@ -653,7 +664,7 @@
653 664 goto err_out;
654 665 }
655 666  
656   - debugf1("%s() now register '%d' blocks for instance %d\n",
  667 + debugf4("%s() now register '%d' blocks for instance %d\n",
657 668 __func__, instance->nr_blocks, idx);
658 669  
659 670 /* register all blocks of this instance */
... ... @@ -669,7 +680,7 @@
669 680 }
670 681 }
671 682  
672   - debugf1("%s() Registered instance %d '%s' kobject\n",
  683 + debugf4("%s() Registered instance %d '%s' kobject\n",
673 684 __func__, idx, instance->name);
674 685  
675 686 return 0;
... ... @@ -848,7 +859,7 @@
848 859 }
849 860  
850 861  
851   - debugf0("%s() calling create-instances, idx=%d\n",
  862 + debugf4("%s() create-instances done, idx=%d\n",
852 863 __func__, edac_dev->dev_idx);
853 864  
854 865 return 0;