Skip to content

Commit

Permalink
fs_inode:Change the type of i_crefs to atomic_int
Browse files Browse the repository at this point in the history 
Summary:
  1.Modified the i_crefs from int16_t to atomic_int
  2.Modified the i_crefs add, delete, read, and initialize interfaces to atomic operations
The purpose of this change is to avoid deadlock in cross-core scenarios, where A Core blocks B Core’s request for a write operation to A Core when A Core requests a read operation to B Core.

Signed-off-by: chenrun1 <[email protected]>
  • Loading branch information
@crafcat7 @xiaoxiang781216
crafcat7 authored and xiaoxiang781216 committed Sep 23, 2024
1 parent 56bcc3b commit 4cec713
Show file tree
Hide file tree
Showing 26 changed files with 47 additions and 134 deletions.
2 changes: 1 addition & 1 deletion arch/arm/src/cxd56xx/cxd56_sph.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ static int sph_open(struct file *filep)
{
/* Exclusive access */

if (filep->f_inode->i_crefs > 2)
if (atomic_load(&filep->f_inode->i_crefs) > 2)
{
return ERROR;
}
Expand Down
4 changes: 2 additions & 2 deletions arch/arm/src/cxd56xx/cxd56_uart0.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ static int uart0_open(struct file *filep)
int stop;
int ret;

if (inode->i_crefs > 2)
if (atomic_load(&inode->i_crefs) > 2)
{
return OK;
}
Expand Down Expand Up @@ -172,7 +172,7 @@ static int uart0_close(struct file *filep)
{
struct inode *inode = filep->f_inode;

if (inode->i_crefs == 2)
if (atomic_load(&inode->i_crefs) == 2)
{
fw_pd_uartdisable(0);
fw_pd_uartuninit(0);
Expand Down
2 changes: 1 addition & 1 deletion crypto/cryptodev.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ static const struct file_operations g_cryptoops =

static struct inode g_cryptoinode =
{
.i_crefs = 1,
.i_crefs = ATOMIC_VAR_INIT(1),
.u.i_ops = &g_cryptofops
};

Expand Down
2 changes: 1 addition & 1 deletion drivers/serial/pty.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ static int pty_close(FAR struct file *filep)

/* Check if the decremented inode reference count would go to zero */

if (inode->i_crefs == 1)
if (atomic_load(&inode->i_crefs) == 1)
{
/* Did the (single) master just close its reference? */

Expand Down
7 changes: 1 addition & 6 deletions fs/inode/fs_inodeaddref.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,7 @@ int inode_addref(FAR struct inode *inode)

if (inode)
{
ret = inode_lock();
if (ret >= 0)
{
inode->i_crefs++;
inode_unlock();
}
atomic_fetch_add(&inode->i_crefs, 1);
}

return ret;
Expand Down
2 changes: 1 addition & 1 deletion fs/inode/fs_inodefind.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ int inode_find(FAR struct inode_search_s *desc)

/* Increment the reference count on the inode */

node->i_crefs++;
atomic_fetch_add(&node->i_crefs, 1);
}

inode_unlock();
Expand Down
37 changes: 1 addition & 36 deletions fs/inode/fs_inoderelease.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,49 +47,14 @@

void inode_release(FAR struct inode *inode)
{
int ret;

if (inode)
{
/* Decrement the references of the inode */

do
{
ret = inode_lock();

/* This only possible error is due to cancellation of the thread.
* We need to try again anyway in this case, otherwise the
* reference count would be wrong.
*/

DEBUGASSERT(ret == OK || ret == -ECANCELED);
}
while (ret < 0);

if (inode->i_crefs)
{
inode->i_crefs--;
}

/* If the subtree was previously deleted and the reference
* count has decrement to zero, then delete the inode
* now.
*/

if (inode->i_crefs <= 0)
if (atomic_fetch_sub(&inode->i_crefs, 1) <= 1)
{
/* If the inode has been properly unlinked, then the peer pointer
* should be NULL.
*/

inode_unlock();

DEBUGASSERT(inode->i_peer == NULL);
inode_free(inode);
}
else
{
inode_unlock();
}
}
}
4 changes: 2 additions & 2 deletions fs/inode/fs_inoderemove.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ static FAR struct inode *inode_unlink(FAR const char *path)

node->i_peer = NULL;
node->i_parent = NULL;
node->i_crefs--;
atomic_fetch_sub(&node->i_crefs, 1);
}

RELEASE_SEARCH(&desc);
Expand Down Expand Up @@ -134,7 +134,7 @@ int inode_remove(FAR const char *path)
* to it
*/

if (node->i_crefs)
if (atomic_load(&node->i_crefs))
{
return -EBUSY;
}
Expand Down
2 changes: 1 addition & 1 deletion fs/inode/fs_inodereserve.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ static FAR struct inode *inode_alloc(FAR const char *name, mode_t mode)
if (node)
{
node->i_ino = g_ino++;
node->i_crefs = 1;
atomic_init(&node->i_crefs, 1);
#ifdef CONFIG_PSEUDOFS_ATTRIBUTES
node->i_mode = mode;
clock_gettime(CLOCK_REALTIME, &node->i_atime);
Expand Down
4 changes: 2 additions & 2 deletions fs/mount/fs_mount.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ int nx_mount(FAR const char *source, FAR const char *target,
if (drvr_inode != NULL)
#endif
{
drvr_inode->i_crefs++;
atomic_fetch_add(&drvr_inode->i_crefs, 1);
}
#endif

Expand Down Expand Up @@ -477,7 +477,7 @@ int nx_mount(FAR const char *source, FAR const char *target,
if (drvr_inode != NULL)
#endif
{
drvr_inode->i_crefs--;
atomic_fetch_sub(&drvr_inode->i_crefs, 1);
}
#endif

Expand Down
3 changes: 1 addition & 2 deletions fs/mount/fs_umount2.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,7 @@ int nx_umount2(FAR const char *target, unsigned int flags)
{
/* Just decrement the reference count (without deleting it) */

DEBUGASSERT(mountpt_inode->i_crefs > 0);
mountpt_inode->i_crefs--;
atomic_fetch_sub(&mountpt_inode->i_crefs, 1);
inode_unlock();
}
else
Expand Down
4 changes: 2 additions & 2 deletions fs/mqueue/mq_open.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ static int nxmq_file_close(FAR struct file *filep)
{
FAR struct inode *inode = filep->f_inode;

if (inode->i_crefs <= 0)
if (atomic_load(&inode->i_crefs) <= 0)
{
FAR struct mqueue_inode_s *msgq = inode->i_private;

Expand Down Expand Up @@ -322,7 +322,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,

/* Set the initial reference count on this inode to one */

inode->i_crefs++;
atomic_fetch_add(&inode->i_crefs, 1);

if (created)
{
Expand Down
2 changes: 1 addition & 1 deletion fs/mqueue/mq_unlink.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@

static void mq_inode_release(FAR struct inode *inode)
{
if (inode->i_crefs <= 1)
if (atomic_load(&inode->i_crefs) <= 1)
{
FAR struct mqueue_inode_s *msgq = inode->i_private;

Expand Down
2 changes: 1 addition & 1 deletion fs/notify/inotify.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ static struct inode g_inotify_inode =
NULL,
NULL,
NULL,
1,
ATOMIC_VAR_INIT(1),
FSNODEFLAG_TYPE_DRIVER,
{
&g_inotify_fops
Expand Down
27 changes: 1 addition & 26 deletions fs/semaphore/sem_close.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ int nxsem_close(FAR sem_t *sem)
{
FAR struct nsem_inode_s *nsem;
struct inode *inode;
int ret;

DEBUGASSERT(sem);

Expand All @@ -81,51 +80,27 @@ int nxsem_close(FAR sem_t *sem)
DEBUGASSERT(nsem->ns_inode);
inode = nsem->ns_inode;

/* Decrement the reference count on the inode */

do
{
ret = inode_lock();

/* The only error that is expected is due to thread cancellation.
* At this point, we must continue to free the semaphore anyway.
*/

DEBUGASSERT(ret == OK || ret == -ECANCELED);
}
while (ret < 0);

if (inode->i_crefs > 0)
{
inode->i_crefs--;
}

/* If the semaphore was previously unlinked and the reference count has
* decremented to zero, then release the semaphore and delete the inode
* now.
*/

if (inode->i_crefs <= 0)
if (atomic_fetch_sub(&inode->i_crefs, 1) <= 1)
{
/* Destroy the semaphore and free the container */

nxsem_destroy(&nsem->ns_sem);
group_free(NULL, nsem);

/* Release and free the inode container. If it has been properly
* unlinked, then the peer pointer should be NULL.
*/

inode_unlock();
#ifdef CONFIG_FS_NOTIFY
notify_close2(inode);
#endif
DEBUGASSERT(inode->i_peer == NULL);
inode_free(inode);
return OK;
}

inode_unlock();
return OK;
}

Expand Down
2 changes: 1 addition & 1 deletion fs/semaphore/sem_open.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ int nxsem_open(FAR sem_t **sem, FAR const char *name, int oflags, ...)
/* Initialize the inode */

INODE_SET_NAMEDSEM(inode);
inode->i_crefs++;
atomic_fetch_add(&inode->i_crefs, 1);

/* Initialize the semaphore */

Expand Down
2 changes: 1 addition & 1 deletion fs/shm/shm_open.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ static int file_shm_open(FAR struct file *shm, FAR const char *name,
INODE_SET_SHM(inode);
inode->u.i_ops = &g_shmfs_operations;
inode->i_private = NULL;
inode->i_crefs++;
atomic_fetch_add(&inode->i_crefs, 1);
}

/* Associate the inode with a file structure */
Expand Down
31 changes: 8 additions & 23 deletions fs/shm/shmfs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,21 +185,13 @@ static int shmfs_release(FAR struct inode *inode)
* The inode is released after this call, hence checking if i_crefs <= 1.
*/

int ret = inode_lock();
if (ret >= 0)
if (inode->i_parent == NULL && atomic_load(&inode->i_crefs) <= 1)
{
if (inode->i_parent == NULL &&
inode->i_crefs <= 1)
{
shmfs_free_object(inode->i_private);
inode->i_private = NULL;
ret = OK;
}

inode_unlock();
shmfs_free_object(inode->i_private);
inode->i_private = NULL;
}

return ret;
return OK;
}

/****************************************************************************
Expand Down Expand Up @@ -267,20 +259,13 @@ static int shmfs_truncate(FAR struct file *filep, off_t length)
#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
static int shmfs_unlink(FAR struct inode *inode)
{
int ret = inode_lock();

if (ret >= 0)
if (atomic_load(&inode->i_crefs) <= 1)
{
if (inode->i_crefs <= 1)
{
shmfs_free_object(inode->i_private);
inode->i_private = NULL;
}

inode_unlock();
shmfs_free_object(inode->i_private);
inode->i_private = NULL;
}

return ret;
return OK;
}
#endif

Expand Down
2 changes: 1 addition & 1 deletion fs/socket/socket.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ static struct inode g_sock_inode =
NULL, /* i_parent */
NULL, /* i_peer */
NULL, /* i_child */
1, /* i_crefs */
ATOMIC_VAR_INIT(1), /* i_crefs */
FSNODEFLAG_TYPE_SOCKET, /* i_flags */
{
&g_sock_fileops /* u */
Expand Down
6 changes: 3 additions & 3 deletions fs/vfs/fs_dir.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ static struct inode g_dir_inode =
NULL,
NULL,
NULL,
1,
ATOMIC_VAR_INIT(1),
0,
{ &g_dir_fileops },
};
Expand Down Expand Up @@ -216,7 +216,7 @@ static off_t seek_pseudodir(FAR struct file *filep, off_t offset)
{
/* Increment the reference count on this next node */

curr->i_crefs++;
atomic_fetch_add(&curr->i_crefs, 1);
}

inode_unlock();
Expand Down Expand Up @@ -383,7 +383,7 @@ static int read_pseudodir(FAR struct fs_dirent_s *dir,
{
/* Increment the reference count on this next node */

pdir->next->i_crefs++;
atomic_fetch_add(&pdir->next->i_crefs, 1);
}

inode_unlock();
Expand Down
2 changes: 1 addition & 1 deletion fs/vfs/fs_epoll.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ static struct inode g_epoll_inode =
NULL, /* i_parent */
NULL, /* i_peer */
NULL, /* i_child */
1, /* i_crefs */
ATOMIC_VAR_INIT(1), /* i_crefs */
FSNODEFLAG_TYPE_DRIVER, /* i_flags */
{
&g_epoll_ops /* u */
Expand Down
2 changes: 1 addition & 1 deletion fs/vfs/fs_eventfd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ static struct inode g_eventfd_inode =
NULL, /* i_parent */
NULL, /* i_peer */
NULL, /* i_child */
1, /* i_crefs */
ATOMIC_VAR_INIT(1), /* i_crefs */
FSNODEFLAG_TYPE_DRIVER, /* i_flags */
{
&g_eventfd_fops /* u */
Expand Down
Loading

10 comments on commit 4cec713

@TakuyaMiyasita
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xiaoxiang781216 @crafcat7

We got the trouble by this PR.

Our platform does not support the atomic instruction although our platfom uses Cortex-M4F.

Of course our CM4F can execute the atomic instruction,
but our platform does not implement the monitor for exclusive access which is described in ARMv7-M Architecture Reference Manual,
and our platform alwas returns the result of atomic instruction as fail.
So this PR will make an inifite loop at our platform,
we use the primask insted of the atomic instruction because our platform is an uniprocessor.

I think the processor which compliants with ARMv7-M is not enough to support the atomic instruction,
it is also needed to implement the monitor for exclusive access.

Please take care for such platform, althogh it might be rare case.

To avoid using the atomic instruction on above platform,
How about following idea?

  1. On the platform which supports the atomic instruction, use the atomic functions.
  2. On the platform which does not support the atomic instruction, use the function described in arch_atomic.c.
    All functions which are described in arch_atomic.c must be needed to rename other than the name of atomic functions. (ex : atomic_fetch_add -> nx_atomic_fetch_add)
    Include "include/nuttx/lib/stdatomic.h" which defines atomic_fetch_add -> nx_atomic_fetch_add.

I think
these atomic functions are implemented as built-in function at GCC, built-in functions are inlined when the source codes are compiled,
to avoid using built-in functions, not to call them.

@xiaoxiang781216
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@crafcat7 will provide a patch, please try it.

@crafcat7
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,I have provided a patch here #14537, which is used to determine whether to use the toolchain's atomic by checking the macro.

In https://en.cppreference.com/w/c/atomic it is mentioned

If the macro constant STDC_NO_ATOMICS(C11) is defined by the compiler, the header <stdatomic.h>, the keyword _Atomic, and all of the names listed here are not provided.
In this case, we will let atomic use the nuttx/lib/stdatomic.h version

For your situation, I think it is possible to solve it by adding the STDC_NO_ATOMICS flag on this basis.

@crafcat7
Copy link
Contributor Author

@crafcat7 crafcat7 commented on 4cec713 Oct 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#14537 (comment) Continue this discussion.

Hi, What I want to ask is whether we can make it work in arch_atomic based on the changes in PR #14537 by adding FLAGS += STDC_NO_ATOMICS in the compilation options of this soc?

I think as long as it declares STDC_NO_ATOMICS, then it will be pointed to arch_atomic
https://github.com/apache/nuttx/pull/14537/files#diff-83e77068636671f3a69415d40cc75ba82be4a8f1eb2e02cfd44b60ae7a32998cR78-R80

@xiaoxiang781216
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid using the atomic instruction on above platform, How about following idea?

  1. On the platform which supports the atomic instruction, use the atomic functions.

Yes, of course.

  1. On the platform which does not support the atomic instruction, use the function described in arch_atomic.c.> All functions which are described in arch_atomic.c must be needed to rename other than the name of atomic functions. (ex : atomic_fetch_add -> nx_atomic_fetch_add)
    Include "include/nuttx/lib/stdatomic.h" which defines atomic_fetch_add -> nx_atomic_fetch_add.

I think these atomic functions are implemented as built-in function at GCC, built-in functions are inlined when the source codes are compiled, to avoid using built-in functions, not to call them.

@crafcat7 please compile on M4/M7/M33/M55 device and compare the dissemble code to find the best method.

@crafcat7
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it on our local stm32-tiny (M3) and FRDM-k64f (M4). Normally, it compiles and connects to the method provided by the toolchain.
Image
Image

I executed the test cases of apps/testing/atomic in the FRDM-K64F environment, and the results were correct
Image

@xiaoxiang781216
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TakuyaMiyasita could you provide the toolchain/version, command line you use and the dissemble output? The local monitor inside cpu core is enough to make ldrex/strex work for UP case, I think.

@TakuyaMiyasita
Copy link
Contributor

@TakuyaMiyasita TakuyaMiyasita commented on 4cec713 Oct 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xiaoxiang781216 @crafcat7

Thank you for your concern.

the toolchain version is here.

./arm-none-eabi-gcc -v
Using built-in specs.
COLLECT_GCC=./arm-none-eabi-gcc
COLLECT_LTO_WRAPPER=/home/lassdk/workdir/lassdk-nx/gcc-arm-none-eabi-10.3-2021.10/bin/../lib/gcc/arm-none-eabi/10.3.1/lto-wrapper
Target: arm-none-eabi
Configured with: /mnt/workspace/workspace/GCC-10-pipeline/jenkins-GCC-10-pipeline-338_20211018_1634516203/src/gcc/configure --target=arm-none-eabi --prefix=/mnt/workspace/workspace/GCC-10-pipeline/jenkins-GCC-10-pipeline-338_20211018_1634516203/install-native --libexecdir=/mnt/workspace/workspace/GCC-10-pipeline/jenkins-GCC-10-pipeline-338_20211018_1634516203/install-native/lib --infodir=/mnt/workspace/workspace/GCC-10-pipeline/jenkins-GCC-10-pipeline-338_20211018_1634516203/install-native/share/doc/gcc-arm-none-eabi/info --mandir=/mnt/workspace/workspace/GCC-10-pipeline/jenkins-GCC-10-pipeline-338_20211018_1634516203/install-native/share/doc/gcc-arm-none-eabi/man --htmldir=/mnt/workspace/workspace/GCC-10-pipeline/jenkins-GCC-10-pipeline-338_20211018_1634516203/install-native/share/doc/gcc-arm-none-eabi/html --pdfdir=/mnt/workspace/workspace/GCC-10-pipeline/jenkins-GCC-10-pipeline-338_20211018_1634516203/install-native/share/doc/gcc-arm-none-eabi/pdf --enable-languages=c,c++ --enable-plugins --disable-decimal-float --disable-libffi --disable-libgomp --disable-libmudflap --disable-libquadmath --disable-libssp --disable-libstdcxx-pch --disable-nls --disable-shared --disable-threads --disable-tls --with-gnu-as --with-gnu-ld --with-newlib --with-headers=yes --with-python-dir=share/gcc-arm-none-eabi --with-sysroot=/mnt/workspace/workspace/GCC-10-pipeline/jenkins-GCC-10-pipeline-338_20211018_1634516203/install-native/arm-none-eabi --build=x86_64-linux-gnu --host=x86_64-linux-gnu --with-gmp=/mnt/workspace/workspace/GCC-10-pipeline/jenkins-GCC-10-pipeline-338_20211018_1634516203/build-native/host-libs/usr --with-mpfr=/mnt/workspace/workspace/GCC-10-pipeline/jenkins-GCC-10-pipeline-338_20211018_1634516203/build-native/host-libs/usr --with-mpc=/mnt/workspace/workspace/GCC-10-pipeline/jenkins-GCC-10-pipeline-338_20211018_1634516203/build-native/host-libs/usr --with-isl=/mnt/workspace/workspace/GCC-10-pipeline/jenkins-GCC-10-pipeline-338_20211018_1634516203/build-native/host-libs/usr --with-libelf=/mnt/workspace/workspace/GCC-10-pipeline/jenkins-GCC-10-pipeline-338_20211018_1634516203/build-native/host-libs/usr --with-host-libstdcxx='-static-libgcc -Wl,-Bstatic,-lstdc++,-Bdynamic -lm' --with-pkgversion='GNU Arm Embedded Toolchain 10.3-2021.10' --with-multilib-list=rmprofile,aprofile
Thread model: single
Supported LTO compression algorithms: zlib
gcc version 10.3.1 20210824 (release) (GNU Arm Embedded Toolchain 10.3-2021.10)

and the part of disassemble code is here.

24028c20 <inode_find>:
24028c20:	b500      	push	{lr}
24028c22:	b085      	sub	sp, #20
24028c24:	9001      	str	r0, [sp, #4]
24028c26:	f7fb fb97 	bl	24024358 <inode_rlock>
24028c2a:	9801      	ldr	r0, [sp, #4]
24028c2c:	f7fb fd82 	bl	24024734 <inode_search>
24028c30:	9003      	str	r0, [sp, #12]
24028c32:	9b03      	ldr	r3, [sp, #12]
24028c34:	2b00      	cmp	r3, #0
24028c36:	db10      	blt.n	24028c5a <inode_find+0x3a>
24028c38:	9b01      	ldr	r3, [sp, #4]
24028c3a:	685b      	ldr	r3, [r3, #4]
24028c3c:	9302      	str	r3, [sp, #8]
24028c3e:	9b02      	ldr	r3, [sp, #8]
24028c40:	330c      	adds	r3, #12
24028c42:	f3bf 8f5f 	dmb	sy
24028c46:	e8d3 2f5f 	ldrexh	r2, [r3]
24028c4a:	f102 0201 	add.w	r2, r2, #1
24028c4e:	e8c3 2f51 	strexh	r1, r2, [r3]
24028c52:	2900      	cmp	r1, #0
24028c54:	d1f7      	bne.n	24028c46 <inode_find+0x26>
24028c56:	f3bf 8f5f 	dmb	sy
24028c5a:	f7fb fb8d 	bl	24024378 <inode_runlock>
24028c5e:	9b03      	ldr	r3, [sp, #12]
24028c60:	4618      	mov	r0, r3
24028c62:	b005      	add	sp, #20
24028c64:	f85d fb04 	ldr.w	pc, [sp], #4

@xiaoxiang781216
Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 commented on 4cec713 Oct 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, the compiler generates the exclusive load/store:

24028c46: e8d3 2f5f ldrexh r2, [r3]
24028c4a: f102 0201 add.w r2, r2, https://github.com/apache/nuttx/pull/1
24028c4e: e8c3 2f51 strexh r1, r2, [r3]

but your chip fail to run it correctly.

The following modification is needed for your chip:

To avoid using the atomic instruction on above platform, How about following idea?

  1. On the platform which supports the atomic instruction, use the atomic functions.
  2. On the platform which does not support the atomic instruction, use the function described in arch_atomic.c.
    All functions which are described in arch_atomic.c must be needed to rename other than the name of atomic functions. (ex : atomic_fetch_add -> nx_atomic_fetch_add)
    Include "include/nuttx/lib/stdatomic.h" which defines atomic_fetch_add -> nx_atomic_fetch_add.

I think these atomic functions are implemented as built-in function at GCC, built-in functions are inlined when the source codes are compiled, to avoid using built-in functions, not to call them.

could you provide a PR?

@TakuyaMiyasita
Copy link
Contributor

@TakuyaMiyasita TakuyaMiyasita commented on 4cec713 Nov 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xiaoxiang781216

could you provide a PR?

I will try it.

Please sign in to comment.