Oliver <ooh...@gmail.com> writes:
> On Fri, Nov 10, 2017 at 1:40 AM, <lukasz.pl...@intel.com> wrote:
>> From: Łukasz Plewa <lukasz.pl...@intel.com>
>>
>> The next version of libpmem will depend on libndctl - as daxctl
>> depends on libpmem, it will make building packages inconvenient.
>> This patch replaces libpmem calls with intrinsic functions what
>> allows to remove a libpmem dependency.
>>
>> Signed-off-by: Łukasz Plewa <lukasz.pl...@intel.com>
>> ---
>> configure.ac | 15 +++++----------
>> daxctl/Makefile.am | 4 ----
>> daxctl/io.c | 23 +++++++++++++++++++----
>> ndctl.spec.in | 7 -------
>> 4 files changed, 24 insertions(+), 25 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 5b10381..33a3e6e 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -32,6 +32,7 @@ AC_PREFIX_DEFAULT([/usr])
>>
>> AC_PROG_SED
>> AC_PROG_MKDIR_P
>> +AC_CANONICAL_HOST
>>
>> AC_ARG_ENABLE([docs],
>> AS_HELP_STRING([--disable-docs],
>> @@ -94,16 +95,10 @@ PKG_CHECK_MODULES([UDEV], [libudev])
>> PKG_CHECK_MODULES([UUID], [uuid])
>> PKG_CHECK_MODULES([JSON], [json-c])
>>
>> -AC_ARG_WITH([libpmem],
>> - AS_HELP_STRING([--with-libpmem],
>> - [Install with libpmem support. @<:@default=no@:>@]),
>> - [],
>> - [with_libpmem=no])
>> -if test "x$with_libpmem" = "xyes"; then
>> - PKG_CHECK_MODULES([PMEM], [libpmem])
>> - AC_DEFINE(ENABLE_DAXIO, 1, [Enable daxctl io])
>> -fi
>> -AM_CONDITIONAL([ENABLE_DAXIO], [test "x$with_libpmem" = "xyes"])
>> +AS_IF([test "x$host_cpu" == "xx86_64"],
>> + [AC_DEFINE([ENABLE_DAXIO], [1], enable daxctl io command)])
>> +AM_CONDITIONAL([ENABLE_DAXIO], [test "x$host_cpu" == "xx86_64"])
>> +
>>
>> AC_ARG_WITH([bash-completion-dir],
>> AS_HELP_STRING([--with-bash-completion-dir[=PATH]],
>> diff --git a/daxctl/Makefile.am b/daxctl/Makefile.am
>> index 81d405e..8e3fd7e 100644
>> --- a/daxctl/Makefile.am
>> +++ b/daxctl/Makefile.am
>> @@ -17,7 +17,3 @@ daxctl_LDADD =\
>> ../libutil.a \
>> $(UUID_LIBS) \
>> $(JSON_LIBS)
>> -
>> -if ENABLE_DAXIO
>> -daxctl_LDADD += $(PMEM_LIBS)
>> -endif
>> diff --git a/daxctl/io.c b/daxctl/io.c
>> index 27e7463..a0f9613 100644
>> --- a/daxctl/io.c
>> +++ b/daxctl/io.c
>> @@ -20,9 +20,9 @@
>> #include <sys/mman.h>
>> #include <fcntl.h>
>> #include <unistd.h>
>> +#include <emmintrin.h>
>> #include <limits.h>
>> #include <libgen.h>
>> -#include <libpmem.h>
>> #include <util/json.h>
>> #include <util/filter.h>
>> #include <util/size.h>
>> @@ -357,6 +357,20 @@ static int clear_badblocks(struct io_dev *dev, uint64_t
>> len)
>> return 0;
>> }
>>
>
>> +#define FLUSH_ALIGN 64
>> +
>> +static void clflush(const void *addr, size_t len)
>> +{
>> + /*
>> + * Loop through cache-line-size (typically 64B) aligned chunks
>> + * covering the given range.
>> + */
>> + for (uintptr_t uptr = (uintptr_t)addr & ~(FLUSH_ALIGN - 1);
>> + uptr < (uintptr_t)addr + len; uptr += FLUSH_ALIGN)
>> + _mm_clflush((char *)uptr);
>> + _mm_sfence();
>> +}
>> +
>
> x86 specific stuff should probably be wrapped in an #ifdef
FLUSH_ALIGN is x86 specific. There's actually a sysctl to tell you the
L1 dcache size, so I'd suggest using that. Calling it clflush doesn't
bug me. I don't see anything else x86 specific there.
If you start using the sysctl, you can get rid of the newly imposed x86
restriction, too.
>> static int64_t __do_io(struct io_dev *dst_dev, struct io_dev *src_dev,
>> uint64_t len, bool zero)
>> {
>> @@ -366,12 +380,13 @@ static int64_t __do_io(struct io_dev *dst_dev, struct
>> io_dev *src_dev,
>> if (zero && dst_dev->is_dax) {
>> dst = (uint8_t *)dst_dev->mmap + dst_dev->offset;
>> memset(dst, 0, len);
>> - pmem_persist(dst, len);
>> + clflush(dst, len);
>> rc = len;
>> } else if (dst_dev->is_dax && src_dev->is_dax) {
>> src = (uint8_t *)src_dev->mmap + src_dev->offset;
>> dst = (uint8_t *)dst_dev->mmap + dst_dev->offset;
>> - pmem_memcpy_persist(dst, src, len);
>
>> + memcpy(dst, src, len);
>> + clflush(dst, len);
>
> I think you would be better off moving this into a seperate
> memcpy_to_pmem() function. That would give you a nice place for
> an arch-independent fallback too.
What's more, as I see this is basically lifting what's in libpmem, I'd
like to point out that pmem_memcpy_persist will use MOVNT where
available. This lifted version doesn't have that optimization.
I don't know, is it impossible to make this work w/o yanking out -lpmem?
Jeff
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
