From 2ed51d026ce010e124fc490b4ef0bb3aecf3a12e Mon Sep 17 00:00:00 2001 From: Ville Juven Date: Wed, 21 Dec 2022 15:13:14 +0200 Subject: [PATCH] fs/streams: Move the file streams from the group structure into TLS This is preparation for flushing streams from user space, like it should be done. - Move tg_streamlist (group, kernel space) -> ta_streamlist (TLS, user space) - Access stream list via tg_info in kernel - Access stream list via TLS in user space - Remove / rename nxsched_get_streams -> lib_getstreams - Remove system call for nxsched_get_streams --- fs/vfs/fs_fdopen.c | 7 ++--- include/nuttx/lib/lib.h | 7 ++++- include/nuttx/sched.h | 18 ------------- include/nuttx/tls.h | 4 +++ include/stdio.h | 8 +++--- include/sys/syscall_lookup.h | 1 - libs/libc/stdio/Make.defs | 1 + libs/libc/stdio/lib_fclose.c | 2 +- libs/libc/stdio/lib_fflush.c | 2 +- .../libc/stdio/lib_libgetstreams.c | 21 +++++---------- libs/libc/stdio/lib_libstream.c | 19 ++++--------- sched/group/group_create.c | 24 +---------------- sched/group/group_leave.c | 27 ------------------- sched/sched/Make.defs | 1 - syscall/syscall.csv | 1 - 15 files changed, 32 insertions(+), 111 deletions(-) rename sched/sched/sched_getstreams.c => libs/libc/stdio/lib_libgetstreams.c (82%) diff --git a/fs/vfs/fs_fdopen.c b/fs/vfs/fs_fdopen.c index db02601568b..5f7bf7005ad 100644 --- a/fs/vfs/fs_fdopen.c +++ b/fs/vfs/fs_fdopen.c @@ -34,6 +34,7 @@ #include #include #include +#include #include "inode/inode.h" @@ -153,11 +154,7 @@ int fs_fdopen(int fd, int oflags, FAR struct tcb_s *tcb, /* Get the stream list from the TCB */ -#ifdef CONFIG_MM_KERNEL_HEAP - slist = tcb->group->tg_streamlist; -#else - slist = &tcb->group->tg_streamlist; -#endif + slist = &tcb->group->tg_info->ta_streamlist; /* Allocate FILE structure */ diff --git a/include/nuttx/lib/lib.h b/include/nuttx/lib/lib.h index 00b41adef7a..a2632c77b4a 100644 --- a/include/nuttx/lib/lib.h +++ b/include/nuttx/lib/lib.h @@ -107,7 +107,12 @@ extern "C" struct task_group_s; void lib_stream_initialize(FAR struct task_group_s *group); void lib_stream_release(FAR struct task_group_s *group); -#endif + +/* Functions contained in lib_getstreams.c **********************************/ + +struct streamlist; +FAR struct streamlist *lib_get_streams(void); +#endif /* CONFIG_FILE_STREAM */ /* Functions defined in lib_srand.c *****************************************/ diff --git a/include/nuttx/sched.h b/include/nuttx/sched.h index fb6b3de1a82..bf8db3ff819 100644 --- a/include/nuttx/sched.h +++ b/include/nuttx/sched.h @@ -495,21 +495,6 @@ struct task_group_s struct filelist tg_filelist; /* Maps file descriptor to file */ -#ifdef CONFIG_FILE_STREAM - /* FILE streams ***********************************************************/ - - /* In a flat, single-heap build. The stream list is allocated with this - * structure. But kernel mode with a kernel allocator, - * it must be separately allocated using a user-space allocator. - */ - -#ifdef CONFIG_MM_KERNEL_HEAP - FAR struct streamlist *tg_streamlist; -#else - struct streamlist tg_streamlist; /* Holds C buffered I/O info */ -#endif -#endif - #ifdef CONFIG_ARCH_ADDRENV /* Address Environment ****************************************************/ @@ -841,9 +826,6 @@ int nxsched_release_tcb(FAR struct tcb_s *tcb, uint8_t ttype); */ FAR struct filelist *nxsched_get_files(void); -#ifdef CONFIG_FILE_STREAM -FAR struct streamlist *nxsched_get_streams(void); -#endif /* CONFIG_FILE_STREAM */ /**************************************************************************** * Name: nxtask_init diff --git a/include/nuttx/tls.h b/include/nuttx/tls.h index 3215e899816..60e7f6cbf91 100644 --- a/include/nuttx/tls.h +++ b/include/nuttx/tls.h @@ -29,6 +29,7 @@ #include #include +#include #include #include @@ -146,6 +147,9 @@ struct task_info_s #if CONFIG_LIBC_MAX_EXITFUNS > 0 struct atexit_list_s ta_exit; /* Exit functions */ #endif +#ifdef CONFIG_FILE_STREAM + struct streamlist ta_streamlist; /* Holds C buffered I/O info */ +#endif }; /* struct pthread_cleanup_s *************************************************/ diff --git a/include/stdio.h b/include/stdio.h index 29ca4e878b9..92a863d3469 100644 --- a/include/stdio.h +++ b/include/stdio.h @@ -32,7 +32,7 @@ #include #include -#include +#include /**************************************************************************** * Pre-processor Definitions @@ -63,9 +63,9 @@ /* The first three _iob entries are reserved for standard I/O */ -#define stdin (&nxsched_get_streams()->sl_std[0]) -#define stdout (&nxsched_get_streams()->sl_std[1]) -#define stderr (&nxsched_get_streams()->sl_std[2]) +#define stdin (&lib_get_streams()->sl_std[0]) +#define stdout (&lib_get_streams()->sl_std[1]) +#define stderr (&lib_get_streams()->sl_std[2]) /* Path to the directory where temporary files can be created */ diff --git a/include/sys/syscall_lookup.h b/include/sys/syscall_lookup.h index 0fcae2aa975..57645b2e994 100644 --- a/include/sys/syscall_lookup.h +++ b/include/sys/syscall_lookup.h @@ -268,7 +268,6 @@ SYSCALL_LOOKUP(futimens, 2) #ifdef CONFIG_FILE_STREAM SYSCALL_LOOKUP(fs_fdopen, 4) - SYSCALL_LOOKUP(nxsched_get_streams, 0) #endif #ifndef CONFIG_DISABLE_MOUNTPOINT diff --git a/libs/libc/stdio/Make.defs b/libs/libc/stdio/Make.defs index 44fd90219c8..57d169fc683 100644 --- a/libs/libc/stdio/Make.defs +++ b/libs/libc/stdio/Make.defs @@ -45,6 +45,7 @@ CSRCS += lib_fputs.c lib_ungetc.c lib_fprintf.c lib_vfprintf.c CSRCS += lib_feof.c lib_ferror.c lib_rewind.c lib_clearerr.c CSRCS += lib_scanf.c lib_vscanf.c lib_fscanf.c lib_vfscanf.c lib_tmpfile.c CSRCS += lib_setbuf.c lib_setvbuf.c lib_libstream.c lib_libfilelock.c +CSRCS += lib_libgetstreams.c endif # Add the stdio directory to the build diff --git a/libs/libc/stdio/lib_fclose.c b/libs/libc/stdio/lib_fclose.c index 54812c7ef29..df6cd9ab3ca 100644 --- a/libs/libc/stdio/lib_fclose.c +++ b/libs/libc/stdio/lib_fclose.c @@ -84,7 +84,7 @@ int fclose(FAR FILE *stream) /* Remove FILE structure from the stream list */ - slist = nxsched_get_streams(); + slist = lib_get_streams(); nxmutex_lock(&slist->sl_lock); for (next = slist->sl_head; next; prev = next, next = next->fs_next) diff --git a/libs/libc/stdio/lib_fflush.c b/libs/libc/stdio/lib_fflush.c index da9c91fa410..532b327264f 100644 --- a/libs/libc/stdio/lib_fflush.c +++ b/libs/libc/stdio/lib_fflush.c @@ -62,7 +62,7 @@ int fflush(FAR FILE *stream) { /* Yes... then this is a request to flush all streams */ - ret = lib_flushall(nxsched_get_streams()); + ret = lib_flushall(lib_get_streams()); } else { diff --git a/sched/sched/sched_getstreams.c b/libs/libc/stdio/lib_libgetstreams.c similarity index 82% rename from sched/sched/sched_getstreams.c rename to libs/libc/stdio/lib_libgetstreams.c index 481149dd5d1..3c31ecae1af 100644 --- a/sched/sched/sched_getstreams.c +++ b/libs/libc/stdio/lib_libgetstreams.c @@ -1,5 +1,5 @@ /**************************************************************************** - * sched/sched/sched_getstreams.c + * libs/libc/stdio/lib_libgetstreams.c * * Licensed to the Apache Software Foundation (ASF) under one or more * contributor license agreements. See the NOTICE file distributed with @@ -23,10 +23,9 @@ ****************************************************************************/ #include -#include #include -#include "sched/sched.h" +#include #ifdef CONFIG_FILE_STREAM @@ -35,7 +34,7 @@ ****************************************************************************/ /**************************************************************************** - * Name: nxsched_get_streams + * Name: lib_get_streams * * Description: * Return a pointer to the streams list for this thread @@ -50,18 +49,12 @@ * ****************************************************************************/ -FAR struct streamlist *nxsched_get_streams(void) +FAR struct streamlist *lib_get_streams(void) { - FAR struct tcb_s *rtcb = this_task(); - FAR struct task_group_s *group = rtcb->group; + FAR struct task_info_s *info; - DEBUGASSERT(group); - -#ifdef CONFIG_MM_KERNEL_HEAP - return group->tg_streamlist; -#else - return &group->tg_streamlist; -#endif + info = task_get_info(); + return &info->ta_streamlist; } #endif /* CONFIG_FILE_STREAM */ diff --git a/libs/libc/stdio/lib_libstream.c b/libs/libc/stdio/lib_libstream.c index 3931744271a..be6a2a1c021 100644 --- a/libs/libc/stdio/lib_libstream.c +++ b/libs/libc/stdio/lib_libstream.c @@ -34,6 +34,7 @@ #include #include #include +#include #include "libc.h" @@ -56,13 +57,8 @@ void lib_stream_initialize(FAR struct task_group_s *group) { FAR struct streamlist *list; -#ifdef CONFIG_MM_KERNEL_HEAP - DEBUGASSERT(group && group->tg_streamlist); - list = group->tg_streamlist; -#else - DEBUGASSERT(group); - list = &group->tg_streamlist; -#endif + DEBUGASSERT(group && group->tg_info); + list = &group->tg_info->ta_streamlist; /* Initialize the list access mutex */ @@ -94,13 +90,8 @@ void lib_stream_release(FAR struct task_group_s *group) { FAR struct streamlist *list; -#ifdef CONFIG_MM_KERNEL_HEAP - DEBUGASSERT(group && group->tg_streamlist); - list = group->tg_streamlist; -#else - DEBUGASSERT(group); - list = &group->tg_streamlist; -#endif + DEBUGASSERT(group && group->tg_info); + list = &group->tg_info->ta_streamlist; /* Destroy the mutex and release the filelist */ diff --git a/sched/group/group_create.c b/sched/group/group_create.c index ece3eacef98..fe7533629a6 100644 --- a/sched/group/group_create.c +++ b/sched/group/group_create.c @@ -149,24 +149,6 @@ int group_allocate(FAR struct task_tcb_s *tcb, uint8_t ttype) { group->tg_flags |= GROUP_FLAG_PRIVILEGED; } - -# if defined(CONFIG_FILE_STREAM) - /* In a flat, single-heap build. The stream list is allocated with the - * group structure. But in a kernel build with a kernel allocator, it - * must be separately allocated using a user-space allocator. - * - * REVISIT: Kernel threads should not require a stream allocation. They - * should not be using C buffered I/O at all. - */ - - group->tg_streamlist = (FAR struct streamlist *) - group_zalloc(group, sizeof(struct streamlist)); - if (!group->tg_streamlist) - { - goto errout_with_group; - } - -# endif /* defined(CONFIG_FILE_STREAM) */ #endif /* defined(CONFIG_MM_KERNEL_HEAP) */ #ifdef HAVE_GROUP_MEMBERS @@ -175,7 +157,7 @@ int group_allocate(FAR struct task_tcb_s *tcb, uint8_t ttype) group->tg_members = kmm_malloc(GROUP_INITIAL_MEMBERS * sizeof(pid_t)); if (!group->tg_members) { - goto errout_with_stream; + goto errout_with_group; } /* Number of members in allocation */ @@ -226,10 +208,6 @@ int group_allocate(FAR struct task_tcb_s *tcb, uint8_t ttype) errout_with_member: #ifdef HAVE_GROUP_MEMBERS kmm_free(group->tg_members); -errout_with_stream: -#endif -#if defined(CONFIG_FILE_STREAM) && defined(CONFIG_MM_KERNEL_HEAP) - group_free(group, group->tg_streamlist); errout_with_group: #endif kmm_free(group); diff --git a/sched/group/group_leave.c b/sched/group/group_leave.c index fa203f69caf..1436a3466b3 100644 --- a/sched/group/group_leave.c +++ b/sched/group/group_leave.c @@ -197,33 +197,6 @@ static inline void group_release(FAR struct task_group_s *group) } #endif -#if defined(CONFIG_FILE_STREAM) && defined(CONFIG_MM_KERNEL_HEAP) - /* In a flat, single-heap build. The stream list is part of the - * group structure and, hence will be freed when the group structure - * is freed. Otherwise, it is separately allocated an must be - * freed here. - */ - -# ifdef CONFIG_BUILD_KERNEL - /* In the kernel build, the unprivileged process's stream list will be - * allocated from with its per-process, private user heap. But in that - * case, there is no reason to do anything here: That allocation resides - * in the user heap which which be completely freed when we destroy the - * process's address environment. - */ - - if ((group->tg_flags & GROUP_FLAG_PRIVILEGED) != 0) -# endif - { - /* But kernel threads are different in this build configuration: Their - * stream lists were allocated from the common, global kernel heap and - * must explicitly freed here. - */ - - group_free(group, group->tg_streamlist); - } -#endif - #ifdef CONFIG_BINFMT_LOADABLE /* If the exiting task was loaded into RAM from a file, then we need to * lease all of the memory resource when the last thread exits the task diff --git a/sched/sched/Make.defs b/sched/sched/Make.defs index 257ff3f40b5..ade87e00bf5 100644 --- a/sched/sched/Make.defs +++ b/sched/sched/Make.defs @@ -23,7 +23,6 @@ CSRCS += sched_addreadytorun.c sched_removereadytorun.c CSRCS += sched_addprioritized.c sched_mergeprioritized.c sched_mergepending.c CSRCS += sched_addblocked.c sched_removeblocked.c CSRCS += sched_gettcb.c sched_verifytcb.c sched_releasetcb.c -CSRCS += sched_getstreams.c CSRCS += sched_setparam.c sched_setpriority.c sched_getparam.c CSRCS += sched_setscheduler.c sched_getscheduler.c CSRCS += sched_yield.c sched_rrgetinterval.c sched_foreach.c diff --git a/syscall/syscall.csv b/syscall/syscall.csv index 13f31bc9847..f5909bf2959 100644 --- a/syscall/syscall.csv +++ b/syscall/syscall.csv @@ -74,7 +74,6 @@ "nx_pthread_exit","nuttx/pthread.h","!defined(CONFIG_DISABLE_PTHREAD)","noreturn","pthread_addr_t" "nx_vsyslog","nuttx/syslog/syslog.h","","int","int","FAR const IPTR char *","FAR va_list *" "nxsched_get_stackinfo","nuttx/sched.h","","int","pid_t","FAR struct stackinfo_s *" -"nxsched_get_streams","nuttx/sched.h","defined(CONFIG_FILE_STREAM)","FAR struct streamlist *" "open","fcntl.h","","int","FAR const char *","int","...","mode_t" "pgalloc", "nuttx/arch.h", "defined(CONFIG_BUILD_KERNEL)", "uintptr_t", "uintptr_t", "unsigned int" "pipe2","unistd.h","defined(CONFIG_PIPES) && CONFIG_DEV_PIPE_SIZE > 0","int","int [2]|FAR int *","int"