|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Minios-devel] [UNIKRAFT PATCH v3 1/2] lib/vfscore: introduce vfscore
Simon Kuenzer <simon.kuenzer@xxxxxxxxx> writes:
> 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 ;-).
Time runs so fast...
>
> 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.
Ok
>
>> +
>> +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.
You are right, protection is needed here too. I will use atomic
operation here. So do not need to disable interrupts. In particular this case.
>
>> +
>> +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.
Ok
>
>> + 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).
Alright, will signal EIO.
>
>> + 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__ */
>>
--
Yuri Volchkov
Software Specialist
NEC Europe Ltd
Kurfürsten-Anlage 36
D-69115 Heidelberg
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |