Received-SPF: pass (google.com: domain of linux-kernel+bounces-75207-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <f7ad3aed-3482-4eee-8c81-8e471916ef82@oracle.com>
Date: Wed, 21 Feb 2024 17:38:39 +0000
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 6/6] fs: xfs: Set FMODE_CAN_ATOMIC_WRITE for
 FS_XFLAG_ATOMICWRITES set
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: hch@lst.de, viro@zeniv.linux.org.uk, brauner@kernel.org,
        dchinner@redhat.com, jack@suse.cz, chandan.babu@oracle.com,
        martin.petersen@oracle.com, linux-kernel@vger.kernel.org,
        linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        tytso@mit.edu, jbongio@google.com, ojaswin@linux.ibm.com
References: <20240124142645.9334-1-john.g.garry@oracle.com>
 <20240124142645.9334-7-john.g.garry@oracle.com>
 <20240202180619.GK6184@frogsfrogsfrogs>
 <7e3b9556-f083-4c14-a48f-46242d1c744b@oracle.com>
 <20240213175954.GV616564@frogsfrogsfrogs>
 <b902bee1-fcfd-4542-8a4e-c6b9861828c9@oracle.com>
 <20240221170031.GI6184@frogsfrogsfrogs>
Content-Language: en-US
From: John Garry <john.g.garry@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <20240221170031.GI6184@frogsfrogsfrogs>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?b3dqWHBzRkY4UGZMWmxLb0NtdG1vMzRSb01TNzBrK3h4V1VIMVhqN1lDVUtH?=
 =?utf-8?B?MEdKMnYyMzQ4QlhBd2xrM05ZWkphTzYrZm50MFYzTkZMdXZpK1JtaWFVZVlC?=
 =?utf-8?B?ZTE0eDRDZERmZkh5Z3ZYVW9TWDBNQVIxVzZQRi9VdDlQV25ua0s1d0NiUFpH?=
 =?utf-8?B?OFZjci9kOTMzM1lVb2l1bkd2MXY5N2t1UjNlS1NzMUUwWDdBWlNQY0xONnpN?=
 =?utf-8?B?YVlGY1NtVGV5WU1pYTF6Z1o5eUhiTTZYdFVmdGRWY3dDUUE5R3gyVVgvcHJk?=
 =?utf-8?B?TURkaDdLamUvUXIyRlA4TytHL0h0NGZUVnM2Y0M2WXNtdk40Z2FIQldyL21k?=
 =?utf-8?B?YWRka24yWEpUNDRGbXJDS0NDdEZBWjViL0JCSTZBRm5hbEF6eXZHS2x5V0tk?=
 =?utf-8?B?a0lVUU56NDB0Z3NBQ2diWi9aT09zbzN5SDM2WDlGcFFWZStpUVp5TWRyTGZ4?=
 =?utf-8?B?UmV2UFZBUjcwODdvZzBzbnZ1R1NzYWJoSWMwcExZL3AvYWpyMzBJY3VqemZC?=
 =?utf-8?B?WTJjejVBYzlXaWU2bVNBZFZBdXJjMXlhTmNaZGZVakJsN2RjQkRaMnZYZStj?=
 =?utf-8?B?UU9PNDZwd0YvbXZYdENVTGFzd2Z4SW1DeGxmTUhYV2RCdGFmYjIzVWFFYnh4?=
 =?utf-8?B?S1ZrTkhPenQ2Qk4rL1ZZWVpGOTNLYTluejM3MnVMRXpPUkpDSFpDUDBabTN5?=
 =?utf-8?B?OHBUK1ZMVFRSMWxDWkZhV1ZLbGIzMlRLVTdyT3JwNS9lU0d1NG9Zb2o0eWdj?=
 =?utf-8?B?SmRCa0hHUVlyNlYvZmYycmhGOHV3MTZMZDV4K1pPV0N1QjM3NHRlSEpMODZi?=
 =?utf-8?B?N1A4WDZyclM4SG1ZeXh3Y21mak5Xb0dVZTF1d3VpWm5PWVNBRHEzcWY1RXlV?=
 =?utf-8?B?Z3M1V29Va3B2VzVtU1BTblZicHJoUmtMOHZvR2tHL2dDYjBic2hQeWpSN21J?=
 =?utf-8?B?bHZrWHArN1VKK083QXZyVUlQMW8zRVhwMmhPMzYvZlZpYWFnQmdSMTlZVXg0?=
 =?utf-8?B?OXdwdXBmWkdHNkNWN1lJK0dXaUlwVk1TNFhOczcwUDZNT1FYSEFiS3dBWk4v?=
 =?utf-8?B?NXNiT2tPNEE2Nm0rWTZrTllLdGJDVzEwM0tUNWpWc3JDUXJpQ3ByV2FVTUdj?=
 =?utf-8?B?ZWdLTGE0Rk5pN2ZwYzY2TmtzRGwweWtTeVVhQlF5T3pBSGcvTFRrcDdLVmpw?=
 =?utf-8?B?RkxobUt0VE94WGFOWUQvWUVQUnYyd1JjSUo5UENUODFnZE43emdqTFhQaUtL?=
 =?utf-8?B?aW1wYXo1em12Sy9wa242TURKOHo3Y3gxRnh5ck05T1BvOE9XaWV3bmtxdjly?=
 =?utf-8?B?QVM2cnBjbnpIajkwNXhwa0thVCt0VFFSNFNDWHR1akVCTVpxL0t2T05PMkgz?=
 =?utf-8?B?VTlvQk5MaW83WTM3YzlLb0RkVG8yclZqR2hrblYzdGx2MWFlQUtWZEdDNlhQ?=
 =?utf-8?B?OExROWVCK1NtRWVmaU9pb2NOL05UQ3oxa1dOWG14OEo5NHVLVmJJRTlFLzB5?=
 =?utf-8?B?bmV1QkFCLzh0YmpTRC9Gb0RWWVZNWnpJS1RGNlRsbVpubi9NSmRwdk1zRkVD?=
 =?utf-8?B?VXJ1Lzc4ZlNEOGlkbHd0WnBCcTFQd1BLWU0rb1BYMkhSZ0V5ZUV5VFVhNTFs?=
 =?utf-8?B?b1poaHAyZVFSdTNRVEhpd1J6NmJWUmNCelduYkp4ci95K05jYlZxanhQbUJl?=
 =?utf-8?B?KzBNZ2J2ZWlkaHFHV2xDdDBJcFVuZUlBUGp0OGc3WStOcTZoWFZJblV6aG5j?=
 =?utf-8?B?cGNkcTJlc0ltSjJTZ25BWXQrTUF3Zlh1VEZObTNINWdOU09PUzR2SHBKNU51?=
 =?utf-8?B?eitrZ1ViNlh5bHVYY29TeUpOeG5JSnRSSkJuWW5BQ0kycTc3V0JwUUNJUmlW?=
 =?utf-8?B?ejN0Qm45N0prZVY3OStDRWJ0RkdwdGtOaWtLMnltZ29ibFM2WENmbmdxc202?=
 =?utf-8?B?NTk2ZzczNjVheVlhNzZuRExLQ2N3Tk00VFJ4OGRuelhKZ0hJNjN6VHR0WW1v?=
 =?utf-8?B?Y0lOZFh2VzF5aFhjT0NMWW1lY0NnTUNnZjdSSWs3eU8zRkdsSFlUNnBlRzFV?=
 =?utf-8?B?NG4vTndubFA4MVhuTnQxL25iSENyV3lpREpwci8rZjY4S3Y4aGFNU0hMM0sy?=
 =?utf-8?Q?l6q5VjtcxdAeN6WxLt7pwS5kh?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	0uYbOInkbbheUhD7aSzLLAWaIYhlzTvCpe7dReO0ZvHskK2xRT/I35jls3vweEay1ZeBMIe9LRnz5w45NYnPKPvBDKrweT3J5tSzeg7BrvkdVjCCZUlyG91XGTMApjoLngw6xLAEiHwBmawUVcIQWBWh4oi3bzyKUSOtTacJAdmNJcdfL2+ppS6Liqule93IwOlbKsrt+Qy7K8HZiaWXgUXO6cqDQpT9F+tARd93pgFHX/mvx+NmuHD+a6KnKy6Z1qx3zpHo6UG8cUgL3Ml1mIE5xuZMbskxpCCvg2vdkTs5vbsccvBLz2TvtE16Tjg9RWYHNw6O1eVtJLeeFFk0QcldsFgXunCsdkdKsm76O9Tolph9ltmfoEoNLXoPGfjo+tuWb3wjDUXLNT3fSpq35rj0MX5NCMnmeuLySffR52aUkKCJg9pMtZWBHIf3LU7jc9S4Grzx0qOubdCe2Qjnup2mSTR5eiePLZQPX0xWG+RvBH6aDRGU3uQuAMzR6KXI8Dcu9au6ljkAngB6HWJJlcDtwvljrS1wznB4yRTaaguUH4qQaWF1hXNPMBIRFkTId0DDGAwjH7tcsiVMKxTuHYbx4RckIVD7D1FpwVDjoXQ=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d6ea3c53-38a6-48a1-afea-08dc3303f384
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB4313.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Feb 2024 17:38:44.5312
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: P9B6+yF78A4AtJBTxlAM0gqrYoOVc3JweTCBtidnSqULCdxC9OmNzCtY24dDSyWs2zUAoN6+cyUG9oChmAvSdw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR10MB5898
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-02-21_05,2024-02-21_02,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 suspectscore=0
 mlxscore=0 spamscore=0 mlxlogscore=999 phishscore=0 bulkscore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2311290000 definitions=main-2402210137
X-Proofpoint-GUID: HDrYK-D-mrZna6dDEvipkEYjyuqqGpzZ
X-Proofpoint-ORIG-GUID: HDrYK-D-mrZna6dDEvipkEYjyuqqGpzZ

On 21/02/2024 17:00, Darrick J. Wong wrote:
>>> Hmm.  Well, if we move towards pushing all the hardware checks out of
>>> xfs/iomap and into whatever goes on underneath submit_bio then I guess
>>> we don't need to check device support here at all.
>> Yeah, I have been thinking about this. But I was still planning on putting a
>> "bdev on atomic write" check here, as you mentioned.
>>
>> But is this a proper method to access the bdev for an xfs inode:
>>
>> STATIC bool
>> xfs_file_can_atomic_write(
>> struct xfs_inode *inode)
>> {
>> 	struct xfs_buftarg *target = xfs_inode_buftarg(inode);
>> 	struct block_device *bdev = target->bt_bdev;
>>
>> 	if (!xfs_inode_atomicwrites(inode))
>> 		return false;
>>
>> 	return bdev_can_atomic_write(bdev);
>> }
> There's still a TOCTOU race problem if the bdev gets reconfigured
> between xfs_file_can_atomic_write and submit_bio.

If that is the case then a check in the bio submit path is required to 
catch any such reconfigure problems - and we effectively have that in 
this series.

I am looking at change some of these XFS bdev_can_atomic_write() checks, 
but would still have a check in the bio submit path.

> 
> However, if you're only using this to advertise the capability via statx
> then I suppose that's fine -- userspace has to have some means of
> discovering the ability at all.  Userspace is also inherently racy.
> 
>> I do notice the dax check in xfs_bmbt_to_iomap() when assigning iomap->bdev,
>> which is creating some doubt?
> Do you mean this?
> 
> 	if (mapping_flags & IOMAP_DAX)
> 		iomap->dax_dev = target->bt_daxdev;
> 	else
> 		iomap->bdev = target->bt_bdev;
> 
> The dax path wants dax_dev set so that it can do the glorified memcpy
> operation, and it doesn't need (or want) a block device.

Yes, so proper to use target->bt_bdev for checks for bdev atomic write 
capability, right?

Thanks,
John