rationale for sysfs attr store/show accepting kobj_attribute?

[Jim Cromie]

On Sun, May 8, 2011 at 3:23 AM, Robert P. J. Day <rpjday at crashcourse.ca> wrote:
>
>  more just a curiosity than anything else, but i'm perusing the
> kobject sample programs in the kernel source directory and in
> kobject-example.c, there's one thing about which i'm a bit puzzled.
>
>  the kernel header file sysfs.h defines the basic attribute structure
> as:
>
> struct attribute {
>        const char              *name;
>        mode_t                  mode;
> #ifdef CONFIG_DEBUG_LOCK_ALLOC
>        struct lock_class_key   *key;
>        struct lock_class_key   skey;
> #endif
> };
>
> in other words, simply the attribute name and mode.  no problem.
>
>  however, the header file kobject.h defines a more informative
> kobj_attribute structure thusly:
>
> struct kobj_attribute {
>        struct attribute attr;
>        ssize_t (*show)(struct kobject *kobj, struct kobj_attribute *attr,
>                        char *buf);
>        ssize_t (*store)(struct kobject *kobj, struct kobj_attribute *attr,
>                         const char *buf, size_t count);
> };
>
> as you can see, it's just a regular sysfs attribute, bundled with the
> respective show and store routines.
>
>  what puzzles me is why those show() and store() routines are defined
> as being passed the larger kobj_attribute structure, rather than the
> simpler sysfs attribute structure.  it doesn't seem as if there's much
> value in having the show() and store() callbacks getting those two
> extra pieces of information.


well, theyre passing pointers, so "larger" doesnt matter.
the larger structure just has more info available.
Now whether pointers to show & store are particularly useful
from inside a show / store routine is debatable, as you noted.
A quick grep didnt show me anything, but my eye isnt
as discerning as Id like.

>
>  in fact, in the sample program kobject-example.c, in the more
> general example, the callback routine needs to check the name of the
> attribute to know how to process it properly so it has to use
> "attr->attr.name" to access the field of the simpler "struct
> attribute" anyway.
>

thats to be expected.
looking up the name is far better than having many separate show/store
routines, each doing essentially the same thing, for
in0, in1, in2 ...

hwmon drivers used to do that (multiple fns, defined via macros),
the situation is much improved now, though I think a few still remain.

FWIW, there are SENSOR_ATTRs, which extend/subclass plain-old
struct attrs (modulo capitalization, approximations),
maybe that possibility is what kobj_attributes are about.

static struct sensor_device_attribute fan_min[] = {
	SENSOR_ATTR(fan1_min, S_IWUSR | S_IRUGO, show_fan_min, set_fan_min, 0),
	SENSOR_ATTR(fan2_min, S_IWUSR | S_IRUGO, show_fan_min, set_fan_min, 1),
	SENSOR_ATTR(fan3_min, S_IWUSR | S_IRUGO, show_fan_min, set_fan_min, 2),
};

hwmon-sysfs.h - hardware monitoring chip driver sysfs defines

struct sensor_device_attribute{
	struct device_attribute dev_attr;
	int index;
};
struct sensor_device_attribute_2 {
	struct device_attribute dev_attr;
	u8 index;
	u8 nr;
};

in device.h:

struct bus_attribute {
	struct attribute	attr;
	ssize_t (*show)(struct bus_type *bus, char *buf);
	ssize_t (*store)(struct bus_type *bus, const char *buf, size_t count);
};
struct driver_attribute {
	struct attribute attr;
	ssize_t (*show)(struct device_driver *driver, char *buf);
	ssize_t (*store)(struct device_driver *driver, const char *buf,
			 size_t count);
};
struct class_attribute {
	struct attribute attr;
	ssize_t (*show)(struct class *class, struct class_attribute *attr,
			char *buf);
	ssize_t (*store)(struct class *class, struct class_attribute *attr,
			const char *buf, size_t count);
};
struct device_attribute {
	struct attribute	attr;
	ssize_t (*show)(struct device *dev, struct device_attribute *attr,
			char *buf);
	ssize_t (*store)(struct device *dev, struct device_attribute *attr,
			 const char *buf, size_t count);
};

I dont fully grok it, but theres clearly a pattern here.


>  long question short -- even though it's clearly an arbitrary
> decision, was there some rationale for having those callback show()
> and store() routines be passed a pointer to the larger attribute
> structure containing the show() and store() routine addresses
> themselves (in this case, kobj_attribute)?  or is everything those
> callback routines might be interested in available in the simpler
> "struct attribute"?
>
>  and, of course, if one wanted to get the enclosing attribute
> structure, there's always "container_of", but i'd just like to know if
> there was a reason for the choice that was made.
>
> rday
>
> --