on 9/16/2023 7:06 PM, Bernd Schubert wrote: > > > On 9/14/23 17:45, Kemeng Shi wrote: >> Commit 670d21c6e17f6 ("fuse: remove reliance on bdi congestion") change how >> congestion_threshold is used and lock in >> fuse_conn_congestion_threshold_write is not needed anymore. >> 1. Access to supe_block is removed along with removing of bdi congestion. >> Then down_read(&fc->killsb) which protecting access to super_block is no >> needed. >> 2. Compare num_background and congestion_threshold without holding >> bg_lock. Then there is no need to hold bg_lock to update >> congestion_threshold. >> >> Signed-off-by: Kemeng Shi <shikemeng@xxxxxxxxxxxxxxx> >> --- >> fs/fuse/control.c | 4 ---- >> 1 file changed, 4 deletions(-) >> >> diff --git a/fs/fuse/control.c b/fs/fuse/control.c >> index 247ef4f76761..c5d7bf80efed 100644 >> --- a/fs/fuse/control.c >> +++ b/fs/fuse/control.c >> @@ -174,11 +174,7 @@ static ssize_t fuse_conn_congestion_threshold_write(struct file *file, >> if (!fc) >> goto out; >> - down_read(&fc->killsb); >> - spin_lock(&fc->bg_lock); >> fc->congestion_threshold = val; >> - spin_unlock(&fc->bg_lock); >> - up_read(&fc->killsb); >> fuse_conn_put(fc); >> out: >> return ret; > > Yeah, I don't see readers holding any of these locks. > I just wonder if it wouldn't be better to use WRITE_ONCE to ensure a single atomic operation to store the value. Sure, WRITE_ONCE looks better. I wonder if we should use READ_ONCE from reader. Would like to get any advice. Thanks! > > > Thanks, > Bernd >