On Fri, 23 Apr 2021 12:24:57 -0400 Eric Farman <far...@linux.ibm.com> wrote:
> On Fri, 2021-04-23 at 09:22 -0400, Matthew Rosato wrote: > > So, this looks OK to me. > > +1 (Thanks for doing the research, Matt) +1 to both the analysis and the thanks :) > > > > > > > > handle any ioctl failure gracefully), but worth a second look. In > > > fact, > > > we already unregister the crw irq unconditionally, so we would > > > likely > > > already have seen any problems for that one, so I assume all is > > > good. > > > > > > > But +1 on driving the path and making sure it works anyway (do a > > double-unregister?) > > Yeah, works fine. Tried skipping the register of the CRW and double- > unregistering the IO IRQ. > > I also tried a combination where I unconditionally unregister the REQ > IRQ, which obviously throws a message when it doesn't exist on the > host. Good, thanks for double-checking. > > That might be nice to clean up so that adding new IRQs in the future is > more intuitive; I'll add it to the list unless you want me to address > it in a v2 of this. (Previously, the addition of the REQ IRQ needed to > add the cleanup of the CRW IRQ. So the next IRQ would need to cleanup > the REQ IRQ.) Yeah, let's just do it on top.