Skip to content

Commit

Permalink
fs/inode: using rwsem lock as inode_lock to avoid deadlock
Browse files Browse the repository at this point in the history
Example:
When executing "df -h" on Core A to view mount information, this
process will traverse inode nodes, thereby holding the inode_lock.
Since the inode type of the mount point may be rpmsgfs, it will fetch statfs
information from another Core B.

Meanwhile, rcS on Core B needs to obtain file information from Core A,
which will be achieved by fetching stat information through rpmsgfs.
When this message arrives at Core A, a deadlock can occur between Core A's
rptun ap and nsh task.

However, both of these places involve read operations only, thus a reader-writer lock
can be utilized to prevent such a deadlock.

Signed-off-by: dongjiuzhu1 <dongjiuzhu1@xiaomi.com>
  • Loading branch information
Donny9 committed Sep 29, 2024
1 parent 2fc97d1 commit adf7dd3
Show file tree
Hide file tree
Showing 29 changed files with 116 additions and 212 deletions.
7 changes: 1 addition & 6 deletions fs/driver/fs_registerblockdriver.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,7 @@ int register_blockdriver(FAR const char *path,
* valid data.
*/

ret = inode_lock();
if (ret < 0)
{
return ret;
}

inode_lock();
ret = inode_reserve(path, mode, &node);
if (ret >= 0)
{
Expand Down
7 changes: 1 addition & 6 deletions fs/driver/fs_registerdriver.c
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,7 @@ int register_driver(FAR const char *path,
* will have a momentarily bad structure.
*/

ret = inode_lock();
if (ret < 0)
{
return ret;
}

inode_lock();
ret = inode_reserve(path, mode, &node);
if (ret >= 0)
{
Expand Down
7 changes: 1 addition & 6 deletions fs/driver/fs_registermtddriver.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,7 @@ int register_mtddriver(FAR const char *path, FAR struct mtd_dev_s *mtd,
* valid data.
*/

ret = inode_lock();
if (ret < 0)
{
return ret;
}

inode_lock();
ret = inode_reserve(path, mode, &node);
if (ret >= 0)
{
Expand Down
7 changes: 1 addition & 6 deletions fs/driver/fs_registerpipedriver.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,7 @@ int register_pipedriver(FAR const char *path,
* will have a momentarily bad structure.
*/

ret = inode_lock();
if (ret < 0)
{
return ret;
}

inode_lock();
ret = inode_reserve(path, mode, &node);
if (ret >= 0)
{
Expand Down
14 changes: 4 additions & 10 deletions fs/driver/fs_unregisterblockdriver.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,11 @@ int unregister_blockdriver(FAR const char *path)
{
int ret;

ret = inode_lock();
if (ret >= 0)
{
ret = inode_remove(path);
inode_unlock();
inode_lock();
ret = inode_remove(path);
inode_unlock();
#ifdef CONFIG_FS_NOTIFY
notify_unlink(path);
notify_unlink(path);
#endif
return OK;
}

inode_unlock();
return ret;
}
14 changes: 4 additions & 10 deletions fs/driver/fs_unregisterdriver.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,17 +55,11 @@ int unregister_driver(FAR const char *path)

/* If unlink failed, only remove inode. */

ret = inode_lock();
if (ret >= 0)
{
ret = inode_remove(path);
inode_unlock();
inode_lock();
ret = inode_remove(path);
inode_unlock();
#ifdef CONFIG_FS_NOTIFY
notify_unlink(path);
notify_unlink(path);
#endif
return OK;
}

inode_unlock();
return ret;
}
14 changes: 4 additions & 10 deletions fs/driver/fs_unregistermtddriver.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,11 @@ int unregister_mtddriver(FAR const char *path)
{
int ret;

ret = inode_lock();
if (ret >= 0)
{
ret = inode_remove(path);
inode_unlock();
inode_lock();
ret = inode_remove(path);
inode_unlock();
#ifdef CONFIG_FS_NOTIFY
notify_unlink(path);
notify_unlink(path);
#endif
return OK;
}

inode_unlock();
return ret;
}
14 changes: 4 additions & 10 deletions fs/driver/fs_unregisterpipedriver.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,12 @@ int unregister_pipedriver(FAR const char *path)
{
int ret;

ret = inode_lock();
if (ret >= 0)
{
ret = inode_remove(path);
inode_unlock();
inode_lock();
ret = inode_remove(path);
inode_unlock();
#ifdef CONFIG_FS_NOTIFY
notify_unlink(path);
notify_unlink(path);
#endif
return OK;
}

inode_unlock();
return ret;
}

Expand Down
18 changes: 6 additions & 12 deletions fs/inode/fs_foreachinode.c
Original file line number Diff line number Diff line change
Expand Up @@ -184,12 +184,9 @@ int foreach_inode(foreach_inode_t handler, FAR void *arg)

/* Start the recursion at the root inode */

ret = inode_lock();
if (ret >= 0)
{
ret = foreach_inodelevel(g_root_inode->i_child, info);
inode_unlock();
}
inode_rlock();
ret = foreach_inodelevel(g_root_inode->i_child, info);
inode_runlock();

/* Free the info structure and return the result */

Expand All @@ -208,12 +205,9 @@ int foreach_inode(foreach_inode_t handler, FAR void *arg)

/* Start the recursion at the root inode */

ret = inode_lock();
if (ret >= 0)
{
ret = foreach_inodelevel(g_root_inode->i_child, &info);
inode_unlock();
}
inode_rlock();
ret = foreach_inodelevel(g_root_inode->i_child, &info);
inode_runlock();

return ret;

Expand Down
46 changes: 33 additions & 13 deletions fs/inode/fs_inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,8 @@
* Included Files
****************************************************************************/

#include <nuttx/config.h>

#include <unistd.h>
#include <assert.h>
#include <errno.h>

#include <nuttx/fs/fs.h>
#include <nuttx/mutex.h>
#include <nuttx/rwsem.h>

#include "inode/inode.h"

Expand All @@ -45,7 +39,7 @@
* Private Data
****************************************************************************/

static rmutex_t g_inode_lock = NXRMUTEX_INITIALIZER;
static rw_semaphore_t g_inode_lock = RWSEM_INITIALIZER;

/****************************************************************************
* Public Functions
Expand All @@ -71,24 +65,50 @@ void inode_initialize(void)
* Name: inode_lock
*
* Description:
* Get exclusive access to the in-memory inode tree (g_inode_sem).
* Get writeable exclusive access to the in-memory inode tree.
*
****************************************************************************/

void inode_lock(void)
{
down_write(&g_inode_lock);
}

/****************************************************************************
* Name: inode_rlock
*
* Description:
* Get readable exclusive access to the in-memory inode tree.
*
****************************************************************************/

int inode_lock(void)
void inode_rlock(void)
{
return nxrmutex_lock(&g_inode_lock);
down_read(&g_inode_lock);
}

/****************************************************************************
* Name: inode_unlock
*
* Description:
* Relinquish exclusive access to the in-memory inode tree (g_inode_sem).
* Relinquish writeable exclusive access to the in-memory inode tree.
*
****************************************************************************/

void inode_unlock(void)
{
DEBUGVERIFY(nxrmutex_unlock(&g_inode_lock));
up_write(&g_inode_lock);
}

/****************************************************************************
* Name: inode_runlock
*
* Description:
* Relinquish read exclusive access to the in-memory inode tree.
*
****************************************************************************/

void inode_runlock(void)
{
up_read(&g_inode_lock);
}
4 changes: 1 addition & 3 deletions fs/inode/fs_inodeaddref.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,10 @@

int inode_addref(FAR struct inode *inode)
{
int ret = OK;

if (inode)
{
atomic_fetch_add(&inode->i_crefs, 1);
}

return ret;
return OK;
}
7 changes: 1 addition & 6 deletions fs/inode/fs_inodefind.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,7 @@ int inode_find(FAR struct inode_search_s *desc)
* references on the node.
*/

ret = inode_lock();
if (ret < 0)
{
return ret;
}

inode_lock();
ret = inode_search(desc);
if (ret >= 0)
{
Expand Down
26 changes: 23 additions & 3 deletions fs/inode/inode.h
Original file line number Diff line number Diff line change
Expand Up @@ -170,22 +170,42 @@ void inode_initialize(void);
* Name: inode_lock
*
* Description:
* Get exclusive access to the in-memory inode tree (tree_sem).
* Get writeable exclusive access to the in-memory inode tree.
*
****************************************************************************/

int inode_lock(void);
void inode_lock(void);

/****************************************************************************
* Name: inode_rlock
*
* Description:
* Get readable exclusive access to the in-memory inode tree.
*
****************************************************************************/

void inode_rlock(void);

/****************************************************************************
* Name: inode_unlock
*
* Description:
* Relinquish exclusive access to the in-memory inode tree (tree_sem).
* Relinquish writeable exclusive access to the in-memory inode tree.
*
****************************************************************************/

void inode_unlock(void);

/****************************************************************************
* Name: inode_runlock
*
* Description:
* Relinquish read exclusive access to the in-memory inode tree.
*
****************************************************************************/

void inode_runlock(void);

/****************************************************************************
* Name: inode_search
*
Expand Down
8 changes: 2 additions & 6 deletions fs/mount/fs_automount.c
Original file line number Diff line number Diff line change
Expand Up @@ -401,11 +401,7 @@ static int automount_findinode(FAR const char *path)

/* Get exclusive access to the in-memory inode tree. */

ret = inode_lock();
if (ret < 0)
{
return ret;
}
inode_rlock();

/* Find the inode */

Expand Down Expand Up @@ -439,7 +435,7 @@ static int automount_findinode(FAR const char *path)

/* Relinquish our exclusive access to the inode try and return the result */

inode_unlock();
inode_runlock();
RELEASE_SEARCH(&desc);
return ret;
}
Expand Down
9 changes: 2 additions & 7 deletions fs/mount/fs_mount.c
Original file line number Diff line number Diff line change
Expand Up @@ -367,12 +367,7 @@ int nx_mount(FAR const char *source, FAR const char *target,
goto errout;
}

ret = inode_lock();
if (ret < 0)
{
goto errout_with_inode;
}

inode_lock();
#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
/* Check if the inode already exists */

Expand Down Expand Up @@ -462,7 +457,7 @@ int nx_mount(FAR const char *source, FAR const char *target,
#else
ret = mops->bind(NULL, data, &fshandle);
#endif
DEBUGVERIFY(inode_lock() >= 0);
inode_lock();
if (ret < 0)
{
/* The inode is unhappy with the driver for some reason. Back out
Expand Down
7 changes: 1 addition & 6 deletions fs/mount/fs_umount2.c
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,7 @@ int nx_umount2(FAR const char *target, unsigned int flags)

/* Hold the semaphore through the unbind logic */

ret = inode_lock();
if (ret < 0)
{
goto errout_with_mountpt;
}

inode_lock();
ret = mountpt_inode->u.i_mops->unbind(mountpt_inode->i_private,
&blkdrvr_inode, flags);
if (ret < 0)
Expand Down
Loading

0 comments on commit adf7dd3

Please sign in to comment.