search for: ccw_io_help

On Wed, 12 Sep 2018 16:02:02 +0200 Halil Pasic <pasic at linux.ibm.com> wrote: > While ccw_io_helper() seems like intended to be exclusive in a sense that > it is supposed to facilitate I/O for at most one thread at any given > time, there is actually nothing ensuring that threads won't pile up at > vcdev->wait_q. If they all threads get woken up and see the status that > belo...

...w_device *vcdev, ccw->cmd_code = CCW_CMD_READ_VQ_CONF; ccw->flags = 0; ccw->count = sizeof(struct vq_config_block); - ccw->cda = (__u32)(unsigned long)(&vcdev->dma_area->config_block); + ccw->cda = (__u32)virt_to_phys(&vcdev->dma_area->config_block); ret = ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_READ_VQ_CONF); if (ret) return ret; @@ -454,7 +454,7 @@ static void virtio_ccw_del_vq(struct virtqueue *vq, struct ccw1 *ccw) } ccw->cmd_code = CCW_CMD_SET_VQ; ccw->flags = 0; - ccw->cda = (__u32)(unsigned long)(info->info_block); + ccw->cda...

...w_device *vcdev, ccw->cmd_code = CCW_CMD_READ_VQ_CONF; ccw->flags = 0; ccw->count = sizeof(struct vq_config_block); - ccw->cda = (__u32)(unsigned long)(&vcdev->dma_area->config_block); + ccw->cda = (__u32)virt_to_phys(&vcdev->dma_area->config_block); ret = ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_READ_VQ_CONF); if (ret) return ret; @@ -454,7 +454,7 @@ static void virtio_ccw_del_vq(struct virtqueue *vq, struct ccw1 *ccw) } ccw->cmd_code = CCW_CMD_SET_VQ; ccw->flags = 0; - ccw->cda = (__u32)(unsigned long)(info->info_block); + ccw->cda...

...t_free; - + } /* Give virtio_ring a chance to accept features. */ vring_transport_features(vdev); @@ -776,7 +778,9 @@ static int virtio_ccw_finalize_features(struct virtio_device *vdev) ccw->flags = 0; ccw->count = sizeof(*features); ccw->cda = (__u32)(unsigned long)features; - ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_WRITE_FEAT); + ret = ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_WRITE_FEAT); + if (ret) + goto out_free; if (vcdev->revision == 0) goto out_free; @@ -788,13 +792,13 @@ static int virtio_ccw_finalize_features(struct virtio_device *vdev) ccw->flags = 0;...

...t_free; - + } /* Give virtio_ring a chance to accept features. */ vring_transport_features(vdev); @@ -776,7 +778,9 @@ static int virtio_ccw_finalize_features(struct virtio_device *vdev) ccw->flags = 0; ccw->count = sizeof(*features); ccw->cda = (__u32)(unsigned long)features; - ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_WRITE_FEAT); + ret = ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_WRITE_FEAT); + if (ret) + goto out_free; if (vcdev->revision == 0) goto out_free; @@ -788,13 +792,13 @@ static int virtio_ccw_finalize_features(struct virtio_device *vdev) ccw->flags = 0;...

...eyword as a struct member name" (2018-09-25 13:28:58 +0200) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git tags/virtio-ccw-20180926 for you to fetch changes up to 1fd7ccf82385b3b14e4b3f009a82ada17ddc1e6f: virtio/s390: fix race in ccw_io_helper() (2018-09-25 15:29:10 +0200) ---------------------------------------------------------------- Two patches fixing races in the virtio-ccw driver. ---------------------------------------------------------------- Halil Pasic (2): virtio/s390: avoid race on vcdev->config virtio/s390: fix r...

On Wed, 19 Sep 2018 15:17:28 +0200 Halil Pasic <pasic at linux.ibm.com> wrote: > On 09/18/2018 08:45 PM, Cornelia Huck wrote: > > We basically have two options: > > - Have a way to queue I/O operations and then handle them in sequence. > > Creates complexity, and is likely overkill. (We already have a kind > > of serialization because we re-submit the channel

On Wed, 19 Sep 2018 18:56:45 +0200 Halil Pasic <pasic at linux.ibm.com> wrote: > On 09/19/2018 04:07 PM, Cornelia Huck wrote: > > Do you spot any other places where we may need to care about concurrent > > processing (like for the ->config area in the previous patch)? > > > > It is hard to tell, because: > * Synchronization external to the transport

On Mon, 24 Sep 2018 14:57:26 +0200 Halil Pasic <pasic at linux.ibm.com> wrote: > On 09/21/2018 03:30 PM, Cornelia Huck wrote: > > On Fri, 21 Sep 2018 14:46:21 +0200 > > Halil Pasic <pasic at linux.ibm.com> wrote: > Again, not what I tried to say. By 'This' I meant 'this race'. How > about? > > """ > This race normally does

...> are available in the Git repository at: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git tags/virtio-ccw-20180926 > > > > for you to fetch changes up to 1fd7ccf82385b3b14e4b3f009a82ada17ddc1e6f: > > > > virtio/s390: fix race in ccw_io_helper() (2018-09-25 15:29:10 +0200) > > > > ---------------------------------------------------------------- > > Two patches fixing races in the virtio-ccw driver. > > > > ---------------------------------------------------------------- > > > > Halil Pasic (...

...a chance to accept features. */ > vring_transport_features(vdev); > > @@ -776,7 +778,9 @@ static int virtio_ccw_finalize_features(struct virtio_device *vdev) > ccw->flags = 0; > ccw->count = sizeof(*features); > ccw->cda = (__u32)(unsigned long)features; > - ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_WRITE_FEAT); > + ret = ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_WRITE_FEAT); > + if (ret) > + goto out_free; > > if (vcdev->revision == 0) > goto out_free; > @@ -788,13 +792,13 @@ static int virtio_ccw_finalize_features(struct virtio_...

...a chance to accept features. */ > vring_transport_features(vdev); > > @@ -776,7 +778,9 @@ static int virtio_ccw_finalize_features(struct virtio_device *vdev) > ccw->flags = 0; > ccw->count = sizeof(*features); > ccw->cda = (__u32)(unsigned long)features; > - ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_WRITE_FEAT); > + ret = ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_WRITE_FEAT); > + if (ret) > + goto out_free; > > if (vcdev->revision == 0) > goto out_free; > @@ -788,13 +792,13 @@ static int virtio_ccw_finalize_features(struct virtio_...

From: Pierre Morel <pmorel at linux.vnet.ibm.com> In virtio_ccw_read_vq_conf() the return value of ccw_io_helper() was not checked. If the configuration could not be read properly, we'd wrongly assume a queue size of 0. Let's propagate any I/O error to virtio_ccw_setup_vq() so it may properly fail. Signed-off-by: Pierre Morel <pmorel at linux.vnet.ibm.com> Signed-off-by: Cornelia Huck <co...

Michael, here's a bugfix for the virtio-ccw driver. Question: How would you like to get patches? This one is formatted against your current linux-next branch; I can prepare a branch for you to pull from as well. Pierre Morel (1): virtio/s390: handle failures of READ_VQ_CONF ccw drivers/s390/virtio/virtio_ccw.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) -- 2.3.8

Michael, here's a bugfix for the virtio-ccw driver. Question: How would you like to get patches? This one is formatted against your current linux-next branch; I can prepare a branch for you to pull from as well. Pierre Morel (1): virtio/s390: handle failures of READ_VQ_CONF ccw drivers/s390/virtio/virtio_ccw.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) -- 2.3.8

Two enhancements for virtio_ccw, on top of latest 1.0 patchset Cornelia Huck (1): virtio_ccw: finalize_features error handling Michael S. Tsirkin (1): virtio_ccw: future-proof finalize_features drivers/s390/kvm/virtio_ccw.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) -- MST

Two enhancements for virtio_ccw, on top of latest 1.0 patchset Cornelia Huck (1): virtio_ccw: finalize_features error handling Michael S. Tsirkin (1): virtio_ccw: future-proof finalize_features drivers/s390/kvm/virtio_ccw.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) -- MST

...= CCW_CMD_SET_IND; - ccw->count = sizeof(&vcdev->indicators); + ccw->count = sizeof(indicators(vcdev)); ccw->cda = (__u32)(unsigned long) indicatorp; } /* Deregister indicators from host. */ - vcdev->indicators = 0; + *indicators(vcdev) = 0; ccw->flags = 0; ret = ccw_io_helper(vcdev, ccw, vcdev->is_thinint ? @@ -656,10 +666,10 @@ static int virtio_ccw_find_vqs(struct virtio_device *vdev, unsigned nvqs, * We need a data area under 2G to communicate. Our payload is * the address of the indicators. */ - indicatorp = kmalloc(sizeof(&vcdev->indicat...

...ccw and config_area should be allocated inside vcdev. > You could either use pointers, or simply allocate vcdev with GDP_DMA. > > This would avoid the kmalloc inside these calls. > > Thanks, > Rusty. But it won't solve the problem of nested sleepers with ccw: ATM is invokes ccw_io_helper to execute commands, and that one calls wait_event to wait for an interrupt. Might be fixable but I think my patch looks like a safer solution for 4.0/3.19, no? -- MST

...ccw and config_area should be allocated inside vcdev. > You could either use pointers, or simply allocate vcdev with GDP_DMA. > > This would avoid the kmalloc inside these calls. > > Thanks, > Rusty. But it won't solve the problem of nested sleepers with ccw: ATM is invokes ccw_io_helper to execute commands, and that one calls wait_event to wait for an interrupt. Might be fixable but I think my patch looks like a safer solution for 4.0/3.19, no? -- MST