Re: [Intel-gfx] [PATCH v2 7/9] vfio/ccw: Remove private->mdev

Eric Farman
2021-09-28T12:17:01Z

On Mon, 2021-09-27 at 09:32 -0300, Jason Gunthorpe wrote:
> On Fri, Sep 24, 2021 at 04:45:02PM -0400, Eric Farman wrote:
> > On Thu, 2021-09-09 at 16:38 -0300, Jason Gunthorpe wrote:
> > > Having a mdev pointer floating about in addition to a struct
> > > vfio_device
> > > is confusing. It is only used for three things:
> > >
> > > - Getting the mdev 'struct device *' - this is the same as
> > > private->vdev.dev
> > >
> > > - Printing the uuid of the mdev in logging. The uuid is also the
> > > dev_name
> > > of the mdev so this is the same string as
> > > dev_name(private->vdev.dev)
> > >
> > > - A weird attempt to fence the vfio_ccw_sch_io_todo() work. This
> > > work
> > > is
> > > only queued during states IDLE/PROCESSING/PENDING and flushed
> > > when
> > > entering CLOSED. Thus the work already cannot run when the mdev
> > > is
> > > NULL.
> > > Remove it.
> > >
> > > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> > > drivers/s390/cio/vfio_ccw_drv.c | 6 ++--
> > > drivers/s390/cio/vfio_ccw_fsm.c | 48 +++++++++++++--------
> > > ----
> > > drivers/s390/cio/vfio_ccw_ops.c | 16 ++++------
> > > drivers/s390/cio/vfio_ccw_private.h | 2 --
> > > include/linux/mdev.h | 4 ---
> > > 5 files changed, 30 insertions(+), 46 deletions(-)
> >
> > I like this patch. Unfortunately it depends on the removal of a
> > hunk in
> > patch 4, which sets the FSM state to different values based on
> > whether
> > private->mdev is NULL or not, so can't go on its own. Need to spend
> > more time thinking about that patch.
>
> The FSM patch is important, really what is happening is the FSM logic
> takes on the roles that was being split all over the place with other
> logic, like this mdev stuff. To make that work we need a FSM that
> makes sense..

No argument from me about that. My point is that I could consume this
patch easier than the FSM patch, and need to get back to that one.

Eric

>
> Jason