Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 1 Dec 2022 16:46:48 +0530
From:   "Ritesh Harjani (IBM)" <ritesh.list@gmail.com>
To:     Jan Kara <jack@suse.cz>
Cc:     Ted Tso <tytso@mit.edu>, linux-ext4@vger.kernel.org,
        Christoph Hellwig <hch@infradead.org>
Subject: Re: [PATCH 7/9] ext4: Move percpu_rwsem protection into
 ext4_writepages()
Message-ID: <20221201111648.ifuumk2m244yx2mr@riteshh-domain>
References: <20221130162435.2324-1-jack@suse.cz>
 <20221130163608.29034-7-jack@suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221130163608.29034-7-jack@suse.cz>
Precedence: bulk

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@gmail.com>


>
> Signed-off-by: Jan Kara <jack@suse.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
>