Re: [PATCH] libata, freezer: avoid block device removal while system is frozen

Rafael J. Wysocki
2013-12-18T21:35:15Z

On Wednesday, December 18, 2013 06:17:26 AM Tejun Heo wrote:
> Hello, Rafael.
>
> On Wed, Dec 18, 2013 at 01:35:13AM +0100, Rafael J. Wysocki wrote:
> > So do I understand correctly that you're talking about kernel threads/worqueues
> > freezing below?
>
> Yeap, I'm strictly talking about kernel freezables.
>
> > > So, are you saying it's really about giving device drivers easy way to
> > > implement suspend/resume?
> >
> > Well, that's a side effect rather than a recommeded interface. A *few* pieces
> > of code need to freeze kernel threads/workqueues, but they should know who they
> > are and they really really should know *why* they need that (the above-mentioned
> > runtime PM workqueue is one example). The rest is just doing that because they
> > can, which may not be entirely reasonable (or because they did that in the past
> > and the original author is not responsive and everyone else does not dare to try
> > removing that).
>
> I see. In the long term, I think the right thing to do is making the
> freezer interface more specific so that only the ones which actually
> need it do so explicitly. Right now, kernel freezables are
> conceptually at a very high level - it's a global task attribute and a
> major knob in workqueue. I suppose most of that is historical but by
> perpetuating the model we're encouraging misuse of freezer in large
> swaths of the kernel. Even in this specific case, both writeback and
> jbd workers have no fundamental reason to be freezable and yet
> they're, eventually developing into totally unnecessary deadlocks.

You're right, but I'm not sure how we can make the interface for workqueues
more specific, for example. I guess we can simply drop create_freezable_workqueue()
so that whoever wants to create a freezable workqueue has to use the right
combination of flags. Can we make it more specific than that?

BTW, pm_start_workqueue(), which is a legitimate user, doesn't even use that macro. :-)

> > They were a lot more of that to start with really. We removed quite a number
> > of try_to_freeze() instances from the kernel a few years ago, but apparently
> > people are taking shortcuts.
>
> Great, so we're at least headed towards the right direction.
>
> > The rule should be to require patch submitters to put in comments explaining
> > why they need their kernel threads/workqueues to be freezable and generally
> > there should be no such things in drivers.
>
> I'm not so sure whether that's something which can be effectively
> enforced given the current high visibility and confusion around
> freezer. I think the only way to get this under control is weed out
> the current spurious users actively, deprecate the existing interface
> and switch the real ones to something a lot more specific.

That'd be fine by me modulo the above remarks.

Thanks,
Rafael