Messages in this thread | | | Date | Mon, 4 Aug 2003 11:01:41 -0700 (PDT) | From | Patrick Mochel <> | Subject | Re: [PATCH]shm to sysfs ready (?) |
| |
> Summary : > > -shm destruction moved to kobject release function > -id seek was trivial.Attrshow rewritten. > -Tested against recursive fgreps-kde-personal benchmark program > -Patched might_sleep problem > -Checking release code in front of all shm_destroy calls > -No more MAJ charset for attributes > -Adding EOL to attr values. > -Working against 2.6.0.test2 > > Is it ok for you ?
A few comments below.
> +#include <linux/kobj_map.h>
You shouldn't need to include this.
> + strcpy(shm_ids.kobj.name, "shm"); > + shm_ids.kobj.parent = NULL; > + shm_ids.kobj.kset = NULL; > + shm_ids.kobj.ktype = NULL; > + kobject_register(&shm_ids.kobj);
The kobject internals rely on the entire kobject being zero-filled. You should be memset'ing it (and probably the entire object) earlier, which saves you from manually initializing the fields above to 0, and prevents you from missing any in the future.
> +static ssize_t shm_attr_show(struct kobject *kobj, struct attribute *attr, char *buf) > +{ > + unsigned int id=simple_strtoul(kobj->name,NULL,10)%SEQ_MULTIPLIER; > + struct shmid_kernel *shp; > + down (&shm_ids.sem); > + shp=shm_lock(id); > + if(!strcmp(attr->name,"key")) > + snprintf(buf, PAGE_SIZE,"%ld\n", (long)shp->shm_perm.key); ...
If you're going to do it this way, I suggest declaring the attributes before the show method, so you can do a pointer comparison, rather than a string compare:
+ if(attr == &shm_attr_key)) + snprintf(buf, PAGE_SIZE,"%ld\n", (long)shp->shm_perm.key); ...
> +#define SHM_ATTR(_name) \ > +static struct attribute shm_attr_##_name={ \ > + .name=__stringify(_name), \ > + .mode=0444, \ > +}; \
Also, you should declare all the attributes here, rather than in the registration function. They will only be defined once, and will not put unnecessary pressure on the stack.
struct kobj_type::default_attrs is designed to hold an array of attributes that are added to each kobject registered of that type. This is done automatically for you, so you don't have to do it manually. (It also checks the errors as it adds each attribute). I suggest using that.
Thanks,
-pat
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |