Re: [PATCH] block: don't show io_timeout if driver has no timeout handler
From: Bart Van Assche <bvanassche@acm.org>
Date: 2019-01-09 00:57:44
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.
quoted hunk ↗ jump to hunk
@@ -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?
quoted hunk ↗ jump to hunk
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.