Re: [PATCH 01/12] usbnet: introduce usbnet 3 command helpers

[Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org>]

On Wednesday 10 October 2012 13:56:16 Ming Lei wrote:
> On Wed, Oct 10, 2012 at 11:19 AM, Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> wrote:
> > On Tue, Oct 9, 2012 at 4:47 PM, Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> wrote:
> >>
> >> Using GFP_KERNEL you preclude using those in resume() and error handling.
> >> Please pass a gfp_t parameter.
> >
> > IMO, it is not a big deal because generally only several bytes are to be
> > allocated inside these helpers.
> 
> Also pm_restrict_gfp_mask()/pm_restore_gfp_mask() have been introduced
> to address the problem for 2 years, looks the gfp_t isn't needed, doesn't it?

No, absolutely not. Introducing them was a mistake and is hiding errors.
Those helpers solve the problem only for the case of _system_ suspend/resume.
However the runtime case has the same problem. So in addition to not solving
the problem, we now have two code paths.
Frankly, those functions should be removed.

Secondly, in this case a similar deadlock exists with error handling.
Again take a device with network and storage functions (a.k.a. cell phone).
The storage function does a reset. And the deadlock happens like this:

reset storage -> pre_reset() -> physical reset -> post_reset() -> net interface
does a control message -> kmalloc(..., GFP_KERNEL) -> VM layer decide
to page out -> IO to storage function -> SCSI layer waits for error handler --> DEADLOCK

Believe me, you won't find a fancy solution for this. Just pass the gfp_t.

	Regards
		Oliver

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