Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: yO/o7iCgxwsHqdEJqwojD4psSV6AxyaqP1Kkh1O0/YrxuS/pk7OCeAZ2dTG4y4TmzvJ66F6AeB
 0cTydn7YjOKA8i3fBPsypCd2ojHg1/c1INjnd5m9lAumzRiCFUxhpzKmp9Nce6ZfAh11o2tUxp
 TtvdcidZpZaXeqsPhTJhTVCRdIiZnC1R8RHXVtSvVuWAgi8gKfTjDAaJvokC7EaeuJcolM2QOA
 Rr/wnAywcg+IfPi/oUYnWalC31wEhOFjizYP2H4pRRxHpigDBsIu883zSylGfZBkIYCecX4F4B
 v8k=
From:   Damien Le Moal <Damien.LeMoal@wdc.com>
To:     Jan Kara <jack@suse.cz>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
CC:     "hch@infradead.org" <hch@infradead.org>,
        Dave Chinner <david@fromorbit.com>,
        "ceph-devel@vger.kernel.org" <ceph-devel@vger.kernel.org>,
        Chao Yu <yuchao0@huawei.com>,
        "Darrick J. Wong" <darrick.wong@oracle.com>,
        Jaegeuk Kim <jaegeuk@kernel.org>,
        Jeff Layton <jlayton@kernel.org>,
        Johannes Thumshirn <jth@kernel.org>,
        "linux-cifs@vger.kernel.org" <linux-cifs@vger.kernel.org>,
        "linux-ext4@vger.kernel.org" <linux-ext4@vger.kernel.org>,
        "linux-f2fs-devel@lists.sourceforge.net" 
        <linux-f2fs-devel@lists.sourceforge.net>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        "linux-xfs@vger.kernel.org" <linux-xfs@vger.kernel.org>,
        Miklos Szeredi <miklos@szeredi.hu>,
        Steve French <sfrench@samba.org>, Ted Tso <tytso@mit.edu>,
        Matthew Wilcox <willy@infradead.org>
Subject: Re: [PATCH 07/11] zonefs: Convert to using invalidate_lock
Thread-Topic: [PATCH 07/11] zonefs: Convert to using invalidate_lock
Thread-Index: AQHXRzU+qXklL5tXkka8lKRMD3f5wg==
Date:   Thu, 13 May 2021 00:34:33 +0000
Message-ID: <DM6PR04MB70813581E891647A893FE741E7519@DM6PR04MB7081.namprd04.prod.outlook.com>
References: <20210512101639.22278-1-jack@suse.cz>
 <20210512134631.4053-7-jack@suse.cz>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 4f8a851b-a28b-477d-0fa4-08d915a6e13a
X-MS-Exchange-CrossTenant-originalarrivaltime: 13 May 2021 00:34:33.6063
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: ltRJzvAvGXVvXjosAbmERv3AR4aVfhIA0oPp1ciTNez8X2y2+9Tx03D3+9r1TbAqKhL6mr0XkfkNfnRS9mme4A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR04MB0813
Precedence: bulk
List-ID: <linux-ext4.vger.kernel.org>
X-Mailing-List: linux-ext4@vger.kernel.org

On 2021/05/12 22:46, Jan Kara wrote:=0A=
> Use invalidate_lock instead of zonefs' private i_mmap_sem. The intended=
=0A=
> purpose is exactly the same.=0A=
> =0A=
> CC: Damien Le Moal <damien.lemoal@wdc.com>=0A=
> CC: Johannes Thumshirn <jth@kernel.org>=0A=
> CC: <linux-fsdevel@vger.kernel.org>=0A=
> Signed-off-by: Jan Kara <jack@suse.cz>=0A=
> ---=0A=
>  fs/zonefs/super.c  | 23 +++++------------------=0A=
>  fs/zonefs/zonefs.h |  7 +++----=0A=
>  2 files changed, 8 insertions(+), 22 deletions(-)=0A=
> =0A=
> diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c=0A=
> index cd145d318b17..da2e95d98677 100644=0A=
> --- a/fs/zonefs/super.c=0A=
> +++ b/fs/zonefs/super.c=0A=
> @@ -462,7 +462,7 @@ static int zonefs_file_truncate(struct inode *inode, =
loff_t isize)=0A=
>  	inode_dio_wait(inode);=0A=
>  =0A=
>  	/* Serialize against page faults */=0A=
> -	down_write(&zi->i_mmap_sem);=0A=
> +	down_write(&inode->i_mapping->invalidate_lock);=0A=
>  =0A=
>  	/* Serialize against zonefs_iomap_begin() */=0A=
>  	mutex_lock(&zi->i_truncate_mutex);=0A=
> @@ -500,7 +500,7 @@ static int zonefs_file_truncate(struct inode *inode, =
loff_t isize)=0A=
>  =0A=
>  unlock:=0A=
>  	mutex_unlock(&zi->i_truncate_mutex);=0A=
> -	up_write(&zi->i_mmap_sem);=0A=
> +	up_write(&inode->i_mapping->invalidate_lock);=0A=
>  =0A=
>  	return ret;=0A=
>  }=0A=
> @@ -575,18 +575,6 @@ static int zonefs_file_fsync(struct file *file, loff=
_t start, loff_t end,=0A=
>  	return ret;=0A=
>  }=0A=
>  =0A=
> -static vm_fault_t zonefs_filemap_fault(struct vm_fault *vmf)=0A=
> -{=0A=
> -	struct zonefs_inode_info *zi =3D ZONEFS_I(file_inode(vmf->vma->vm_file)=
);=0A=
> -	vm_fault_t ret;=0A=
> -=0A=
> -	down_read(&zi->i_mmap_sem);=0A=
> -	ret =3D filemap_fault(vmf);=0A=
> -	up_read(&zi->i_mmap_sem);=0A=
> -=0A=
> -	return ret;=0A=
> -}=0A=
> -=0A=
>  static vm_fault_t zonefs_filemap_page_mkwrite(struct vm_fault *vmf)=0A=
>  {=0A=
>  	struct inode *inode =3D file_inode(vmf->vma->vm_file);=0A=
> @@ -607,16 +595,16 @@ static vm_fault_t zonefs_filemap_page_mkwrite(struc=
t vm_fault *vmf)=0A=
>  	file_update_time(vmf->vma->vm_file);=0A=
>  =0A=
>  	/* Serialize against truncates */=0A=
> -	down_read(&zi->i_mmap_sem);=0A=
> +	down_read(&inode->i_mapping->invalidate_lock);=0A=
>  	ret =3D iomap_page_mkwrite(vmf, &zonefs_iomap_ops);=0A=
> -	up_read(&zi->i_mmap_sem);=0A=
> +	up_read(&inode->i_mapping->invalidate_lock);=0A=
>  =0A=
>  	sb_end_pagefault(inode->i_sb);=0A=
>  	return ret;=0A=
>  }=0A=
>  =0A=
>  static const struct vm_operations_struct zonefs_file_vm_ops =3D {=0A=
> -	.fault		=3D zonefs_filemap_fault,=0A=
> +	.fault		=3D filemap_fault,=0A=
>  	.map_pages	=3D filemap_map_pages,=0A=
>  	.page_mkwrite	=3D zonefs_filemap_page_mkwrite,=0A=
>  };=0A=
> @@ -1158,7 +1146,6 @@ static struct inode *zonefs_alloc_inode(struct supe=
r_block *sb)=0A=
>  =0A=
>  	inode_init_once(&zi->i_vnode);=0A=
>  	mutex_init(&zi->i_truncate_mutex);=0A=
> -	init_rwsem(&zi->i_mmap_sem);=0A=
>  	zi->i_wr_refcnt =3D 0;=0A=
>  =0A=
>  	return &zi->i_vnode;=0A=
> diff --git a/fs/zonefs/zonefs.h b/fs/zonefs/zonefs.h=0A=
> index 51141907097c..7b147907c328 100644=0A=
> --- a/fs/zonefs/zonefs.h=0A=
> +++ b/fs/zonefs/zonefs.h=0A=
> @@ -70,12 +70,11 @@ struct zonefs_inode_info {=0A=
>  	 * and changes to the inode private data, and in particular changes to=
=0A=
>  	 * a sequential file size on completion of direct IO writes.=0A=
>  	 * Serialization of mmap read IOs with truncate and syscall IO=0A=
> -	 * operations is done with i_mmap_sem in addition to i_truncate_mutex.=
=0A=
> -	 * Only zonefs_seq_file_truncate() takes both lock (i_mmap_sem first,=
=0A=
> -	 * i_truncate_mutex second).=0A=
> +	 * operations is done with invalidate_lock in addition to=0A=
> +	 * i_truncate_mutex.  Only zonefs_seq_file_truncate() takes both lock=
=0A=
> +	 * (invalidate_lock first, i_truncate_mutex second).=0A=
>  	 */=0A=
>  	struct mutex		i_truncate_mutex;=0A=
> -	struct rw_semaphore	i_mmap_sem;=0A=
>  =0A=
>  	/* guarded by i_truncate_mutex */=0A=
>  	unsigned int		i_wr_refcnt;=0A=
> =0A=
=0A=
Looks OK to me for zonefs. This is a nice cleanup.=0A=
=0A=
Acked-by: Damien Le Moal <damien.lemoal@wdc.com>=0A=
=0A=
-- =0A=
Damien Le Moal=0A=
Western Digital Research=0A=