From 286ac59eb48c4839032a0646b6fddecd479187ca Mon Sep 17 00:00:00 2001 From: hujun5 Date: Fri, 15 Nov 2024 10:30:51 +0800 Subject: [PATCH 1/2] add spinlock_type.h reason: Due to incomplete handling of spinlock_t in arch/spinlock.h, it should not be used directly by external code. Furthermore, because pthread.h and nuttx/spinlock.h have a circular dependency, pthread.h cannot successfully include nuttx/spinlock.h. Therefore, we have split spinlock_type.h from spinlock.h. Signed-off-by: hujun5 --- include/nuttx/spinlock.h | 46 +----------------- include/nuttx/spinlock_type.h | 90 +++++++++++++++++++++++++++++++++++ include/pthread.h | 2 +- 3 files changed, 93 insertions(+), 45 deletions(-) create mode 100644 include/nuttx/spinlock_type.h diff --git a/include/nuttx/spinlock.h b/include/nuttx/spinlock.h index 31d4becd08a63..59ff61dc8a239 100644 --- a/include/nuttx/spinlock.h +++ b/include/nuttx/spinlock.h @@ -41,6 +41,8 @@ # include #endif +#include + #undef EXTERN #if defined(__cplusplus) #define EXTERN extern "C" @@ -50,50 +52,6 @@ extern "C" #define EXTERN extern #endif -#if defined(CONFIG_RW_SPINLOCK) -typedef int rwlock_t; -# define RW_SP_UNLOCKED 0 -# define RW_SP_READ_LOCKED 1 -# define RW_SP_WRITE_LOCKED -1 -#endif - -#ifndef CONFIG_SPINLOCK -# define SP_UNLOCKED 0 /* The Un-locked state */ -# define SP_LOCKED 1 /* The Locked state */ - -typedef uint8_t spinlock_t; -#elif defined(CONFIG_TICKET_SPINLOCK) - -union spinlock_u -{ - struct - { - unsigned short owner; - unsigned short next; - } tickets; - unsigned int value; -}; -typedef union spinlock_u spinlock_t; - -# define SP_UNLOCKED (union spinlock_u){{0, 0}} -# define SP_LOCKED (union spinlock_u){{0, 1}} - -#else - -/* The architecture specific spinlock.h header file must also provide the - * following: - * - * SP_LOCKED - A definition of the locked state value (usually 1) - * SP_UNLOCKED - A definition of the unlocked state value (usually 0) - * spinlock_t - The type of a spinlock memory object. - * - * SP_LOCKED and SP_UNLOCKED must be constants of type spinlock_t. - */ - -#include - -#endif /* CONFIG_SPINLOCK */ - /**************************************************************************** * Pre-processor Definitions ****************************************************************************/ diff --git a/include/nuttx/spinlock_type.h b/include/nuttx/spinlock_type.h new file mode 100644 index 0000000000000..939cc96791a92 --- /dev/null +++ b/include/nuttx/spinlock_type.h @@ -0,0 +1,90 @@ +/**************************************************************************** + * include/nuttx/spinlock_type.h + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +#ifndef __INCLUDE_NUTTX_SPINLOCK_TYPE_H +#define __INCLUDE_NUTTX_SPINLOCK_TYPE_H + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include + +#undef EXTERN +#if defined(__cplusplus) +#define EXTERN extern "C" +extern "C" +{ +#else +#define EXTERN extern +#endif + +#if defined(CONFIG_RW_SPINLOCK) +typedef int rwlock_t; +# define RW_SP_UNLOCKED 0 +# define RW_SP_READ_LOCKED 1 +# define RW_SP_WRITE_LOCKED -1 +#endif + +#ifndef CONFIG_SPINLOCK +# define SP_UNLOCKED 0 /* The Un-locked state */ +# define SP_LOCKED 1 /* The Locked state */ + +typedef uint8_t spinlock_t; +#elif defined(CONFIG_TICKET_SPINLOCK) + +union spinlock_u +{ + struct + { + unsigned short owner; + unsigned short next; + } tickets; + unsigned int value; +}; +typedef union spinlock_u spinlock_t; + +# define SP_UNLOCKED (union spinlock_u){{0, 0}} +# define SP_LOCKED (union spinlock_u){{0, 1}} + +#else + +/* The architecture specific spinlock.h header file must also provide the + * following: + * + * SP_LOCKED - A definition of the locked state value (usually 1) + * SP_UNLOCKED - A definition of the unlocked state value (usually 0) + * spinlock_t - The type of a spinlock memory object. + * + * SP_LOCKED and SP_UNLOCKED must be constants of type spinlock_t. + */ + +#include + +#endif /* CONFIG_SPINLOCK */ + +#undef EXTERN +#if defined(__cplusplus) +} +#endif + +#endif /* __INCLUDE_NUTTX_SPINLOCK_TYPE_H */ diff --git a/include/pthread.h b/include/pthread.h index e0866c8462d51..191bbf4e8bf15 100644 --- a/include/pthread.h +++ b/include/pthread.h @@ -49,7 +49,7 @@ * SP_LOCKED and SP_UNLOCKED must constants of type spinlock_t. */ -# include +# include #endif /**************************************************************************** From bf396e660d49d96902413a63030622b503f76488 Mon Sep 17 00:00:00 2001 From: hujun5 Date: Fri, 29 Nov 2024 21:50:04 +0800 Subject: [PATCH 2/2] fs: use small lock to protect filelist Signed-off-by: hujun5 --- fs/inode/fs_files.c | 19 ++++++++++--------- include/nuttx/fs/fs.h | 2 ++ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/fs/inode/fs_files.c b/fs/inode/fs_files.c index e3d7b60cebe9c..42cd054759a04 100644 --- a/fs/inode/fs_files.c +++ b/fs/inode/fs_files.c @@ -72,7 +72,7 @@ static FAR struct file *files_fget_by_index(FAR struct filelist *list, FAR struct file *filep; irqstate_t flags; - flags = spin_lock_irqsave(NULL); + flags = spin_lock_irqsave(&list->fl_lock); filep = &list->fl_files[l1][l2]; #ifdef CONFIG_FS_REFCOUNT @@ -111,7 +111,7 @@ static FAR struct file *files_fget_by_index(FAR struct filelist *list, } #endif - spin_unlock_irqrestore(NULL, flags); + spin_unlock_irqrestore(&list->fl_lock, flags); return filep; } @@ -165,7 +165,7 @@ static int files_extend(FAR struct filelist *list, size_t row) } while (++i < row); - flags = spin_lock_irqsave(NULL); + flags = spin_lock_irqsave(&list->fl_lock); /* To avoid race condition, if the file list is updated by other threads * and list rows is greater or equal than temp list, @@ -174,7 +174,7 @@ static int files_extend(FAR struct filelist *list, size_t row) if (orig_rows != list->fl_rows && list->fl_rows >= row) { - spin_unlock_irqrestore(NULL, flags); + spin_unlock_irqrestore(&list->fl_lock, flags); for (j = orig_rows; j < i; j++) { @@ -196,7 +196,7 @@ static int files_extend(FAR struct filelist *list, size_t row) list->fl_files = files; list->fl_rows = row; - spin_unlock_irqrestore(NULL, flags); + spin_unlock_irqrestore(&list->fl_lock, flags); if (tmp != NULL && tmp != &list->fl_prefile) { @@ -371,6 +371,7 @@ void files_initlist(FAR struct filelist *list) list->fl_crefs = 1; list->fl_files = &list->fl_prefile; list->fl_prefile = list->fl_prefiles; + spin_lock_init(&list->fl_lock); } /**************************************************************************** @@ -589,13 +590,13 @@ int file_allocate_from_tcb(FAR struct tcb_s *tcb, FAR struct inode *inode, /* Find free file */ - flags = spin_lock_irqsave(NULL); + flags = spin_lock_irqsave(&list->fl_lock); for (; ; i++, j = 0) { if (i >= list->fl_rows) { - spin_unlock_irqrestore(NULL, flags); + spin_unlock_irqrestore(&list->fl_lock, flags); ret = files_extend(list, i + 1); if (ret < 0) @@ -603,7 +604,7 @@ int file_allocate_from_tcb(FAR struct tcb_s *tcb, FAR struct inode *inode, return ret; } - flags = spin_lock_irqsave(NULL); + flags = spin_lock_irqsave(&list->fl_lock); } do @@ -632,7 +633,7 @@ int file_allocate_from_tcb(FAR struct tcb_s *tcb, FAR struct inode *inode, } found: - spin_unlock_irqrestore(NULL, flags); + spin_unlock_irqrestore(&list->fl_lock, flags); if (addref) { diff --git a/include/nuttx/fs/fs.h b/include/nuttx/fs/fs.h index d9aa023b8383b..1ef5dba359d6c 100644 --- a/include/nuttx/fs/fs.h +++ b/include/nuttx/fs/fs.h @@ -49,6 +49,7 @@ #include #include #include +#include /**************************************************************************** * Pre-processor Definitions @@ -491,6 +492,7 @@ struct file struct filelist { + spinlock_t fl_lock; /* Manage access to the file list */ uint8_t fl_rows; /* The number of rows of fl_files array */ uint8_t fl_crefs; /* The references to filelist */ FAR struct file **fl_files; /* The pointer of two layer file descriptors array */