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 BDBC5C77B7A for ; Tue, 30 May 2023 19:43:41 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 12299280003; Tue, 30 May 2023 15:43:41 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0D1C0280001; Tue, 30 May 2023 15:43:41 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E8D43280003; Tue, 30 May 2023 15:43:40 -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 D7484280001 for ; Tue, 30 May 2023 15:43:40 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id A86861C723A for ; Tue, 30 May 2023 19:43:40 +0000 (UTC) X-FDA: 80847946200.07.82411DC Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf23.hostedemail.com (Postfix) with ESMTP id 3FF4714001F for ; Tue, 30 May 2023 19:43:38 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=AQzZWEim; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf23.hostedemail.com: domain of mpatocka@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=mpatocka@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1685475818; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=61jSLilvI6Kq8tZP1TgtpGiMrTcQZMGT4z3p0LusAeA=; b=vx3K7MDeOXgxTkyZePjlNDlSniVfyuOcIVNy/E0BtHyzdf0keokL8lqjoLwHTh6gr4dtrn Fzde6JzEEL8Zpki1US6xloHstNAwXHm4xsUGVqEM4I/yoBOkd4QcztYijJvbEM3dL7SzZL JOxDYc/B4mR8gaxzKRpDOiShuCVdOGM= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=AQzZWEim; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf23.hostedemail.com: domain of mpatocka@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=mpatocka@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1685475818; a=rsa-sha256; cv=none; b=nufKxtqH5j2k95avqqk60bixXH7bLYzBrJ4n3N76qy2f9gs42jXJlXfHTsvpFd68r7pshW B5vyecOWLiCdIwIxTldwW+nZnMkgLR1oxkR1is+eh6f4zv+yhgN9j2v1BB3wI2w6UaZf2w 10DmGQno4oPZe1exR1SMsVPOFYth+zE= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1685475817; 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: in-reply-to:in-reply-to:references:references; bh=61jSLilvI6Kq8tZP1TgtpGiMrTcQZMGT4z3p0LusAeA=; b=AQzZWEimlSADXKYe63gZ79BIX+JMOZ5zpaGGt0ZiSWrkK27zGCzNltF47rEw83E5xnJPDU 4cdJyl4r/+sZ2yh/ol77hyHptwzAVrllGB0jWHgArvgeMtO7jLFMXsbG5Vr4lAKw6dzVBD DGYbPk71MdyPtYHrSH/oVAEVV/ptYEk= 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-142-MsdeEUCPPlCHG2sXrVx-fA-1; Tue, 30 May 2023 15:43:35 -0400 X-MC-Unique: MsdeEUCPPlCHG2sXrVx-fA-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 903E8800B2A; Tue, 30 May 2023 19:43:34 +0000 (UTC) Received: from file01.intranet.prod.int.rdu2.redhat.com (file01.intranet.prod.int.rdu2.redhat.com [10.11.5.7]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5346B2166B25; Tue, 30 May 2023 19:43:34 +0000 (UTC) Received: from file01.intranet.prod.int.rdu2.redhat.com (localhost [127.0.0.1]) by file01.intranet.prod.int.rdu2.redhat.com (8.14.4/8.14.4) with ESMTP id 34UJhY4b022142; Tue, 30 May 2023 15:43:34 -0400 Received: from localhost (mpatocka@localhost) by file01.intranet.prod.int.rdu2.redhat.com (8.14.4/8.14.4/Submit) with ESMTP id 34UJhXMb022117; Tue, 30 May 2023 15:43:33 -0400 X-Authentication-Warning: file01.intranet.prod.int.rdu2.redhat.com: mpatocka owned process doing -bs Date: Tue, 30 May 2023 15:43:33 -0400 (EDT) From: Mikulas Patocka X-X-Sender: mpatocka@file01.intranet.prod.int.rdu2.redhat.com To: Mike Snitzer cc: Johannes Thumshirn , "axboe @ kernel . dk" , shaggy@kernel.org, damien.lemoal@wdc.com, cluster-devel@redhat.com, kch@nvidia.com, agruenba@redhat.com, linux-mm@kvack.org, Damien Le Moal , jfs-discussion@lists.sourceforge.net, willy@infradead.org, ming.lei@redhat.com, linux-block@vger.kernel.org, song@kernel.org, dm-devel@redhat.com, rpeterso@redhat.com, linux-fsdevel@vger.kernel.org, linux-raid@vger.kernel.org, hch@lst.de Subject: Re: [PATCH v5 16/20] dm-crypt: check if adding pages to clone bio fails In-Reply-To: Message-ID: References: <20230502101934.24901-1-johannes.thumshirn@wdc.com> <20230502101934.24901-17-johannes.thumshirn@wdc.com> User-Agent: Alpine 2.21 (LRH 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Scanned-By: MIMEDefang 3.1 on 10.11.54.6 X-Rspamd-Queue-Id: 3FF4714001F X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: 5bq6nid7bycc4387dz1e8nn5ym1pn16h X-HE-Tag: 1685475818-216617 X-HE-Meta: U2FsdGVkX19TE+4/oJMYPHOATi29+kQRqrSPy5d0o+O/aXWzl1dENNnczgqN9aCntd27IYERmj0/zqMo5MOeCQ0IaWHlt11JbLEHGXKEh9anQglJ53Szk/HCUYHgJCJVn7mIrEPIIpbZWWHccgm/iEB42lvKSf50gP1fb1DwiWFOEDnoYQFWlN+tmbckSeijVuCs+K9yhc+JPRL9rPi7irLKX90lDMUoDUxroyKU1zePRwI6duv6w18wxakemYHaCo8u/QzFgvZI9yzWL+Rzx+fpuPriLml030ibw7cB7wmN8V1BfjoLcXuwjX/MCh6bWI3nSIXNzzr7xj4aLERnOsgphbF5meVinLbIlLpPp8JmOzxdo8gMaDBf+tFA4k9UmcZcQDc8PTSmyBmHPbl0o0EhUUs1jXAu+MrutX5rC+5SPEiYUQYavsQKqOYZEzKR1HYgwIHO57OALhUD8wNdIV8Dtk1wPI0RXQ6PPsjwNlvSxN6vPR75bknw99L76ISYCBZuMoZm1aQPWkP7SH4rZ7B2F3YjmaJYiTGM9Kt2UJoaSRu7FIJvnKlU80pPG0Zfi/vfC5Vbjz2aOAI4LtF/jHBK5OSnMvqzVTZ22UTvOyhrYutbFbVmWVD04KTtV8Pk2NXz/bxOzwSKt2jE9cnkV4kvDSb/Gd+GxBzZyFIpTWXNyzCZZSNULnX80Sb63lmA/e9uD+TuJkZgGGF+1f2Q3tOqaFMQi31bBKdtzeoRvJzzrHuYYZQtLAkRsqxSxYjcXJ1dgpiRz+pDpzLpq6MLa1uiil9e6PUxCGoE4cr59hRbCHdHKe0xyRoIZyc7FHUZZXR83lWv4AF1+PMskL5hP3NwsnqnEm48qhUfxwrevInLHwPwACQ5XBHIFOPPsQ69vbZQt2hZ7ONYgOUDlk6B9ZcKuZnAi76Q5ztG7bOwm/0uakHi8KDLQe5unTPw8TRzKyghO8vRkS90td5+JW6 Ps3E0rGD YsodoPHQN1nptrUWoORIgrkP0z0I3WZJlGWB6xmCMJG5G50r7oKp7T5HAObD7Xt+b6O0+BIlChejNLJrrMauedb6JwMQvfxBLr9xmYP3hzdrYs+DHwzRtJQ5lC2Bkya4yJDA5Fx8k4IIMvgwSc+J/kEmeIGmYDFunqVEAf60FNZfryWYB7lrwn/yaLe9uN+yF3OutiPuzDcEqYXlea/iiBwU4VCtHMpD4XrKLTEBH9kqJAE8= 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: On Tue, 30 May 2023, Mike Snitzer wrote: > On Tue, May 30 2023 at 11:13P -0400, > Mikulas Patocka wrote: > > > Hi > > > > I nack this. This just adds code that can't ever be executed. > > > > dm-crypt already allocates enough entries in the vector (see "unsigned int > > nr_iovecs = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;"), so bio_add_page can't > > fail. > > > > If you want to add __must_check to bio_add_page, you should change the > > dm-crypt code to: > > if (!bio_add_page(clone, page, len, 0)) { > > WARN(1, "this can't happen"); > > return NULL; > > } > > and not write recovery code for a can't-happen case. > > Thanks for the review Mikulas. But the proper way forward, in the > context of this patchset, is to simply change bio_add_page() to > __bio_add_page() > > Subject becomes: "dm crypt: use __bio_add_page to add single page to clone bio" > > And header can explain that "crypt_alloc_buffer() already allocates > enough entries in the clone bio's vector, so bio_add_page can't fail". > > Mike Yes, __bio_add_page would look nicer. But bio_add_page can merge adjacent pages into a single bvec entry and __bio_add_page can't (I don't know how often the merging happens or what is the performance implication of non-merging). I think that for the next merge window, we can apply this patch: https://listman.redhat.com/archives/dm-devel/2023-May/054046.html which makes this discussion irrelevant. (you can change bio_add_page to __bio_add_page in it) Mikukas