Re: [PATCH] RFC: mmc: block: replace semaphore with freezing
From: Arnd Bergmann <arnd@arndb.de>
Date: 2016-11-22 09:16:20
Also in:
linux-mmc
On Tuesday, November 22, 2016 9:54:18 AM CET Linus Walleij wrote:
On Wed, Nov 16, 2016 at 5:32 PM, Arnd Bergmann [off-list ref] wrote:quoted
On Wednesday, November 16, 2016 4:20:47 PM CET Linus Walleij wrote:quoted
On Wed, Nov 16, 2016 at 1:46 PM, Rafael J. Wysocki [off-list ref] wrote:quoted
Well, we had a session at the KS regarding usage of the freezer on kernel threads and the conclusion was to get rid of that (as opposed to freezing user space, which is necessary IMO). So this change would go in the opposite direction.Aha so I should not make this thread look like everyone else, instead everyone else should look like this thread, haha Ah well, I'll just drop it.It would still be good to remove the semaphore and do something else, as we also want to remove all semaphores. We could check "mq->flags & MMC_QUEUE_SUSPENDED" in the kthread to see if the queue is currently suspended, and otherwise go to sleep there, and then call wake_up() in the resume function.Hm... so simply: if (mq->flags & MMC_QUEUE_SUSPENDED) schedule(); ?
Something like that.
This whole kthread business is pretty messy. I would prefer if I could just convert it to a workqueue. Just that it's not very simple the way it speculates around in the request queue from the block layer.
I don't see how that would work, but might be worth trying. After doing that, a simple flush_workqueue() might be enough to take care of the suspend operation.
quoted
While looking at that code, I just noticed that access to mq->flags is racy and should be fixed as well.I guess we should take the queue lock &q->lock around accessing the flags.
Yes, either that, or use set_bit/test_bit/test_and_set_bit for
atomic access. For instance, this one
if (mq->flags & MMC_QUEUE_NEW_REQUEST) {
mq->flags &= ~MMC_QUEUE_NEW_REQUEST;
Could be
if (test_and_clear(MMC_QUEUE_NEW_REQUEST_BIT, &mq->flags))
...
Arnd