Re: [PATCH v3 1/3] virtio-balloon: tweak config_changed implementation
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
> > Signed-off-by: Wei Wang
> > Reviewed-by: Cornelia Huck
> > Reviewed-by: Halil Pasic
>
> 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
>
> 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,
> > +>config_read_bitmap))
> > + return;
> > +
> > + queue_work(vb->balloon_wq, >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(>stop_update_lock, flags);
> > - if (!vb->stop_update)
> > - queue_work(system_freezable_wq,
> > - >update_balloon_size_work);
> > - spin_unlock_irqrestore(>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, >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(>stop_update_lock, flags);
> > - if (!vb->stop_update) {
> > - queue_work(vb->balloon_wq,
> > - >report_free_page_work);
> > - }
> > - spin_unlock_irqrestore(>stop_update_lock, flags);
> > - }
> > +
Re: [PATCH v3 1/3] virtio-balloon: tweak config_changed implementation
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 > >>> Signed-off-by: Wei Wang > >>> Reviewed-by: Cornelia Huck > >>> Reviewed-by: Halil Pasic > >> 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 > > 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: sta...@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. -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 1/3] virtio-balloon: tweak config_changed implementation
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 >>> Signed-off-by: Wei Wang >>> Reviewed-by: Cornelia Huck >>> Reviewed-by: Halil Pasic >> 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 > 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: sta...@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? ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 1/3] virtio-balloon: tweak config_changed implementation
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 > > Signed-off-by: Wei Wang > > Reviewed-by: Cornelia Huck > > Reviewed-by: Halil Pasic > 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 > >>> 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: sta...@vger.kernel.org in the message body? Then it's all automatically handled by ack attaching scripts. -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 1/3] virtio-balloon: tweak config_changed implementation
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 > > > > > Signed-off-by: Wei Wang > > > > > Reviewed-by: Cornelia Huck > > > > > Reviewed-by: Halil Pasic > > > > 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 > > > 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? -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 1/3] virtio-balloon: tweak config_changed implementation
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 > Signed-off-by: Wei Wang > Reviewed-by: Cornelia Huck > Reviewed-by: Halil Pasic 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 >>> 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. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 1/3] virtio-balloon: tweak config_changed implementation
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 Signed-off-by: Wei Wang Reviewed-by: Cornelia Huck Reviewed-by: Halil Pasic 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 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 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 1/3] virtio-balloon: tweak config_changed implementation
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 >>> Signed-off-by: Wei Wang >>> Reviewed-by: Cornelia Huck >>> Reviewed-by: Halil Pasic >> 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 > > 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? ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 1/3] virtio-balloon: tweak config_changed implementation
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 Signed-off-by: Wei Wang Reviewed-by: Cornelia Huck Reviewed-by: Halil Pasic 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 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 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 1/3] virtio-balloon: tweak config_changed implementation
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
> Signed-off-by: Wei Wang
> Reviewed-by: Cornelia Huck
> Reviewed-by: Halil Pasic
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
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,
> + >config_read_bitmap))
> + return;
> +
> + queue_work(vb->balloon_wq, >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(>stop_update_lock, flags);
> - if (!vb->stop_update)
> - queue_work(system_freezable_wq,
> ->update_balloon_size_work);
> - spin_unlock_irqrestore(>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, >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(>stop_update_lock, flags);
> - if (!vb->stop_update) {
> - queue_work(vb->balloon_wq,
> ->report_free_page_work);
> - }
> - spin_unlock_irqrestore(>stop_update_lock, flags);
> - }
> + spin_lock_irqsave(>stop_update_lock, flags);
> + if (!vb->stop_update) {
> + queue_work(system_freezable_wq,
> +>update_balloon_size_work);
> + virtio_balloon_queue_free_page_work(vb);
> }
> + spin_unlock_irqrestore(>stop_update_lock, flags);
> }
>
> static void update_balloon_size(struct
Re: [PATCH v3 1/3] virtio-balloon: tweak config_changed implementation
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
> Signed-off-by: Wei Wang
> Reviewed-by: Cornelia Huck
> Reviewed-by: Halil Pasic
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,
> + >config_read_bitmap))
> + return;
> +
> + queue_work(vb->balloon_wq, >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(>stop_update_lock, flags);
> - if (!vb->stop_update)
> - queue_work(system_freezable_wq,
> ->update_balloon_size_work);
> - spin_unlock_irqrestore(>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, >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(>stop_update_lock, flags);
> - if (!vb->stop_update) {
> - queue_work(vb->balloon_wq,
> ->report_free_page_work);
> - }
> - spin_unlock_irqrestore(>stop_update_lock, flags);
> - }
> + spin_lock_irqsave(>stop_update_lock, flags);
> + if (!vb->stop_update) {
> + queue_work(system_freezable_wq,
> +>update_balloon_size_work);
> + virtio_balloon_queue_free_page_work(vb);
> }
> + spin_unlock_irqrestore(>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)
>
Re: [PATCH v3 1/3] virtio-balloon: tweak config_changed implementation
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
> Signed-off-by: Wei Wang
> Reviewed-by: Cornelia Huck
> Reviewed-by: Halil Pasic
> ---
> 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,
> + >config_read_bitmap))
> + return;
> +
> + queue_work(vb->balloon_wq, >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(>stop_update_lock, flags);
> - if (!vb->stop_update)
> - queue_work(system_freezable_wq,
> ->update_balloon_size_work);
> - spin_unlock_irqrestore(>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, >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(>stop_update_lock, flags);
> - if (!vb->stop_update) {
> - queue_work(vb->balloon_wq,
> ->report_free_page_work);
> - }
> - spin_unlock_irqrestore(>stop_update_lock, flags);
> - }
> + spin_lock_irqsave(>stop_update_lock, flags);
> + if (!vb->stop_update) {
> + queue_work(system_freezable_wq,
> +>update_balloon_size_work);
> + virtio_balloon_queue_free_page_work(vb);
> }
> + spin_unlock_irqrestore(>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
[PATCH v3 1/3] virtio-balloon: tweak config_changed implementation
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
Signed-off-by: Wei Wang
Reviewed-by: Cornelia Huck
Reviewed-by: Halil Pasic
---
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,
+>config_read_bitmap))
+ return;
+
+ queue_work(vb->balloon_wq, >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(>stop_update_lock, flags);
- if (!vb->stop_update)
- queue_work(system_freezable_wq,
- >update_balloon_size_work);
- spin_unlock_irqrestore(>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, >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(>stop_update_lock, flags);
- if (!vb->stop_update) {
- queue_work(vb->balloon_wq,
- >report_free_page_work);
- }
- spin_unlock_irqrestore(>stop_update_lock, flags);
- }
+ spin_lock_irqsave(>stop_update_lock, flags);
+ if (!vb->stop_update) {
+ queue_work(system_freezable_wq,
+ >update_balloon_size_work);
+ virtio_balloon_queue_free_page_work(vb);
}
+ spin_unlock_irqrestore(>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,
+ >config_read_bitmap))
+ virtio_cread(vb->vdev, struct virtio_balloon_config,
+
