On Mon, 2019-01-07 at 20:52 +0800, Weiping Zhang wrote:
> If the low level driver has no timerout handler, the
^^^^^^^^
timeout?
> +static umode_t queue_attr_visible(struct kobject *kobj, struct attribute
> *attr,
> + int n)
> +{
> + struct request_queue *q =
> + container_of(kobj, struct request_queue, kobj);
> +
> + if (attr == &queue_io_timeout_entry.attr) {
> + if (!q->mq_ops || !q->mq_ops->timeout)
> + return 0;
> + }
Are there any legacy block drivers left? Do we really need the mq_ops test?
Additionally, please combine the two nested if-statements into a single
if-statement.
> @@ -942,6 +961,14 @@ int blk_register_queue(struct gendisk *disk)
> goto unlock;
> }
>
> + ret = sysfs_create_group(&q->kobj, &queue_attr_group);
> + if (ret) {
> + kobject_del(&q->kobj);
> + blk_trace_remove_sysfs(dev);
> + kobject_put(&dev->kobj);
> + goto unlock;
> + }
Are you sure the "goto unlock" is OK here? Shouldn't kobject_del() be called
to undo the kobject_add() call if sysfs_create_group() fails?
> if (queue_is_mq(q)) {
> __blk_mq_register_dev(dev, q);
> blk_mq_debugfs_register(q);
> @@ -958,6 +985,7 @@ int blk_register_queue(struct gendisk *disk)
> if (ret) {
> mutex_unlock(&q->sysfs_lock);
> kobject_uevent(&q->kobj, KOBJ_REMOVE);
> + sysfs_remove_group(&q->kobj, &queue_attr_group);
> kobject_del(&q->kobj);
> blk_trace_remove_sysfs(dev);
> kobject_put(&dev->kobj);
> @@ -1006,6 +1034,7 @@ void blk_unregister_queue(struct gendisk *disk)
> blk_mq_unregister_dev(disk_to_dev(disk), q);
> mutex_unlock(&q->sysfs_lock);
>
> + sysfs_remove_group(&q->kobj, &queue_attr_group);
> kobject_uevent(&q->kobj, KOBJ_REMOVE);
> kobject_del(&q->kobj);
> blk_trace_remove_sysfs(disk_to_dev(disk));
Is it necessary to call sysfs_remove_group() explicitly? Isn't this something
kobject_del() does automatically?
Thanks,
Bart.
