linux-ext4 - Re: [PATCH 7/9] ext4: Move percpu_rwsem protection into ext4

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221201111648.ifuumk2m244yx2mr@riteshh-domain>
Date:   Thu, 1 Dec 2022 16:46:48 +0530
From:   "Ritesh Harjani (IBM)" <ritesh.list@...il.com>
To:     Jan Kara <jack@...e.cz>
Cc:     Ted Tso <tytso@....edu>, linux-ext4@...r.kernel.org,
        Christoph Hellwig <hch@...radead.org>
Subject: Re: [PATCH 7/9] ext4: Move percpu_rwsem protection into
 ext4_writepages()

On 22/11/30 05:35PM, Jan Kara wrote:
> Move protection by percpu_rwsem from ext4_do_writepages() to
> ext4_writepages(). We will not want to grab this protection during
> transaction commits as that would be prone to deadlocks and the
> protection is not needed. Move the shutdown state checking as well since
> we want to be able to complete commit while the shutdown is in progress.

Yup. Sounds good. Please feel free to add:
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@...il.com>


>
> Signed-off-by: Jan Kara <jack@...e.cz>
> ---
>  fs/ext4/inode.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index fbea77ab470f..00c4d12f8270 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -2718,10 +2718,6 @@ static int ext4_do_writepages(struct mpage_da_data *mpd)
>  	struct blk_plug plug;
>  	bool give_up_on_write = false;
>
> -	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
> -		return -EIO;
> -
> -	percpu_down_read(&sbi->s_writepages_rwsem);
>  	trace_ext4_writepages(inode, wbc);
>
>  	/*
> @@ -2930,20 +2926,28 @@ static int ext4_do_writepages(struct mpage_da_data *mpd)
>  out_writepages:
>  	trace_ext4_writepages_result(inode, wbc, ret,
>  				     nr_to_write - wbc->nr_to_write);
> -	percpu_up_read(&sbi->s_writepages_rwsem);
>  	return ret;
>  }
>
>  static int ext4_writepages(struct address_space *mapping,
>  			   struct writeback_control *wbc)
>  {
> +	struct super_block *sb = mapping->host->i_sb;
>  	struct mpage_da_data mpd = {
>  		.inode = mapping->host,
>  		.wbc = wbc,
>  		.can_map = 1,
>  	};
> +	int ret;
> +
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(sb))))
> +		return -EIO;
>
> -	return ext4_do_writepages(&mpd);
> +	percpu_down_read(&EXT4_SB(sb)->s_writepages_rwsem);
> +	ret = ext4_do_writepages(&mpd);
> +	percpu_up_read(&EXT4_SB(sb)->s_writepages_rwsem);
> +
> +	return ret;
>  }
>
>  static int ext4_dax_writepages(struct address_space *mapping,
> --
> 2.35.3
>