Received-SPF: pass (google.com: domain of linux-kernel+bounces-5185-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <76c85021-dd9e-49e3-80e3-25a17c7ca455@oracle.com>
Date: Tue, 19 Dec 2023 12:41:37 +0000
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 00/16] block atomic writes
To: Christoph Hellwig <hch@lst.de>, "Darrick J. Wong" <djwong@kernel.org>
Cc: axboe@kernel.dk, kbusch@kernel.org, sagi@grimberg.me, jejb@linux.ibm.com,
        martin.petersen@oracle.com, viro@zeniv.linux.org.uk,
        brauner@kernel.org, dchinner@redhat.com, jack@suse.cz,
        linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-nvme@lists.infradead.org, linux-xfs@vger.kernel.org,
        linux-fsdevel@vger.kernel.org, tytso@mit.edu, jbongio@google.com,
        linux-scsi@vger.kernel.org, ming.lei@redhat.com, jaswin@linux.ibm.com,
        bvanassche@acm.org
References: <20231212110844.19698-1-john.g.garry@oracle.com>
 <20231212163246.GA24594@lst.de>
 <b8b0a9d7-88d2-45a9-877a-ecc5e0f1e645@oracle.com>
 <20231213154409.GA7724@lst.de>
 <c729b03c-b1d1-4458-9983-113f8cd752cd@oracle.com>
 <20231219051456.GB3964019@frogsfrogsfrogs> <20231219052121.GA338@lst.de>
Content-Language: en-US
From: John Garry <john.g.garry@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <20231219052121.GA338@lst.de>
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?WjM3NGtJUzRuN3RkazBTL0pESG8vOHN6dlNRODBuUTRuZDRFTW1Oa1JqVk5s?=
 =?utf-8?B?emRaYlNuNmg1Z1UrdVZza05ZV3B2V3RZWHVjcndpdUJMTk5IZHdISnc1Zkxq?=
 =?utf-8?B?dE84V0hJaGlDL2xHci90MTUvQlVQU2hEdW5uYkxXZXNXdWlXWnNCdlN1YlAr?=
 =?utf-8?B?NzJQSnp0ZDNmOGE0d1VsQ1A0V3Z4SjNyckRTLzJTUmJNWkdZL29yMlcrbzBC?=
 =?utf-8?B?SGM5SG5MdXJBeDl0TGlVQjFoYU9hMHphcmh3SWVxWU9hRVl2UHplSDdyMGFG?=
 =?utf-8?B?VXFTcnRJUVJ3Q05XRTU1OEU3UmNydXhKSUprc09TUXNzMTh2all3WW16em1B?=
 =?utf-8?B?U015bEE1d0JLZ3c5ZVRZUkVzNGF4TFBRMktHa3VxWi9DeVhMRjdEWHhrQXBs?=
 =?utf-8?B?VFcvMjh5V001bnRtMWRQM1FnUVdtd2YwdTJEM1dsOElpdnhiWlhaeTM2RFV6?=
 =?utf-8?B?V3UxNVh0TXNoaGZqbFAra3JmcU1iVlVYYXh3SE1GY1ZPWG4xYkxKNGZQQ2ov?=
 =?utf-8?B?alYxRzF2MUFVaTZ0MlQ0elNBTXRKSXlnejROb1JNUkhHZUFVMXZmcUQ4Q1Q0?=
 =?utf-8?B?ckNZdE4xYVRTYjZYVW9mZHMxYkUvRVl1SW9ib2pzY0IxMjcxUkMreTRxbEpy?=
 =?utf-8?B?M2FGWms2MmZMNnRmcE1LK1QvSDhZMnZINVlOYUt2SEllaTd6NXVXT0NOUy91?=
 =?utf-8?B?bFV3WEtRdWhnOTcwTnZKaklnT1NENFZ3SGcxaElJRXV0emxNOUFhbzZ3ZTRB?=
 =?utf-8?B?RU5aeGNjTDJ5MXFsWWIwZTZzQ1lzTjZsakJDbzMrWlJ6RnhPT1hRTkJDL202?=
 =?utf-8?B?VzgvYldoRldiSDFWUTU2YTM0TXJreHl0SVZmMjA2NmJHR093ak9RbWNha2Zn?=
 =?utf-8?B?WnN2YnFoQnQ2UUM1ZXdxYkE2RmF0NjVvdldDdndlM3Q4SUlKNVp5dS93dnFK?=
 =?utf-8?B?cnFveDlTSnNMWGxjRlg2WUtmVEd6S2FTTzJDN1Q0M3ZUWmNDZUk5SW9zZHFo?=
 =?utf-8?B?TW9BVHdnTTJ2Y0lIZDRFYUhZaG1FcFRDdmNQUWJWblZKU3ZPWVpqMTU2S3Vx?=
 =?utf-8?B?alRTUVZHMittelgzNHh6L2t0SnFKWmdxWGpnMmExNnJtaXVyWkh5dnBnNVcv?=
 =?utf-8?B?ZUhVSDVTODZPM2J4QTJ6KzkxaGVNTmU0Y2kzQVVHY2ZieC9sSE1JcWVwU2oz?=
 =?utf-8?B?WGVUeDZ5bFNoRHFDREhwbGhNc3R5MnIzbldvOHFwazZpTTU3UUV3Z2xuVkU4?=
 =?utf-8?B?aDIxMHBTb3lkekF6cmtONnNXTnlOTDkwK2hFMVBTRU0vd0lvVnNTNUNlR1hQ?=
 =?utf-8?B?WGpCK3MxWkNtQkgySFc5YXdjTmZvZG9lWWIwZ09pVDJLZ2lPNmEvenpLU2RN?=
 =?utf-8?B?eXFBM29wTWZiUURscUNsNVU3RDR6N3oxWkxUc05MejdtOUhOWU9nZGtCVTNU?=
 =?utf-8?B?TzNkZE1nZkYyQnpDNkJRUHppVXQ4d0VOMmh3SXFkSkZFTVZLREh6OUowd2Qx?=
 =?utf-8?B?dStLc3pGOXR6amc3TkwxTjhrdmwwNVVGdmhIa0FJK25Sc0QzSTMzZzZBT2FS?=
 =?utf-8?B?QWNrREtoUnlmNnNjc1ZtNFl2NzlRY2tya2g0d0ZIYkVsejlERWpSemd4UHFr?=
 =?utf-8?B?NW50TDBBSVdYWXYxcGV2cWwrYkhkMjNJQXFvcnd6cEFBMEZHdDFvVXZPUHBr?=
 =?utf-8?B?S1RZOWZ3UlVvaU5BU29JZXJGL2s3MzNCa3duSEhMT05Kd1NxR1JRaFI1bEp3?=
 =?utf-8?B?dU5wVjJqeENIUTQwOHlYV2pXRVhYQkd5SmRQT1JZdVJsME5EZytEZ2tleXdC?=
 =?utf-8?B?VGxWdlpkeEx4cjFPVndBUXI1eldRMHoycUZlcThUR1g3TkhnSUl2SXJ3MUo0?=
 =?utf-8?B?RXhwZHZMVWcxb2x0aXRVRmdZcW1WeGkrYWJSVlQzZXFSQnNyTDllQ1BobGd5?=
 =?utf-8?B?Qlk4bjZPc2ROSmVsMzlUUzdML3BSRDZJOC9iakRUdW5CMkdwcFllZzlMMHJp?=
 =?utf-8?B?VlIyRWo4Z01TNlFzSDhnTnA5cC9IdVE2ODFjbjdjeHp3YWFKZW9rMkcwNHBo?=
 =?utf-8?B?WUVuVmVlTVFldlh2cmptMTQyMWFydDh1T1U3WFZDTWhrZm9PQ3cwMGZQcDRM?=
 =?utf-8?Q?IzcCTukZzd5d5NB3xbnVQkyFi?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	xNQoJhgQS6WbbLy4YZrJJ3jYz7yCjGI/ghloXUzfkXUMHSazFIJn5BHXQaE7ZRyg0by+KF/soikraIIn7gtD8BEJ3bfrvsP0K8ZXdE7b5GnMJKWT8ERDDPrz+a0eFmc7myl+IOMJ4Pw9Xibp30hMiDVE1UuRPttL1yzNiT8ZaGvYTn6hk9Le0RIoCDs42+ct/xF2So4r3BHKuNWHo/T1a7KV6NXbdJ3ItDQWkZ/+CY8aewYToK7PGGgDc7r18kwjpmtHKPjW4WOQhyKbLeLbIa9UmHkRDEEuBtynTasWgMeAE2GtLg73qNHxklsqMsTIF2GdaEnum21l+YVMsETtwr3QluYh3OxQW1MvZdGwuETh2lc6Nxff7PHKQkYWRiv/BMngiBZnUyIFwphm6uRL2huG0mie6yFjpwnxOxrp89c/g2sDRjsRKGWcmpf6hPiIBa7nqVJ1cQOW5E/t4+4Ab/XpCvzVZgIu/sk4ZxGFeHQbrtDeyrRBxKHvH+K+BAdAPEQGO+o2MYPCyTJxwjwisIB0hRfR9qUyVD+aDbtEEzWRCSivKyCenvnS03kCQSSaOevgc731k94qBAuxXaINEMLxp54Yp6k/hRJX5X6/Ku4=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 41b95cf6-70a1-4de6-7e2c-08dc008fda5e
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB4313.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Dec 2023 12:41:42.6146
 (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: nkg5UKTFwleZBeAhwhjD8/Jnm4c6OpU3crYMEX2S9ia839JBaDOzgxRh0pjvw9wGaBYKSPQS/pT+V2vXThjavA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV8PR10MB7917
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-12-19_07,2023-12-14_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 spamscore=0 mlxscore=0
 mlxlogscore=999 bulkscore=0 adultscore=0 suspectscore=0 phishscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311290000
 definitions=main-2312190094
X-Proofpoint-ORIG-GUID: 16ywSFLrKwLUcu9918SKbWgXkkiI32AQ
X-Proofpoint-GUID: 16ywSFLrKwLUcu9918SKbWgXkkiI32AQ

On 19/12/2023 05:21, Christoph Hellwig wrote:
> On Mon, Dec 18, 2023 at 09:14:56PM -0800, Darrick J. Wong wrote:
>> /me stumbles back in with plenty of covidbrain to go around.
>>
>> So ... Christoph, you're asking for a common API for
>> sysadmins/applications to signal to the filesystem that they want all
>> data allocations aligned to a given value, right?
>>
>> e.g. if a nvme device advertises a capability for untorn writes between
>> $lbasize and 64k, then we need a way to set up each untorn-file with
>> some alignment between $lbasize and 64k?
>>
>> or if cxl storage becomes not ung-dly expensive, then we'd want a way to
>> set up files with an alignment of 2M (x86) or 512M (arm64lol) to take
>> advantage of PMD mmap mappings?
> 
> The most important point is to not mix these up.
> 
> If we want to use a file for atomic writes I should tell the fs about
> it, and preferably in a way that does not require me to know about weird
> internal implementation details of every file system.  I really just
> want to use atomic writes.  Preferably I'd just start using them after
> asking for the limits.  But that's obviously not going to work for
> file systems that use the hardware offload and don't otherwise align
> to the limit (which would suck for very small files anyway :))
> 
> So as a compromise I tell the file system before writing or otherwise
> adding any data [1] to file that I want to use atomic writes so that
> the fs can take the right decisions.
> 
> [1] reflinking data into a such marked file will be ... interesting.
> 

How about something based on fcntl, like below? We will prob also 
require some per-FS flag for enabling atomic writes without HW support. 
That flag might be also useful for XFS for differentiating forcealign 
for atomic writes with just forcealign.

---8<----

diff --git a/fs/fcntl.c b/fs/fcntl.c
index c80a6acad742..d4a50655565a 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -313,6 +313,15 @@ static long fcntl_rw_hint(struct file *file, 
unsigned int cmd,
  	}
  }

+static long fcntl_atomic_write(struct file *file, unsigned int cmd,
+			  unsigned long arg)
+{
+	if (file->f_op->enable_atomic_writes)
+		return file->f_op->enable_atomic_writes(file, arg);
+
+	return -EOPNOTSUPP;
+}
+
  static long do_fcntl(int fd, unsigned int cmd, unsigned long arg,
  		struct file *filp)
  {
@@ -419,6 +428,9 @@ static long do_fcntl(int fd, unsigned int cmd, 
unsigned long arg,
  	case F_SET_RW_HINT:
  		err = fcntl_rw_hint(filp, cmd, arg);
  		break;
+	case F_SET_ATOMIC_WRITE_SIZE:
+		err = fcntl_atomic_write(filp, cmd, arg);
+		break;
  	default:
  		break;
  	}
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index e33e5e13b95f..dba206cbe1ab 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1478,6 +1478,46 @@ xfs_file_mmap(
  	return 0;
  }

+STATIC int
+xfs_enable_atomic_writes(
+	struct file	*file,
+	unsigned int unit_max)
+{
+	struct xfs_inode *ip = XFS_I(file_inode(file));
+	struct mnt_idmap *idmap = file_mnt_idmap(file);
+	struct dentry *dentry = file->f_path.dentry;
+	struct xfs_buftarg	*target = xfs_inode_buftarg(ip);
+	struct block_device	*bdev = target->bt_bdev;
+	struct request_queue	*q = bdev->bd_queue;
+	struct inode		*inode = &ip->i_vnode;
+
+	if (queue_atomic_write_unit_max_bytes(q) == 0) {
+		if (unit_max != 0) {
+			/* We expect unbounded CoW size if no HW support */
+			return -ENOTBLK;
+		}
+		/* Do something for CoW support ... */
+
+		return 0;
+	}
+
+	if (!unit_max)
+		return -EINVAL;
+
+	/* bodge alert */
+	if (inode->i_op->fileattr_set) {
+		struct fileattr fa = {
+			.fsx_extsize = unit_max,
+			.fsx_xflags = FS_XFLAG_EXTSIZE | FS_XFLAG_FORCEALIGN,
+			.fsx_valid = 1,
+		};
+
+		return inode->i_op->fileattr_set(idmap, dentry, &fa);
+	}
+
+	return -EOPNOTSUPP;
+}
+
  const struct file_operations xfs_file_operations = {
  	.llseek		= xfs_file_llseek,
  	.read_iter	= xfs_file_read_iter,
@@ -1498,6 +1538,7 @@ const struct file_operations xfs_file_operations = {
  	.fallocate	= xfs_file_fallocate,
  	.fadvise	= xfs_file_fadvise,
  	.remap_file_range = xfs_file_remap_range,
+	.enable_atomic_writes = xfs_enable_atomic_writes,
  };

  const struct file_operations xfs_dir_file_operations = {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 98b7a7a8c42e..a715f98057b8 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1956,6 +1956,7 @@ struct file_operations {
  	int (*uring_cmd)(struct io_uring_cmd *ioucmd, unsigned int issue_flags);
  	int (*uring_cmd_iopoll)(struct io_uring_cmd *, struct io_comp_batch *,
  				unsigned int poll_flags);
+	int (*enable_atomic_writes)(struct file *, unsigned int unit_max);
  } __randomize_layout;

  /* Wrap a directory iterator that needs exclusive inode access */
diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
index 6c80f96049bd..69780c49622e 100644
--- a/include/uapi/linux/fcntl.h
+++ b/include/uapi/linux/fcntl.h
@@ -56,6 +56,8 @@
  #define F_GET_FILE_RW_HINT	(F_LINUX_SPECIFIC_BASE + 13)
  #define F_SET_FILE_RW_HINT	(F_LINUX_SPECIFIC_BASE + 14)

+#define F_SET_ATOMIC_WRITE_SIZE  (F_LINUX_SPECIFIC_BASE + 15)
+
  /*
   * Valid hint values for F_{GET,SET}_RW_HINT. 0 is "not set", or can be
   * used to clear any hints previously set.
-- 


--->8----