Received-SPF: pass (google.com: domain of linux-nfs+bounces-3646-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <8e23be47-e542-4bb8-8da7-da7801c98e42@hpe.com>
Date: Tue, 11 Jun 2024 17:47:12 +0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/2] filemap: Convert generic_perform_write() to support
 large folios
To: Christoph Hellwig <hch@lst.de>,
        Trond Myklebust
	<trond.myklebust@hammerspace.com>,
        Anna Schumaker <anna@kernel.org>,
        "Matthew
 Wilcox" <willy@infradead.org>
CC: <linux-nfs@vger.kernel.org>, <linux-fsdevel@vger.kernel.org>,
        <linux-mm@kvack.org>
References: <20240527163616.1135968-1-hch@lst.de>
 <20240527163616.1135968-2-hch@lst.de>
Content-Language: en-US
From: Shaun Tancheff <shaun.tancheff@hpe.com>
In-Reply-To: <20240527163616.1135968-2-hch@lst.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?a0srZ0Zibk41OVViNkthNi9iMW9vQS84MEpXSmtQb3Z1NTVhWm1KS0h1bUNx?=
 =?utf-8?B?MnA0YThWYmQwSDlkV09JR1NsL1FMbGcrcmZrQTVGY205NWhXRWdjaU15em9T?=
 =?utf-8?B?c1YwSjVNZnRCZ2s3aTJaalByRWQ4NGZWSVVTVFBwYnRzR2oyMEg1dHo1aXVZ?=
 =?utf-8?B?RUwwTHc0WjViT09Bck14ZDBzdHNrSVNqaXF2bGpWL2dpRGVteTRrWVhYN095?=
 =?utf-8?B?Q1hxMndVOXV4emxIaExseERLaGtJNWJBM2NUc2k5ZDhVYVZJQ3pGYVRCTUZt?=
 =?utf-8?B?cjNQaldXdStRWS90eXBnOTBmdVlDbmd3djI5RER6ZCs0V05ZZ2pFOTFrSVBZ?=
 =?utf-8?B?eFZyUFQzWGU2ZkcrZmZGM2w2WXJnWVBEY2Z2SFg5K3p5TkJMZlZhbUdiZytV?=
 =?utf-8?B?NFgvenQ4RURDM0N2OC94K3hFdVRCM0RaeUUwWjFBblFxN1hQZC9sY3c5RTcz?=
 =?utf-8?B?NnhIci9HcGh6Ti83N0hEM3J2VWFiK3VQWnJocDJKbU1hWmw0TVd5L0pFQUM4?=
 =?utf-8?B?c1FWbDg3TDk0a09INUhrc0ltY2ZHRU5GdlJzRUxNRHlBc3ByejBJazRzKzZG?=
 =?utf-8?B?Nk9mY0JqWXhIUUwwd3hodGxmYisxS1hrenRQaFRHb3ZtZ1QyclFUMEc1SXdD?=
 =?utf-8?B?YVpQUU5NQmk0MEllMG1YeTZXdzVpMjM2QzZzNXFCOU55Z2JIR1ozZXovTUNn?=
 =?utf-8?B?TTRIc3dBREt6K1JCdWRYRFRtSk5GQUdkSlJNRjdXbDB6Y2hoNEF1WlN2Q1ZE?=
 =?utf-8?B?NW5CcGowYjMrbWE2Slh3YkMxb1FaZFQwaTI4VndwOWpnSTdwelZQdkN3ME5W?=
 =?utf-8?B?YjJsTFk5elpOS3ZFeUZKc0RMWU9QdlpLc085dG1aVHhuN1RYL2swR2s4NjZD?=
 =?utf-8?B?R3NIKzNKc1ZRUU1BM3pNWEkyYWozKzA2N0xPQVB5RzdEUkJOVUZDQTJ4WHVq?=
 =?utf-8?B?cDg0UWZmVWV3SVFiZUwraDR3MVNmYjVRSWhqa2dhRkVIZEtiNTAvTjZvZDVP?=
 =?utf-8?B?WGN4UmtiQnJhcnlqalNvS293NXI3Qld3Um5HRTJBeURzVlhtOXAwODFjVHRD?=
 =?utf-8?B?M09uZmhhNnhKVFZNVEw5WVFwbURrdXZtSHRSREk5cW95K0NtVmtFRnYyb28r?=
 =?utf-8?B?L1FFK2IrR0gxOHgzMWthVHJRQ2tnVm1pQnBWZ3B5SzF4QUVwRmZkdDZIVlJR?=
 =?utf-8?B?TUttQVVRRGE5dDloYUIyS1phWVh6azY2cGtBbnRybU1qRTdFckpwc00wR1BT?=
 =?utf-8?B?OVBCb0lscDVXYnlIdy9BSWNuY0s5T0hKY21sNkFlNzBVY05DaHZ2VHJuSnFS?=
 =?utf-8?B?dnNFVno5QVJOcUNHc2tWdERHM0tuRzY3WkFxeFRncXR3aFArWFlmU1BTK0VC?=
 =?utf-8?B?V0hjQ2pqRGlkYThBcW1pNU1PaHVpcXpVbXRnMVVGMk1xYnNHWDYxQi9QWnRm?=
 =?utf-8?B?YUQ5UEpnZ0FwNmIyeHhVRXdnMEFTTlpHUTN3TDdWdTh1VUlMZWljTk9lR21D?=
 =?utf-8?B?ZksrMHh6eFpKbXp0TUNxQVBHVnB1bGNPSkJ6bUk3Q3FZenJYQlNQV05sbVVy?=
 =?utf-8?B?ZkZudEFIeDJXeTM0eTNaSEhFTzRyTG1uU3BDQ1E4QjBVR0YyRXQvUUZleVVl?=
 =?utf-8?B?bEM3QzRJRDR5YjMyZkczTFpJUGpMdysxcm5mNm1KMTB0eW5sWWh2U1VCbHE0?=
 =?utf-8?B?OGJLSE5TNnNMUS9IMTl0amFSbjROV1Z6SUp1aWJ1R0Q2WW5obE15ZmR6b2ZM?=
 =?utf-8?B?emNZVUZ5Wi9uMHl1UHJKSWZUeFN2M1VvWlZuOWltMEZNTUYycWNKUU1NK2s5?=
 =?utf-8?B?N0NlcGUxb2tpMXExL3FHRzJmVDdld3Rqb25yT2dlVWNpR2VGc2NITFE2dnZu?=
 =?utf-8?B?UG40a1pXUVdBZkhJd3BQaGkxNFZ4MzlYb0M1c0NKdTJ5Tm5iYWFFNTNmWHl6?=
 =?utf-8?B?QUZGaUFWbTdhbWhleTFlK1oxWEJDRVlsVnZXNkxqTnBmeDVtbGJlczBuWWYx?=
 =?utf-8?B?bU51QlNPMnQ1Y05VTVFTK3FveTRpYkRabW91N1hnclliUDRpWk1LNTZwTmR0?=
 =?utf-8?B?b3ZHanZJYjVKT1dXSHRzOUhSRDUwRjJBR0lXT2RoZEkydklOZno0bjF4MENR?=
 =?utf-8?Q?SADH/ybgJQ+PwiYqAgyF5oLQE?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 0a56e543-cc8f-4817-b0f8-08dc8a03e4e4
X-MS-Exchange-CrossTenant-AuthSource: SJ0PR84MB1697.NAMPRD84.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Jun 2024 10:47:31.0517
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 105b2061-b669-4b31-92ac-24d304d195dc
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: OSGKXHG6dvAqNB8CdR5msAvMaJHMM7xkeS7Tv7nyYSibmrnQSVuEUIVG/u0xieNBvByfr12XDTucAbNHBH5fkQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR84MB3913
X-OriginatorOrg: hpe.com
X-Proofpoint-ORIG-GUID: yPFUXbeTuFJmA-wDLoEAjseP2FNsC0-r
X-Proofpoint-GUID: yPFUXbeTuFJmA-wDLoEAjseP2FNsC0-r
X-HPE-SCL: -1
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.680,FMLib:17.12.28.16
 definitions=2024-06-11_07,2024-06-11_01,2024-05-17_01
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1011 malwarescore=0
 bulkscore=0 phishscore=0 suspectscore=0 adultscore=0 lowpriorityscore=0
 priorityscore=1501 spamscore=0 mlxlogscore=999 mlxscore=0 impostorscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2405010000
 definitions=main-2406110081

On 5/27/24 23:36, Christoph Hellwig wrote:

> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
>
> Modelled after the loop in iomap_write_iter(), copy larger chunks from
> userspace if the filesystem has created large folios.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> [hch: use mapping_max_folio_size to keep supporting file systems that do
>   not support large folios]
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   mm/filemap.c | 40 +++++++++++++++++++++++++---------------
>   1 file changed, 25 insertions(+), 15 deletions(-)
>
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 382c3d06bfb10c..860728e26ccf32 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -3981,21 +3981,24 @@ ssize_t generic_perform_write(struct kiocb *iocb, struct iov_iter *i)
>   	loff_t pos = iocb->ki_pos;
>   	struct address_space *mapping = file->f_mapping;
>   	const struct address_space_operations *a_ops = mapping->a_ops;
> +	size_t chunk = mapping_max_folio_size(mapping);

Better to default chunk to PAGE_SIZE for backward compat
+       size_t chunk = PAGE_SIZE;

>   	long status = 0;
>   	ssize_t written = 0;
>   

Have fs opt in to large folio support:

+       if (mapping_large_folio_support(mapping))
+               chunk = PAGE_SIZE << MAX_PAGECACHE_ORDER;

>   	do {
>   		struct page *page;
> -		unsigned long offset;	/* Offset into pagecache page */
> -		unsigned long bytes;	/* Bytes to write to page */
> +		struct folio *folio;
> +		size_t offset;		/* Offset into folio */
> +		size_t bytes;		/* Bytes to write to folio */
>   		size_t copied;		/* Bytes copied from user */
>   		void *fsdata = NULL;
>   
> -		offset = (pos & (PAGE_SIZE - 1));
> -		bytes = min_t(unsigned long, PAGE_SIZE - offset,
> -						iov_iter_count(i));
> +		bytes = iov_iter_count(i);
> +retry:
> +		offset = pos & (chunk - 1);
> +		bytes = min(chunk - offset, bytes);
> +		balance_dirty_pages_ratelimited(mapping);
>   
> -again:
>   		/*
>   		 * Bring in the user page that we will copy from _first_.
>   		 * Otherwise there's a nasty deadlock on copying from the
> @@ -4017,11 +4020,16 @@ ssize_t generic_perform_write(struct kiocb *iocb, struct iov_iter *i)
>   		if (unlikely(status < 0))
>   			break;
>   
> +		folio = page_folio(page);
> +		offset = offset_in_folio(folio, pos);
> +		if (bytes > folio_size(folio) - offset)
> +			bytes = folio_size(folio) - offset;
> +
>   		if (mapping_writably_mapped(mapping))
> -			flush_dcache_page(page);
> +			flush_dcache_folio(folio);
>   
> -		copied = copy_page_from_iter_atomic(page, offset, bytes, i);
> -		flush_dcache_page(page);
> +		copied = copy_folio_from_iter_atomic(folio, offset, bytes, i);
> +		flush_dcache_folio(folio);
>   
>   		status = a_ops->write_end(file, mapping, pos, bytes, copied,
>   						page, fsdata);
> @@ -4039,14 +4047,16 @@ ssize_t generic_perform_write(struct kiocb *iocb, struct iov_iter *i)
>   			 * halfway through, might be a race with munmap,
>   			 * might be severe memory pressure.
>   			 */
> -			if (copied)
> +			if (chunk > PAGE_SIZE)
> +				chunk /= 2;
> +			if (copied) {
>   				bytes = copied;
> -			goto again;
> +				goto retry;
> +			}
> +		} else {
> +			pos += status;
> +			written += status;
>   		}
> -		pos += status;
> -		written += status;
> -
> -		balance_dirty_pages_ratelimited(mapping);
>   	} while (iov_iter_count(i));
>   
>   	if (!written)

Tested with Lustre with large folios and kernel 6.6 with this patch (and suggested changes).

Tested-by: Shaun Tancheff <shaun.tancheff@hpe.com>