Hi David,
Sorry about the delay.
On 2/8/21 11:51 AM, David Gibson wrote:
On Tue, Jan 19, 2021 at 12:40:31PM +0530, Shivaprasad G Bhat wrote:
Thanks for the comments!
On 12/28/20 2:08 PM, David Gibson wrote:
On Mon, Dec 21, 2020 at 01:08:53PM +0100, Greg Kurz wrote:
...
The overall idea looks good but I think you should consider using
a thread pool to implement it. See below.
I am not convinced, however. Specifically, attaching this to the DRC
doesn't make sense to me. We're adding exactly one DRC related async
hcall, and I can't really see much call for another one. We could
have other async hcalls - indeed we already have one for HPT resizing
- but attaching this to DRCs doesn't help for those.
The semantics of the hcall made me think, if this is going to be
re-usable for future if implemented at DRC level.
It would only be re-usable for operations that are actually connected
to DRCs. It doesn't seem to me particularly likely that we'll ever
have more asynchronous hcalls that are also associated with DRCs.
Okay
Other option
is to move the async-hcall-state/list into the NVDIMMState structure
in include/hw/mem/nvdimm.h and handle it with machine->nvdimms_state
at a global level.
I'm ok with either of two options:
A) Implement this ad-hoc for this specific case, making whatever
simplifications you can based on this specific case.
I am simplifying it to nvdimm use-case alone and limiting the scope.
B) Implement a general mechanism for async hcalls that is *not* tied
to DRCs. Then use that for the existing H_RESIZE_HPT_PREPARE call as
well as this new one.
Hope you are okay with using the pool based approach that Greg
Honestly a thread pool seems like it might be overkill for this
application.
I think its appropriate here as that is what is being done by virtio-pmem
too for flush requests. The aio infrastructure simplifies lot of the
thread handling usage. Please suggest if you think there are better ways.
I am sending the next version addressing all the comments from you and Greg.
Thanks,
Shivaprasad
