[2/2] super: don't bother with WARN_ON_ONCE()

Commit Message

Christian Brauner Nov. 27, 2023, 11:51 a.m. UTC

We hold our own active reference and we've checked it above.

Signed-off-by: Christian Brauner <[e-mail]>
---
 fs/super.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Comments

Christoph Hellwig Nov. 27, 2023, 1:59 p.m. UTC | #1

On Mon, Nov 27, 2023 at 12:51:31PM +0100, Christian Brauner wrote:
> diff --git a/fs/super.c b/fs/super.c
> index f3227b22879d..844ca13e9d93 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -2067,10 +2067,7 @@ int freeze_super(struct super_block *sb, enum freeze_holder who)
>  	/* Release s_umount to preserve sb_start_write -> s_umount ordering */
>  	super_unlock_excl(sb);
>  	sb_wait_write(sb, SB_FREEZE_WRITE);
> -	if (!super_lock_excl(sb)) {
> -		WARN_ON_ONCE("Dying superblock while freezing!");
> -		return -EINVAL;
> -	}
> +	__super_lock_excl(sb);

This looks ok, but I still find these locking helper so horrible to
follow..

Christian Brauner Nov. 27, 2023, 2:53 p.m. UTC | #2

On Mon, Nov 27, 2023 at 02:59:45PM +0100, Christoph Hellwig wrote:
> This looks ok, but I still find these locking helper so horrible to
> follow..

What do you still find objectionable?

Christoph Hellwig Nov. 27, 2023, 4:48 p.m. UTC | #3

On Mon, Nov 27, 2023 at 03:53:36PM +0100, Christian Brauner wrote:
> On Mon, Nov 27, 2023 at 02:59:45PM +0100, Christoph Hellwig wrote:
> > This looks ok, but I still find these locking helper so horrible to
> > follow..
> 
> What do you still find objectionable?

Same thing as last time.  The __ helpers that take the share/exclusive
trip me off every single time I have to follow them.   Just open coding
the calls to the rw_semaphore helpers is a lot easier to read in
general, but for anything complex that actually needs an enum with
EXCL and SHARED in it would at least makes it clear what is happening.

Patch

diff --git a/fs/super.c b/fs/super.c
index f3227b22879d..844ca13e9d93 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -2067,10 +2067,7 @@  int freeze_super(struct super_block *sb, enum freeze_holder who)
 	/* Release s_umount to preserve sb_start_write -> s_umount ordering */
 	super_unlock_excl(sb);
 	sb_wait_write(sb, SB_FREEZE_WRITE);
-	if (!super_lock_excl(sb)) {
-		WARN_ON_ONCE("Dying superblock while freezing!");
-		return -EINVAL;
-	}
+	__super_lock_excl(sb);
 
 	/* Now we go and block page faults... */
 	sb->s_writers.frozen = SB_FREEZE_PAGEFAULT;