Linux-NVDIMM Archive on lore.kernel.org
 help / color / Atom feed
From: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
To: "Michal Suchánek" <msuchanek@suse.de>
Cc: linuxppc-dev@lists.ozlabs.org, mpe@ellerman.id.au,
	linux-nvdimm@lists.01.org, Jan Kara <jack@suse.cz>
Subject: Re: [PATCH v6 6/8] powerpc/pmem: Avoid the barrier in flush routines
Date: Tue, 30 Jun 2020 02:10:15 +0530
Message-ID: <87lfk5hahc.fsf@linux.ibm.com> (raw)
In-Reply-To: <20200629160940.GU21462@kitsune.suse.cz>

Michal Suchánek <msuchanek@suse.de> writes:

> Hello,
>
> On Mon, Jun 29, 2020 at 07:27:20PM +0530, Aneesh Kumar K.V wrote:
>> nvdimm expect the flush routines to just mark the cache clean. The barrier
>> that mark the store globally visible is done in nvdimm_flush().
>> 
>> Update the papr_scm driver to a simplified nvdim_flush callback that do
>> only the required barrier.
>> 
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>>  arch/powerpc/lib/pmem.c                   |  6 ------
>>  arch/powerpc/platforms/pseries/papr_scm.c | 13 +++++++++++++
>>  2 files changed, 13 insertions(+), 6 deletions(-)
>> 
>> diff --git a/arch/powerpc/lib/pmem.c b/arch/powerpc/lib/pmem.c
>> index 5a61aaeb6930..21210fa676e5 100644
>> --- a/arch/powerpc/lib/pmem.c
>> +++ b/arch/powerpc/lib/pmem.c
>> @@ -19,9 +19,6 @@ static inline void __clean_pmem_range(unsigned long start, unsigned long stop)
>>  
>>  	for (i = 0; i < size >> shift; i++, addr += bytes)
>>  		asm volatile(PPC_DCBSTPS(%0, %1): :"i"(0), "r"(addr): "memory");
>> -
>> -
>> -	asm volatile(PPC_PHWSYNC ::: "memory");
>>  }
>>  
>>  static inline void __flush_pmem_range(unsigned long start, unsigned long stop)
>> @@ -34,9 +31,6 @@ static inline void __flush_pmem_range(unsigned long start, unsigned long stop)
>>  
>>  	for (i = 0; i < size >> shift; i++, addr += bytes)
>>  		asm volatile(PPC_DCBFPS(%0, %1): :"i"(0), "r"(addr): "memory");
>> -
>> -
>> -	asm volatile(PPC_PHWSYNC ::: "memory");
>>  }
>>  
>>  static inline void clean_pmem_range(unsigned long start, unsigned long stop)
>> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
>> index 9c569078a09f..9a9a0766f8b6 100644
>> --- a/arch/powerpc/platforms/pseries/papr_scm.c
>> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
>> @@ -630,6 +630,18 @@ static int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc,
>>  
>>  	return 0;
>>  }
>> +/*
>> + * We have made sure the pmem writes are done such that before calling this
>> + * all the caches are flushed/clean. We use dcbf/dcbfps to ensure this. Here
>> + * we just need to add the necessary barrier to make sure the above flushes
>> + * are have updated persistent storage before any data access or data transfer
>> + * caused by subsequent instructions is initiated.
>> + */
>> +static int papr_scm_flush_sync(struct nd_region *nd_region, struct bio *bio)
>> +{
>> +	arch_pmem_flush_barrier();
>> +	return 0;
>> +}
>>  
>>  static ssize_t flags_show(struct device *dev,
>>  			  struct device_attribute *attr, char *buf)
>> @@ -743,6 +755,7 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
>>  	ndr_desc.mapping = &mapping;
>>  	ndr_desc.num_mappings = 1;
>>  	ndr_desc.nd_set = &p->nd_set;
>> +	ndr_desc.flush = papr_scm_flush_sync;
>
> AFAICT currently the only device that implements flush is virtio_pmem.
> How does the nfit driver get away without implementing flush?

generic_nvdimm_flush does the required barrier for nfit. The reason for
adding ndr_desc.flush call back for papr_scm was to avoid the usage
of iomem based deep flushing (ndr_region_data.flush_wpq) which is not
supported by papr_scm.

BTW we do return NULL for ndrd_get_flush_wpq() on power. So the upstream
code also does the same thing, but in a different way.


> Also the flush takes arguments that are completely unused but a user of
> the pmem region must assume they are used, and call flush() on the
> region rather than arch_pmem_flush_barrier() directly.

The bio argument can help a pmem driver to do range based flushing in
case of pmem_make_request. If bio is null then we must assume a full
device flush. 

>This may not
> work well with md as discussed with earlier iteration of the patchest.
>

dm-writecache needs some major changes to work with asynchronous pmem
devices. 

-aneesh
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

  reply index

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-29 13:57 [PATCH v6 0/8] Support new pmem flush and sync instructions for POWER Aneesh Kumar K.V
2020-06-29 13:57 ` [PATCH v6 1/8] powerpc/pmem: Restrict papr_scm to P8 and above Aneesh Kumar K.V
2020-06-29 13:57 ` [PATCH v6 2/8] powerpc/pmem: Add new instructions for persistent storage and sync Aneesh Kumar K.V
2020-06-29 13:57 ` [PATCH v6 3/8] powerpc/pmem: Add flush routines using new pmem store and sync instruction Aneesh Kumar K.V
2020-06-29 13:57 ` [PATCH v6 4/8] libnvdimm/nvdimm/flush: Allow architecture to override the flush barrier Aneesh Kumar K.V
2020-06-29 18:53   ` kernel test robot
2020-06-29 20:27     ` Aneesh Kumar K.V
2020-06-29 19:27   ` kernel test robot
2020-06-29 20:29   ` [PATCH updated] " Aneesh Kumar K.V
2020-06-30  1:32     ` Dan Williams
2020-06-30  5:01       ` Aneesh Kumar K.V
2020-06-30  7:06         ` Dan Williams
2020-06-30  7:22           ` Aneesh Kumar K.V
2020-06-30  7:53             ` Aneesh Kumar K.V
2020-06-30 12:48             ` Aneesh Kumar K.V
2020-06-30 19:21               ` Dan Williams
2020-06-29 13:57 ` [PATCH v6 5/8] powerpc/pmem/of_pmem: Update of_pmem to use the new barrier instruction Aneesh Kumar K.V
2020-06-30  1:38   ` Dan Williams
2020-06-30  5:05     ` Aneesh Kumar K.V
2020-06-30  7:16       ` Dan Williams
2020-06-29 13:57 ` [PATCH v6 6/8] powerpc/pmem: Avoid the barrier in flush routines Aneesh Kumar K.V
2020-06-29 16:09   ` Michal Suchánek
2020-06-29 20:40     ` Aneesh Kumar K.V [this message]
2020-06-30  1:50       ` Dan Williams
2020-06-30  8:54         ` Michal Suchánek
2020-06-30  9:20           ` Aneesh Kumar K.V
2020-06-30 19:45             ` Dan Williams
2020-07-01  3:09               ` Aneesh Kumar K.V
2020-07-01  5:08                 ` Dan Williams
2020-06-29 13:57 ` [PATCH v6 7/8] powerpc/pmem: Add WARN_ONCE to catch the wrong usage of pmem flush functions Aneesh Kumar K.V
2020-06-30  1:52   ` Dan Williams
2020-06-30  5:05     ` Aneesh Kumar K.V
2020-06-29 13:57 ` [PATCH v6 8/8] powerpc/pmem: Initialize pmem device on newer hardware Aneesh Kumar K.V

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 \
    --in-reply-to=87lfk5hahc.fsf@linux.ibm.com \
    --to=aneesh.kumar@linux.ibm.com \
    --cc=jack@suse.cz \
    --cc=linux-nvdimm@lists.01.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=msuchanek@suse.de \
    /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

Linux-NVDIMM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nvdimm/0 linux-nvdimm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nvdimm linux-nvdimm/ https://lore.kernel.org/linux-nvdimm \
		linux-nvdimm@lists.01.org
	public-inbox-index linux-nvdimm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.01.lists.linux-nvdimm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git