| Message ID | 20221213154437.15480-4-akrowiak@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | improve AP queue reset processing | expand |
On 2022-12-13 16:44, Tony Krowiak wrote: > To eliminate the repeated calls to the PQAP(ZAPQ) function to verify > that > a reset in progress completed successfully and ensure that error > response > codes get appropriately logged, let's call the apq_reset_check() > function > when the ZAPQ response code indicates that a reset that is already in > progress. > > Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com> > --- > drivers/s390/crypto/vfio_ap_ops.c | 24 +++++++++++++----------- > 1 file changed, 13 insertions(+), 11 deletions(-) > > diff --git a/drivers/s390/crypto/vfio_ap_ops.c > b/drivers/s390/crypto/vfio_ap_ops.c > index a5530a46cf31..5bf2d93ae8af 100644 > --- a/drivers/s390/crypto/vfio_ap_ops.c > +++ b/drivers/s390/crypto/vfio_ap_ops.c > @@ -33,7 +33,7 @@ > static int vfio_ap_mdev_reset_queues(struct ap_queue_table *qtable); > static struct vfio_ap_queue *vfio_ap_find_queue(int apqn); > static const struct vfio_device_ops vfio_ap_matrix_dev_ops; > -static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q, unsigned > int retry); > +static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q); > > /** > * get_update_locks_for_kvm: Acquire the locks required to dynamically > update a > @@ -1632,8 +1632,7 @@ static int apq_reset_check(struct vfio_ap_queue > *q) > return ret; > } > > -static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q, > - unsigned int retry) > +static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q) > { > struct ap_queue_status status; > int ret; > @@ -1648,12 +1647,15 @@ static int vfio_ap_mdev_reset_queue(struct > vfio_ap_queue *q, > ret = 0; > break; > case AP_RESPONSE_RESET_IN_PROGRESS: > - if (retry--) { > - msleep(20); > - goto retry_zapq; > - } > - ret = -EBUSY; > - break; > + /* > + * There is a reset issued by another process in progress. Let's > wait > + * for that to complete. Since we have no idea whether it was a RAPQ > or > + * ZAPQ, then if it completes successfully, let's issue the ZAPQ. > + */ > + ret = apq_reset_check(q); > + if (ret) > + break; > + goto retry_zapq; > case AP_RESPONSE_Q_NOT_AVAIL: > case AP_RESPONSE_DECONFIGURED: > case AP_RESPONSE_CHECKSTOPPED: > @@ -1688,7 +1690,7 @@ static int vfio_ap_mdev_reset_queues(struct > ap_queue_table *qtable) > struct vfio_ap_queue *q; > > hash_for_each(qtable->queues, loop_cursor, q, mdev_qnode) { > - ret = vfio_ap_mdev_reset_queue(q, 1); > + ret = vfio_ap_mdev_reset_queue(q); > /* > * Regardless whether a queue turns out to be busy, or > * is not operational, we need to continue resetting > @@ -1931,7 +1933,7 @@ void vfio_ap_mdev_remove_queue(struct ap_device > *apdev) > } > } > > - vfio_ap_mdev_reset_queue(q, 1); > + vfio_ap_mdev_reset_queue(q); > dev_set_drvdata(&apdev->device, NULL); > kfree(q); > release_update_locks_for_mdev(matrix_mdev); Reviewed-by: Harald Freudenberger <freude@linux.ibm.com>
On 12/15/22 5:51 AM, Harald Freudenberger wrote: > On 2022-12-13 16:44, Tony Krowiak wrote: >> To eliminate the repeated calls to the PQAP(ZAPQ) function to verify >> that >> a reset in progress completed successfully and ensure that error >> response >> codes get appropriately logged, let's call the apq_reset_check() >> function >> when the ZAPQ response code indicates that a reset that is already in >> progress. >> >> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com> >> --- >> drivers/s390/crypto/vfio_ap_ops.c | 24 +++++++++++++----------- >> 1 file changed, 13 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/s390/crypto/vfio_ap_ops.c >> b/drivers/s390/crypto/vfio_ap_ops.c >> index a5530a46cf31..5bf2d93ae8af 100644 >> --- a/drivers/s390/crypto/vfio_ap_ops.c >> +++ b/drivers/s390/crypto/vfio_ap_ops.c >> @@ -33,7 +33,7 @@ >> static int vfio_ap_mdev_reset_queues(struct ap_queue_table *qtable); >> static struct vfio_ap_queue *vfio_ap_find_queue(int apqn); >> static const struct vfio_device_ops vfio_ap_matrix_dev_ops; >> -static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q, unsigned >> int retry); >> +static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q); >> >> /** >> * get_update_locks_for_kvm: Acquire the locks required to >> dynamically update a >> @@ -1632,8 +1632,7 @@ static int apq_reset_check(struct vfio_ap_queue >> *q) >> return ret; >> } >> >> -static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q, >> - unsigned int retry) >> +static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q) >> { >> struct ap_queue_status status; >> int ret; >> @@ -1648,12 +1647,15 @@ static int vfio_ap_mdev_reset_queue(struct >> vfio_ap_queue *q, >> ret = 0; >> break; >> case AP_RESPONSE_RESET_IN_PROGRESS: >> - if (retry--) { >> - msleep(20); >> - goto retry_zapq; >> - } >> - ret = -EBUSY; >> - break; >> + /* >> + * There is a reset issued by another process in progress. >> Let's wait >> + * for that to complete. Since we have no idea whether it >> was a RAPQ or >> + * ZAPQ, then if it completes successfully, let's issue the >> ZAPQ. >> + */ >> + ret = apq_reset_check(q); >> + if (ret) >> + break; >> + goto retry_zapq; >> case AP_RESPONSE_Q_NOT_AVAIL: >> case AP_RESPONSE_DECONFIGURED: >> case AP_RESPONSE_CHECKSTOPPED: >> @@ -1688,7 +1690,7 @@ static int vfio_ap_mdev_reset_queues(struct >> ap_queue_table *qtable) >> struct vfio_ap_queue *q; >> >> hash_for_each(qtable->queues, loop_cursor, q, mdev_qnode) { >> - ret = vfio_ap_mdev_reset_queue(q, 1); >> + ret = vfio_ap_mdev_reset_queue(q); >> /* >> * Regardless whether a queue turns out to be busy, or >> * is not operational, we need to continue resetting >> @@ -1931,7 +1933,7 @@ void vfio_ap_mdev_remove_queue(struct ap_device >> *apdev) >> } >> } >> >> - vfio_ap_mdev_reset_queue(q, 1); >> + vfio_ap_mdev_reset_queue(q); >> dev_set_drvdata(&apdev->device, NULL); >> kfree(q); >> release_update_locks_for_mdev(matrix_mdev); > > Reviewed-by: Harald Freudenberger <freude@linux.ibm.com> Thanks for the review.
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index a5530a46cf31..5bf2d93ae8af 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -33,7 +33,7 @@ static int vfio_ap_mdev_reset_queues(struct ap_queue_table *qtable); static struct vfio_ap_queue *vfio_ap_find_queue(int apqn); static const struct vfio_device_ops vfio_ap_matrix_dev_ops; -static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q, unsigned int retry); +static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q); /** * get_update_locks_for_kvm: Acquire the locks required to dynamically update a @@ -1632,8 +1632,7 @@ static int apq_reset_check(struct vfio_ap_queue *q) return ret; } -static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q, - unsigned int retry) +static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q) { struct ap_queue_status status; int ret; @@ -1648,12 +1647,15 @@ static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q, ret = 0; break; case AP_RESPONSE_RESET_IN_PROGRESS: - if (retry--) { - msleep(20); - goto retry_zapq; - } - ret = -EBUSY; - break; + /* + * There is a reset issued by another process in progress. Let's wait + * for that to complete. Since we have no idea whether it was a RAPQ or + * ZAPQ, then if it completes successfully, let's issue the ZAPQ. + */ + ret = apq_reset_check(q); + if (ret) + break; + goto retry_zapq; case AP_RESPONSE_Q_NOT_AVAIL: case AP_RESPONSE_DECONFIGURED: case AP_RESPONSE_CHECKSTOPPED: @@ -1688,7 +1690,7 @@ static int vfio_ap_mdev_reset_queues(struct ap_queue_table *qtable) struct vfio_ap_queue *q; hash_for_each(qtable->queues, loop_cursor, q, mdev_qnode) { - ret = vfio_ap_mdev_reset_queue(q, 1); + ret = vfio_ap_mdev_reset_queue(q); /* * Regardless whether a queue turns out to be busy, or * is not operational, we need to continue resetting @@ -1931,7 +1933,7 @@ void vfio_ap_mdev_remove_queue(struct ap_device *apdev) } } - vfio_ap_mdev_reset_queue(q, 1); + vfio_ap_mdev_reset_queue(q); dev_set_drvdata(&apdev->device, NULL); kfree(q); release_update_locks_for_mdev(matrix_mdev);
To eliminate the repeated calls to the PQAP(ZAPQ) function to verify that a reset in progress completed successfully and ensure that error response codes get appropriately logged, let's call the apq_reset_check() function when the ZAPQ response code indicates that a reset that is already in progress. Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com> --- drivers/s390/crypto/vfio_ap_ops.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-)