From: Vaibhav Jain <email@example.com> To: "Aneesh Kumar K.V" <firstname.lastname@example.org>, Shivaprasad G Bhat <email@example.com>, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org Cc: email@example.com Subject: Re: [PATCH v3] powerpc/papr_scm: Implement support for H_SCM_FLUSH hcall Date: Wed, 14 Apr 2021 14:51:36 +0530 [thread overview] Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <email@example.com> "Aneesh Kumar K.V" <firstname.lastname@example.org> writes: > Vaibhav Jain <email@example.com> writes: > >> Hi Shiva, >> >> Apologies for a late review but something just caught my eye while >> working on a different patch. >> >> Shivaprasad G Bhat <firstname.lastname@example.org> writes: >> >>> Add support for ND_REGION_ASYNC capability if the device tree >>> indicates 'ibm,hcall-flush-required' property in the NVDIMM node. >>> Flush is done by issuing H_SCM_FLUSH hcall to the hypervisor. >>> >>> If the flush request failed, the hypervisor is expected to >>> to reflect the problem in the subsequent nvdimm H_SCM_HEALTH call. >>> >>> This patch prevents mmap of namespaces with MAP_SYNC flag if the >>> nvdimm requires an explicit flush. >>> >>> References: >>>  https://github.com/avocado-framework-tests/avocado-misc-tests/blob/master/memory/ndctl.py.data/map_sync.c >>> >>> Signed-off-by: Shivaprasad G Bhat <email@example.com> >>> --- >>> v2 - https://www.spinics.net/lists/kvm-ppc/msg18799.html >>> Changes from v2: >>> - Fixed the commit message. >>> - Add dev_dbg before the H_SCM_FLUSH hcall >>> >>> v1 - https://www.spinics.net/lists/kvm-ppc/msg18272.html >>> Changes from v1: >>> - Hcall semantics finalized, all changes are to accomodate them. >>> >>> Documentation/powerpc/papr_hcalls.rst | 14 ++++++++++ >>> arch/powerpc/include/asm/hvcall.h | 3 +- >>> arch/powerpc/platforms/pseries/papr_scm.c | 40 +++++++++++++++++++++++++++++ >>> 3 files changed, 56 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/powerpc/papr_hcalls.rst b/Documentation/powerpc/papr_hcalls.rst >>> index 48fcf1255a33..648f278eea8f 100644 >>> --- a/Documentation/powerpc/papr_hcalls.rst >>> +++ b/Documentation/powerpc/papr_hcalls.rst >>> @@ -275,6 +275,20 @@ Health Bitmap Flags: >>> Given a DRC Index collect the performance statistics for NVDIMM and copy them >>> to the resultBuffer. >>> >>> +**H_SCM_FLUSH** >>> + >>> +| Input: *drcIndex, continue-token* >>> +| Out: *continue-token* >>> +| Return Value: *H_SUCCESS, H_Parameter, H_P2, H_BUSY* >>> + >>> +Given a DRC Index Flush the data to backend NVDIMM device. >>> + >>> +The hcall returns H_BUSY when the flush takes longer time and the hcall needs >>> +to be issued multiple times in order to be completely serviced. The >>> +*continue-token* from the output to be passed in the argument list of >>> +subsequent hcalls to the hypervisor until the hcall is completely serviced >>> +at which point H_SUCCESS or other error is returned by the hypervisor. >>> + >> Does the hcall semantic also include measures to trigger a barrier/fence >> (like pm_wmb()) so that all the stores before the hcall are gauranteed >> to have hit the pmem controller ? > > It is upto the hypervisor to implement the right sequence to ensure the > guarantee. The hcall doesn't expect the store to reach the platform > buffers. Since the asyc_flush function is also called for performing deep_flush from libnvdimm and as the hcall doesnt gaurantee stores to reach the platform buffers, hence papr_scm_pmem_flush() should issue pm_wmb() before the hcall. This would ensure papr_scm_pmem_flush() semantics are consistent that to generic_nvdimm_flush(). Also, adding the statement "The hcall doesn't expect the store to reach the platform buffers" to the hcall documentation would be good to have. > > >> >> If not then the papr_scm_pmem_flush() introduced below should issue a >> fence like pm_wmb() before issuing the flush hcall. >> >> If yes then this behaviour should also be documented with the hcall >> semantics above. >> > > IIUC fdatasync on the backend file is enough to ensure the > guaraantee. Such a request will have REQ_PREFLUSH and REQ_FUA which will > do the necessary barrier on the backing device holding the backend file. > Right, but thats assuming nvdimm is backed by a regular file in hypervisor which may not always be the case. > -aneesh -- Cheers ~ Vaibhav _______________________________________________ Linux-nvdimm mailing list -- firstname.lastname@example.org To unsubscribe send an email to email@example.com
next prev parent reply other threads:[~2021-04-14 9:21 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-03-29 17:36 Shivaprasad G Bhat 2021-03-30 4:56 ` Aneesh Kumar K.V 2021-03-31 10:20 ` Michael Ellerman 2021-04-05 3:47 ` Aneesh Kumar K.V 2021-04-13 14:13 ` Vaibhav Jain 2021-04-14 5:20 ` Aneesh Kumar K.V 2021-04-14 9:21 ` Vaibhav Jain [this message] 2021-04-19 3:59 ` Michael Ellerman
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH v3] powerpc/papr_scm: Implement support for H_SCM_FLUSH hcall' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).