Re: [PATCH] block: don't show io_timeout if driver has no timeout handler

[Bart Van Assche <bvanassche@acm.org>]

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.