Re: [PATCH V2 6/6] block: split .sysfs_lock into two locks

[Ming Lei <ming.lei@redhat.com>]

On Fri, Aug 23, 2019 at 09:46:48AM -0700, Bart Van Assche wrote:
> On 8/21/19 2:15 AM, Ming Lei wrote:
> > @@ -966,7 +966,7 @@ int blk_register_queue(struct gendisk *disk)
> >   		return ret;
> >   	/* Prevent changes through sysfs until registration is completed. */
> > -	mutex_lock(&q->sysfs_lock);
> > +	mutex_lock(&q->sysfs_dir_lock);
> >   	ret = kobject_add(&q->kobj, kobject_get(&dev->kobj), "%s", "queue");
> >   	if (ret < 0) {
> > @@ -987,26 +987,37 @@ int blk_register_queue(struct gendisk *disk)
> >   		blk_mq_debugfs_register(q);
> >   	}
> > -	kobject_uevent(&q->kobj, KOBJ_ADD);
> > -
> > -	wbt_enable_default(q);
> > -
> > -	blk_throtl_register_queue(q);
> > -
> > +	/*
> > +	 * The queue's kobject ADD uevent isn't sent out, also the
> > +	 * flag of QUEUE_FLAG_REGISTERED isn't set yet, so elevator
> > +	 * switch won't happen at all.
> > +	 */
> >   	if (q->elevator) {
> > -		ret = elv_register_queue(q);
> > +		ret = elv_register_queue(q, false);
> >   		if (ret) {
> 
> The above changes seems risky to me. In contrast with what the comment
> suggests, user space code is not required to wait for KOBJ_ADD event to
> start using sysfs attributes. I think user space code *can* write into the
> request queue I/O scheduler sysfs attribute after the kobject_add() call has
> finished and before kobject_uevent(&q->kobj, KOBJ_ADD) is called.

Yeah, one crazy userspace may simply poll on sysfs entres and start to
READ/WRITE before seeing the KOBJ_ADD event.

However, we have another protection via the queue flag QUEUE_FLAG_REGISTERED,
which is set after everything is done. So if userspace's early store
comes, elevator switch still can't happen because the flag is checked in
__elevator_change().

thanks,
Ming