Re: [PATCH] vfio/ccw: Remove WARN_ON during shutdown

[Eric Farman <farman@linux.ibm.com>]

On Fri, 2023-02-10 at 14:30 -0700, Alex Williamson wrote:
> On Fri, 10 Feb 2023 18:42:27 +0100
> Eric Farman <farman@linux.ibm.com> wrote:
> 
> > The logic in vfio_ccw_sch_shutdown() always assumed that the input
> > subchannel would point to a vfio_ccw_private struct, without
> > checking
> > that one exists. The blamed commit put in a check for this
> > scenario,
> > to prevent the possibility of a missing private.
> > 
> > The trouble is that check was put alongside a WARN_ON(), presuming
> > that such a scenario would be a cause for concern. But this can be
> > triggered by binding a subchannel to vfio-ccw, and rebooting the
> > system before starting the mdev (via "mdevctl start" or similar)
> > or after stopping it. In those cases, shutdown doesn't need to
> > worry because either the private was never allocated, or it was
> > cleaned up by vfio_ccw_mdev_remove().
> > 
> > Remove the WARN_ON() piece of this check, since there are plausible
> > scenarios where private would be NULL in this path.
> > 
> > Fixes: 9e6f07cd1eaa ("vfio/ccw: create a parent struct")
> > Signed-off-by: Eric Farman <farman@linux.ibm.com>
> > ---
> >  drivers/s390/cio/vfio_ccw_drv.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/s390/cio/vfio_ccw_drv.c
> > b/drivers/s390/cio/vfio_ccw_drv.c
> > index 54aba7cceb33..ff538a086fc7 100644
> > --- a/drivers/s390/cio/vfio_ccw_drv.c
> > +++ b/drivers/s390/cio/vfio_ccw_drv.c
> > @@ -225,7 +225,7 @@ static void vfio_ccw_sch_shutdown(struct
> > subchannel *sch)
> >         struct vfio_ccw_parent *parent = dev_get_drvdata(&sch-
> > >dev);
> >         struct vfio_ccw_private *private = dev_get_drvdata(&parent-
> > >dev);
> >  
> > -       if (WARN_ON(!private))
> > +       if (!private)
> >                 return;
> >  
> >         vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_CLOSE);
> 
> I see I'm on the To: line here, is this intended to go through the
> vfio
> tree rather than s390? 

Either way. I put you as "to" as the blamed commit went via vfio, but I
could ask Heiko or Vasili to take it if that's easier.

Eric