[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Minios-devel] [UNIKRAFT PATCH v3 1/2] lib/vfscore: introduce vfscore
Hey,
I really like this patch. It is nice to read. I have just a few minor
comments regarding thread-safety: It is actually not needed right now
but we may forget when we decide to integrate a preemptive scheduler.
You are also allowed to put 2018 in the copyright header ;-).
Thanks,
Simon
On 14.06.2018 16:53, Yuri Volchkov wrote:
The initial implementation of the vfs abstraction layer. The task of
the library is to allocate file descriptors, and redirect generic
calls to the responsible library.
At this point only 3 basic calls are redirected: close(), read() and
write().
A target library (e.g. lwip) needs to provide an actual implementation
of the functions, by setting pointers in struct vfscore_fops. For
example:
struct lwip_file {
struct vfscore_file vfscore_file;
int private_fd;
};
int socket(int domain, int type, int protocol)
{
struct lwip_file *file;
int ret = vfscore_alloc_fd();
file = uk_malloc(uk_alloc_get_default(), sizeof(*file));
file->vfscore_file.fops = &lwip_fops;
file->private_fd = lwip_socket(blah, blah, blah);
vfscore_install_fd(ret, &file->vfscore_file);
}
If user calls 'write', vfscore will resolve corresponding struct
vfscore_file and will pass it to the registered (in the fops) write
function. The latter could get it's wrapper struct via __containerof()
Signed-off-by: Yuri Volchkov <yuri.volchkov@xxxxxxxxx>
---
lib/Makefile.uk | 1 +
lib/nolibc/include/unistd.h | 51 +++++++++++++++
lib/vfscore/Makefile.uk | 6 ++
lib/vfscore/export.syms | 7 +++
lib/vfscore/fd.c | 99 ++++++++++++++++++++++++++++++
lib/vfscore/file.c | 93 ++++++++++++++++++++++++++++
lib/vfscore/include/vfscore/file.h | 60 ++++++++++++++++++
7 files changed, 317 insertions(+)
create mode 100644 lib/nolibc/include/unistd.h
create mode 100644 lib/vfscore/Makefile.uk
create mode 100644 lib/vfscore/export.syms
create mode 100644 lib/vfscore/fd.c
create mode 100644 lib/vfscore/file.c
create mode 100644 lib/vfscore/include/vfscore/file.h
diff --git a/lib/Makefile.uk b/lib/Makefile.uk
index 21c3dd6..d5b31e5 100644
--- a/lib/Makefile.uk
+++ b/lib/Makefile.uk
@@ -13,3 +13,4 @@ $(eval $(call
_import_lib,$(CONFIG_UK_BASE)/lib/ukallocbbuddy))
$(eval $(call _import_lib,$(CONFIG_UK_BASE)/lib/uksched))
$(eval $(call _import_lib,$(CONFIG_UK_BASE)/lib/ukschedcoop))
$(eval $(call _import_lib,$(CONFIG_UK_BASE)/lib/fdt))
+$(eval $(call _import_lib,$(CONFIG_UK_BASE)/lib/vfscore))
diff --git a/lib/nolibc/include/unistd.h b/lib/nolibc/include/unistd.h
new file mode 100644
index 0000000..b723b6a
--- /dev/null
+++ b/lib/nolibc/include/unistd.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Yuri Volchkov <yuri.volchkov@xxxxxxxxx>
+ *
+ *
+ * Copyright (c) 2017, NEC Europe Ltd., NEC Corporation. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the copyright holder nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
+ */
+
+#ifndef __UNISTD_H__
+#define __UNISTD_H__
+#include <stdint.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+int close(int fd);
+ssize_t write(int fd, const void *buf, size_t count);
+ssize_t read(int fd, void *buf, size_t count);
+
+#ifdef __cplusplus
+}
+#endif
+#endif /* __UNISTD_H__ */
diff --git a/lib/vfscore/Makefile.uk b/lib/vfscore/Makefile.uk
new file mode 100644
index 0000000..fa56c8e
--- /dev/null
+++ b/lib/vfscore/Makefile.uk
@@ -0,0 +1,6 @@
+$(eval $(call addlib,libvfscore))
+
+CINCLUDES-y += -I$(LIBVFSCORE_BASE)/include
+
+LIBVFSCORE_SRCS-y += $(LIBVFSCORE_BASE)/fd.c
+LIBVFSCORE_SRCS-y += $(LIBVFSCORE_BASE)/file.c
diff --git a/lib/vfscore/export.syms b/lib/vfscore/export.syms
new file mode 100644
index 0000000..9e229a1
--- /dev/null
+++ b/lib/vfscore/export.syms
@@ -0,0 +1,7 @@
+vfscore_alloc_fd
+vfscore_put_fd
+vfscore_install_fd
+vfscore_get_file
+close
+write
+read
diff --git a/lib/vfscore/fd.c b/lib/vfscore/fd.c
new file mode 100644
index 0000000..85761ea
--- /dev/null
+++ b/lib/vfscore/fd.c
@@ -0,0 +1,99 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Yuri Volchkov <yuri.volchkov@xxxxxxxxx>
+ *
+ *
+ * Copyright (c) 2017, NEC Europe Ltd., NEC Corporation. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the copyright holder nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
+ */
+
+#include <string.h>
+#include <uk/essentials.h>
+#include <uk/arch/atomic.h>
+#include <uk/assert.h>
+#include <vfscore/file.h>
+
+#define FDTABLE_MAX_FILES (sizeof(uint64_t) * 8)
+
+struct fdtable {
+ uint64_t bitmap;
+ uint32_t fd_start;
+ struct vfscore_file *files[FDTABLE_MAX_FILES];
+};
+struct fdtable fdtable;
+
+int vfscore_alloc_fd(void)
+{
+ int ret = ukarch_find_lsbit(~fdtable.bitmap);
+
+ if (!ret)
+ return ret;
+
+ fdtable.bitmap |= (uint64_t) 1 << ret;
+
+ return ret;
+}
Could you guard this function by saving and restoring the interrupt
flags? finding the bit and reserving it should be a atomic operation.
We may run this together with a preemptive scheduler in the future.
+
+void vfscore_put_fd(int fd)
+{
+ UK_ASSERT(fd < (int) FDTABLE_MAX_FILES);
+ /* Currently it is not allowed to free std(in|out|err) */
+ UK_ASSERT(fd > 2);
+
+ fdtable.bitmap ^= (uint64_t) 1 << fd;
+}
Save the interrupt flags also before removing the bit and restore them
afterwards.
+
+void vfscore_install_fd(int fd, struct vfscore_file *file)
+{
+ UK_ASSERT(fd < (int) FDTABLE_MAX_FILES);
+ UK_ASSERT(fd > 2);
+ UK_ASSERT(file);
+
+ file->fd = fd;
+ fdtable.files[fd] = file;
+}
+
+struct vfscore_file *vfscore_get_file(int fd)
+{
+ UK_ASSERT(fd < (int) FDTABLE_MAX_FILES);
+ UK_ASSERT(fd > 2);
+
+ if (!(fdtable.bitmap & ((uint64_t) 1 << fd)))
+ return NULL;
+
The if-statement should probably also get guarded with interrupts off.
+ return fdtable.files[fd];
+}
+
+__constructor static void fdtable_init(void)
+{
+ memset(&fdtable, 0, sizeof(fdtable));
+
+ /* reserve stdin, stdout and stderr */
+ fdtable.bitmap = 7;
+}
diff --git a/lib/vfscore/file.c b/lib/vfscore/file.c
new file mode 100644
index 0000000..1f2c968
--- /dev/null
+++ b/lib/vfscore/file.c
@@ -0,0 +1,93 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Yuri Volchkov <yuri.volchkov@xxxxxxxxx>
+ *
+ *
+ * Copyright (c) 2017, NEC Europe Ltd., NEC Corporation. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the copyright holder nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
+ */
+
+#include <unistd.h>
+#include <errno.h>
+#include <uk/print.h>
+#include <vfscore/file.h>
+#include <uk/assert.h>
+
+
+int close(int fd)
+{
+ struct vfscore_file *file = vfscore_get_file(fd);
+
+ if (!file) {
+ uk_printd(DLVL_WARN, "no such file descriptor: %d\n", fd);
+ errno = EBADF;
+ return -1;
+ }
+
+ UK_ASSERT(file->fops->close);
Hum, you want have the assertion here? Maybe you want to use an if
statement instead. It may be possible that some files do not need a
close or can't even be closed (e.g., stdin, stdout, stderr).
+ return file->fops->close(file);
+}
+
+ssize_t write(int fd, const void *buf, size_t count)
+{
+ struct vfscore_file *file = vfscore_get_file(fd);
+
+ if (!file) {
+ uk_printd(DLVL_WARN, "no such file descriptor: %d\n", fd);
+ errno = EBADF;
+ return -1;
+ }
+
+ if (!file->fops->write) {
+ uk_printd(DLVL_WARN, "file does not have write op: %d\n", fd);
+ errno = EINVAL;
+ return -1;
+ }
+
+ return file->fops->write(file, buf, count);
+}
+
+ssize_t read(int fd, void *buf, size_t count)
+{
+ struct vfscore_file *file = vfscore_get_file(fd);
+
+ if (!file) {
+ uk_printd(DLVL_WARN, "no such file descriptor: %d\n", fd);
+ errno = EBADF;
+ return -1;
+ }
+
+ if (!file->fops->read) {
+ uk_printd(DLVL_WARN, "file does not have read op: %d\n", fd);
+ errno = EINVAL;
+ return -1;
+ }
+
+ return file->fops->read(file, buf, count);
+}
diff --git a/lib/vfscore/include/vfscore/file.h
b/lib/vfscore/include/vfscore/file.h
new file mode 100644
index 0000000..2c3d72b
--- /dev/null
+++ b/lib/vfscore/include/vfscore/file.h
@@ -0,0 +1,60 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Yuri Volchkov <yuri.volchkov@xxxxxxxxx>
+ *
+ *
+ * Copyright (c) 2017, NEC Europe Ltd., NEC Corporation. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the copyright holder nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
+ */
+
+#ifndef __VFSCORE_FILE_H__
+#define __VFSCORE_FILE_H__
+
+#include <stdint.h>
+#include <sys/types.h>
+
+struct vfscore_file;
+
+struct vfscore_fops {
+ int (*close)(struct vfscore_file *);
+ ssize_t (*write)(struct vfscore_file *, const void *, size_t);
+ ssize_t (*read)(struct vfscore_file *, void *, size_t);
+};
+
+struct vfscore_file {
+ int fd;
+ struct vfscore_fops *fops;
+};
+
+int vfscore_alloc_fd(void);
+void vfscore_put_fd(int fd);
+void vfscore_install_fd(int fd, struct vfscore_file *file);
+struct vfscore_file *vfscore_get_file(int fd);
+
+#endif /* __VFSCORE_FILE_H__ */
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
