Messages in this thread Patch in this message | | | From | Elias Oltmanns <> | Subject | Block: Trouble with kobject initialisation for blk_cmd_filter | Date | Fri, 05 Sep 2008 18:48:07 +0200 |
| |
Hi all,
current usage of the kobject in struct blk_cmd_filter is flawed. Doing # modprobe -r sd-mod && modprobe sd-mod while, for instance, a usb-stick is plugged in currently results in nasty warnings and a dump_stack(). Since blk_cmd_filter is embedded in struct request_queue, I don't see the need for a kobject anyway. What about the much simpler option of a struct attribute_group in this particular case?
This would imply that the cmd_filter subdirectory would appear under sda/queue/ rather than sda/ (which is probably the right place) but, alas, we have to keep compatibility in mind. So I've made some changes to sysfs along the way in order to provide a legacy symlink. I'd have to seperate these two changes for submission but I wanted to know your opinion about it all first.
Thinking about it now makes me wonder whether this is too much for a rc patch and whether we should just allocate the struct blk_cmd_filter dynamically and have done with it. Anyway, tell me what you think.
Regards,
Elias
--- Applies to 2.6.27-rc5-git7.
block/blk-sysfs.c | 6 -- block/blk.h | 6 ++ block/cmd-filter.c | 113 +++++++++++++++---------------------------------- fs/sysfs/symlink.c | 48 ++++++++++++++++---- include/linux/blkdev.h | 1 include/linux/sysfs.h | 2 6 files changed, 81 insertions(+), 95 deletions(-)
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c index 304ec73..0120c8e 100644 --- a/block/blk-sysfs.c +++ b/block/blk-sysfs.c @@ -9,12 +9,6 @@ #include "blk.h" -struct queue_sysfs_entry { - struct attribute attr; - ssize_t (*show)(struct request_queue *, char *); - ssize_t (*store)(struct request_queue *, const char *, size_t); -}; - static ssize_t queue_var_show(unsigned int var, char *page) { diff --git a/block/blk.h b/block/blk.h index c79f30e..9ab0d6a 100644 --- a/block/blk.h +++ b/block/blk.h @@ -10,6 +10,12 @@ extern struct kmem_cache *blk_requestq_cachep; extern struct kobj_type blk_queue_ktype; +struct queue_sysfs_entry { + struct attribute attr; + ssize_t (*show)(struct request_queue *, char *); + ssize_t (*store)(struct request_queue *, const char *, size_t); +}; + void init_request_from_bio(struct request *req, struct bio *bio); void blk_rq_bio_prep(struct request_queue *q, struct request *rq, struct bio *bio); diff --git a/block/cmd-filter.c b/block/cmd-filter.c index 228b644..fc0f0b2 100644 --- a/block/cmd-filter.c +++ b/block/cmd-filter.c @@ -26,6 +26,8 @@ #include <scsi/scsi.h> #include <linux/cdrom.h> +#include "blk.h" + int blk_verify_command(struct blk_cmd_filter *filter, unsigned char *cmd, int has_write_perm) { @@ -50,9 +52,9 @@ int blk_verify_command(struct blk_cmd_filter *filter, EXPORT_SYMBOL(blk_verify_command); /* and now, the sysfs stuff */ -static ssize_t rcf_cmds_show(struct blk_cmd_filter *filter, char *page, - int rw) +static ssize_t rcf_cmds_show(struct request_queue *q, char *page, int rw) { + struct blk_cmd_filter *filter = &q->cmd_filter; char *npage = page; unsigned long *okbits; int i; @@ -76,24 +78,27 @@ static ssize_t rcf_cmds_show(struct blk_cmd_filter *filter, char *page, return npage - page; } -static ssize_t rcf_readcmds_show(struct blk_cmd_filter *filter, char *page) +static ssize_t rcf_readcmds_show(struct request_queue *q, char *page) { - return rcf_cmds_show(filter, page, READ); + return rcf_cmds_show(q, page, READ); } -static ssize_t rcf_writecmds_show(struct blk_cmd_filter *filter, - char *page) +static ssize_t rcf_writecmds_show(struct request_queue *q, char *page) { - return rcf_cmds_show(filter, page, WRITE); + return rcf_cmds_show(q, page, WRITE); } -static ssize_t rcf_cmds_store(struct blk_cmd_filter *filter, - const char *page, size_t count, int rw) +static ssize_t rcf_cmds_store(struct request_queue *q, const char *page, + size_t count, int rw) { + struct blk_cmd_filter *filter = &q->cmd_filter; unsigned long okbits[BLK_SCSI_CMD_PER_LONG], *target_okbits; int cmd, set; char *p, *status; + if (!capable(CAP_SYS_RAWIO)) + return -EPERM; + if (rw == READ) { memcpy(&okbits, filter->read_ok, sizeof(okbits)); target_okbits = filter->read_ok; @@ -128,31 +133,25 @@ static ssize_t rcf_cmds_store(struct blk_cmd_filter *filter, return count; } -static ssize_t rcf_readcmds_store(struct blk_cmd_filter *filter, - const char *page, size_t count) +static ssize_t rcf_readcmds_store(struct request_queue *q, const char *page, + size_t count) { - return rcf_cmds_store(filter, page, count, READ); + return rcf_cmds_store(q, page, count, READ); } -static ssize_t rcf_writecmds_store(struct blk_cmd_filter *filter, - const char *page, size_t count) +static ssize_t rcf_writecmds_store(struct request_queue *q, const char *page, + size_t count) { - return rcf_cmds_store(filter, page, count, WRITE); + return rcf_cmds_store(q, page, count, WRITE); } -struct rcf_sysfs_entry { - struct attribute attr; - ssize_t (*show)(struct blk_cmd_filter *, char *); - ssize_t (*store)(struct blk_cmd_filter *, const char *, size_t); -}; - -static struct rcf_sysfs_entry rcf_readcmds_entry = { +static struct queue_sysfs_entry rcf_readcmds_entry = { .attr = { .name = "read_table", .mode = S_IRUGO | S_IWUSR }, .show = rcf_readcmds_show, .store = rcf_readcmds_store, }; -static struct rcf_sysfs_entry rcf_writecmds_entry = { +static struct queue_sysfs_entry rcf_writecmds_entry = { .attr = {.name = "write_table", .mode = S_IRUGO | S_IWUSR }, .show = rcf_writecmds_show, .store = rcf_writecmds_store, @@ -164,72 +163,30 @@ static struct attribute *default_attrs[] = { NULL, }; -#define to_rcf(atr) container_of((atr), struct rcf_sysfs_entry, attr) - -static ssize_t -rcf_attr_show(struct kobject *kobj, struct attribute *attr, char *page) -{ - struct rcf_sysfs_entry *entry = to_rcf(attr); - struct blk_cmd_filter *filter; - - filter = container_of(kobj, struct blk_cmd_filter, kobj); - if (entry->show) - return entry->show(filter, page); - - return 0; -} - -static ssize_t -rcf_attr_store(struct kobject *kobj, struct attribute *attr, - const char *page, size_t length) -{ - struct rcf_sysfs_entry *entry = to_rcf(attr); - struct blk_cmd_filter *filter; - - if (!capable(CAP_SYS_RAWIO)) - return -EPERM; - - if (!entry->store) - return -EINVAL; - - filter = container_of(kobj, struct blk_cmd_filter, kobj); - return entry->store(filter, page, length); -} - -static struct sysfs_ops rcf_sysfs_ops = { - .show = rcf_attr_show, - .store = rcf_attr_store, -}; - -static struct kobj_type rcf_ktype = { - .sysfs_ops = &rcf_sysfs_ops, - .default_attrs = default_attrs, +static struct attribute_group rcf_attr_group = { + .name = "cmd_filter", + .attrs = default_attrs, }; int blk_register_filter(struct gendisk *disk) { + struct kobject *kobj = &disk->queue->kobj; int ret; - struct blk_cmd_filter *filter = &disk->queue->cmd_filter; - struct kobject *parent = kobject_get(disk->holder_dir->parent); - if (!parent) - return -ENODEV; + ret = sysfs_create_group(kobj, &rcf_attr_group); + if (!ret) + ret = sysfs_create_link_to_group(disk->holder_dir->parent, + kobj, rcf_attr_group.name, + rcf_attr_group.name); + WARN_ON(ret); - ret = kobject_init_and_add(&filter->kobj, &rcf_ktype, parent, - "%s", "cmd_filter"); - - if (ret < 0) - return ret; - - return 0; + return ret; } EXPORT_SYMBOL(blk_register_filter); void blk_unregister_filter(struct gendisk *disk) { - struct blk_cmd_filter *filter = &disk->queue->cmd_filter; - - kobject_put(&filter->kobj); - kobject_put(disk->holder_dir->parent); + sysfs_remove_link(disk->holder_dir->parent, rcf_attr_group.name); + sysfs_remove_group(&disk->queue->kobj, &rcf_attr_group); } EXPORT_SYMBOL(blk_unregister_filter); diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c index a3ba217..d2813a1 100644 --- a/fs/sysfs/symlink.c +++ b/fs/sysfs/symlink.c @@ -19,22 +19,16 @@ #include "sysfs.h" -static int sysfs_do_create_link(struct kobject *kobj, struct kobject *target, +static int sysfs_sd_create_link(struct sysfs_dirent *parent_sd, + struct sysfs_dirent *target_sd, const char *name, int warn) { - struct sysfs_dirent *parent_sd = NULL; - struct sysfs_dirent *target_sd = NULL; struct sysfs_dirent *sd = NULL; struct sysfs_addrm_cxt acxt; int error; BUG_ON(!name); - if (!kobj) - parent_sd = &sysfs_root; - else - parent_sd = kobj->sd; - error = -EFAULT; if (!parent_sd) goto out_put; @@ -43,8 +37,8 @@ static int sysfs_do_create_link(struct kobject *kobj, struct kobject *target, * sysfs_assoc_lock. Fetch target_sd from it. */ spin_lock(&sysfs_assoc_lock); - if (target->sd) - target_sd = sysfs_get(target->sd); + if (target_sd) + target_sd = sysfs_get(target_sd); spin_unlock(&sysfs_assoc_lock); error = -ENOENT; @@ -77,6 +71,19 @@ static int sysfs_do_create_link(struct kobject *kobj, struct kobject *target, return error; } +static int sysfs_do_create_link(struct kobject *kobj, struct kobject *target, + const char *name, int warn) +{ + struct sysfs_dirent *parent_sd; + + if (!kobj) + parent_sd = &sysfs_root; + else + parent_sd = kobj->sd; + + return sysfs_sd_create_link(parent_sd, target->sd, name, warn); +} + /** * sysfs_create_link - create symlink between two objects. * @kobj: object whose directory we're creating the link in. @@ -104,6 +111,26 @@ int sysfs_create_link_nowarn(struct kobject *kobj, struct kobject *target, return sysfs_do_create_link(kobj, target, name, 0); } +int sysfs_create_link_to_group(struct kobject *kobj, struct kobject *target, + const char *group, const char *linkname) +{ + struct sysfs_dirent *sd; + int ret; + + BUG_ON(!kobj || !target); + + sd = sysfs_get_dirent(target->sd, group); + if (!sd) { + WARN(!sd, KERN_WARNING "sysfs group %p not found for " + "kobject '%s'\n", group, kobject_name(target)); + return -ENOENT; + } + + ret = sysfs_sd_create_link(kobj->sd, sd, linkname, 1); + sysfs_put(sd); + return ret; +} + /** * sysfs_remove_link - remove symlink in object's directory. * @kobj: object we're acting for. @@ -213,4 +240,5 @@ const struct inode_operations sysfs_symlink_inode_operations = { EXPORT_SYMBOL_GPL(sysfs_create_link); +EXPORT_SYMBOL_GPL(sysfs_create_link_to_group); EXPORT_SYMBOL_GPL(sysfs_remove_link); diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 44710d7..ca616b7 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -286,7 +286,6 @@ struct blk_queue_tag { struct blk_cmd_filter { unsigned long read_ok[BLK_SCSI_CMD_PER_LONG]; unsigned long write_ok[BLK_SCSI_CMD_PER_LONG]; - struct kobject kobj; }; struct request_queue diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h index 37fa241..e9e0971 100644 --- a/include/linux/sysfs.h +++ b/include/linux/sysfs.h @@ -116,6 +116,8 @@ int sysfs_add_file_to_group(struct kobject *kobj, const struct attribute *attr, const char *group); void sysfs_remove_file_from_group(struct kobject *kobj, const struct attribute *attr, const char *group); +int sysfs_create_link_to_group(struct kobject *kobj, struct kobject *target, + const char *grpname, const char *linkname); void sysfs_notify(struct kobject *kobj, char *dir, char *attr);
| |