* [PATCH 0/3] binder: Use kmap_local_page() in binder_alloc.c @ 2022-04-23 10:24 Fabio M. De Francesco 2022-04-23 10:24 ` [PATCH 1/3] binder: Use memset_page() in binder_alloc_clear_buf() Fabio M. De Francesco ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Fabio M. De Francesco @ 2022-04-23 10:24 UTC (permalink / raw) To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos, Martijn Coenen, Joel Fernandes, Christian Brauner, Hridya Valsaraju, Suren Baghdasaryan, Ira Weiny, linux-kernel Cc: Fabio M. De Francesco Use kmap_local_page() in binder_alloc.c because kmap() and kmap_atomic() are being deprecated and kmap_local_page is preferred where it is feasible. With kmap_local_page(), the mapping is per thread, CPU local and not globally visible. Furthermore, the mapping can be acquired from any context, including interrupts. Fabio M. De Francesco (3): binder: Use memset_page() in binder_alloc_clear_buf() binder: Use kmap_local_page() in binder_alloc_copy_user_to_buffer() binder: Use kmap_local_page() in binder_alloc_get_page() drivers/android/binder_alloc.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] binder: Use memset_page() in binder_alloc_clear_buf() 2022-04-23 10:24 [PATCH 0/3] binder: Use kmap_local_page() in binder_alloc.c Fabio M. De Francesco @ 2022-04-23 10:24 ` Fabio M. De Francesco 2022-04-23 15:43 ` Todd Kjos 2022-04-23 10:24 ` [PATCH 2/3] binder: Use kmap_local_page() in binder_alloc_copy_user_to_buffer() Fabio M. De Francesco 2022-04-23 10:24 ` [PATCH 3/3] binder: Use kmap_local_page() in binder_alloc_get_page() Fabio M. De Francesco 2 siblings, 1 reply; 11+ messages in thread From: Fabio M. De Francesco @ 2022-04-23 10:24 UTC (permalink / raw) To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos, Martijn Coenen, Joel Fernandes, Christian Brauner, Hridya Valsaraju, Suren Baghdasaryan, Ira Weiny, linux-kernel Cc: Fabio M. De Francesco The use of kmap() is being deprecated in favor of kmap_local_page() where it is feasible. With kmap_local_page(), the mapping is per thread, CPU local and not globally visible. binder_alloc_clear_buf() is a function where the use of kmap_local_page() in place of kmap() is correctly suited because the mapping is local to the thread. Therefore, use kmap_local_page() / kunmap_local() but, instead of open coding these two functions and adding a memset() of the virtual address of the mapping, prefer memset_page(). Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> --- drivers/android/binder_alloc.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c index 2ac1008a5f39..0b3f2f569053 100644 --- a/drivers/android/binder_alloc.c +++ b/drivers/android/binder_alloc.c @@ -1175,14 +1175,11 @@ static void binder_alloc_clear_buf(struct binder_alloc *alloc, unsigned long size; struct page *page; pgoff_t pgoff; - void *kptr; page = binder_alloc_get_page(alloc, buffer, buffer_offset, &pgoff); size = min_t(size_t, bytes, PAGE_SIZE - pgoff); - kptr = kmap(page) + pgoff; - memset(kptr, 0, size); - kunmap(page); + memset_page(page, pgoff, 0, size); bytes -= size; buffer_offset += size; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] binder: Use memset_page() in binder_alloc_clear_buf() 2022-04-23 10:24 ` [PATCH 1/3] binder: Use memset_page() in binder_alloc_clear_buf() Fabio M. De Francesco @ 2022-04-23 15:43 ` Todd Kjos 0 siblings, 0 replies; 11+ messages in thread From: Todd Kjos @ 2022-04-23 15:43 UTC (permalink / raw) To: Fabio M. De Francesco Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos, Martijn Coenen, Joel Fernandes, Christian Brauner, Hridya Valsaraju, Suren Baghdasaryan, Ira Weiny, linux-kernel On Sat, Apr 23, 2022 at 3:24 AM Fabio M. De Francesco <fmdefrancesco@gmail.com> wrote: > > The use of kmap() is being deprecated in favor of kmap_local_page() > where it is feasible. With kmap_local_page(), the mapping is per > thread, CPU local and not globally visible. > > binder_alloc_clear_buf() is a function where the use of kmap_local_page() > in place of kmap() is correctly suited because the mapping is local to the > thread. > > Therefore, use kmap_local_page() / kunmap_local() but, instead of open > coding these two functions and adding a memset() of the virtual address > of the mapping, prefer memset_page(). > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> Acked-by: Todd Kjos <tkjos@google.com> > --- > drivers/android/binder_alloc.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c > index 2ac1008a5f39..0b3f2f569053 100644 > --- a/drivers/android/binder_alloc.c > +++ b/drivers/android/binder_alloc.c > @@ -1175,14 +1175,11 @@ static void binder_alloc_clear_buf(struct binder_alloc *alloc, > unsigned long size; > struct page *page; > pgoff_t pgoff; > - void *kptr; > > page = binder_alloc_get_page(alloc, buffer, > buffer_offset, &pgoff); > size = min_t(size_t, bytes, PAGE_SIZE - pgoff); > - kptr = kmap(page) + pgoff; > - memset(kptr, 0, size); > - kunmap(page); > + memset_page(page, pgoff, 0, size); > bytes -= size; > buffer_offset += size; > } > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] binder: Use kmap_local_page() in binder_alloc_copy_user_to_buffer() 2022-04-23 10:24 [PATCH 0/3] binder: Use kmap_local_page() in binder_alloc.c Fabio M. De Francesco 2022-04-23 10:24 ` [PATCH 1/3] binder: Use memset_page() in binder_alloc_clear_buf() Fabio M. De Francesco @ 2022-04-23 10:24 ` Fabio M. De Francesco 2022-04-23 15:43 ` Todd Kjos 2022-04-23 10:24 ` [PATCH 3/3] binder: Use kmap_local_page() in binder_alloc_get_page() Fabio M. De Francesco 2 siblings, 1 reply; 11+ messages in thread From: Fabio M. De Francesco @ 2022-04-23 10:24 UTC (permalink / raw) To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos, Martijn Coenen, Joel Fernandes, Christian Brauner, Hridya Valsaraju, Suren Baghdasaryan, Ira Weiny, linux-kernel Cc: Fabio M. De Francesco The use of kmap() is being deprecated in favor of kmap_local_page() where it is feasible. With kmap_local_page(), the mapping is per thread, CPU local and not globally visible. binder_alloc_copy_user_to_buffer() is a function where the use of kmap_local_page() in place of kmap() is correctly suited because the mapping is local to the thread. Therefore, use kmap_local_page() / kunmap_local(). Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> --- drivers/android/binder_alloc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c index 0b3f2f569053..0875c463c002 100644 --- a/drivers/android/binder_alloc.c +++ b/drivers/android/binder_alloc.c @@ -1217,9 +1217,9 @@ binder_alloc_copy_user_to_buffer(struct binder_alloc *alloc, page = binder_alloc_get_page(alloc, buffer, buffer_offset, &pgoff); size = min_t(size_t, bytes, PAGE_SIZE - pgoff); - kptr = kmap(page) + pgoff; + kptr = kmap_local_page(page) + pgoff; ret = copy_from_user(kptr, from, size); - kunmap(page); + kunmap_local(kptr); if (ret) return bytes - size + ret; bytes -= size; -- 2.34.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] binder: Use kmap_local_page() in binder_alloc_copy_user_to_buffer() 2022-04-23 10:24 ` [PATCH 2/3] binder: Use kmap_local_page() in binder_alloc_copy_user_to_buffer() Fabio M. De Francesco @ 2022-04-23 15:43 ` Todd Kjos 0 siblings, 0 replies; 11+ messages in thread From: Todd Kjos @ 2022-04-23 15:43 UTC (permalink / raw) To: Fabio M. De Francesco Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos, Martijn Coenen, Joel Fernandes, Christian Brauner, Hridya Valsaraju, Suren Baghdasaryan, Ira Weiny, linux-kernel On Sat, Apr 23, 2022 at 3:24 AM Fabio M. De Francesco <fmdefrancesco@gmail.com> wrote: > > The use of kmap() is being deprecated in favor of kmap_local_page() > where it is feasible. With kmap_local_page(), the mapping is per > thread, CPU local and not globally visible. > > binder_alloc_copy_user_to_buffer() is a function where the use of > kmap_local_page() in place of kmap() is correctly suited because > the mapping is local to the thread. > > Therefore, use kmap_local_page() / kunmap_local(). > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> Acked-by: Todd Kjos <tkjos@google.com> > --- > drivers/android/binder_alloc.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c > index 0b3f2f569053..0875c463c002 100644 > --- a/drivers/android/binder_alloc.c > +++ b/drivers/android/binder_alloc.c > @@ -1217,9 +1217,9 @@ binder_alloc_copy_user_to_buffer(struct binder_alloc *alloc, > page = binder_alloc_get_page(alloc, buffer, > buffer_offset, &pgoff); > size = min_t(size_t, bytes, PAGE_SIZE - pgoff); > - kptr = kmap(page) + pgoff; > + kptr = kmap_local_page(page) + pgoff; > ret = copy_from_user(kptr, from, size); > - kunmap(page); > + kunmap_local(kptr); > if (ret) > return bytes - size + ret; > bytes -= size; > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] binder: Use kmap_local_page() in binder_alloc_get_page() 2022-04-23 10:24 [PATCH 0/3] binder: Use kmap_local_page() in binder_alloc.c Fabio M. De Francesco 2022-04-23 10:24 ` [PATCH 1/3] binder: Use memset_page() in binder_alloc_clear_buf() Fabio M. De Francesco 2022-04-23 10:24 ` [PATCH 2/3] binder: Use kmap_local_page() in binder_alloc_copy_user_to_buffer() Fabio M. De Francesco @ 2022-04-23 10:24 ` Fabio M. De Francesco 2022-04-23 15:44 ` Todd Kjos 2022-04-23 16:02 ` Christophe JAILLET 2 siblings, 2 replies; 11+ messages in thread From: Fabio M. De Francesco @ 2022-04-23 10:24 UTC (permalink / raw) To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos, Martijn Coenen, Joel Fernandes, Christian Brauner, Hridya Valsaraju, Suren Baghdasaryan, Ira Weiny, linux-kernel Cc: Fabio M. De Francesco The use of kmap_atomic() is being deprecated in favor of kmap_local_page() where it is feasible. Each call of kmap_atomic() in the kernel creates a non-preemptible section and disable pagefaults. This could be a source of unwanted latency, so it should be only used if it is absolutely required, otherwise kmap_local_page() should be preferred. With kmap_local_page(), the mapping is per thread, CPU local and not globally visible. Furthermore, the mapping can be acquired from any context (including interrupts). Therefore, use kmap_local_page() / kunmap_local() in place of kmap_atomic() / kunmap_atomic(). Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> --- drivers/android/binder_alloc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c index 0875c463c002..058595cc7ff0 100644 --- a/drivers/android/binder_alloc.c +++ b/drivers/android/binder_alloc.c @@ -1250,17 +1250,17 @@ static int binder_alloc_do_buffer_copy(struct binder_alloc *alloc, page = binder_alloc_get_page(alloc, buffer, buffer_offset, &pgoff); size = min_t(size_t, bytes, PAGE_SIZE - pgoff); - base_ptr = kmap_atomic(page); + base_ptr = kmap_local_page(page); tmpptr = base_ptr + pgoff; if (to_buffer) memcpy(tmpptr, ptr, size); else memcpy(ptr, tmpptr, size); /* - * kunmap_atomic() takes care of flushing the cache + * kunmap_local() takes care of flushing the cache * if this device has VIVT cache arch */ - kunmap_atomic(base_ptr); + kunmap_local(base_ptr); bytes -= size; pgoff = 0; ptr = ptr + size; -- 2.34.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] binder: Use kmap_local_page() in binder_alloc_get_page() 2022-04-23 10:24 ` [PATCH 3/3] binder: Use kmap_local_page() in binder_alloc_get_page() Fabio M. De Francesco @ 2022-04-23 15:44 ` Todd Kjos 2022-04-23 16:02 ` Christophe JAILLET 1 sibling, 0 replies; 11+ messages in thread From: Todd Kjos @ 2022-04-23 15:44 UTC (permalink / raw) To: Fabio M. De Francesco Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos, Martijn Coenen, Joel Fernandes, Christian Brauner, Hridya Valsaraju, Suren Baghdasaryan, Ira Weiny, linux-kernel On Sat, Apr 23, 2022 at 3:24 AM Fabio M. De Francesco <fmdefrancesco@gmail.com> wrote: > > The use of kmap_atomic() is being deprecated in favor of kmap_local_page() > where it is feasible. Each call of kmap_atomic() in the kernel creates > a non-preemptible section and disable pagefaults. This could be a source > of unwanted latency, so it should be only used if it is absolutely > required, otherwise kmap_local_page() should be preferred. > > With kmap_local_page(), the mapping is per thread, CPU local and not > globally visible. Furthermore, the mapping can be acquired from any context > (including interrupts). > > Therefore, use kmap_local_page() / kunmap_local() in place of > kmap_atomic() / kunmap_atomic(). > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> Acked-by: Todd Kjos <tkjos@google.com> > --- > drivers/android/binder_alloc.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c > index 0875c463c002..058595cc7ff0 100644 > --- a/drivers/android/binder_alloc.c > +++ b/drivers/android/binder_alloc.c > @@ -1250,17 +1250,17 @@ static int binder_alloc_do_buffer_copy(struct binder_alloc *alloc, > page = binder_alloc_get_page(alloc, buffer, > buffer_offset, &pgoff); > size = min_t(size_t, bytes, PAGE_SIZE - pgoff); > - base_ptr = kmap_atomic(page); > + base_ptr = kmap_local_page(page); > tmpptr = base_ptr + pgoff; > if (to_buffer) > memcpy(tmpptr, ptr, size); > else > memcpy(ptr, tmpptr, size); > /* > - * kunmap_atomic() takes care of flushing the cache > + * kunmap_local() takes care of flushing the cache > * if this device has VIVT cache arch > */ > - kunmap_atomic(base_ptr); > + kunmap_local(base_ptr); > bytes -= size; > pgoff = 0; > ptr = ptr + size; > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] binder: Use kmap_local_page() in binder_alloc_get_page() @ 2022-04-23 16:02 ` Christophe JAILLET 0 siblings, 0 replies; 11+ messages in thread From: Christophe JAILLET @ 2022-04-23 16:02 UTC (permalink / raw) To: Fabio M. De Francesco, Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos, Martijn Coenen, Joel Fernandes, Christian Brauner, Hridya Valsaraju, Suren Baghdasaryan, Ira Weiny, linux-kernel Hi, Le 23/04/2022 à 12:24, Fabio M. De Francesco a écrit : > The use of kmap_atomic() is being deprecated in favor of kmap_local_page() > where it is feasible. Each call of kmap_atomic() in the kernel creates > a non-preemptible section and disable pagefaults. This could be a source > of unwanted latency, so it should be only used if it is absolutely > required, otherwise kmap_local_page() should be preferred. > > With kmap_local_page(), the mapping is per thread, CPU local and not > globally visible. Furthermore, the mapping can be acquired from any context > (including interrupts). > > Therefore, use kmap_local_page() / kunmap_local() in place of > kmap_atomic() / kunmap_atomic(). > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> > --- > drivers/android/binder_alloc.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c > index 0875c463c002..058595cc7ff0 100644 > --- a/drivers/android/binder_alloc.c > +++ b/drivers/android/binder_alloc.c > @@ -1250,17 +1250,17 @@ static int binder_alloc_do_buffer_copy(struct binder_alloc *alloc, > page = binder_alloc_get_page(alloc, buffer, > buffer_offset, &pgoff); > size = min_t(size_t, bytes, PAGE_SIZE - pgoff); > - base_ptr = kmap_atomic(page); > + base_ptr = kmap_local_page(page); > tmpptr = base_ptr + pgoff; > if (to_buffer) > memcpy(tmpptr, ptr, size); > else > memcpy(ptr, tmpptr, size); in the same spirit as patch 1/3, memcpy_to_page() and memcpy_from_page() looks good candidate to avoid code duplication. Not checked in details, but looks mostly the same. Just my 2c. CJ > /* > - * kunmap_atomic() takes care of flushing the cache > + * kunmap_local() takes care of flushing the cache > * if this device has VIVT cache arch > */ > - kunmap_atomic(base_ptr); > + kunmap_local(base_ptr); > bytes -= size; > pgoff = 0; > ptr = ptr + size; ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] binder: Use kmap_local_page() in binder_alloc_get_page() @ 2022-04-23 16:02 ` Christophe JAILLET 0 siblings, 0 replies; 11+ messages in thread From: Christophe JAILLET @ 2022-04-23 16:02 UTC (permalink / raw) To: linux-kernel Hi, Le 23/04/2022 à 12:24, Fabio M. De Francesco a écrit : > The use of kmap_atomic() is being deprecated in favor of kmap_local_page() > where it is feasible. Each call of kmap_atomic() in the kernel creates > a non-preemptible section and disable pagefaults. This could be a source > of unwanted latency, so it should be only used if it is absolutely > required, otherwise kmap_local_page() should be preferred. > > With kmap_local_page(), the mapping is per thread, CPU local and not > globally visible. Furthermore, the mapping can be acquired from any context > (including interrupts). > > Therefore, use kmap_local_page() / kunmap_local() in place of > kmap_atomic() / kunmap_atomic(). > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> > --- > drivers/android/binder_alloc.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c > index 0875c463c002..058595cc7ff0 100644 > --- a/drivers/android/binder_alloc.c > +++ b/drivers/android/binder_alloc.c > @@ -1250,17 +1250,17 @@ static int binder_alloc_do_buffer_copy(struct binder_alloc *alloc, > page = binder_alloc_get_page(alloc, buffer, > buffer_offset, &pgoff); > size = min_t(size_t, bytes, PAGE_SIZE - pgoff); > - base_ptr = kmap_atomic(page); > + base_ptr = kmap_local_page(page); > tmpptr = base_ptr + pgoff; > if (to_buffer) > memcpy(tmpptr, ptr, size); > else > memcpy(ptr, tmpptr, size); in the same spirit as patch 1/3, memcpy_to_page() and memcpy_from_page() looks good candidate to avoid code duplication. Not checked in details, but looks mostly the same. Just my 2c. CJ > /* > - * kunmap_atomic() takes care of flushing the cache > + * kunmap_local() takes care of flushing the cache > * if this device has VIVT cache arch > */ > - kunmap_atomic(base_ptr); > + kunmap_local(base_ptr); > bytes -= size; > pgoff = 0; > ptr = ptr + size; ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] binder: Use kmap_local_page() in binder_alloc_get_page() 2022-04-23 16:02 ` Christophe JAILLET (?) @ 2022-04-24 9:39 ` Fabio M. De Francesco 2022-04-25 15:52 ` Todd Kjos -1 siblings, 1 reply; 11+ messages in thread From: Fabio M. De Francesco @ 2022-04-24 9:39 UTC (permalink / raw) To: Todd Kjos, linux-kernel, Christophe JAILLET Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Martijn Coenen, Joel Fernandes, Christian Brauner, Hridya Valsaraju, Suren Baghdasaryan, Ira Weiny On sabato 23 aprile 2022 18:02:48 CEST Christophe JAILLET wrote: > Hi, > > Le 23/04/2022 à 12:24, Fabio M. De Francesco a écrit : > > The use of kmap_atomic() is being deprecated in favor of kmap_local_page() > > where it is feasible. Each call of kmap_atomic() in the kernel creates > > a non-preemptible section and disable pagefaults. This could be a source > > of unwanted latency, so it should be only used if it is absolutely > > required, otherwise kmap_local_page() should be preferred. > > > > With kmap_local_page(), the mapping is per thread, CPU local and not > > globally visible. Furthermore, the mapping can be acquired from any context > > (including interrupts). > > > > Therefore, use kmap_local_page() / kunmap_local() in place of > > kmap_atomic() / kunmap_atomic(). > > > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> > > --- > > drivers/android/binder_alloc.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/android/binder_alloc.c b/drivers/android/ binder_alloc.c > > index 0875c463c002..058595cc7ff0 100644 > > --- a/drivers/android/binder_alloc.c > > +++ b/drivers/android/binder_alloc.c > > @@ -1250,17 +1250,17 @@ static int binder_alloc_do_buffer_copy(struct binder_alloc *alloc, > > page = binder_alloc_get_page(alloc, buffer, > > buffer_offset, &pgoff); > > size = min_t(size_t, bytes, PAGE_SIZE - pgoff); > > - base_ptr = kmap_atomic(page); > > + base_ptr = kmap_local_page(page); > > tmpptr = base_ptr + pgoff; > > if (to_buffer) > > memcpy(tmpptr, ptr, size); > > else > > memcpy(ptr, tmpptr, size); > > in the same spirit as patch 1/3, memcpy_to_page() and memcpy_from_page() > looks good candidate to avoid code duplication. Hello Christophe, Todd, I had thought to use memcpy_to_page() and memcpy_from_page() (exactly as I did in other conversions I have been working on during the latest couple of weeks). However, I decided to avoid to use them for I should also have deleted the comment which is before "kunmap_local(base_ptr);". I don't know how much Maintainers think it is necessary to make readers notice that "kunmap_local() takes care of flushing the cache []" (exactly as kunmap_atomic() does). Actually I'd delete that comment that looks redundant and unnecessary to me, but I cannot know if Todd wants it to remain there. @Todd: Can you please say what you think about this topic? Should I leave the patch as-is or I should use memcpy_{to,from}_page() and delete that comment? I won't send any v2 unless I have your confirmation. Thanks, Fabio > > Not checked in details, but looks mostly the same. > > Just my 2c. > > CJ > > > /* > > - * kunmap_atomic() takes care of flushing the cache > > + * kunmap_local() takes care of flushing the cache > > * if this device has VIVT cache arch > > */ > > - kunmap_atomic(base_ptr); > > + kunmap_local(base_ptr); > > bytes -= size; > > pgoff = 0; > > ptr = ptr + size; > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] binder: Use kmap_local_page() in binder_alloc_get_page() 2022-04-24 9:39 ` Fabio M. De Francesco @ 2022-04-25 15:52 ` Todd Kjos 0 siblings, 0 replies; 11+ messages in thread From: Todd Kjos @ 2022-04-25 15:52 UTC (permalink / raw) To: Fabio M. De Francesco Cc: Todd Kjos, linux-kernel, Christophe JAILLET, Greg Kroah-Hartman, Arve Hjønnevåg, Martijn Coenen, Joel Fernandes, Christian Brauner, Hridya Valsaraju, Suren Baghdasaryan, Ira Weiny On Sun, Apr 24, 2022 at 2:40 AM Fabio M. De Francesco <fmdefrancesco@gmail.com> wrote: > > On sabato 23 aprile 2022 18:02:48 CEST Christophe JAILLET wrote: > > Hi, > > > > Le 23/04/2022 à 12:24, Fabio M. De Francesco a écrit : > > > The use of kmap_atomic() is being deprecated in favor of > kmap_local_page() > > > where it is feasible. Each call of kmap_atomic() in the kernel creates > > > a non-preemptible section and disable pagefaults. This could be a > source > > > of unwanted latency, so it should be only used if it is absolutely > > > required, otherwise kmap_local_page() should be preferred. > > > > > > With kmap_local_page(), the mapping is per thread, CPU local and not > > > globally visible. Furthermore, the mapping can be acquired from any > context > > > (including interrupts). > > > > > > Therefore, use kmap_local_page() / kunmap_local() in place of > > > kmap_atomic() / kunmap_atomic(). > > > > > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> > > > --- > > > drivers/android/binder_alloc.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/android/binder_alloc.c b/drivers/android/ > binder_alloc.c > > > index 0875c463c002..058595cc7ff0 100644 > > > --- a/drivers/android/binder_alloc.c > > > +++ b/drivers/android/binder_alloc.c > > > @@ -1250,17 +1250,17 @@ static int binder_alloc_do_buffer_copy(struct > binder_alloc *alloc, > > > page = binder_alloc_get_page(alloc, buffer, > > > buffer_offset, > &pgoff); > > > size = min_t(size_t, bytes, PAGE_SIZE - pgoff); > > > - base_ptr = kmap_atomic(page); > > > + base_ptr = kmap_local_page(page); > > > tmpptr = base_ptr + pgoff; > > > if (to_buffer) > > > memcpy(tmpptr, ptr, size); > > > else > > > memcpy(ptr, tmpptr, size); > > > > in the same spirit as patch 1/3, memcpy_to_page() and memcpy_from_page() > > looks good candidate to avoid code duplication. > > Hello Christophe, Todd, > > I had thought to use memcpy_to_page() and memcpy_from_page() (exactly as I > did in other conversions I have been working on during the latest couple of > weeks). > > However, I decided to avoid to use them for I should also have deleted the > comment which is before "kunmap_local(base_ptr);". > > I don't know how much Maintainers think it is necessary to make readers > notice that "kunmap_local() takes care of flushing the cache []" (exactly > as kunmap_atomic() does). Actually I'd delete that comment that looks > redundant and unnecessary to me, but I cannot know if Todd wants it to > remain there. > > @Todd: Can you please say what you think about this topic? Should I leave > the patch as-is or I should use memcpy_{to,from}_page() and delete that > comment? I'm fine with using memcpy_{to,from}_page() and removing the comment. > > I won't send any v2 unless I have your confirmation. > > Thanks, > > Fabio > > > > > Not checked in details, but looks mostly the same. > > > > Just my 2c. > > > > CJ > > > > > /* > > > - * kunmap_atomic() takes care of flushing the cache > > > + * kunmap_local() takes care of flushing the cache > > > * if this device has VIVT cache arch > > > */ > > > - kunmap_atomic(base_ptr); > > > + kunmap_local(base_ptr); > > > bytes -= size; > > > pgoff = 0; > > > ptr = ptr + size; > > > > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-04-25 15:52 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-23 10:24 [PATCH 0/3] binder: Use kmap_local_page() in binder_alloc.c Fabio M. De Francesco 2022-04-23 10:24 ` [PATCH 1/3] binder: Use memset_page() in binder_alloc_clear_buf() Fabio M. De Francesco 2022-04-23 15:43 ` Todd Kjos 2022-04-23 10:24 ` [PATCH 2/3] binder: Use kmap_local_page() in binder_alloc_copy_user_to_buffer() Fabio M. De Francesco 2022-04-23 15:43 ` Todd Kjos 2022-04-23 10:24 ` [PATCH 3/3] binder: Use kmap_local_page() in binder_alloc_get_page() Fabio M. De Francesco 2022-04-23 15:44 ` Todd Kjos 2022-04-23 16:02 ` Christophe JAILLET 2022-04-23 16:02 ` Christophe JAILLET 2022-04-24 9:39 ` Fabio M. De Francesco 2022-04-25 15:52 ` Todd Kjos
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.