sys/dev/mrsas/mrsas.c
1197	is there an upper bound here? And why only for panicstr != NULL? I get it's only while we're panicing, but why do we have to do this when we panic and not otherwise.

avg added inline comments.Apr 4 2019, 5:46 PM

sys/dev/mrsas/mrsas.c
1197	I copied this code verbatim from mrsas_detach, so there is no upper limit in both places. And it's actually the other way around, we do this only when we are not panic-ing. This code wakes another thread and waits for it to clear a flag. But the other thread, of course, cannot run if we are in panic.

remove a wrong panicstr check that was left by accident

This looks good to me, however I'd wait a few days to see if Scott can spot something that I can't.

sys/dev/mrsas/mrsas.c
1197	I'm glad to see you got rid of the panicstr here, that's what I was really commenting about.

This revision is now accepted and ready to land.Apr 9 2019, 5:20 PM

In D19817#426388, @imp wrote:

This looks good to me, however I'd wait a few days to see if Scott can spot something that I can't.

I also sent a link to this review to Kashyap Desai.
Waiting for his / Broadcom's reply as well.

avg added inline comments.Apr 9 2019, 7:07 PM

sys/dev/mrsas/mrsas.c
1197	I was so sure that I removed it that I just didn't see it.

sys/dev/mrsas/mrsas.c
1199	This can be done with pps_ratecheck() instead.
1209	This isn't incorrect, but I'm not comfortable with `mesas_shutdown_ctlr()` having an unbounded timeout.

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

sys/

dev/

mrsas/

30 lines

Diff 55797