lkml.org 
[lkml]   [2018]   [Jun]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2 10/10] vfio: ccw: Let user wait when busy on IO
    From
    Date
    On 25/05/2018 16:04, Heiko Carstens wrote:
    > On Fri, May 25, 2018 at 12:21:18PM +0200, Pierre Morel wrote:
    >> In the current implementation, we do not want to start a new SSCH
    >> command before the last one ends.
    >>
    >> Currently the user needs to poll on the -EBUSY error to
    >> wait before sending a new request.
    >>
    >> Let's be friendly with global warming and let the user sleep
    >> until he may send a new request.
    >>
    >> Let's make the caller wait until the last SSCH ends.
    >>
    >> Signed-off-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
    >> ---
    >> drivers/s390/cio/vfio_ccw_fsm.c | 4 ++++
    >> drivers/s390/cio/vfio_ccw_ops.c | 6 ++++++
    >> drivers/s390/cio/vfio_ccw_private.h | 1 +
    >> 3 files changed, 11 insertions(+)
    >>
    >> diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c
    >> index c37052d..97b74a1 100644
    >> --- a/drivers/s390/cio/vfio_ccw_fsm.c
    >> +++ b/drivers/s390/cio/vfio_ccw_fsm.c
    >> @@ -200,6 +200,10 @@ static int fsm_irq(struct vfio_ccw_private *private)
    >>
    >> if (private->io_trigger)
    >> eventfd_signal(private->io_trigger, 1);
    >> +
    >> + if (private->io_completion)
    >> + complete(private->io_completion);
    >> +
    >> return VFIO_CCW_STATE_IDLE;
    >> }
    >>
    >> diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
    >> index b202e73..39beb6e 100644
    >> --- a/drivers/s390/cio/vfio_ccw_ops.c
    >> +++ b/drivers/s390/cio/vfio_ccw_ops.c
    >> @@ -183,6 +183,7 @@ static ssize_t vfio_ccw_mdev_write(struct mdev_device *mdev,
    >> struct vfio_ccw_private *private;
    >> struct ccw_io_region *region;
    >> union scsw *scsw;
    >> + DECLARE_COMPLETION_ONSTACK(completion);
    >>
    >> if (*ppos + count > sizeof(*region))
    >> return -EINVAL;
    >> @@ -196,6 +197,11 @@ static ssize_t vfio_ccw_mdev_write(struct mdev_device *mdev,
    >> scsw = (union scsw *) &region->scsw_area;
    >> switch (scsw->cmd.fctl) {
    >> case SCSW_FCTL_START_FUNC:
    >> + if (private->state == VFIO_CCW_STATE_BUSY) {
    >> + private->io_completion = &completion;
    >> + if (wait_for_completion_interruptible(&completion))
    >> + return -EINTR;
    >> + }
    > What prevents a state change between checking the state and before
    > private->io_completion is set? If that happens you would end with an
    > endless wait.
    >
    > Similarly, you would have memory corruption if the task would be
    > interrupted and if the function would be left, ending up with a stale
    > private->io_completion completion pointer.
    > The complete(private->io_completion) call will then write to a memory
    > location that might already be reused.
    >
    > Just my 0.02 after having a very very short look ;)

    Right, completely false, I should pay a little more (at least) attention.

    Thanks to have had a very very short (but sharp)look.

    Pierre


    --
    Pierre Morel
    Linux/KVM/QEMU in Böblingen - Germany

    \
     
     \ /
      Last update: 2018-06-05 15:03    [W:4.555 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site