| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <4C85926A-2FB5-418B-8F76-3055FA213D6C@dubeyko.com>
Date: Tue, 9 Aug 2022 15:18:37 -0700
From: Viacheslav Dubeyko <slava@...eyko.com>
To: "Fabio M. De Francesco" <fmdefrancesco@...il.com>
Cc: "Matthew Wilcox (Oracle)" <willy@...radead.org>,
Ira Weiny <ira.weiny@...el.com>, Jens Axboe <axboe@...nel.dk>,
Andrew Morton <akpm@...ux-foundation.org>,
Bart Van Assche <bvanassche@....org>,
Kees Cook <keescook@...omium.org>,
Muchun Song <songmuchun@...edance.com>,
Linux FS Devel <linux-fsdevel@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/4] hfsplus: Convert kmap() to kmap_local_page() in
btree.c
> On Aug 9, 2022, at 1:31 PM, Fabio M. De Francesco <fmdefrancesco@...il.com> wrote:
>
> kmap() is being deprecated in favor of kmap_local_page().
>
> There are two main problems with kmap(): (1) It comes with an overhead as
> mapping space is restricted and protected by a global lock for
> synchronization and (2) it also requires global TLB invalidation when the
> kmap’s pool wraps and it might block when the mapping space is fully
> utilized until a slot becomes available.
>
> With kmap_local_page() the mappings are per thread, CPU local, can take
> page faults, and can be called from any context (including interrupts).
> It is faster than kmap() in kernels with HIGHMEM enabled. Furthermore,
> the tasks can be preempted and, when they are scheduled to run again, the
> kernel virtual addresses are restored and are still valid.
>
> Since its use in btree.c is safe everywhere, it should be preferred.
>
> Therefore, replace kmap() with kmap_local_page() in btree.c.
>
> Tested in a QEMU/KVM x86_32 VM, 6GB RAM, booting a kernel with
> HIGHMEM64GB enabled.
>
> Cc: Viacheslav Dubeyko <slava@...eyko.com>
> Suggested-by: Ira Weiny <ira.weiny@...el.com>
> Reviewed-by: Ira Weiny <ira.weiny@...el.com>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@...il.com>
> ---
Looks good.
Reviewed-by: Viacheslav Dubeyko <slava@...eyko.com>
Thanks,
Slava.
> fs/hfsplus/btree.c | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
> index 3a917a9a4edd..9e1732a2b92a 100644
> --- a/fs/hfsplus/btree.c
> +++ b/fs/hfsplus/btree.c
> @@ -163,7 +163,7 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id)
> goto free_inode;
>
> /* Load the header */
> - head = (struct hfs_btree_header_rec *)(kmap(page) +
> + head = (struct hfs_btree_header_rec *)(kmap_local_page(page) +
> sizeof(struct hfs_bnode_desc));
> tree->root = be32_to_cpu(head->root);
> tree->leaf_count = be32_to_cpu(head->leaf_count);
> @@ -240,12 +240,12 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id)
> (tree->node_size + PAGE_SIZE - 1) >>
> PAGE_SHIFT;
>
> - kunmap(page);
> + kunmap_local(head);
> put_page(page);
> return tree;
>
> fail_page:
> - kunmap(page);
> + kunmap_local(head);
> put_page(page);
> free_inode:
> tree->inode->i_mapping->a_ops = &hfsplus_aops;
> @@ -292,7 +292,7 @@ int hfs_btree_write(struct hfs_btree *tree)
> return -EIO;
> /* Load the header */
> page = node->page[0];
> - head = (struct hfs_btree_header_rec *)(kmap(page) +
> + head = (struct hfs_btree_header_rec *)(kmap_local_page(page) +
> sizeof(struct hfs_bnode_desc));
>
> head->root = cpu_to_be32(tree->root);
> @@ -304,7 +304,7 @@ int hfs_btree_write(struct hfs_btree *tree)
> head->attributes = cpu_to_be32(tree->attributes);
> head->depth = cpu_to_be16(tree->depth);
>
> - kunmap(page);
> + kunmap_local(head);
> set_page_dirty(page);
> hfs_bnode_put(node);
> return 0;
> @@ -395,7 +395,7 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
>
> off += node->page_offset;
> pagep = node->page + (off >> PAGE_SHIFT);
> - data = kmap(*pagep);
> + data = kmap_local_page(*pagep);
> off &= ~PAGE_MASK;
> idx = 0;
>
> @@ -408,7 +408,7 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
> idx += i;
> data[off] |= m;
> set_page_dirty(*pagep);
> - kunmap(*pagep);
> + kunmap_local(data);
> tree->free_nodes--;
> mark_inode_dirty(tree->inode);
> hfs_bnode_put(node);
> @@ -418,14 +418,14 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
> }
> }
> if (++off >= PAGE_SIZE) {
> - kunmap(*pagep);
> - data = kmap(*++pagep);
> + kunmap_local(data);
> + data = kmap_local_page(*++pagep);
> off = 0;
> }
> idx += 8;
> len--;
> }
> - kunmap(*pagep);
> + kunmap_local(data);
> nidx = node->next;
> if (!nidx) {
> hfs_dbg(BNODE_MOD, "create new bmap node\n");
> @@ -441,7 +441,7 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
> off = off16;
> off += node->page_offset;
> pagep = node->page + (off >> PAGE_SHIFT);
> - data = kmap(*pagep);
> + data = kmap_local_page(*pagep);
> off &= ~PAGE_MASK;
> }
> }
> @@ -491,7 +491,7 @@ void hfs_bmap_free(struct hfs_bnode *node)
> }
> off += node->page_offset + nidx / 8;
> page = node->page[off >> PAGE_SHIFT];
> - data = kmap(page);
> + data = kmap_local_page(page);
> off &= ~PAGE_MASK;
> m = 1 << (~nidx & 7);
> byte = data[off];
> @@ -499,13 +499,13 @@ void hfs_bmap_free(struct hfs_bnode *node)
> pr_crit("trying to free free bnode "
> "%u(%d)\n",
> node->this, node->type);
> - kunmap(page);
> + kunmap_local(data);
> hfs_bnode_put(node);
> return;
> }
> data[off] = byte & ~m;
> set_page_dirty(page);
> - kunmap(page);
> + kunmap_local(data);
> hfs_bnode_put(node);
> tree->free_nodes++;
> mark_inode_dirty(tree->inode);
> --
> 2.37.1
>
Powered by blists - more mailing lists