Re: [PATCH] RFC: mmc: block: replace semaphore with freezing

[Arnd Bergmann <arnd@arndb.de>]

On Tuesday, November 22, 2016 9:54:18 AM CET Linus Walleij wrote:
> On Wed, Nov 16, 2016 at 5:32 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Wednesday, November 16, 2016 4:20:47 PM CET Linus Walleij wrote:
> >> On Wed, Nov 16, 2016 at 1:46 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> >>
> >> > 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.

> > 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