Re: [PATCH 2/4] swait: add the missing killable swaits

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jun 29, 2017 at 10:40 AM, Davidlohr Bueso <dave@xxxxxxxxxxxx> wrote:
>
> For all the above, what do you think of my 'sswait' proposal?

I see no actual users of such a specialty interface.

I don't think we've *ever* had any actual problems with our current
wait-queues, apart from the RT issues, which were not about the
waitqueues themselves, but purely about RT itself.

So without some very compelling reason, I'd not want to add yet
another wait-queue.

I actually think swait is pure garbage. Most users only wake up one
process anyway, and using swait for that is stupid. If you only wake
up one, you might as well just have a single process pointer, not a
wait list at all, and then use "wake_up_process()".

There is *one* single user of swake_up_all(), and that one looks like
bogus crap also: it does it outside of the spinlock that could have
been used to protect the queue - p,lus I'm not sure there's really a
queue anyway, since I think it's just the grace-period kthread that is
there.

kvm uses swait, but doesn't use swake_up_all(), so it always just
wakes up a single waiter. That may be the right thing to do. Or it's
just another bug. I don't know. The KVM use looks broken too, since it
does

        if (swait_active(wqp)) {
                swake_up(wqp);

which is racy wrt new waiters, which implies that there is some
upper-level synchronization - possibly the same "only one thread
anyway".

So swake really looks like crap. It has crap semantics, it has crap
users, it's just broken.

The last thing we want to do is to create something _else_ specialized
like this.

                Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux