Page MenuHomeFreeBSD

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

Authored by markj on Jul 17 2020, 2:40 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Mar 29, 8:50 AM
Unknown Object (File)
Jan 18 2024, 7:03 PM
Unknown Object (File)
Jan 4 2024, 9:41 PM
Unknown Object (File)
Dec 20 2023, 3:00 AM
Unknown Object (File)
Dec 12 2023, 6:11 AM
Unknown Object (File)
Dec 11 2023, 12:50 PM
Unknown Object (File)
Nov 6 2023, 7:14 PM
Unknown Object (File)
Oct 31 2023, 1:04 PM
Subscribers

Details

Summary

PR: 240545
MFC after: 1 week

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 32437
Build 29914: arc lint + arc unit

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

This one can happen in bootloader.

sys/dev/usb/usb_transfer.c
1346

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