From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8CD1CEE644C for ; Fri, 15 Sep 2023 09:38:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2468A6B0328; Fri, 15 Sep 2023 05:38:27 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1F7156B032A; Fri, 15 Sep 2023 05:38:27 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0BF226B032B; Fri, 15 Sep 2023 05:38:27 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id F0B0C6B0328 for ; Fri, 15 Sep 2023 05:38:26 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id C5623160B1D for ; Fri, 15 Sep 2023 09:38:26 +0000 (UTC) X-FDA: 81238331412.21.B231EC5 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf08.hostedemail.com (Postfix) with ESMTP id C966B160008 for ; Fri, 15 Sep 2023 09:38:24 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=INbdH8Th; spf=pass (imf08.hostedemail.com: domain of dhowells@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=dhowells@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1694770705; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=1kkiG5L/a/xceIw2PPIGBdgyPVq7y6221yleAsPibq4=; b=xAgvzuvM6XjDUI6dFRnvXLTmBtHtfgF2F4ylvqazQLTfp0cbAsDf5FJ8YQ+TCTC/ASv4re fdn0H54gtiOokOtL9qWI2YzQFMWfQe6XYAh+icG8sqISXYbwBANjBfSd11k8GLBBlGcO5T MJzjYwmiZIGWvVXbAZQCCCgqai9kngM= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=INbdH8Th; spf=pass (imf08.hostedemail.com: domain of dhowells@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=dhowells@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1694770705; a=rsa-sha256; cv=none; b=NK/ZljrUeo7o4tSP5C8aRarFvNQ1BdqaBvqAQbnBbdYUfJje9Qu7ZdqGrnltsO7WpJR/SE 7YiT24Njpofx4bWpFXEg6uyVpusVKBri5ZDNJg9TIMHSWj8QtnVMhA8u/62b2peopqvCTw vCsOjaHhyvaxxBfrhHjOG5guLDrxfvg= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1694770704; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=1kkiG5L/a/xceIw2PPIGBdgyPVq7y6221yleAsPibq4=; b=INbdH8ThtlJSAJJ8dGpV++RhoRTveJIMU0t6n5WceyQE6+Hd54rVx5j7NNvNfW8UMIne4A G4YdZ5agV81zM9zMsYrcu86I+qTsBhJPDSDEwNoX7xnv80srsxKDEWvaIdrt/U9WmFrm3X 1EY/0ZGi/jmYQNBtmEpC476XAJM4RA0= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-260-Zfaqv3swO7urejhYiLIkNA-1; Fri, 15 Sep 2023 05:38:17 -0400 X-MC-Unique: Zfaqv3swO7urejhYiLIkNA-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 0647681DA99; Fri, 15 Sep 2023 09:38:17 +0000 (UTC) Received: from warthog.procyon.org.uk (unknown [10.42.28.216]) by smtp.corp.redhat.com (Postfix) with ESMTP id D167428FD; Fri, 15 Sep 2023 09:38:14 +0000 (UTC) Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 3798903 From: David Howells In-Reply-To: <445a78b0ff3047fea20d3c8058a5ff6a@AcuMS.aculab.com> References: <445a78b0ff3047fea20d3c8058a5ff6a@AcuMS.aculab.com> <20230913165648.2570623-1-dhowells@redhat.com> <20230913165648.2570623-6-dhowells@redhat.com> To: David Laight Cc: dhowells@redhat.com, Al Viro , Linus Torvalds , Jens Axboe , Christoph Hellwig , "Christian Brauner" , Matthew Wilcox , "Jeff Layton" , "linux-fsdevel@vger.kernel.org" , "linux-block@vger.kernel.org" , "linux-mm@kvack.org" , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v4 05/13] iov: Move iterator functions to a header file MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-ID: <3369773.1694770694.1@warthog.procyon.org.uk> Content-Transfer-Encoding: quoted-printable Date: Fri, 15 Sep 2023 10:38:14 +0100 Message-ID: <3369774.1694770694@warthog.procyon.org.uk> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.5 X-Rspamd-Queue-Id: C966B160008 X-Rspam-User: X-Stat-Signature: 6nynwb4wfsnxf8xnmea9ttgy64jn5try X-Rspamd-Server: rspam01 X-HE-Tag: 1694770704-82597 X-HE-Meta: U2FsdGVkX18i/MwZRCiRoqXX8yfpiwaKk5NKsgiICvS5yNsaYZ40J8PCZGoedOe+ne2P10efATHCk+UbcCheSuhLBVwvyve1Lo2wWTRBrxKcGnz4jTCsN3ss7fKE4n5jYXWU3SbyecCCTLULnBrtJfEyTGj3x6BJX6X//U3xNcPU5Ye8m6I6Psq/zIta48jlhv7FfYfDerCNpq/BHHnPMR4TKjfGbYXJSL5Xv/ANqeFN00LbtV4KWIiGDs71zXcY5o/5NASPt5FnLCNBcUFBV8Wva/SwkrXDPqKXgr05fR514Lxd/gzQ/ACUPWHaUEAayUNbAacikQ26dWwp7AtI7iPr+LSkz4z721oFrzTKifwStwPHw44ognUTm0/zzeAVGarZawG3Kf5GMa03+VCU/qjDDnilByK8br2rvgKDztCTMKtnfc3Of0UVdwC8fCL4789/qxhlvV/OSZ6xHXo/pfVCRWWaZZsBHARygUHbLFs4BVm8m+Yc0lEPPHstCpEJOkGnvdH0QTBZ2eYj/VqxqYWLBRa4Q8hO+uevXMXe3T0UImzSp3elH0KUWYj2clwXBNyMst9EmB45wAzZ0WOTD3ZvdMAX/qAnfU0n1hBgAW0ARjsw5U7BgENJPygQUb9lv5wMdGmD8CmASi2V4VA7ciCRoCkGqllPXOLLcMynB2a6MLAzO/p1uappG7oNvNdyLAEKxG+GIsgEP3AlfYPV7IeoXcOoBcNORxa7EjhJk69XtAuWxlbJgRCLXGnLNb+4nXLUK3a1yR+RyZY5ceuKdujKnR0/RVcHgI0xNoIICh+zHR/dS+WUiRGAebS7JWzFMRGnZUv1yW5tb7aknfCZZahXezZoFgZv9iYoWcAbB46s5OhThRexCMRUl85kwDuefQHMlEodxn9IB2f/DVWd6vOSm4fVlaEJuNI+ENuSSSRXsLzo5fKAWxxBudr8hg1SyBXoAyZ7HCMJpNwLCGd 8Xyk1lTo 8SnCZdZzD3pRmP8QK4srdDGIBsUzqx6osWEu/s971aIFamLG4eABsRDO194H/eO2d4eMocxNKLer4qHF12jw4eKShZ0ipYx4Ba1RbVG8hjuLSaw2qvQxmm+tD25TQdmiI+GzP4nf/46bj5ssL4Kyfdycw4YWtNYZOpi1fSu3lCiAD91pzQC4AEm4veeB2VokTbTuCgkc64/zDA88K+UC1NBtBXaxjek3sckDOVemwC/BM6PrXqrdewIq68wZYE6zLYaIo X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: David Laight wrote: > > Move the iterator functions to a header file so that other operations = that > > need to scan over an iterator can be added. For instance, the rbd dri= ver > > could use this to scan a buffer to see if it is all zeros and libceph = could > > use this to generate a crc. > = > These all look a bit big for being more generally inlined. > = > I know you want to avoid the indirect call in the normal cases, > but maybe it would be ok for other uses? So you'd advocate for something like: size_t generic_iterate(struct iov_iter *iter, size_t len, void *priv, void *priv2, iov_ustep_f ustep, iov_step_f step) { return iterate_and_advance2(iter, len, priv, priv2, ustep, step); } EXPORT_SYMBOL(generic_iterate); in lib/iov_iter.c and then call that from the places that want to use it? I tried benchmarking that (see attached patch - it needs to go on top of m= y iov patches). Running the insmod thrice and then filtering out and sortin= g the results: iov_kunit_benchmark_bvec: avg 3174 uS, stddev 68 uS iov_kunit_benchmark_bvec: avg 3176 uS, stddev 61 uS iov_kunit_benchmark_bvec: avg 3180 uS, stddev 64 uS iov_kunit_benchmark_bvec_outofline: avg 3678 uS, stddev 4 uS iov_kunit_benchmark_bvec_outofline: avg 3678 uS, stddev 5 uS iov_kunit_benchmark_bvec_outofline: avg 3679 uS, stddev 6 uS iov_kunit_benchmark_xarray: avg 3560 uS, stddev 5 uS iov_kunit_benchmark_xarray: avg 3560 uS, stddev 6 uS iov_kunit_benchmark_xarray: avg 3570 uS, stddev 16 uS iov_kunit_benchmark_xarray_outofline: avg 4125 uS, stddev 13 uS iov_kunit_benchmark_xarray_outofline: avg 4125 uS, stddev 2 uS iov_kunit_benchmark_xarray_outofline: avg 4125 uS, stddev 6 uS It adds almost 16% overhead: (gdb) p 4125/3560.0 $2 =3D 1.1587078651685394 (gdb) p 3678/3174.0 $3 =3D 1.1587901701323251 I'm guessing a lot of that is due to function pointer mitigations. Now, part of the code size expansion can be mitigated by using, say, iterate_and_advance_kernel() if you know you aren't going to encounter user-backed iterators, or even using, say, iterate_bvec() if you know you'= re only going to see a specific iterator type. David --- iov_iter: Benchmark out of line generic iterator diff --git a/include/linux/iov_iter.h b/include/linux/iov_iter.h index 2ebb86c041b6..8f562e80473b 100644 --- a/include/linux/iov_iter.h +++ b/include/linux/iov_iter.h @@ -293,4 +293,7 @@ size_t iterate_and_advance_kernel(struct iov_iter *ite= r, size_t len, void *priv, return progress; } = +size_t generic_iterate(struct iov_iter *iter, size_t len, void *priv, voi= d *priv2, + iov_ustep_f ustep, iov_step_f step); + #endif /* _LINUX_IOV_ITER_H */ diff --git a/lib/iov_iter.c b/lib/iov_iter.c index 8f7a10c4a295..f9643dd02676 100644 --- a/lib/iov_iter.c +++ b/lib/iov_iter.c @@ -1684,3 +1684,10 @@ ssize_t iov_iter_extract_pages(struct iov_iter *i, return -EFAULT; } EXPORT_SYMBOL_GPL(iov_iter_extract_pages); + +size_t generic_iterate(struct iov_iter *iter, size_t len, void *priv, voi= d *priv2, + iov_ustep_f ustep, iov_step_f step) +{ + return iterate_and_advance2(iter, len, priv, priv2, ustep, step); +} +EXPORT_SYMBOL(generic_iterate); diff --git a/lib/kunit_iov_iter.c b/lib/kunit_iov_iter.c index cc9c64663a73..f208516a68c9 100644 --- a/lib/kunit_iov_iter.c +++ b/lib/kunit_iov_iter.c @@ -18,6 +18,7 @@ #include #include #include +#include #include = MODULE_DESCRIPTION("iov_iter testing"); @@ -1571,6 +1572,124 @@ static void __init iov_kunit_benchmark_xarray(stru= ct kunit *test) KUNIT_SUCCEED(); } = +static noinline +size_t shovel_to_user_iter(void __user *iter_to, size_t progress, + size_t len, void *from, void *priv2) +{ + if (should_fail_usercopy()) + return len; + if (access_ok(iter_to, len)) { + from +=3D progress; + instrument_copy_to_user(iter_to, from, len); + len =3D raw_copy_to_user(iter_to, from, len); + } + return len; +} + +static noinline +size_t shovel_to_kernel_iter(void *iter_to, size_t progress, + size_t len, void *from, void *priv2) +{ + memcpy(iter_to, from + progress, len); + return 0; +} + +/* + * Time copying 256MiB through an ITER_BVEC with an out-of-line copier + * function. + */ +static void __init iov_kunit_benchmark_bvec_outofline(struct kunit *test) +{ + struct iov_iter iter; + struct bio_vec *bvec; + struct page *page; + unsigned int samples[IOV_KUNIT_NR_SAMPLES]; + ktime_t a, b; + ssize_t copied; + size_t size =3D 256 * 1024 * 1024, npages =3D size / PAGE_SIZE; + void *scratch; + int i; + + /* Allocate a page and tile it repeatedly in the buffer. */ + page =3D alloc_page(GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, page); + kunit_add_action_or_reset(test, iov_kunit_free_page, page); + + bvec =3D kunit_kmalloc_array(test, npages, sizeof(bvec[0]), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, bvec); + for (i =3D 0; i < npages; i++) + bvec_set_page(&bvec[i], page, PAGE_SIZE, 0); + + /* Create a single large buffer to copy to/from. */ + scratch =3D iov_kunit_create_source(test, npages); + + /* Perform and time a bunch of copies. */ + kunit_info(test, "Benchmarking copy_to_iter() over BVEC:\n"); + for (i =3D 0; i < IOV_KUNIT_NR_SAMPLES; i++) { + iov_iter_bvec(&iter, ITER_DEST, bvec, npages, size); + a =3D ktime_get_real(); + copied =3D generic_iterate(&iter, size, scratch, NULL, + shovel_to_user_iter, + shovel_to_kernel_iter); + b =3D ktime_get_real(); + KUNIT_EXPECT_EQ(test, copied, size); + samples[i] =3D ktime_to_us(ktime_sub(b, a)); + } + + iov_kunit_benchmark_print_stats(test, samples); + KUNIT_SUCCEED(); +} + +/* + * Time copying 256MiB through an ITER_XARRAY with an out-of-line copier + * function. + */ +static void __init iov_kunit_benchmark_xarray_outofline(struct kunit *tes= t) +{ + struct iov_iter iter; + struct xarray *xarray; + struct page *page; + unsigned int samples[IOV_KUNIT_NR_SAMPLES]; + ktime_t a, b; + ssize_t copied; + size_t size =3D 256 * 1024 * 1024, npages =3D size / PAGE_SIZE; + void *scratch; + int i; + + /* Allocate a page and tile it repeatedly in the buffer. */ + page =3D alloc_page(GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, page); + kunit_add_action_or_reset(test, iov_kunit_free_page, page); + + xarray =3D iov_kunit_create_xarray(test); + + for (i =3D 0; i < npages; i++) { + void *x =3D xa_store(xarray, i, page, GFP_KERNEL); + + KUNIT_ASSERT_FALSE(test, xa_is_err(x)); + } + + /* Create a single large buffer to copy to/from. */ + scratch =3D iov_kunit_create_source(test, npages); + + /* Perform and time a bunch of copies. */ + kunit_info(test, "Benchmarking copy_to_iter() over XARRAY:\n"); + for (i =3D 0; i < IOV_KUNIT_NR_SAMPLES; i++) { + iov_iter_xarray(&iter, ITER_DEST, xarray, 0, size); + a =3D ktime_get_real(); + + copied =3D generic_iterate(&iter, size, scratch, NULL, + shovel_to_user_iter, + shovel_to_kernel_iter); + b =3D ktime_get_real(); + KUNIT_EXPECT_EQ(test, copied, size); + samples[i] =3D ktime_to_us(ktime_sub(b, a)); + } + + iov_kunit_benchmark_print_stats(test, samples); + KUNIT_SUCCEED(); +} + static struct kunit_case __refdata iov_kunit_cases[] =3D { KUNIT_CASE(iov_kunit_copy_to_ubuf), KUNIT_CASE(iov_kunit_copy_from_ubuf), @@ -1593,6 +1712,8 @@ static struct kunit_case __refdata iov_kunit_cases[]= =3D { KUNIT_CASE(iov_kunit_benchmark_bvec), KUNIT_CASE(iov_kunit_benchmark_bvec_split), KUNIT_CASE(iov_kunit_benchmark_xarray), + KUNIT_CASE(iov_kunit_benchmark_bvec_outofline), + KUNIT_CASE(iov_kunit_benchmark_xarray_outofline), {} }; =