From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shivaprasad G Bhat Date: Tue, 23 Mar 2021 07:54:13 +0000 Subject: Re: [RFC Qemu PATCH v2 1/2] spapr: drc: Add support for async hcalls at the drc level Message-Id: <0c3591b8-e2bd-3a7a-112f-e410bca4434f@linux.ibm.com> List-Id: References: <160674929554.2492771.17651548703390170573.stgit@lep8c.aus.stglabs.ibm.com> <160674938210.2492771.1728601884822491679.stgit@lep8c.aus.stglabs.ibm.com> <20201221130853.15c8ddfd@bahia.lan> <20201228083800.GN6952@yekko.fritz.box> <3b47312a-217f-8df5-0bfd-1a653598abad@linux.ibm.com> <20210208062122.GA40668@yekko.fritz.box> In-Reply-To: <20210208062122.GA40668@yekko.fritz.box> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: David Gibson Cc: Greg Kurz , xiaoguangrong.eric@gmail.com, mst@redhat.com, imammedo@redhat.com, qemu-devel@nongnu.org, qemu-ppc@nongnu.org, linux-nvdimm@lists.01.org, aneesh.kumar@linux.ibm.com, kvm-ppc@vger.kernel.org, shivaprasadbhat@gmail.com, bharata@linux.vnet.ibm.com, linuxppc-dev@lists.ozlabs.org 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