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 X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_2 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5A7E9C0044D for ; Wed, 4 Dec 2019 07:53:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 2DC672073B for ; Wed, 4 Dec 2019 07:53:56 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=dubeyko-com.20150623.gappssmtp.com header.i=@dubeyko-com.20150623.gappssmtp.com header.b="R15GZEj9" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727101AbfLDHxz (ORCPT ); Wed, 4 Dec 2019 02:53:55 -0500 Received: from mail-lj1-f194.google.com ([209.85.208.194]:37347 "EHLO mail-lj1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726166AbfLDHxy (ORCPT ); Wed, 4 Dec 2019 02:53:54 -0500 Received: by mail-lj1-f194.google.com with SMTP id u17so6949109lja.4 for ; Tue, 03 Dec 2019 23:53:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dubeyko-com.20150623.gappssmtp.com; s=20150623; h=message-id:subject:from:to:cc:date:in-reply-to:references :mime-version:content-transfer-encoding; bh=o7BdwW/nndIBcsxZWW8vbo3uXBPbU7tTjh33V04X4ek=; b=R15GZEj9KTLTNO0VqqzBo4GIYJtcpTb2kJpdaZtblji+D+RrmcI9rTjDxe+QmhairT Dbdver0Sq7o39O5yoG4UDoGmJmo9vH5HMuLXGO7h9I6dmFkSgcGzoSd18zm/LtsqK2mF lUVjO8y7iCVF2gvIFJREkER6zjHAaENjaCcLtLpziuR4Pq2w3Z8Wlhe3XDpq/lUxhE9O T4/3msAlwIt0YUvQb44lViYxzbj8rFEkqw+r1HWH3ILYKZPpWy2mnzcedUuXScZGEePd QqT5lFUQEHQIvzSg5F3M5PVgRTSf84jSg366orX8ATHSyfkMZ7VBL4bHLLLgCBkyqZGQ zDxQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:mime-version:content-transfer-encoding; bh=o7BdwW/nndIBcsxZWW8vbo3uXBPbU7tTjh33V04X4ek=; b=Rn9xCCv/+r7LzotzlhqdIRJxuXzjmmq5ZGt80ueZsSllht3UeNTcFvIGD1NS/6kXZN neUwvSyoICiE9jp0xJaWs6cX2S9GWQKqzXQbM9uZyIyX8klr00nd7IWxp52gwo2dwO5k 3BWDHC4UBs1p0Y6l2bG4dIyAFbINQ7XRkPNiqU73lIoKlvprRbLvDI8zA0uYfmlP6yVb 9DC0sphdeF/K0NCGNV7v+RIvKc6g0Mb3DcNINwGcS4Kt5ZOZ4k46mcDKpJaduCfyB2bo UzXl7mmo6hXqvhMHQiS40L3pz1zQflX8dAYm/2iVDjOsjr6uKjO3hdGDaN7HoKvqRgLO Bi6A== X-Gm-Message-State: APjAAAXle9EaptTiaISxEAEFHUAimMS484UUi4jp+fbV6Vd1Y8AlVICA MfCxc0Ojw0h/se0C9HpypwtH/Q== X-Google-Smtp-Source: APXvYqw9cr0AJeOMuYf9c3iwqsmg8RWLKAie3QTXWVS6sDwM7krFflTW5KVptECDL2Qepv39q9RcUg== X-Received: by 2002:a2e:9e97:: with SMTP id f23mr1068434ljk.89.1575446031396; Tue, 03 Dec 2019 23:53:51 -0800 (PST) Received: from msk1wst115n.omp.ru (mail.omprussia.ru. [5.134.221.218]) by smtp.gmail.com with ESMTPSA id x23sm2807809lff.24.2019.12.03.23.53.50 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 03 Dec 2019 23:53:50 -0800 (PST) Message-ID: <96a288281d9d84f11dcc06e62a1ff20e2bb2f776.camel@dubeyko.com> Subject: Re: [PATCH] fs-verity: implement readahead for FS_IOC_ENABLE_VERITY From: Vyacheslav Dubeyko To: Eric Biggers , linux-fscrypt@vger.kernel.org Cc: linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-fsdevel@vger.kernel.org, Victor Hsieh Date: Wed, 04 Dec 2019 10:53:50 +0300 In-Reply-To: <20191203193001.66906-1-ebiggers@kernel.org> References: <20191203193001.66906-1-ebiggers@kernel.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5-0ubuntu0.18.04.1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org On Tue, 2019-12-03 at 11:30 -0800, Eric Biggers wrote: > From: Eric Biggers > > When it builds the first level of the Merkle tree, > FS_IOC_ENABLE_VERITY > sequentially reads each page of the file using read_mapping_page(). > This works fine if the file's data is already in pagecache, which > should > normally be the case, since this ioctl is normally used immediately > after writing out the file. > > But in any other case this implementation performs very poorly, since > only one page is read at a time. > > Fix this by implementing readahead using the functions from > mm/readahead.c. > > This improves performance in the uncached case by about 20x, as seen > in > the following benchmarks done on a 250MB file (on x86_64 with SHA- > NI): > > FS_IOC_ENABLE_VERITY uncached (before) 3.299s > FS_IOC_ENABLE_VERITY uncached (after) 0.160s > FS_IOC_ENABLE_VERITY cached 0.147s > sha256sum uncached 0.191s > sha256sum cached 0.145s > > Note: we could instead switch to kernel_read(). But that would mean > we'd no longer be hashing the data directly from the pagecache, which > is > a nice optimization of its own. And using kernel_read() would > require > allocating another temporary buffer, hashing the data and tree pages > separately, and explicitly zero-padding the last page -- so it > wouldn't > really be any simpler than direct pagecache access, at least for now. > > Signed-off-by: Eric Biggers > --- > fs/verity/enable.c | 46 ++++++++++++++++++++++++++++++++++++++++-- > ---- > 1 file changed, 40 insertions(+), 6 deletions(-) > > diff --git a/fs/verity/enable.c b/fs/verity/enable.c > index eabc6ac19906..f7eaffa60196 100644 > --- a/fs/verity/enable.c > +++ b/fs/verity/enable.c > @@ -13,14 +13,44 @@ > #include > #include > > -static int build_merkle_tree_level(struct inode *inode, unsigned int > level, > +/* > + * Read a file data page for Merkle tree construction. Do > aggressive readahead, > + * since we're sequentially reading the entire file. > + */ > +static struct page *read_file_data_page(struct inode *inode, > + struct file_ra_state *ra, > + struct file *filp, > + pgoff_t index, > + pgoff_t num_pages_in_file) > +{ > + struct page *page; > + > + page = find_get_page(inode->i_mapping, index); > + if (!page || !PageUptodate(page)) { > + if (page) > + put_page(page); It looks like that there is not necessary check here. If we have NULL pointer on page then we will not enter inside. But if we have valid pointer on page then we have double check inside. Am I correct? > + page_cache_sync_readahead(inode->i_mapping, ra, filp, > + index, num_pages_in_file - > index); > + page = read_mapping_page(inode->i_mapping, index, > NULL); > + if (IS_ERR(page)) > + return page; Could we recieve the NULL pointer here? Is callee ready to process theNULL return value? Thanks, Viacheslav Dubeyko.