search for: cmd_id_receiv

...;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_receive...

...;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_receive...

Since virtio-ccw doesn't work with accessing to the config registers inside an interrupt context, this patch series avoids that issue by moving the config register accesses to the related workqueue contexts. Wei Wang (2): virtio-balloon: tweak config_changed implementation virtio-balloon: improve update_balloon_size_func drivers/virtio/virtio_balloon.c | 59

...;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); There's one problem with this approach: previously updating the cmd_id_received here would immediately stop the report in send_free_pages. With this approach we are waiting for the wq to schedule, which might be blocked waiting for report to complete. So host can no longer quickly stop t...

Since virtio-ccw doesn't work with accessing to the config space inside an interrupt context, this patch series avoids that issue by moving the config register accesses to the related workqueue contexts. v1->v2 ChangeLog: - add config_read_bitmap to indicate to the workqueue callbacks about the necessity of reading the related config fields. Wei Wang (2): virtio-balloon: tweak

Since virtio-ccw doesn't work with accessing to the config space inside an interrupt context, this patch series avoids that issue by moving the config register accesses to the related workqueue contexts. v2->v3 ChangeLog: - rename cmd_id_received to cmd_id_received_cache, and have call sites read the latest value via virtio_balloon_cmd_id_received. (Still kept Cornelia and Halil's reviewed-by as it's a minor change) - remove zeroing vb->num_free_page_blocks in probe since vb is allocated via kzalloc. v1-&g...

...tem_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_c...

...tem_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_c...

...FT; > > > + > > > + /* > > > + * If a stop id or a new cmd id was just received from host, stop > > > + * the reporting, and return 1 to indicate an active stop. > > > + */ > > > + if (virtio32_to_cpu(vb->vdev, vb->cmd_id_use) != vb->cmd_id_received) > > > + return 1; functions returning int should return 0 or -errno on failure, positive return code should indicate progress. If you want a boolean, use bool pls. > > > + > > this access to cmd_id_use and cmd_id_received without locks > > bothers me. Pls docum...

...FT; > > > + > > > + /* > > > + * If a stop id or a new cmd id was just received from host, stop > > > + * the reporting, and return 1 to indicate an active stop. > > > + */ > > > + if (virtio32_to_cpu(vb->vdev, vb->cmd_id_use) != vb->cmd_id_received) > > > + return 1; functions returning int should return 0 or -errno on failure, positive return code should indicate progress. If you want a boolean, use bool pls. > > > + > > this access to cmd_id_use and cmd_id_received without locks > > bothers me. Pls docum...

...> > > + * If a stop id or a new cmd id was just received from host, stop > > > > > + * the reporting, and return 1 to indicate an active stop. > > > > > + */ > > > > > + if (virtio32_to_cpu(vb->vdev, vb->cmd_id_use) != vb- > > >cmd_id_received) > > > > > + return 1; > > > > functions returning int should return 0 or -errno on failure, positive return > > code should indicate progress. > > > > If you want a boolean, use bool pls. > > OK. I plan to change 1 to -EBUSY to indicate the...

...> > > + * If a stop id or a new cmd id was just received from host, stop > > > > > + * the reporting, and return 1 to indicate an active stop. > > > > > + */ > > > > > + if (virtio32_to_cpu(vb->vdev, vb->cmd_id_use) != vb- > > >cmd_id_received) > > > > > + return 1; > > > > functions returning int should return 0 or -errno on failure, positive return > > code should indicate progress. > > > > If you want a boolean, use bool pls. > > OK. I plan to change 1 to -EBUSY to indicate the...

.../* The balloon servicing is delegated to a freezable workqueue. */ > struct work_struct update_balloon_stats_work; > @@ -63,6 +76,13 @@ struct virtio_balloon { > spinlock_t stop_update_lock; > bool stop_update; > > + /* The new cmd id received from host */ > + uint32_t cmd_id_received; > + /* The cmd id that is in use */ > + __virtio32 cmd_id_use; > + /* Buffer to store the stop sign */ > + __virtio32 stop_cmd_id; > + > /* Waiting for host to ack the pages we released. */ > wait_queue_head_t acked; > > @@ -320,17 +340,6 @@ static void stats_han...

.../* The balloon servicing is delegated to a freezable workqueue. */ > struct work_struct update_balloon_stats_work; > @@ -63,6 +76,13 @@ struct virtio_balloon { > spinlock_t stop_update_lock; > bool stop_update; > > + /* The new cmd id received from host */ > + uint32_t cmd_id_received; > + /* The cmd id that is in use */ > + __virtio32 cmd_id_use; > + /* Buffer to store the stop sign */ > + __virtio32 stop_cmd_id; > + > /* Waiting for host to ack the pages we released. */ > wait_queue_head_t acked; > > @@ -320,17 +340,6 @@ static void stats_han...

...ones). > This "hack" makes the random crashes go away, but the balloon interface itself does not work. (setting the value to anything will hang the guest). As patch 1 also modifies the main path, there seem to be additional issues, maybe endianess Looking at things like + vb->cmd_id_received = 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, + VIRTIO_BALLOON_CMD_ID_STOP); Why is cmd_id_received not using cpu_to_virtio32?

...driver couldn't be aware if the added hints are stale > > > or not, > > > > > > No - I mean that driver has code that compares two values and stops > > reporting. Can one of the values be stale? > > The driver compares "vb->cmd_id_use != vb->cmd_id_received" to decide if it needs to stop reporting hints, and cmd_id_received is what the driver reads from host (host notifies the driver to read for the latest value). If host sends a new cmd id, it will notify the guest to read again. I'm not sure how that could be a stale cmd id (or maybe I mi...

...driver couldn't be aware if the added hints are stale > > > or not, > > > > > > No - I mean that driver has code that compares two values and stops > > reporting. Can one of the values be stale? > > The driver compares "vb->cmd_id_use != vb->cmd_id_received" to decide if it needs to stop reporting hints, and cmd_id_received is what the driver reads from host (host notifies the driver to read for the latest value). If host sends a new cmd id, it will notify the guest to read again. I'm not sure how that could be a stale cmd id (or maybe I mi...

...irtio_balloon *vb) { > > int err; > > @@ -620,6 +630,7 @@ static int send_free_pages(struct virtio_balloon *vb) > > * stop the reporting. > > */ > > cmd_id_active = virtio32_to_cpu(vb->vdev, vb- > >cmd_id_active); > > + virtio_balloon_read_cmd_id_received(vb); [1] > > if (cmd_id_active != vb->cmd_id_received) > > break; > > > > @@ -637,11 +648,9 @@ static int send_free_pages(struct virtio_balloon > *vb) > > return 0; > > } > > > > -static void report_free_page_func(struct work_st...

...2_t len = nr_pages << PAGE_SHIFT; >> + >> + /* >> + * If a stop id or a new cmd id was just received from host, stop >> + * the reporting, and return 1 to indicate an active stop. >> + */ >> + if (virtio32_to_cpu(vb->vdev, vb->cmd_id_use) != vb->cmd_id_received) >> + return 1; >> + > this access to cmd_id_use and cmd_id_received without locks > bothers me. Pls document why it's safe. OK. Probably we could add below to the above comments: cmd_id_use and cmd_id_received don't need to be accessed under locks because the report...

...ock; > bool stop_update; > > + /* Command buffers to start and stop the reporting of hints to host */ > + struct virtio_balloon_free_page_hints_cmd cmd_start; > + struct virtio_balloon_free_page_hints_cmd cmd_stop; > + > + /* The cmd id received from host */ > + uint32_t cmd_id_received; > + /* The cmd id that is actively in use */ > + uint32_t cmd_id_active; > + > /* Waiting for host to ack the pages we released. */ > wait_queue_head_t acked; > You want u32 types. > @@ -326,17 +353,6 @@ static void stats_handle_request(struct virtio_balloon *vb) &gt...