On Sat, Apr 15, 2023 at 10:44:15PM +0800, Kevin Lo wrote:
> On Fri, Apr 14, 2023 at 02:01:29PM +0200, Alexander Bluhm wrote:
> > I think you are trying to change the kernel in the wrong direction.
> > It should not fail, but handle the requests. Panic if there is a
> > bug.
> >
> > Why do you think M_CANFAIL is a good thing at this place?
>
> Because M_WAITOK will not return NULl, I think adding M_CANFAIL will
> keep the mallocarray call unchanged.
The goal is not to handle errors by failing, but to prevent them.
Better keep M_WAITOK, avoid M_CANFAIL, and remove the check.
Discussion in the hackroom concluded that M_CANFAIL is for input
from userland that can be unlimited large. If userland requests
too much, it can fail. But it is not for normal operation of the
kernel.
M_NOWAIT should be used when the kernel has a good way to deal with
a temporary failure, e.g. drop the packet.
M_CANFAIL | M_WAITOK deals with user input that cannot be fullfilled
permanently and should be reported as an error.
M_WAITOK is for all other cases. If it panics, fix the underlying
bug that requested unrealistic memory size.
Here use number of queues which should be reasonable low and limited
by the driver code. Keep M_WAITOK and remove the error check.
By the way, M_DEVBUF is also wrong. It should be M_TEMP as it is
freed in the same function and not stored permanently. But I wont
fix that now as malloc(9) type usage is far from consistent.
ok?
bluhm
Index: dev/pci/if_igc.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/pci/if_igc.c,v
retrieving revision 1.12
diff -u -p -r1.12 if_igc.c
--- dev/pci/if_igc.c 9 Mar 2023 00:13:47 -0000 1.12
+++ dev/pci/if_igc.c 21 Apr 2023 18:25:35 -0000
@@ -1209,9 +1209,8 @@ igc_rxrinfo(struct igc_softc *sc, struct
struct rx_ring *rxr;
int error, i, n = 0;
- if ((ifr = mallocarray(sc->sc_nqueues, sizeof(*ifr), M_DEVBUF,
- M_WAITOK | M_ZERO)) == NULL)
- return ENOMEM;
+ ifr = mallocarray(sc->sc_nqueues, sizeof(*ifr), M_DEVBUF,
+ M_WAITOK | M_ZERO);
for (i = 0; i < sc->sc_nqueues; i++) {
rxr = &sc->rx_rings[i];
Index: dev/pci/if_ix.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/pci/if_ix.c,v
retrieving revision 1.192
diff -u -p -r1.192 if_ix.c
--- dev/pci/if_ix.c 6 Feb 2023 20:27:44 -0000 1.192
+++ dev/pci/if_ix.c 21 Apr 2023 18:25:35 -0000
@@ -640,9 +640,8 @@ ixgbe_rxrinfo(struct ix_softc *sc, struc
u_int n = 0;
if (sc->num_queues > 1) {
- if ((ifr = mallocarray(sc->num_queues, sizeof(*ifr), M_DEVBUF,
- M_WAITOK | M_ZERO)) == NULL)
- return (ENOMEM);
+ ifr = mallocarray(sc->num_queues, sizeof(*ifr), M_DEVBUF,
+ M_WAITOK | M_ZERO);
} else
ifr = &ifr1;
Index: dev/pci/if_oce.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/pci/if_oce.c,v
retrieving revision 1.106
diff -u -p -r1.106 if_oce.c
--- dev/pci/if_oce.c 11 Mar 2022 18:00:48 -0000 1.106
+++ dev/pci/if_oce.c 21 Apr 2023 18:25:35 -0000
@@ -902,9 +902,8 @@ oce_rxrinfo(struct oce_softc *sc, struct
u_int n = 0;
if (sc->sc_nrq > 1) {
- if ((ifr = mallocarray(sc->sc_nrq, sizeof(*ifr), M_DEVBUF,
- M_WAITOK | M_ZERO)) == NULL)
- return (ENOMEM);
+ ifr = mallocarray(sc->sc_nrq, sizeof(*ifr), M_DEVBUF,
+ M_WAITOK | M_ZERO);
} else
ifr = &ifr1;
