Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
IronPort-SDR: Rg4ifiyeJd9+f7d6hx8/aP6CSUlQWArKV0QD3UtCF5TZWBjVFVx8B1q41bhNCDBFs2LnIQ6wPg
 sxOqIt5xswkGs3D3+TsAjvKcjotFseQQR+nqSKXrsuZBM2/5bhrNAlMEGT1KKbLPdP9g4Fz8oU
 LRtB8rq80sH+RNPEZ5RA42D7qqlcGJWKj8a9zk7mnRiDn9NQX2Os1sJ5/CJLxcFwSoo75LUCup
 LNBLZtCxIGQzsR4+AzCdcJ1/ozRNvvToJAedrIx86/UyvAu7K6XPvcITepScYLLNP7gJfEHyuI
 swg=
From:   Damien Le Moal <Damien.LeMoal@wdc.com>
To:     =?iso-8859-1?Q?Javier_Gonz=E1lez?= <javier@javigon.com>,
        "jaegeuk@kernel.org" <jaegeuk@kernel.org>,
        "yuchao0@huawei.com" <yuchao0@huawei.com>
CC:     "linux-f2fs-devel@lists.sourceforge.net" 
        <linux-f2fs-devel@lists.sourceforge.net>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        =?iso-8859-1?Q?Javier_Gonz=E1lez?= <javier.gonz@samsung.com>
Subject: Re: [PATCH] f2fs: disble physical prealloc in LSF mount
Thread-Topic: [PATCH] f2fs: disble physical prealloc in LSF mount
Thread-Index: AQHVoRNC1jbxwLDWckincxKmSLcqwA==
Date:   Mon, 25 Nov 2019 00:48:37 +0000
Message-ID: <BYAPR04MB58166AE029D919C6610D8404E74A0@BYAPR04MB5816.namprd04.prod.outlook.com>
References: <20191122085952.12754-1-javier@javigon.com>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 44aa8961-3a55-4780-b902-08d771413592
X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Nov 2019 00:48:37.7897
 (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: Do9BI5q2xGmaeTapLAo0uw2X8QJSJ+q1r9zLCUzK2GrFKgL++n524A8gZsY0dT5dHu8quGCotl2LsJoLEmCNBw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR04MB4182
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2019/11/22 18:00, Javier Gonz=E1lez wrote:=0A=
> From: Javier Gonz=E1lez <javier.gonz@samsung.com>=0A=
> =0A=
> Fix file system corruption when using LFS mount (e.g., in zoned=0A=
> devices). Seems like the fallback into buffered I/O creates an=0A=
> inconsistency if the application is assuming both read and write DIO. I=
=0A=
> can easily reproduce a corruption with a simple RocksDB test.=0A=
> =0A=
> Might be that the f2fs_forced_buffered_io path brings some problems too,=
=0A=
> but I have not seen other failures besides this one.=0A=
> =0A=
> Problem reproducible without a zoned block device, simply by forcing=0A=
> LFS mount:=0A=
> =0A=
>   $ sudo mkfs.f2fs -f -m /dev/nvme0n1=0A=
>   $ sudo mount /dev/nvme0n1 /mnt/f2fs=0A=
>   $ sudo  /opt/rocksdb/db_bench  --benchmarks=3Dfillseq --use_existing_db=
=3D0=0A=
>   --use_direct_reads=3Dtrue --use_direct_io_for_flush_and_compaction=3Dtr=
ue=0A=
>   --db=3D/mnt/f2fs --num=3D5000 --value_size=3D1048576 --verify_checksum=
=3D1=0A=
>   --block_size=3D65536=0A=
> =0A=
> Note that the options that cause the problem are:=0A=
>   --use_direct_reads=3Dtrue --use_direct_io_for_flush_and_compaction=3Dtr=
ue=0A=
> =0A=
> Fixes: f9d6d0597698 ("f2fs: fix out-place-update DIO write")=0A=
> =0A=
> Signed-off-by: Javier Gonz=E1lez <javier.gonz@samsung.com>=0A=
> ---=0A=
>  fs/f2fs/data.c | 3 ---=0A=
>  1 file changed, 3 deletions(-)=0A=
> =0A=
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c=0A=
> index 5755e897a5f0..b045dd6ab632 100644=0A=
> --- a/fs/f2fs/data.c=0A=
> +++ b/fs/f2fs/data.c=0A=
> @@ -1081,9 +1081,6 @@ int f2fs_preallocate_blocks(struct kiocb *iocb, str=
uct iov_iter *from)=0A=
>  			return err;=0A=
>  	}=0A=
>  =0A=
> -	if (direct_io && allow_outplace_dio(inode, iocb, from))=0A=
> -		return 0;=0A=
=0A=
Since for LFS mode, all DIOs can end up out of place, I think that it=0A=
may be better to change allow_outplace_dio() to always return true in=0A=
the case of LFS mode. So may be something like:=0A=
=0A=
static inline int allow_outplace_dio(struct inode *inode,=0A=
			struct kiocb *iocb, struct iov_iter *iter)=0A=
{=0A=
	struct f2fs_sb_info *sbi =3D F2FS_I_SB(inode);=0A=
	int rw =3D iov_iter_rw(iter);=0A=
=0A=
	return test_opt(sbi, LFS) ||=0A=
	 	(rw =3D=3D WRITE && !block_unaligned_IO(inode, iocb, iter));=0A=
}=0A=
=0A=
instead of the original:=0A=
=0A=
static inline int allow_outplace_dio(struct inode *inode,=0A=
			struct kiocb *iocb, struct iov_iter *iter)=0A=
{=0A=
	struct f2fs_sb_info *sbi =3D F2FS_I_SB(inode);=0A=
	int rw =3D iov_iter_rw(iter);=0A=
=0A=
	return (test_opt(sbi, LFS) && (rw =3D=3D WRITE) &&=0A=
				!block_unaligned_IO(inode, iocb, iter));=0A=
}=0A=
=0A=
Thoughts ?=0A=
=0A=
> -=0A=
>  	if (is_inode_flag_set(inode, FI_NO_PREALLOC))=0A=
>  		return 0;=0A=
>  =0A=
> =0A=
=0A=
=0A=
-- =0A=
Damien Le Moal=0A=
Western Digital Research=0A=