From: Mikulas Patocka <mpatocka@redhat.com>
To: Huaisheng Ye <yehs2007@zoho.com>
Cc: snitzer@redhat.com, agk@redhat.com, dan.j.williams@intel.com,
hch@lst.de, jack@suse.cz, corbet@lwn.net, dm-devel@redhat.com,
linux-kernel@vger.kernel.org, linux-nvdimm@lists.01.org,
linux-doc@vger.kernel.org, Huaisheng Ye <yehs1@lenovo.com>
Subject: Re: [PATCH v3 0/5] Optimize writecache when using pmem as cache
Date: Mon, 4 Feb 2019 06:27:34 -0500 (EST) [thread overview]
Message-ID: <alpine.LRH.2.02.1902040606380.28217@file01.intranet.prod.int.rdu2.redhat.com> (raw)
In-Reply-To: <20190131022955.9920-1-yehs2007@zoho.com>
On Thu, 31 Jan 2019, Huaisheng Ye wrote:
> From: Huaisheng Ye <yehs1@lenovo.com>
>
> This patch set could be used for dm-writecache when use persistent
> memory as cache data device.
>
> Patch 1 and 2 go towards removing unused parameter and codes which
> actually doesn't really work.
I agree that there is some unused variables and code, but I would let it
be as it is. The processors can write data to persistent memory either by
using non-temporal stores (the movnti instruction) or by normal stores
followed by the clwb instruction.
Currently, the movnti instruction is faster - however, it may be possible
that with some newer processors, the clwb instruction could be faster -
and in that case, we need the code that you have removed.
I would like to keep both flush strategies around (movnti and clwb), so
that we can test how do they perform on various processors.
Unfortunatelly, some upstream developers hate code with #ifdefs :-(
Note that compiler optimizations already remove the unused parameter and
the impossible code behind "if (WC_MODE_PMEM(wc)) if (!WC_MODE_PMEM(wc))".
> Patch 3 and 4 are targeted at solving problem fn ctr failed to work
> due to invalid magic or version, which is caused by the super block
> of pmem has messy data stored.
LVM zeros the beginning of new logical volumes, so there should be no
problem with it. If the user wants to use the writecache target without
LVM, he should zero the superblock with dd if=/dev/zero of=/dev/pmem0
bs=4k count=1
Note that other device mapper targets also follow this policy - for
example see drivers/md/dm-snap-persistent.c:
if (le32_to_cpu(dh->magic) == 0) {
*new_snapshot = 1;
return 0;
}
if (le32_to_cpu(dh->magic) != SNAP_MAGIC) {
DMWARN("Invalid or corrupt snapshot");
r = -ENXIO;
goto bad;
}
So, I think there is no need for these patches - dm-writecache just does
what others targets do.
> Patch 5 is used for getting the status of seq_count.
It may be accepted if other LVM team members find some use for this value.
Mikulas
> Changes Since v2:
> - seq_count is important for flush operations, output it within status
> for debugging and analyzing code behavior.
> [1]: https://lkml.org/lkml/2019/1/3/43
> [2]: https://lkml.org/lkml/2019/1/9/6
>
> Huaisheng Ye (5):
> dm-writecache: remove unused size to writecache_flush_region
> dm-writecache: get rid of memory_data flush to writecache_flush_entry
> dm-writecache: expand pmem_reinit for struct dm_writecache
> Documentation/device-mapper: add optional parameter reinit
> dm-writecache: output seq_count within status
>
> Documentation/device-mapper/writecache.txt | 4 ++++
> drivers/md/dm-writecache.c | 23 +++++++++++++----------
> 2 files changed, 17 insertions(+), 10 deletions(-)
>
> --
> 1.8.3.1
>
>
next prev parent reply other threads:[~2019-02-04 11:27 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-31 2:29 [PATCH v3 0/5] Optimize writecache when using pmem as cache Huaisheng Ye
2019-01-31 2:29 ` [PATCH v3 1/5] dm-writecache: remove unused size to writecache_flush_region Huaisheng Ye
2019-01-31 2:29 ` [PATCH v3 2/5] dm-writecache: get rid of memory_data flush to writecache_flush_entry Huaisheng Ye
2019-01-31 2:29 ` [PATCH v3 3/5] dm-writecache: expand pmem_reinit for struct dm_writecache Huaisheng Ye
2019-01-31 2:29 ` [PATCH v3 4/5] Documentation/device-mapper: add optional parameter reinit Huaisheng Ye
2019-01-31 2:29 ` [PATCH v3 5/5] dm-writecache: output seq_count within status Huaisheng Ye
2019-02-04 11:27 ` Mikulas Patocka [this message]
2019-02-11 6:47 ` [External] Re: [PATCH v3 0/5] Optimize writecache when using pmem as cache Huaisheng HS1 Ye
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=alpine.LRH.2.02.1902040606380.28217@file01.intranet.prod.int.rdu2.redhat.com \
--to=mpatocka@redhat.com \
--cc=agk@redhat.com \
--cc=corbet@lwn.net \
--cc=dan.j.williams@intel.com \
--cc=dm-devel@redhat.com \
--cc=hch@lst.de \
--cc=jack@suse.cz \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nvdimm@lists.01.org \
--cc=snitzer@redhat.com \
--cc=yehs1@lenovo.com \
--cc=yehs2007@zoho.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).