Page MenuHomeFreeBSD

usb(4): Stop checking for failures from malloc(M_WAITOK).
ClosedPublic

Authored by markj on Jul 17 2020, 2:40 PM.

Details

Summary

PR: 240545
MFC after: 1 week

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markj requested review of this revision.Jul 17 2020, 2:40 PM

The USB driver is also used in boot loader environments where malloc may fail even if M_WAITOK is set. Maybe M_WAITOK should be conditionally set?

I've marked the two cases which can happen from bootloader.

Maybe you can do something like this:

#define USB_HAVE_MALLOC_WAITOK 0

#define USB_HAVE_MALLOC_WAITOK 1

In:

sys/dev/usb/usb_freebsd_loader.h:#define USB_HAVE_UGEN 0
sys/dev/usb/usb_freebsd.h:#define USB_HAVE_UGEN 1

sys/dev/usb/usb_device.c
1788 ↗(On Diff #74573)

This one can happen in bootloader.

sys/dev/usb/usb_transfer.c
1346 ↗(On Diff #74573)

This one can happen in bootloader.

  • Add USB_HAVE_MALLOC_WAITOK, use it in two places.
  • Remove more M_WAITOK handling in ugen_ioctl_post().

I've marked the two cases which can happen from bootloader.

Maybe you can do something like this:

#define USB_HAVE_MALLOC_WAITOK 0

#define USB_HAVE_MALLOC_WAITOK 1

In:

sys/dev/usb/usb_freebsd_loader.h:#define USB_HAVE_UGEN 0
sys/dev/usb/usb_freebsd.h:#define USB_HAVE_UGEN 1

Hmm, thanks. I added some checking for the cases you pointed out. It might make sense to have a usb_malloc() that can take three flags: USB_NOWAIT (always can fail), USB_WAITOK (always expected to succeed), USB_TRYWAIT (use M_WAITOK if the platform guarantees that it will succeed, else use M_NOWAIT).

This revision is now accepted and ready to land.Jul 20 2020, 5:44 PM

Hmm, thanks. I added some checking for the cases you pointed out. It might make sense to have a usb_malloc() that can take three flags: USB_NOWAIT (always can fail), USB_WAITOK (always expected to succeed), USB_TRYWAIT (use M_WAITOK if the platform guarantees that it will succeed, else use M_NOWAIT).

I like the current approach too ...