| Message ID | 1546844466-38079-2-git-send-email-wei.w.wang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio-balloon: tweak config_changed | expand |
On Mon, Jan 07, 2019 at 03:01:04PM +0800, Wei Wang wrote: > virtio-ccw has deadlock issues with reading the config space inside the > interrupt context, so we tweak the virtballoon_changed implementation > by moving the config read operations into the related workqueue contexts. > The config_read_bitmap is used as a flag to the workqueue callbacks > about the related config fields that need to be read. > > The cmd_id_received is also renamed to cmd_id_received_cache, and > the value should be obtained via virtio_balloon_cmd_id_received. > > Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> > Signed-off-by: Wei Wang <wei.w.wang@intel.com> > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > Reviewed-by: Halil Pasic <pasic@linux.ibm.com> > --- > drivers/virtio/virtio_balloon.c | 98 +++++++++++++++++++++++++++-------------- > 1 file changed, 65 insertions(+), 33 deletions(-) > > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c > index 728ecd1..fb12fe2 100644 > --- a/drivers/virtio/virtio_balloon.c > +++ b/drivers/virtio/virtio_balloon.c > @@ -61,6 +61,10 @@ enum virtio_balloon_vq { > VIRTIO_BALLOON_VQ_MAX > }; > > +enum virtio_balloon_config_read { > + VIRTIO_BALLOON_CONFIG_READ_CMD_ID = 0, > +}; > + > struct virtio_balloon { > struct virtio_device *vdev; > struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *free_page_vq; > @@ -77,14 +81,20 @@ struct virtio_balloon { > /* Prevent updating balloon when it is being canceled. */ > spinlock_t stop_update_lock; > bool stop_update; > + /* Bitmap to indicate if reading the related config fields are needed */ > + unsigned long config_read_bitmap; > > /* The list of allocated free pages, waiting to be given back to mm */ > struct list_head free_page_list; > spinlock_t free_page_list_lock; > /* The number of free page blocks on the above list */ > unsigned long num_free_page_blocks; > - /* The cmd id received from host */ > - u32 cmd_id_received; > + /* > + * The cmd id received from host. > + * Read it via virtio_balloon_cmd_id_received to get the latest value > + * sent from host. > + */ > + u32 cmd_id_received_cache; > /* The cmd id that is actively in use */ > __virtio32 cmd_id_active; > /* Buffer to store the stop sign */ > @@ -390,37 +400,31 @@ static unsigned long return_free_pages_to_mm(struct virtio_balloon *vb, > return num_returned; > } > > +static void virtio_balloon_queue_free_page_work(struct virtio_balloon *vb) > +{ > + if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) > + return; > + > + /* No need to queue the work if the bit was already set. */ > + if (test_and_set_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, > + &vb->config_read_bitmap)) > + return; > + > + queue_work(vb->balloon_wq, &vb->report_free_page_work); > +} > + > static void virtballoon_changed(struct virtio_device *vdev) > { > struct virtio_balloon *vb = vdev->priv; > unsigned long flags; > - s64 diff = towards_target(vb); > - > - if (diff) { > - spin_lock_irqsave(&vb->stop_update_lock, flags); > - if (!vb->stop_update) > - queue_work(system_freezable_wq, > - &vb->update_balloon_size_work); > - spin_unlock_irqrestore(&vb->stop_update_lock, flags); > - } > > - if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) { > - virtio_cread(vdev, struct virtio_balloon_config, > - free_page_report_cmd_id, &vb->cmd_id_received); > - if (vb->cmd_id_received == VIRTIO_BALLOON_CMD_ID_DONE) { > - /* Pass ULONG_MAX to give back all the free pages */ > - return_free_pages_to_mm(vb, ULONG_MAX); > - } else if (vb->cmd_id_received != VIRTIO_BALLOON_CMD_ID_STOP && > - vb->cmd_id_received != > - virtio32_to_cpu(vdev, vb->cmd_id_active)) { > - spin_lock_irqsave(&vb->stop_update_lock, flags); > - if (!vb->stop_update) { > - queue_work(vb->balloon_wq, > - &vb->report_free_page_work); > - } > - spin_unlock_irqrestore(&vb->stop_update_lock, flags); > - } > + spin_lock_irqsave(&vb->stop_update_lock, flags); > + if (!vb->stop_update) { > + queue_work(system_freezable_wq, > + &vb->update_balloon_size_work); > + virtio_balloon_queue_free_page_work(vb); > } > + spin_unlock_irqrestore(&vb->stop_update_lock, flags); > } > > static void update_balloon_size(struct virtio_balloon *vb) > @@ -527,6 +531,17 @@ static int init_vqs(struct virtio_balloon *vb) > return 0; > } > > +static u32 virtio_balloon_cmd_id_received(struct virtio_balloon *vb) > +{ > + if (test_and_clear_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, > + &vb->config_read_bitmap)) > + virtio_cread(vb->vdev, struct virtio_balloon_config, > + free_page_report_cmd_id, > + &vb->cmd_id_received_cache); > + > + return vb->cmd_id_received_cache; > +} > + > static int send_cmd_id_start(struct virtio_balloon *vb) > { > struct scatterlist sg; > @@ -537,7 +552,8 @@ static int send_cmd_id_start(struct virtio_balloon *vb) > while (virtqueue_get_buf(vq, &unused)) > ; > > - vb->cmd_id_active = cpu_to_virtio32(vb->vdev, vb->cmd_id_received); > + vb->cmd_id_active = virtio32_to_cpu(vb->vdev, > + virtio_balloon_cmd_id_received(vb)); You switches cpu_to_virtio32 to over to virtio32_to_cpu. That will produce a sparse warning I think. Pls always run the static checker when you submit patches. > sg_init_one(&sg, &vb->cmd_id_active, sizeof(vb->cmd_id_active)); > err = virtqueue_add_outbuf(vq, &sg, 1, &vb->cmd_id_active, GFP_KERNEL); > if (!err) > @@ -620,7 +636,8 @@ static int send_free_pages(struct virtio_balloon *vb) > * stop the reporting. > */ > cmd_id_active = virtio32_to_cpu(vb->vdev, vb->cmd_id_active); > - if (cmd_id_active != vb->cmd_id_received) > + if (unlikely(cmd_id_active != > + virtio_balloon_cmd_id_received(vb))) > break; > > /* > @@ -637,11 +654,9 @@ static int send_free_pages(struct virtio_balloon *vb) > return 0; > } > > -static void report_free_page_func(struct work_struct *work) > +static void virtio_balloon_report_free_page(struct virtio_balloon *vb) > { > int err; > - struct virtio_balloon *vb = container_of(work, struct virtio_balloon, > - report_free_page_work); > struct device *dev = &vb->vdev->dev; > > /* Start by sending the received cmd id to host with an outbuf. */ > @@ -659,6 +674,23 @@ static void report_free_page_func(struct work_struct *work) > dev_err(dev, "Failed to send a stop id, err = %d\n", err); > } > > +static void report_free_page_func(struct work_struct *work) > +{ > + struct virtio_balloon *vb = container_of(work, struct virtio_balloon, > + report_free_page_work); > + u32 cmd_id_received; > + > + cmd_id_received = virtio_balloon_cmd_id_received(vb); > + if (cmd_id_received == VIRTIO_BALLOON_CMD_ID_DONE) { > + /* Pass ULONG_MAX to give back all the free pages */ > + return_free_pages_to_mm(vb, ULONG_MAX); > + } else if (cmd_id_received != VIRTIO_BALLOON_CMD_ID_STOP && > + cmd_id_received != > + virtio32_to_cpu(vb->vdev, vb->cmd_id_active)) { > + virtio_balloon_report_free_page(vb); > + } > +} > + > #ifdef CONFIG_BALLOON_COMPACTION > /* > * virtballoon_migratepage - perform the balloon page migration on behalf of > @@ -885,7 +917,7 @@ static int virtballoon_probe(struct virtio_device *vdev) > goto out_del_vqs; > } > INIT_WORK(&vb->report_free_page_work, report_free_page_func); > - vb->cmd_id_received = VIRTIO_BALLOON_CMD_ID_STOP; > + vb->cmd_id_received_cache = VIRTIO_BALLOON_CMD_ID_STOP; > vb->cmd_id_active = cpu_to_virtio32(vb->vdev, > VIRTIO_BALLOON_CMD_ID_STOP); > vb->cmd_id_stop = cpu_to_virtio32(vb->vdev, > -- > 2.7.4
On Mon, Jan 07, 2019 at 03:01:04PM +0800, Wei Wang wrote: > virtio-ccw has deadlock issues with reading the config space inside the > interrupt context, so we tweak the virtballoon_changed implementation > by moving the config read operations into the related workqueue contexts. > The config_read_bitmap is used as a flag to the workqueue callbacks > about the related config fields that need to be read. > > The cmd_id_received is also renamed to cmd_id_received_cache, and > the value should be obtained via virtio_balloon_cmd_id_received. > > Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> > Signed-off-by: Wei Wang <wei.w.wang@intel.com> > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > Reviewed-by: Halil Pasic <pasic@linux.ibm.com> Please include a Fixes tag to list the problematic commit. See Documentation/process/submitting-patches.rst for the appropriate format. > --- > drivers/virtio/virtio_balloon.c | 98 +++++++++++++++++++++++++++-------------- > 1 file changed, 65 insertions(+), 33 deletions(-) > > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c > index 728ecd1..fb12fe2 100644 > --- a/drivers/virtio/virtio_balloon.c > +++ b/drivers/virtio/virtio_balloon.c > @@ -61,6 +61,10 @@ enum virtio_balloon_vq { > VIRTIO_BALLOON_VQ_MAX > }; > > +enum virtio_balloon_config_read { > + VIRTIO_BALLOON_CONFIG_READ_CMD_ID = 0, > +}; > + > struct virtio_balloon { > struct virtio_device *vdev; > struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *free_page_vq; > @@ -77,14 +81,20 @@ struct virtio_balloon { > /* Prevent updating balloon when it is being canceled. */ > spinlock_t stop_update_lock; > bool stop_update; > + /* Bitmap to indicate if reading the related config fields are needed */ > + unsigned long config_read_bitmap; > > /* The list of allocated free pages, waiting to be given back to mm */ > struct list_head free_page_list; > spinlock_t free_page_list_lock; > /* The number of free page blocks on the above list */ > unsigned long num_free_page_blocks; > - /* The cmd id received from host */ > - u32 cmd_id_received; > + /* > + * The cmd id received from host. > + * Read it via virtio_balloon_cmd_id_received to get the latest value > + * sent from host. > + */ > + u32 cmd_id_received_cache; > /* The cmd id that is actively in use */ > __virtio32 cmd_id_active; > /* Buffer to store the stop sign */ > @@ -390,37 +400,31 @@ static unsigned long return_free_pages_to_mm(struct virtio_balloon *vb, > return num_returned; > } > > +static void virtio_balloon_queue_free_page_work(struct virtio_balloon *vb) > +{ > + if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) > + return; > + > + /* No need to queue the work if the bit was already set. */ > + if (test_and_set_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, > + &vb->config_read_bitmap)) > + return; > + > + queue_work(vb->balloon_wq, &vb->report_free_page_work); > +} > + > static void virtballoon_changed(struct virtio_device *vdev) > { > struct virtio_balloon *vb = vdev->priv; > unsigned long flags; > - s64 diff = towards_target(vb); > - > - if (diff) { > - spin_lock_irqsave(&vb->stop_update_lock, flags); > - if (!vb->stop_update) > - queue_work(system_freezable_wq, > - &vb->update_balloon_size_work); > - spin_unlock_irqrestore(&vb->stop_update_lock, flags); > - } > > - if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) { > - virtio_cread(vdev, struct virtio_balloon_config, > - free_page_report_cmd_id, &vb->cmd_id_received); > - if (vb->cmd_id_received == VIRTIO_BALLOON_CMD_ID_DONE) { > - /* Pass ULONG_MAX to give back all the free pages */ > - return_free_pages_to_mm(vb, ULONG_MAX); > - } else if (vb->cmd_id_received != VIRTIO_BALLOON_CMD_ID_STOP && > - vb->cmd_id_received != > - virtio32_to_cpu(vdev, vb->cmd_id_active)) { > - spin_lock_irqsave(&vb->stop_update_lock, flags); > - if (!vb->stop_update) { > - queue_work(vb->balloon_wq, > - &vb->report_free_page_work); > - } > - spin_unlock_irqrestore(&vb->stop_update_lock, flags); > - } > + spin_lock_irqsave(&vb->stop_update_lock, flags); > + if (!vb->stop_update) { > + queue_work(system_freezable_wq, > + &vb->update_balloon_size_work); > + virtio_balloon_queue_free_page_work(vb); > } > + spin_unlock_irqrestore(&vb->stop_update_lock, flags); > } > > static void update_balloon_size(struct virtio_balloon *vb) > @@ -527,6 +531,17 @@ static int init_vqs(struct virtio_balloon *vb) > return 0; > } > > +static u32 virtio_balloon_cmd_id_received(struct virtio_balloon *vb) > +{ > + if (test_and_clear_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, > + &vb->config_read_bitmap)) > + virtio_cread(vb->vdev, struct virtio_balloon_config, > + free_page_report_cmd_id, > + &vb->cmd_id_received_cache); > + > + return vb->cmd_id_received_cache; > +} > + > static int send_cmd_id_start(struct virtio_balloon *vb) > { > struct scatterlist sg; > @@ -537,7 +552,8 @@ static int send_cmd_id_start(struct virtio_balloon *vb) > while (virtqueue_get_buf(vq, &unused)) > ; > > - vb->cmd_id_active = cpu_to_virtio32(vb->vdev, vb->cmd_id_received); > + vb->cmd_id_active = virtio32_to_cpu(vb->vdev, > + virtio_balloon_cmd_id_received(vb)); > sg_init_one(&sg, &vb->cmd_id_active, sizeof(vb->cmd_id_active)); > err = virtqueue_add_outbuf(vq, &sg, 1, &vb->cmd_id_active, GFP_KERNEL); > if (!err) > @@ -620,7 +636,8 @@ static int send_free_pages(struct virtio_balloon *vb) > * stop the reporting. > */ > cmd_id_active = virtio32_to_cpu(vb->vdev, vb->cmd_id_active); > - if (cmd_id_active != vb->cmd_id_received) > + if (unlikely(cmd_id_active != > + virtio_balloon_cmd_id_received(vb))) > break; > > /* > @@ -637,11 +654,9 @@ static int send_free_pages(struct virtio_balloon *vb) > return 0; > } > > -static void report_free_page_func(struct work_struct *work) > +static void virtio_balloon_report_free_page(struct virtio_balloon *vb) > { > int err; > - struct virtio_balloon *vb = container_of(work, struct virtio_balloon, > - report_free_page_work); > struct device *dev = &vb->vdev->dev; > > /* Start by sending the received cmd id to host with an outbuf. */ > @@ -659,6 +674,23 @@ static void report_free_page_func(struct work_struct *work) > dev_err(dev, "Failed to send a stop id, err = %d\n", err); > } > > +static void report_free_page_func(struct work_struct *work) > +{ > + struct virtio_balloon *vb = container_of(work, struct virtio_balloon, > + report_free_page_work); > + u32 cmd_id_received; > + > + cmd_id_received = virtio_balloon_cmd_id_received(vb); > + if (cmd_id_received == VIRTIO_BALLOON_CMD_ID_DONE) { > + /* Pass ULONG_MAX to give back all the free pages */ > + return_free_pages_to_mm(vb, ULONG_MAX); > + } else if (cmd_id_received != VIRTIO_BALLOON_CMD_ID_STOP && > + cmd_id_received != > + virtio32_to_cpu(vb->vdev, vb->cmd_id_active)) { > + virtio_balloon_report_free_page(vb); > + } > +} > + > #ifdef CONFIG_BALLOON_COMPACTION > /* > * virtballoon_migratepage - perform the balloon page migration on behalf of > @@ -885,7 +917,7 @@ static int virtballoon_probe(struct virtio_device *vdev) > goto out_del_vqs; > } > INIT_WORK(&vb->report_free_page_work, report_free_page_func); > - vb->cmd_id_received = VIRTIO_BALLOON_CMD_ID_STOP; > + vb->cmd_id_received_cache = VIRTIO_BALLOON_CMD_ID_STOP; > vb->cmd_id_active = cpu_to_virtio32(vb->vdev, > VIRTIO_BALLOON_CMD_ID_STOP); > vb->cmd_id_stop = cpu_to_virtio32(vb->vdev, > -- > 2.7.4
On 07.01.2019 08:01, Wei Wang wrote: > virtio-ccw has deadlock issues with reading the config space inside the > interrupt context, so we tweak the virtballoon_changed implementation > by moving the config read operations into the related workqueue contexts. > The config_read_bitmap is used as a flag to the workqueue callbacks > about the related config fields that need to be read. > > The cmd_id_received is also renamed to cmd_id_received_cache, and > the value should be obtained via virtio_balloon_cmd_id_received. > > Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> > Signed-off-by: Wei Wang <wei.w.wang@intel.com> > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > Reviewed-by: Halil Pasic <pasic@linux.ibm.com> Together with virtio_pci: use queue idx instead of array idx to set up the vq virtio: don't allocate vqs when names[i] = NULL Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> as already said, would be good to add cc stable (and a Fixes: tag) > --- > drivers/virtio/virtio_balloon.c | 98 +++++++++++++++++++++++++++-------------- > 1 file changed, 65 insertions(+), 33 deletions(-) > > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c > index 728ecd1..fb12fe2 100644 > --- a/drivers/virtio/virtio_balloon.c > +++ b/drivers/virtio/virtio_balloon.c > @@ -61,6 +61,10 @@ enum virtio_balloon_vq { > VIRTIO_BALLOON_VQ_MAX > }; > > +enum virtio_balloon_config_read { > + VIRTIO_BALLOON_CONFIG_READ_CMD_ID = 0, > +}; > + > struct virtio_balloon { > struct virtio_device *vdev; > struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *free_page_vq; > @@ -77,14 +81,20 @@ struct virtio_balloon { > /* Prevent updating balloon when it is being canceled. */ > spinlock_t stop_update_lock; > bool stop_update; > + /* Bitmap to indicate if reading the related config fields are needed */ > + unsigned long config_read_bitmap; > > /* The list of allocated free pages, waiting to be given back to mm */ > struct list_head free_page_list; > spinlock_t free_page_list_lock; > /* The number of free page blocks on the above list */ > unsigned long num_free_page_blocks; > - /* The cmd id received from host */ > - u32 cmd_id_received; > + /* > + * The cmd id received from host. > + * Read it via virtio_balloon_cmd_id_received to get the latest value > + * sent from host. > + */ > + u32 cmd_id_received_cache; > /* The cmd id that is actively in use */ > __virtio32 cmd_id_active; > /* Buffer to store the stop sign */ > @@ -390,37 +400,31 @@ static unsigned long return_free_pages_to_mm(struct virtio_balloon *vb, > return num_returned; > } > > +static void virtio_balloon_queue_free_page_work(struct virtio_balloon *vb) > +{ > + if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) > + return; > + > + /* No need to queue the work if the bit was already set. */ > + if (test_and_set_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, > + &vb->config_read_bitmap)) > + return; > + > + queue_work(vb->balloon_wq, &vb->report_free_page_work); > +} > + > static void virtballoon_changed(struct virtio_device *vdev) > { > struct virtio_balloon *vb = vdev->priv; > unsigned long flags; > - s64 diff = towards_target(vb); > - > - if (diff) { > - spin_lock_irqsave(&vb->stop_update_lock, flags); > - if (!vb->stop_update) > - queue_work(system_freezable_wq, > - &vb->update_balloon_size_work); > - spin_unlock_irqrestore(&vb->stop_update_lock, flags); > - } > > - if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) { > - virtio_cread(vdev, struct virtio_balloon_config, > - free_page_report_cmd_id, &vb->cmd_id_received); > - if (vb->cmd_id_received == VIRTIO_BALLOON_CMD_ID_DONE) { > - /* Pass ULONG_MAX to give back all the free pages */ > - return_free_pages_to_mm(vb, ULONG_MAX); > - } else if (vb->cmd_id_received != VIRTIO_BALLOON_CMD_ID_STOP && > - vb->cmd_id_received != > - virtio32_to_cpu(vdev, vb->cmd_id_active)) { > - spin_lock_irqsave(&vb->stop_update_lock, flags); > - if (!vb->stop_update) { > - queue_work(vb->balloon_wq, > - &vb->report_free_page_work); > - } > - spin_unlock_irqrestore(&vb->stop_update_lock, flags); > - } > + spin_lock_irqsave(&vb->stop_update_lock, flags); > + if (!vb->stop_update) { > + queue_work(system_freezable_wq, > + &vb->update_balloon_size_work); > + virtio_balloon_queue_free_page_work(vb); > } > + spin_unlock_irqrestore(&vb->stop_update_lock, flags); > } > > static void update_balloon_size(struct virtio_balloon *vb) > @@ -527,6 +531,17 @@ static int init_vqs(struct virtio_balloon *vb) > return 0; > } > > +static u32 virtio_balloon_cmd_id_received(struct virtio_balloon *vb) > +{ > + if (test_and_clear_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, > + &vb->config_read_bitmap)) > + virtio_cread(vb->vdev, struct virtio_balloon_config, > + free_page_report_cmd_id, > + &vb->cmd_id_received_cache); > + > + return vb->cmd_id_received_cache; > +} > + > static int send_cmd_id_start(struct virtio_balloon *vb) > { > struct scatterlist sg; > @@ -537,7 +552,8 @@ static int send_cmd_id_start(struct virtio_balloon *vb) > while (virtqueue_get_buf(vq, &unused)) > ; > > - vb->cmd_id_active = cpu_to_virtio32(vb->vdev, vb->cmd_id_received); > + vb->cmd_id_active = virtio32_to_cpu(vb->vdev, > + virtio_balloon_cmd_id_received(vb)); > sg_init_one(&sg, &vb->cmd_id_active, sizeof(vb->cmd_id_active)); > err = virtqueue_add_outbuf(vq, &sg, 1, &vb->cmd_id_active, GFP_KERNEL); > if (!err) > @@ -620,7 +636,8 @@ static int send_free_pages(struct virtio_balloon *vb) > * stop the reporting. > */ > cmd_id_active = virtio32_to_cpu(vb->vdev, vb->cmd_id_active); > - if (cmd_id_active != vb->cmd_id_received) > + if (unlikely(cmd_id_active != > + virtio_balloon_cmd_id_received(vb))) > break; > > /* > @@ -637,11 +654,9 @@ static int send_free_pages(struct virtio_balloon *vb) > return 0; > } > > -static void report_free_page_func(struct work_struct *work) > +static void virtio_balloon_report_free_page(struct virtio_balloon *vb) > { > int err; > - struct virtio_balloon *vb = container_of(work, struct virtio_balloon, > - report_free_page_work); > struct device *dev = &vb->vdev->dev; > > /* Start by sending the received cmd id to host with an outbuf. */ > @@ -659,6 +674,23 @@ static void report_free_page_func(struct work_struct *work) > dev_err(dev, "Failed to send a stop id, err = %d\n", err); > } > > +static void report_free_page_func(struct work_struct *work) > +{ > + struct virtio_balloon *vb = container_of(work, struct virtio_balloon, > + report_free_page_work); > + u32 cmd_id_received; > + > + cmd_id_received = virtio_balloon_cmd_id_received(vb); > + if (cmd_id_received == VIRTIO_BALLOON_CMD_ID_DONE) { > + /* Pass ULONG_MAX to give back all the free pages */ > + return_free_pages_to_mm(vb, ULONG_MAX); > + } else if (cmd_id_received != VIRTIO_BALLOON_CMD_ID_STOP && > + cmd_id_received != > + virtio32_to_cpu(vb->vdev, vb->cmd_id_active)) { > + virtio_balloon_report_free_page(vb); > + } > +} > + > #ifdef CONFIG_BALLOON_COMPACTION > /* > * virtballoon_migratepage - perform the balloon page migration on behalf of > @@ -885,7 +917,7 @@ static int virtballoon_probe(struct virtio_device *vdev) > goto out_del_vqs; > } > INIT_WORK(&vb->report_free_page_work, report_free_page_func); > - vb->cmd_id_received = VIRTIO_BALLOON_CMD_ID_STOP; > + vb->cmd_id_received_cache = VIRTIO_BALLOON_CMD_ID_STOP; > vb->cmd_id_active = cpu_to_virtio32(vb->vdev, > VIRTIO_BALLOON_CMD_ID_STOP); > vb->cmd_id_stop = cpu_to_virtio32(vb->vdev, >
On 01/07/2019 09:49 PM, Christian Borntraeger wrote: > > On 07.01.2019 08:01, Wei Wang wrote: >> virtio-ccw has deadlock issues with reading the config space inside the >> interrupt context, so we tweak the virtballoon_changed implementation >> by moving the config read operations into the related workqueue contexts. >> The config_read_bitmap is used as a flag to the workqueue callbacks >> about the related config fields that need to be read. >> >> The cmd_id_received is also renamed to cmd_id_received_cache, and >> the value should be obtained via virtio_balloon_cmd_id_received. >> >> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> >> Signed-off-by: Wei Wang <wei.w.wang@intel.com> >> Reviewed-by: Cornelia Huck <cohuck@redhat.com> >> Reviewed-by: Halil Pasic <pasic@linux.ibm.com> > Together with > virtio_pci: use queue idx instead of array idx to set up the vq > virtio: don't allocate vqs when names[i] = NULL > > Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> OK. I don't plan to send a new version of the above patches as no changes needed so far. Michael, if the above two patches look good to you, please help add the related tested-by and reviewed-by tags. Thanks. Best, Wei
On 08.01.2019 06:35, Wei Wang wrote: > On 01/07/2019 09:49 PM, Christian Borntraeger wrote: >> >> On 07.01.2019 08:01, Wei Wang wrote: >>> virtio-ccw has deadlock issues with reading the config space inside the >>> interrupt context, so we tweak the virtballoon_changed implementation >>> by moving the config read operations into the related workqueue contexts. >>> The config_read_bitmap is used as a flag to the workqueue callbacks >>> about the related config fields that need to be read. >>> >>> The cmd_id_received is also renamed to cmd_id_received_cache, and >>> the value should be obtained via virtio_balloon_cmd_id_received. >>> >>> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> >>> Signed-off-by: Wei Wang <wei.w.wang@intel.com> >>> Reviewed-by: Cornelia Huck <cohuck@redhat.com> >>> Reviewed-by: Halil Pasic <pasic@linux.ibm.com> >> Together with >> virtio_pci: use queue idx instead of array idx to set up the vq >> virtio: don't allocate vqs when names[i] = NULL >> >> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> > > OK. I don't plan to send a new version of the above patches as no changes needed so far. > > Michael, if the above two patches look good to you, please help add the related tested-by > and reviewed-by tags. Thanks. Can we also make sure that virtio_pci: use queue idx instead of array idx to set up the vq virtio: don't allocate vqs when names[i] = NULL also land in stable?
On 01/08/2019 04:46 PM, Christian Borntraeger wrote: > > On 08.01.2019 06:35, Wei Wang wrote: >> On 01/07/2019 09:49 PM, Christian Borntraeger wrote: >>> On 07.01.2019 08:01, Wei Wang wrote: >>>> virtio-ccw has deadlock issues with reading the config space inside the >>>> interrupt context, so we tweak the virtballoon_changed implementation >>>> by moving the config read operations into the related workqueue contexts. >>>> The config_read_bitmap is used as a flag to the workqueue callbacks >>>> about the related config fields that need to be read. >>>> >>>> The cmd_id_received is also renamed to cmd_id_received_cache, and >>>> the value should be obtained via virtio_balloon_cmd_id_received. >>>> >>>> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> >>>> Signed-off-by: Wei Wang <wei.w.wang@intel.com> >>>> Reviewed-by: Cornelia Huck <cohuck@redhat.com> >>>> Reviewed-by: Halil Pasic <pasic@linux.ibm.com> >>> Together with >>> virtio_pci: use queue idx instead of array idx to set up the vq >>> virtio: don't allocate vqs when names[i] = NULL >>> >>> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> >> OK. I don't plan to send a new version of the above patches as no changes needed so far. >> >> Michael, if the above two patches look good to you, please help add the related tested-by >> and reviewed-by tags. Thanks. > Can we also make sure that > > virtio_pci: use queue idx instead of array idx to set up the vq > virtio: don't allocate vqs when names[i] = NULL > > also land in stable? > You could also send the request to stable after it gets merged to Linus' tree. The stable review committee will decide whether to take it. Please see Option 2: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html Best, Wei
On 09.01.2019 11:35, Wei Wang wrote: > On 01/08/2019 04:46 PM, Christian Borntraeger wrote: >> >> On 08.01.2019 06:35, Wei Wang wrote: >>> On 01/07/2019 09:49 PM, Christian Borntraeger wrote: >>>> On 07.01.2019 08:01, Wei Wang wrote: >>>>> virtio-ccw has deadlock issues with reading the config space inside the >>>>> interrupt context, so we tweak the virtballoon_changed implementation >>>>> by moving the config read operations into the related workqueue contexts. >>>>> The config_read_bitmap is used as a flag to the workqueue callbacks >>>>> about the related config fields that need to be read. >>>>> >>>>> The cmd_id_received is also renamed to cmd_id_received_cache, and >>>>> the value should be obtained via virtio_balloon_cmd_id_received. >>>>> >>>>> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> >>>>> Signed-off-by: Wei Wang <wei.w.wang@intel.com> >>>>> Reviewed-by: Cornelia Huck <cohuck@redhat.com> >>>>> Reviewed-by: Halil Pasic <pasic@linux.ibm.com> >>>> Together with >>>> virtio_pci: use queue idx instead of array idx to set up the vq >>>> virtio: don't allocate vqs when names[i] = NULL >>>> >>>> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> >>> OK. I don't plan to send a new version of the above patches as no changes needed so far. >>> >>> Michael, if the above two patches look good to you, please help add the related tested-by >>> and reviewed-by tags. Thanks. >> Can we also make sure that >> >> virtio_pci: use queue idx instead of array idx to set up the vq >> virtio: don't allocate vqs when names[i] = NULL >> >> also land in stable? >> > > You could also send the request to stable after it gets merged to Linus' tree. > The stable review committee will decide whether to take it. > > Please see Option 2: > > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html > Those patches are not upstream yet, Correct? Michael, can you add the stable tag before submitting? If not, can you give me a heads up when doing the pull request so that I can ping the stable folks.
On Wed, Jan 09, 2019 at 06:35:01PM +0800, Wei Wang wrote: > On 01/08/2019 04:46 PM, Christian Borntraeger wrote: > > > > On 08.01.2019 06:35, Wei Wang wrote: > > > On 01/07/2019 09:49 PM, Christian Borntraeger wrote: > > > > On 07.01.2019 08:01, Wei Wang wrote: > > > > > virtio-ccw has deadlock issues with reading the config space inside the > > > > > interrupt context, so we tweak the virtballoon_changed implementation > > > > > by moving the config read operations into the related workqueue contexts. > > > > > The config_read_bitmap is used as a flag to the workqueue callbacks > > > > > about the related config fields that need to be read. > > > > > > > > > > The cmd_id_received is also renamed to cmd_id_received_cache, and > > > > > the value should be obtained via virtio_balloon_cmd_id_received. > > > > > > > > > > Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> > > > > > Signed-off-by: Wei Wang <wei.w.wang@intel.com> > > > > > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > > > > > Reviewed-by: Halil Pasic <pasic@linux.ibm.com> > > > > Together with > > > > virtio_pci: use queue idx instead of array idx to set up the vq > > > > virtio: don't allocate vqs when names[i] = NULL > > > > > > > > Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> > > > OK. I don't plan to send a new version of the above patches as no changes needed so far. > > > > > > Michael, if the above two patches look good to you, please help add the related tested-by > > > and reviewed-by tags. Thanks. > > Can we also make sure that > > > > virtio_pci: use queue idx instead of array idx to set up the vq > > virtio: don't allocate vqs when names[i] = NULL > > > > also land in stable? > > > > You could also send the request to stable after it gets merged to Linus' > tree. > The stable review committee will decide whether to take it. > > Please see Option 2: > > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html > > > Best, > Wei That's mostly for 3rd party reporters. Why not Cc stable directly in the patches?
On Wed, Jan 09, 2019 at 01:07:16PM +0100, Christian Borntraeger wrote: > On 09.01.2019 11:35, Wei Wang wrote: > > On 01/08/2019 04:46 PM, Christian Borntraeger wrote: > >> > >> On 08.01.2019 06:35, Wei Wang wrote: > >>> On 01/07/2019 09:49 PM, Christian Borntraeger wrote: > >>>> On 07.01.2019 08:01, Wei Wang wrote: > >>>>> virtio-ccw has deadlock issues with reading the config space inside the > >>>>> interrupt context, so we tweak the virtballoon_changed implementation > >>>>> by moving the config read operations into the related workqueue contexts. > >>>>> The config_read_bitmap is used as a flag to the workqueue callbacks > >>>>> about the related config fields that need to be read. > >>>>> > >>>>> The cmd_id_received is also renamed to cmd_id_received_cache, and > >>>>> the value should be obtained via virtio_balloon_cmd_id_received. > >>>>> > >>>>> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> > >>>>> Signed-off-by: Wei Wang <wei.w.wang@intel.com> > >>>>> Reviewed-by: Cornelia Huck <cohuck@redhat.com> > >>>>> Reviewed-by: Halil Pasic <pasic@linux.ibm.com> > >>>> Together with > >>>> virtio_pci: use queue idx instead of array idx to set up the vq > >>>> virtio: don't allocate vqs when names[i] = NULL > >>>> > >>>> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> > >>> OK. I don't plan to send a new version of the above patches as no changes needed so far. > >>> > >>> Michael, if the above two patches look good to you, please help add the related tested-by > >>> and reviewed-by tags. Thanks. > >> Can we also make sure that > >> > >> virtio_pci: use queue idx instead of array idx to set up the vq > >> virtio: don't allocate vqs when names[i] = NULL > >> > >> also land in stable? > >> > > > > You could also send the request to stable after it gets merged to Linus' tree. > > The stable review committee will decide whether to take it. > > > > Please see Option 2: > > > > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html > > > > Those patches are not upstream yet, Correct? > > Michael, > > can you add the stable tag before submitting? If not, can you give me a heads up when doing the > pull request so that I can ping the stable folks. Can you reply to patches that you feel are needed on stable with just Cc: stable@vger.kernel.org in the message body? Then it's all automatically handled by ack attaching scripts.
On 09.01.2019 15:52, Michael S. Tsirkin wrote: > On Wed, Jan 09, 2019 at 01:07:16PM +0100, Christian Borntraeger wrote: >> On 09.01.2019 11:35, Wei Wang wrote: >>> On 01/08/2019 04:46 PM, Christian Borntraeger wrote: >>>> >>>> On 08.01.2019 06:35, Wei Wang wrote: >>>>> On 01/07/2019 09:49 PM, Christian Borntraeger wrote: >>>>>> On 07.01.2019 08:01, Wei Wang wrote: >>>>>>> virtio-ccw has deadlock issues with reading the config space inside the >>>>>>> interrupt context, so we tweak the virtballoon_changed implementation >>>>>>> by moving the config read operations into the related workqueue contexts. >>>>>>> The config_read_bitmap is used as a flag to the workqueue callbacks >>>>>>> about the related config fields that need to be read. >>>>>>> >>>>>>> The cmd_id_received is also renamed to cmd_id_received_cache, and >>>>>>> the value should be obtained via virtio_balloon_cmd_id_received. >>>>>>> >>>>>>> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> >>>>>>> Signed-off-by: Wei Wang <wei.w.wang@intel.com> >>>>>>> Reviewed-by: Cornelia Huck <cohuck@redhat.com> >>>>>>> Reviewed-by: Halil Pasic <pasic@linux.ibm.com> >>>>>> Together with >>>>>> virtio_pci: use queue idx instead of array idx to set up the vq >>>>>> virtio: don't allocate vqs when names[i] = NULL >>>>>> >>>>>> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> >>>>> OK. I don't plan to send a new version of the above patches as no changes needed so far. >>>>> >>>>> Michael, if the above two patches look good to you, please help add the related tested-by >>>>> and reviewed-by tags. Thanks. >>>> Can we also make sure that >>>> >>>> virtio_pci: use queue idx instead of array idx to set up the vq >>>> virtio: don't allocate vqs when names[i] = NULL >>>> >>>> also land in stable? >>>> >>> >>> You could also send the request to stable after it gets merged to Linus' tree. >>> The stable review committee will decide whether to take it. >>> >>> Please see Option 2: >>> >>> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html >>> >> >> Those patches are not upstream yet, Correct? >> >> Michael, >> >> can you add the stable tag before submitting? If not, can you give me a heads up when doing the >> pull request so that I can ping the stable folks. > > Can you reply to patches that you feel are needed on stable with just > > Cc: stable@vger.kernel.org > > in the message body? > > Then it's all automatically handled by ack attaching scripts. Done. But this only works if those patches are not already part of a tree. I guess they have to go via your tree, correct?
On Wed, Jan 09, 2019 at 07:22:50PM +0100, Christian Borntraeger wrote: > > On 09.01.2019 15:52, Michael S. Tsirkin wrote: > > On Wed, Jan 09, 2019 at 01:07:16PM +0100, Christian Borntraeger wrote: > >> On 09.01.2019 11:35, Wei Wang wrote: > >>> On 01/08/2019 04:46 PM, Christian Borntraeger wrote: > >>>> > >>>> On 08.01.2019 06:35, Wei Wang wrote: > >>>>> On 01/07/2019 09:49 PM, Christian Borntraeger wrote: > >>>>>> On 07.01.2019 08:01, Wei Wang wrote: > >>>>>>> virtio-ccw has deadlock issues with reading the config space inside the > >>>>>>> interrupt context, so we tweak the virtballoon_changed implementation > >>>>>>> by moving the config read operations into the related workqueue contexts. > >>>>>>> The config_read_bitmap is used as a flag to the workqueue callbacks > >>>>>>> about the related config fields that need to be read. > >>>>>>> > >>>>>>> The cmd_id_received is also renamed to cmd_id_received_cache, and > >>>>>>> the value should be obtained via virtio_balloon_cmd_id_received. > >>>>>>> > >>>>>>> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> > >>>>>>> Signed-off-by: Wei Wang <wei.w.wang@intel.com> > >>>>>>> Reviewed-by: Cornelia Huck <cohuck@redhat.com> > >>>>>>> Reviewed-by: Halil Pasic <pasic@linux.ibm.com> > >>>>>> Together with > >>>>>> virtio_pci: use queue idx instead of array idx to set up the vq > >>>>>> virtio: don't allocate vqs when names[i] = NULL > >>>>>> > >>>>>> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> > >>>>> OK. I don't plan to send a new version of the above patches as no changes needed so far. > >>>>> > >>>>> Michael, if the above two patches look good to you, please help add the related tested-by > >>>>> and reviewed-by tags. Thanks. > >>>> Can we also make sure that > >>>> > >>>> virtio_pci: use queue idx instead of array idx to set up the vq > >>>> virtio: don't allocate vqs when names[i] = NULL > >>>> > >>>> also land in stable? > >>>> > >>> > >>> You could also send the request to stable after it gets merged to Linus' tree. > >>> The stable review committee will decide whether to take it. > >>> > >>> Please see Option 2: > >>> > >>> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html > >>> > >> > >> Those patches are not upstream yet, Correct? > >> > >> Michael, > >> > >> can you add the stable tag before submitting? If not, can you give me a heads up when doing the > >> pull request so that I can ping the stable folks. > > > > Can you reply to patches that you feel are needed on stable with just > > > > Cc: stable@vger.kernel.org > > > > in the message body? > > > > Then it's all automatically handled by ack attaching scripts. > > Done. But this only works if those patches are not already part of a tree. I guess they have to go via > your tree, correct? Yes. It works because I rebase my tree.
On Mon, Jan 07, 2019 at 02:49:03PM +0100, Christian Borntraeger wrote: > > > On 07.01.2019 08:01, Wei Wang wrote: > > virtio-ccw has deadlock issues with reading the config space inside the > > interrupt context, so we tweak the virtballoon_changed implementation > > by moving the config read operations into the related workqueue contexts. > > The config_read_bitmap is used as a flag to the workqueue callbacks > > about the related config fields that need to be read. > > > > The cmd_id_received is also renamed to cmd_id_received_cache, and > > the value should be obtained via virtio_balloon_cmd_id_received. > > > > Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> > > Signed-off-by: Wei Wang <wei.w.wang@intel.com> > > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > > Reviewed-by: Halil Pasic <pasic@linux.ibm.com> > > Together with > virtio_pci: use queue idx instead of array idx to set up the vq > virtio: don't allocate vqs when names[i] = NULL > > Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> > > as already said, would be good to add cc stable (and a Fixes: tag) I don't think you did that. Did it manually as the pull needed some manual handling but I won't next time and you will have to send it to Greg yourself. > > > --- > > drivers/virtio/virtio_balloon.c | 98 +++++++++++++++++++++++++++-------------- > > 1 file changed, 65 insertions(+), 33 deletions(-) > > > > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c > > index 728ecd1..fb12fe2 100644 > > --- a/drivers/virtio/virtio_balloon.c > > +++ b/drivers/virtio/virtio_balloon.c > > @@ -61,6 +61,10 @@ enum virtio_balloon_vq { > > VIRTIO_BALLOON_VQ_MAX > > }; > > > > +enum virtio_balloon_config_read { > > + VIRTIO_BALLOON_CONFIG_READ_CMD_ID = 0, > > +}; > > + > > struct virtio_balloon { > > struct virtio_device *vdev; > > struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *free_page_vq; > > @@ -77,14 +81,20 @@ struct virtio_balloon { > > /* Prevent updating balloon when it is being canceled. */ > > spinlock_t stop_update_lock; > > bool stop_update; > > + /* Bitmap to indicate if reading the related config fields are needed */ > > + unsigned long config_read_bitmap; > > > > /* The list of allocated free pages, waiting to be given back to mm */ > > struct list_head free_page_list; > > spinlock_t free_page_list_lock; > > /* The number of free page blocks on the above list */ > > unsigned long num_free_page_blocks; > > - /* The cmd id received from host */ > > - u32 cmd_id_received; > > + /* > > + * The cmd id received from host. > > + * Read it via virtio_balloon_cmd_id_received to get the latest value > > + * sent from host. > > + */ > > + u32 cmd_id_received_cache; > > /* The cmd id that is actively in use */ > > __virtio32 cmd_id_active; > > /* Buffer to store the stop sign */ > > @@ -390,37 +400,31 @@ static unsigned long return_free_pages_to_mm(struct virtio_balloon *vb, > > return num_returned; > > } > > > > +static void virtio_balloon_queue_free_page_work(struct virtio_balloon *vb) > > +{ > > + if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) > > + return; > > + > > + /* No need to queue the work if the bit was already set. */ > > + if (test_and_set_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, > > + &vb->config_read_bitmap)) > > + return; > > + > > + queue_work(vb->balloon_wq, &vb->report_free_page_work); > > +} > > + > > static void virtballoon_changed(struct virtio_device *vdev) > > { > > struct virtio_balloon *vb = vdev->priv; > > unsigned long flags; > > - s64 diff = towards_target(vb); > > - > > - if (diff) { > > - spin_lock_irqsave(&vb->stop_update_lock, flags); > > - if (!vb->stop_update) > > - queue_work(system_freezable_wq, > > - &vb->update_balloon_size_work); > > - spin_unlock_irqrestore(&vb->stop_update_lock, flags); > > - } > > > > - if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) { > > - virtio_cread(vdev, struct virtio_balloon_config, > > - free_page_report_cmd_id, &vb->cmd_id_received); > > - if (vb->cmd_id_received == VIRTIO_BALLOON_CMD_ID_DONE) { > > - /* Pass ULONG_MAX to give back all the free pages */ > > - return_free_pages_to_mm(vb, ULONG_MAX); > > - } else if (vb->cmd_id_received != VIRTIO_BALLOON_CMD_ID_STOP && > > - vb->cmd_id_received != > > - virtio32_to_cpu(vdev, vb->cmd_id_active)) { > > - spin_lock_irqsave(&vb->stop_update_lock, flags); > > - if (!vb->stop_update) { > > - queue_work(vb->balloon_wq, > > - &vb->report_free_page_work); > > - } > > - spin_unlock_irqrestore(&vb->stop_update_lock, flags); > > - } > > + spin_lock_irqsave(&vb->stop_update_lock, flags); > > + if (!vb->stop_update) { > > + queue_work(system_freezable_wq, > > + &vb->update_balloon_size_work); > > + virtio_balloon_queue_free_page_work(vb); > > } > > + spin_unlock_irqrestore(&vb->stop_update_lock, flags); > > } > > > > static void update_balloon_size(struct virtio_balloon *vb) > > @@ -527,6 +531,17 @@ static int init_vqs(struct virtio_balloon *vb) > > return 0; > > } > > > > +static u32 virtio_balloon_cmd_id_received(struct virtio_balloon *vb) > > +{ > > + if (test_and_clear_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, > > + &vb->config_read_bitmap)) > > + virtio_cread(vb->vdev, struct virtio_balloon_config, > > + free_page_report_cmd_id, > > + &vb->cmd_id_received_cache); > > + > > + return vb->cmd_id_received_cache; > > +} > > + > > static int send_cmd_id_start(struct virtio_balloon *vb) > > { > > struct scatterlist sg; > > @@ -537,7 +552,8 @@ static int send_cmd_id_start(struct virtio_balloon *vb) > > while (virtqueue_get_buf(vq, &unused)) > > ; > > > > - vb->cmd_id_active = cpu_to_virtio32(vb->vdev, vb->cmd_id_received); > > + vb->cmd_id_active = virtio32_to_cpu(vb->vdev, > > + virtio_balloon_cmd_id_received(vb)); > > sg_init_one(&sg, &vb->cmd_id_active, sizeof(vb->cmd_id_active)); > > err = virtqueue_add_outbuf(vq, &sg, 1, &vb->cmd_id_active, GFP_KERNEL); > > if (!err) > > @@ -620,7 +636,8 @@ static int send_free_pages(struct virtio_balloon *vb) > > * stop the reporting. > > */ > > cmd_id_active = virtio32_to_cpu(vb->vdev, vb->cmd_id_active); > > - if (cmd_id_active != vb->cmd_id_received) > > + if (unlikely(cmd_id_active != > > + virtio_balloon_cmd_id_received(vb))) > > break; > > > > /* > > @@ -637,11 +654,9 @@ static int send_free_pages(struct virtio_balloon *vb) > > return 0; > > } > > > > -static void report_free_page_func(struct work_struct *work) > > +static void virtio_balloon_report_free_page(struct virtio_balloon *vb) > > { > > int err; > > - struct virtio_balloon *vb = container_of(work, struct virtio_balloon, > > - report_free_page_work); > > struct device *dev = &vb->vdev->dev; > > > > /* Start by sending the received cmd id to host with an outbuf. */ > > @@ -659,6 +674,23 @@ static void report_free_page_func(struct work_struct *work) > > dev_err(dev, "Failed to send a stop id, err = %d\n", err); > > } > > > > +static void report_free_page_func(struct work_struct *work) > > +{ > > + struct virtio_balloon *vb = container_of(work, struct virtio_balloon, > > + report_free_page_work); > > + u32 cmd_id_received; > > + > > + cmd_id_received = virtio_balloon_cmd_id_received(vb); > > + if (cmd_id_received == VIRTIO_BALLOON_CMD_ID_DONE) { > > + /* Pass ULONG_MAX to give back all the free pages */ > > + return_free_pages_to_mm(vb, ULONG_MAX); > > + } else if (cmd_id_received != VIRTIO_BALLOON_CMD_ID_STOP && > > + cmd_id_received != > > + virtio32_to_cpu(vb->vdev, vb->cmd_id_active)) { > > + virtio_balloon_report_free_page(vb); > > + } > > +} > > + > > #ifdef CONFIG_BALLOON_COMPACTION > > /* > > * virtballoon_migratepage - perform the balloon page migration on behalf of > > @@ -885,7 +917,7 @@ static int virtballoon_probe(struct virtio_device *vdev) > > goto out_del_vqs; > > } > > INIT_WORK(&vb->report_free_page_work, report_free_page_func); > > - vb->cmd_id_received = VIRTIO_BALLOON_CMD_ID_STOP; > > + vb->cmd_id_received_cache = VIRTIO_BALLOON_CMD_ID_STOP; > > vb->cmd_id_active = cpu_to_virtio32(vb->vdev, > > VIRTIO_BALLOON_CMD_ID_STOP); > > vb->cmd_id_stop = cpu_to_virtio32(vb->vdev, > >
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index 728ecd1..fb12fe2 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -61,6 +61,10 @@ enum virtio_balloon_vq { VIRTIO_BALLOON_VQ_MAX }; +enum virtio_balloon_config_read { + VIRTIO_BALLOON_CONFIG_READ_CMD_ID = 0, +}; + struct virtio_balloon { struct virtio_device *vdev; struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *free_page_vq; @@ -77,14 +81,20 @@ struct virtio_balloon { /* Prevent updating balloon when it is being canceled. */ spinlock_t stop_update_lock; bool stop_update; + /* Bitmap to indicate if reading the related config fields are needed */ + unsigned long config_read_bitmap; /* The list of allocated free pages, waiting to be given back to mm */ struct list_head free_page_list; spinlock_t free_page_list_lock; /* The number of free page blocks on the above list */ unsigned long num_free_page_blocks; - /* The cmd id received from host */ - u32 cmd_id_received; + /* + * The cmd id received from host. + * Read it via virtio_balloon_cmd_id_received to get the latest value + * sent from host. + */ + u32 cmd_id_received_cache; /* The cmd id that is actively in use */ __virtio32 cmd_id_active; /* Buffer to store the stop sign */ @@ -390,37 +400,31 @@ static unsigned long return_free_pages_to_mm(struct virtio_balloon *vb, return num_returned; } +static void virtio_balloon_queue_free_page_work(struct virtio_balloon *vb) +{ + if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) + return; + + /* No need to queue the work if the bit was already set. */ + if (test_and_set_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, + &vb->config_read_bitmap)) + return; + + queue_work(vb->balloon_wq, &vb->report_free_page_work); +} + static void virtballoon_changed(struct virtio_device *vdev) { struct virtio_balloon *vb = vdev->priv; unsigned long flags; - s64 diff = towards_target(vb); - - if (diff) { - spin_lock_irqsave(&vb->stop_update_lock, flags); - if (!vb->stop_update) - queue_work(system_freezable_wq, - &vb->update_balloon_size_work); - spin_unlock_irqrestore(&vb->stop_update_lock, flags); - } - if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) { - virtio_cread(vdev, struct virtio_balloon_config, - free_page_report_cmd_id, &vb->cmd_id_received); - if (vb->cmd_id_received == VIRTIO_BALLOON_CMD_ID_DONE) { - /* Pass ULONG_MAX to give back all the free pages */ - return_free_pages_to_mm(vb, ULONG_MAX); - } else if (vb->cmd_id_received != VIRTIO_BALLOON_CMD_ID_STOP && - vb->cmd_id_received != - virtio32_to_cpu(vdev, vb->cmd_id_active)) { - spin_lock_irqsave(&vb->stop_update_lock, flags); - if (!vb->stop_update) { - queue_work(vb->balloon_wq, - &vb->report_free_page_work); - } - spin_unlock_irqrestore(&vb->stop_update_lock, flags); - } + spin_lock_irqsave(&vb->stop_update_lock, flags); + if (!vb->stop_update) { + queue_work(system_freezable_wq, + &vb->update_balloon_size_work); + virtio_balloon_queue_free_page_work(vb); } + spin_unlock_irqrestore(&vb->stop_update_lock, flags); } static void update_balloon_size(struct virtio_balloon *vb) @@ -527,6 +531,17 @@ static int init_vqs(struct virtio_balloon *vb) return 0; } +static u32 virtio_balloon_cmd_id_received(struct virtio_balloon *vb) +{ + if (test_and_clear_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID, + &vb->config_read_bitmap)) + virtio_cread(vb->vdev, struct virtio_balloon_config, + free_page_report_cmd_id, + &vb->cmd_id_received_cache); + + return vb->cmd_id_received_cache; +} + static int send_cmd_id_start(struct virtio_balloon *vb) { struct scatterlist sg; @@ -537,7 +552,8 @@ static int send_cmd_id_start(struct virtio_balloon *vb) while (virtqueue_get_buf(vq, &unused)) ; - vb->cmd_id_active = cpu_to_virtio32(vb->vdev, vb->cmd_id_received); + vb->cmd_id_active = virtio32_to_cpu(vb->vdev, + virtio_balloon_cmd_id_received(vb)); sg_init_one(&sg, &vb->cmd_id_active, sizeof(vb->cmd_id_active)); err = virtqueue_add_outbuf(vq, &sg, 1, &vb->cmd_id_active, GFP_KERNEL); if (!err) @@ -620,7 +636,8 @@ static int send_free_pages(struct virtio_balloon *vb) * stop the reporting. */ cmd_id_active = virtio32_to_cpu(vb->vdev, vb->cmd_id_active); - if (cmd_id_active != vb->cmd_id_received) + if (unlikely(cmd_id_active != + virtio_balloon_cmd_id_received(vb))) break; /* @@ -637,11 +654,9 @@ static int send_free_pages(struct virtio_balloon *vb) return 0; } -static void report_free_page_func(struct work_struct *work) +static void virtio_balloon_report_free_page(struct virtio_balloon *vb) { int err; - struct virtio_balloon *vb = container_of(work, struct virtio_balloon, - report_free_page_work); struct device *dev = &vb->vdev->dev; /* Start by sending the received cmd id to host with an outbuf. */ @@ -659,6 +674,23 @@ static void report_free_page_func(struct work_struct *work) dev_err(dev, "Failed to send a stop id, err = %d\n", err); } +static void report_free_page_func(struct work_struct *work) +{ + struct virtio_balloon *vb = container_of(work, struct virtio_balloon, + report_free_page_work); + u32 cmd_id_received; + + cmd_id_received = virtio_balloon_cmd_id_received(vb); + if (cmd_id_received == VIRTIO_BALLOON_CMD_ID_DONE) { + /* Pass ULONG_MAX to give back all the free pages */ + return_free_pages_to_mm(vb, ULONG_MAX); + } else if (cmd_id_received != VIRTIO_BALLOON_CMD_ID_STOP && + cmd_id_received != + virtio32_to_cpu(vb->vdev, vb->cmd_id_active)) { + virtio_balloon_report_free_page(vb); + } +} + #ifdef CONFIG_BALLOON_COMPACTION /* * virtballoon_migratepage - perform the balloon page migration on behalf of @@ -885,7 +917,7 @@ static int virtballoon_probe(struct virtio_device *vdev) goto out_del_vqs; } INIT_WORK(&vb->report_free_page_work, report_free_page_func); - vb->cmd_id_received = VIRTIO_BALLOON_CMD_ID_STOP; + vb->cmd_id_received_cache = VIRTIO_BALLOON_CMD_ID_STOP; vb->cmd_id_active = cpu_to_virtio32(vb->vdev, VIRTIO_BALLOON_CMD_ID_STOP); vb->cmd_id_stop = cpu_to_virtio32(vb->vdev,