diff mbox series

[20/21] erofs: kill use_vmap module parameter

Message ID 20190901055130.30572-21-hsiangkao@aol.com (mailing list archive)
State New, archived
Headers show
Series erofs: patchset addressing Christoph's comments | expand

Commit Message

Gao Xiang Sept. 1, 2019, 5:51 a.m. UTC 
From: Gao Xiang <gaoxiang25@huawei.com>

As Christoph said [1],
"vm_map_ram is supposed to generally behave better.  So if
it doesn't please report that that to the arch maintainer
and linux-mm so that they can look into the issue.  Having
user make choices of deep down kernel internals is just
a horrible interface.

Please talk to maintainers of other bits of the kernel
if you see issues and / or need enhancements. "

Let's redo the previous conclusion and kill the vmap
approach.

[1] https://lore.kernel.org/r/20190830165533.GA10909@infradead.org/
Reported-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 Documentation/filesystems/erofs.txt |  4 ----
 fs/erofs/decompressor.c             | 12 +-----------
 2 files changed, 1 insertion(+), 15 deletions(-)

Comments

Christoph Hellwig Sept. 2, 2019, 12:31 p.m. UTC | #1 
> @@ -224,9 +220,6 @@ static void *erofs_vmap(struct page **pages, unsigned int count)
>  {
>  	int i = 0;
>  
> -	if (use_vmap)
> -		return vmap(pages, count, VM_MAP, PAGE_KERNEL);
> -
>  	while (1) {
>  		void *addr = vm_map_ram(pages, count, -1, PAGE_KERNEL);

I think you can just open code this in the caller.

>  static void erofs_vunmap(const void *mem, unsigned int count)
>  {
> -	if (!use_vmap)
> -		vm_unmap_ram(mem, count);
> -	else
> -		vunmap(mem);
> +	vm_unmap_ram(mem, count);
>  }

And this wrapper can go away entirely.

And don't forget to report your performance observations to the arm64
maintainers!
Gao Xiang Sept. 2, 2019, 12:43 p.m. UTC | #2 
Hi Christoph,

On Mon, Sep 02, 2019 at 05:31:24AM -0700, Christoph Hellwig wrote:
> > @@ -224,9 +220,6 @@ static void *erofs_vmap(struct page **pages, unsigned int count)
> >  {
> >  	int i = 0;
> >  
> > -	if (use_vmap)
> > -		return vmap(pages, count, VM_MAP, PAGE_KERNEL);
> > -
> >  	while (1) {
> >  		void *addr = vm_map_ram(pages, count, -1, PAGE_KERNEL);
> 
> I think you can just open code this in the caller.

Yes, the only one user... will fix...

> 
> >  static void erofs_vunmap(const void *mem, unsigned int count)
> >  {
> > -	if (!use_vmap)
> > -		vm_unmap_ram(mem, count);
> > -	else
> > -		vunmap(mem);
> > +	vm_unmap_ram(mem, count);
> >  }
> 
> And this wrapper can go away entirely.

Got it. will fix.

> 
> And don't forget to report your performance observations to the arm64
> maintainers!

In my observation, vm_map_ram always performs better...
If there are something strange later, I will report to them
immediately... :)

Thanks,
Gao Xiang
diff mbox series

Patch

diff --git a/Documentation/filesystems/erofs.txt b/Documentation/filesystems/erofs.txt
index c3b5f603b2b6..b0c085326e2e 100644
--- a/Documentation/filesystems/erofs.txt
+++ b/Documentation/filesystems/erofs.txt
@@ -67,10 +67,6 @@  cache_strategy=%s      Select a strategy for cached decompression from now on:
                                    It still does in-place I/O decompression
                                    for the rest compressed physical clusters.
 
-Module parameters
-=================
-use_vmap=[0|1]         Use vmap() instead of vm_map_ram() (default 0).
-
 On-disk details
 ===============
 
diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
index bb2944c96c89..af273d89e62c 100644
--- a/fs/erofs/decompressor.c
+++ b/fs/erofs/decompressor.c
@@ -28,10 +28,6 @@  struct z_erofs_decompressor {
 	char *name;
 };
 
-static bool use_vmap;
-module_param(use_vmap, bool, 0444);
-MODULE_PARM_DESC(use_vmap, "Use vmap() instead of vm_map_ram() (default 0)");
-
 static int z_erofs_lz4_prepare_destpages(struct z_erofs_decompress_req *rq,
 					 struct list_head *pagepool)
 {
@@ -224,9 +220,6 @@  static void *erofs_vmap(struct page **pages, unsigned int count)
 {
 	int i = 0;
 
-	if (use_vmap)
-		return vmap(pages, count, VM_MAP, PAGE_KERNEL);
-
 	while (1) {
 		void *addr = vm_map_ram(pages, count, -1, PAGE_KERNEL);
 
@@ -240,10 +233,7 @@  static void *erofs_vmap(struct page **pages, unsigned int count)
 
 static void erofs_vunmap(const void *mem, unsigned int count)
 {
-	if (!use_vmap)
-		vm_unmap_ram(mem, count);
-	else
-		vunmap(mem);
+	vm_unmap_ram(mem, count);
 }
 
 static int z_erofs_decompress_generic(struct z_erofs_decompress_req *rq,