From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id A1D3CA00BE; Wed, 3 Jun 2020 14:08:13 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id DDC851C439; Wed, 3 Jun 2020 14:08:12 +0200 (CEST) Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id B85211C436 for ; Wed, 3 Jun 2020 14:08:11 +0200 (CEST) Received: from 2606-a000-111b-4634-0000-0000-0000-1bf2.inf6.spectrum.com ([2606:a000:111b:4634::1bf2] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from ) id 1jgSBc-0001qr-V8; Wed, 03 Jun 2020 08:08:05 -0400 Date: Wed, 3 Jun 2020 08:07:59 -0400 From: Neil Horman To: Dmitry Kozlyuk Cc: dev@dpdk.org, Dmitry Malloy , Narcisa Ana Maria Vasile , Fady Bader , Tal Shnaiderman , Thomas Monjalon , Anatoly Burakov , Bruce Richardson Message-ID: <20200603120759.GA426574@hmswarspite.think-freely.org> References: <20200525003720.6410-1-dmitry.kozliuk@gmail.com> <20200602230329.17838-1-dmitry.kozliuk@gmail.com> <20200602230329.17838-3-dmitry.kozliuk@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200602230329.17838-3-dmitry.kozliuk@gmail.com> X-Spam-Score: -2.9 (--) X-Spam-Status: No Subject: Re: [dpdk-dev] [PATCH v6 02/11] eal: introduce internal wrappers for file operations X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, Jun 03, 2020 at 02:03:20AM +0300, Dmitry Kozlyuk wrote: > Introduce OS-independent wrappers in order to support common EAL code > on Unix and Windows: > > * eal_file_create: open an existing file. > * eal_file_open: create a file or open it if exists. > * eal_file_lock: lock or unlock an open file. > * eal_file_truncate: enforce a given size for an open file. > > Implementation for Linux and FreeBSD is placed in "unix" subdirectory, > which is intended for common code between the two. These thin wrappers > require no special maintenance. > > Common code supporting multi-process doesn't use the new wrappers, > because it is inherently Unix-specific and would impose excessive > requirements on the wrappers. > > Signed-off-by: Dmitry Kozlyuk > --- > MAINTAINERS | 3 + > lib/librte_eal/common/eal_common_fbarray.c | 31 ++++----- > lib/librte_eal/common/eal_private.h | 74 +++++++++++++++++++++ > lib/librte_eal/freebsd/Makefile | 4 ++ > lib/librte_eal/linux/Makefile | 4 ++ > lib/librte_eal/meson.build | 4 ++ > lib/librte_eal/unix/eal_file.c | 76 ++++++++++++++++++++++ > lib/librte_eal/unix/meson.build | 6 ++ > 8 files changed, 183 insertions(+), 19 deletions(-) > create mode 100644 lib/librte_eal/unix/eal_file.c > create mode 100644 lib/librte_eal/unix/meson.build > > diff --git a/MAINTAINERS b/MAINTAINERS > index d2b286701..1d9aff26d 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -323,6 +323,9 @@ FreeBSD UIO > M: Bruce Richardson > F: kernel/freebsd/nic_uio/ > > +Unix shared files > +F: lib/librte_eal/unix/ > + > Windows support > M: Harini Ramakrishnan > M: Omar Cardona > diff --git a/lib/librte_eal/common/eal_common_fbarray.c b/lib/librte_eal/common/eal_common_fbarray.c > index 4f8f1af73..81ce4bd42 100644 > --- a/lib/librte_eal/common/eal_common_fbarray.c > +++ b/lib/librte_eal/common/eal_common_fbarray.c > @@ -8,8 +8,8 @@ > #include > #include > #include > -#include > #include > +#include > > #include > #include > @@ -85,10 +85,8 @@ resize_and_map(int fd, void *addr, size_t len) > char path[PATH_MAX]; > void *map_addr; > > - if (ftruncate(fd, len)) { > + if (eal_file_truncate(fd, len)) { > RTE_LOG(ERR, EAL, "Cannot truncate %s\n", path); > - /* pass errno up the chain */ > - rte_errno = errno; > return -1; > } > > @@ -772,15 +770,15 @@ rte_fbarray_init(struct rte_fbarray *arr, const char *name, unsigned int len, > * and see if we succeed. If we don't, someone else is using it > * already. > */ > - fd = open(path, O_CREAT | O_RDWR, 0600); > + fd = eal_file_create(path); > if (fd < 0) { > RTE_LOG(DEBUG, EAL, "%s(): couldn't open %s: %s\n", > - __func__, path, strerror(errno)); > - rte_errno = errno; > + __func__, path, rte_strerror(rte_errno)); > goto fail; > - } else if (flock(fd, LOCK_EX | LOCK_NB)) { > + } else if (eal_file_lock( > + fd, EAL_FLOCK_EXCLUSIVE, EAL_FLOCK_RETURN)) { > RTE_LOG(DEBUG, EAL, "%s(): couldn't lock %s: %s\n", > - __func__, path, strerror(errno)); > + __func__, path, rte_strerror(rte_errno)); > rte_errno = EBUSY; > goto fail; > } > @@ -789,10 +787,8 @@ rte_fbarray_init(struct rte_fbarray *arr, const char *name, unsigned int len, > * still attach to it, but no other process could reinitialize > * it. > */ > - if (flock(fd, LOCK_SH | LOCK_NB)) { > - rte_errno = errno; > + if (eal_file_lock(fd, EAL_FLOCK_SHARED, EAL_FLOCK_RETURN)) > goto fail; > - } > > if (resize_and_map(fd, data, mmap_len)) > goto fail; > @@ -888,17 +884,14 @@ rte_fbarray_attach(struct rte_fbarray *arr) > > eal_get_fbarray_path(path, sizeof(path), arr->name); > > - fd = open(path, O_RDWR); > + fd = eal_file_open(path, true); > if (fd < 0) { > - rte_errno = errno; > goto fail; > } > > /* lock the file, to let others know we're using it */ > - if (flock(fd, LOCK_SH | LOCK_NB)) { > - rte_errno = errno; > + if (eal_file_lock(fd, EAL_FLOCK_SHARED, EAL_FLOCK_RETURN)) > goto fail; > - } > > if (resize_and_map(fd, data, mmap_len)) > goto fail; > @@ -1025,7 +1018,7 @@ rte_fbarray_destroy(struct rte_fbarray *arr) > * has been detached by all other processes > */ > fd = tmp->fd; > - if (flock(fd, LOCK_EX | LOCK_NB)) { > + if (eal_file_lock(fd, EAL_FLOCK_EXCLUSIVE, EAL_FLOCK_RETURN)) { > RTE_LOG(DEBUG, EAL, "Cannot destroy fbarray - another process is using it\n"); > rte_errno = EBUSY; > ret = -1; > @@ -1042,7 +1035,7 @@ rte_fbarray_destroy(struct rte_fbarray *arr) > * we're still holding an exclusive lock, so drop it to > * shared. > */ > - flock(fd, LOCK_SH | LOCK_NB); > + eal_file_lock(fd, EAL_FLOCK_SHARED, EAL_FLOCK_RETURN); > > ret = -1; > goto out; > diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h > index 869ce183a..727f26881 100644 > --- a/lib/librte_eal/common/eal_private.h > +++ b/lib/librte_eal/common/eal_private.h > @@ -420,4 +420,78 @@ eal_malloc_no_trace(const char *type, size_t size, unsigned int align); > > void eal_free_no_trace(void *addr); > > +/** > + * Create a file or open it if exits. > + * > + * Newly created file is only accessible to the owner (0600 equivalent). > + * Returned descriptor is always read/write. > + * > + * @param path > + * Path to the file. > + * @return > + * Open file descriptor on success, (-1) on failure and rte_errno is set. > + */ > +int > +eal_file_create(const char *path); > + > +/** > + * Open an existing file. > + * > + * @param path > + * Path to the file. > + * @param writable > + * Whether to open file read/write or read-only. > + * @return > + * Open file descriptor on success, (-1) on failure and rte_errno is set. > + */ > +int > +eal_file_open(const char *path, bool writable); > + > +/** File locking operation. */ > +enum eal_flock_op { > + EAL_FLOCK_SHARED, /**< Acquire a shared lock. */ > + EAL_FLOCK_EXCLUSIVE, /**< Acquire an exclusive lock. */ > + EAL_FLOCK_UNLOCK /**< Release a previously taken lock. */ > +}; > + > +/** Behavior on file locking conflict. */ > +enum eal_flock_mode { > + EAL_FLOCK_WAIT, /**< Wait until the file gets unlocked to lock it. */ > + EAL_FLOCK_RETURN /**< Return immediately if the file is locked. */ > +}; > + > +/** > + * Lock or unlock the file. > + * > + * On failure @code rte_errno @endcode is set to the error code > + * specified by POSIX flock(3) description. > + * > + * @param fd > + * Opened file descriptor. > + * @param op > + * Operation to perform. > + * @param mode > + * Behavior on conflict. > + * @return > + * 0 on success, (-1) on failure. > + */ > +int > +eal_file_lock(int fd, enum eal_flock_op op, enum eal_flock_mode mode); > + > +/** > + * Truncate or extend the file to the specified size. > + * > + * On failure @code rte_errno @endcode is set to the error code > + * specified by POSIX ftruncate(3) description. > + * > + * @param fd > + * Opened file descriptor. > + * @param size > + * Desired file size. > + * @return > + * 0 on success, (-1) on failure. > + */ > +int > +eal_file_truncate(int fd, ssize_t size); > + > #endif /* _EAL_PRIVATE_H_ */ > diff --git a/lib/librte_eal/freebsd/Makefile b/lib/librte_eal/freebsd/Makefile > index af95386d4..0f8741d96 100644 > --- a/lib/librte_eal/freebsd/Makefile > +++ b/lib/librte_eal/freebsd/Makefile > @@ -7,6 +7,7 @@ LIB = librte_eal.a > > ARCH_DIR ?= $(RTE_ARCH) > VPATH += $(RTE_SDK)/lib/librte_eal/$(ARCH_DIR) > +VPATH += $(RTE_SDK)/lib/librte_eal/unix > VPATH += $(RTE_SDK)/lib/librte_eal/common > > CFLAGS += -I$(SRCDIR)/include > @@ -74,6 +75,9 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_FREEBSD) += rte_service.c > SRCS-$(CONFIG_RTE_EXEC_ENV_FREEBSD) += rte_random.c > SRCS-$(CONFIG_RTE_EXEC_ENV_FREEBSD) += rte_reciprocal.c > > +# from unix dir > +SRCS-$(CONFIG_RTE_EXEC_ENV_FREEBSD) += eal_file.c > + > # from arch dir > SRCS-$(CONFIG_RTE_EXEC_ENV_FREEBSD) += rte_cpuflags.c > SRCS-$(CONFIG_RTE_EXEC_ENV_FREEBSD) += rte_hypervisor.c > diff --git a/lib/librte_eal/linux/Makefile b/lib/librte_eal/linux/Makefile > index 48cc34844..331489f99 100644 > --- a/lib/librte_eal/linux/Makefile > +++ b/lib/librte_eal/linux/Makefile > @@ -7,6 +7,7 @@ LIB = librte_eal.a > > ARCH_DIR ?= $(RTE_ARCH) > VPATH += $(RTE_SDK)/lib/librte_eal/$(ARCH_DIR) > +VPATH += $(RTE_SDK)/lib/librte_eal/unix > VPATH += $(RTE_SDK)/lib/librte_eal/common > > CFLAGS += -I$(SRCDIR)/include > @@ -81,6 +82,9 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUX) += rte_service.c > SRCS-$(CONFIG_RTE_EXEC_ENV_LINUX) += rte_random.c > SRCS-$(CONFIG_RTE_EXEC_ENV_LINUX) += rte_reciprocal.c > > +# from unix dir > +SRCS-$(CONFIG_RTE_EXEC_ENV_LINUX) += eal_file.c > + > # from arch dir > SRCS-$(CONFIG_RTE_EXEC_ENV_LINUX) += rte_cpuflags.c > SRCS-$(CONFIG_RTE_EXEC_ENV_LINUX) += rte_hypervisor.c > diff --git a/lib/librte_eal/meson.build b/lib/librte_eal/meson.build > index e301f4558..8d492897d 100644 > --- a/lib/librte_eal/meson.build > +++ b/lib/librte_eal/meson.build > @@ -6,6 +6,10 @@ subdir('include') > > subdir('common') > > +if not is_windows > + subdir('unix') > +endif > + > dpdk_conf.set('RTE_EXEC_ENV_' + exec_env.to_upper(), 1) > subdir(exec_env) > > diff --git a/lib/librte_eal/unix/eal_file.c b/lib/librte_eal/unix/eal_file.c > new file mode 100644 > index 000000000..7b3ffa629 > --- /dev/null > +++ b/lib/librte_eal/unix/eal_file.c > @@ -0,0 +1,76 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2020 Dmitry Kozlyuk > + */ > + > +#include > +#include > +#include > + > +#include > + > +#include "eal_private.h" > + > +int > +eal_file_create(const char *path) > +{ > + int ret; > + > + ret = open(path, O_CREAT | O_RDWR, 0600); > + if (ret < 0) > + rte_errno = errno; > + > + return ret; > +} > + You don't need this call if you support the oflags option in the open call below. > +int > +eal_file_open(const char *path, bool writable) > +{ > + int ret, flags; > + > + flags = writable ? O_RDWR : O_RDONLY; > + ret = open(path, flags); > + if (ret < 0) > + rte_errno = errno; > + > + return ret; > +} > + why are you changing this api from the posix file format (with oflags specified). As far as I can see both unix and windows platforms support that > +int > +eal_file_truncate(int fd, ssize_t size) > +{ > + int ret; > + > + ret = ftruncate(fd, size); > + if (ret) > + rte_errno = errno; > + > + return ret; > +} > + > +int > +eal_file_lock(int fd, enum eal_flock_op op, enum eal_flock_mode mode) > +{ > + int sys_flags = 0; > + int ret; > + > + if (mode == EAL_FLOCK_RETURN) > + sys_flags |= LOCK_NB; > + > + switch (op) { > + case EAL_FLOCK_EXCLUSIVE: > + sys_flags |= LOCK_EX; > + break; > + case EAL_FLOCK_SHARED: > + sys_flags |= LOCK_SH; > + break; > + case EAL_FLOCK_UNLOCK: > + sys_flags |= LOCK_UN; > + break; > + } > + > + ret = flock(fd, sys_flags); > + if (ret) > + rte_errno = errno; > + > + return ret; > +} > diff --git a/lib/librte_eal/unix/meson.build b/lib/librte_eal/unix/meson.build > new file mode 100644 > index 000000000..21029ba1a > --- /dev/null > +++ b/lib/librte_eal/unix/meson.build > @@ -0,0 +1,6 @@ > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright(c) 2020 Dmitry Kozlyuk > + > +sources += files( > + 'eal_file.c', > +) > -- > 2.25.4 > >