Re: [PATCH]shm to sysfs ready (?)

From: Patrick Mochel (mochel@osdl.org)
Date: Mon Aug 04 2003 - 13:01:41 EST


> 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/



This archive was generated by hypermail 2b29 : Thu Aug 07 2003 - 22:00:24 EST