| Message ID | 20201124060755.1405602-4-ira.weiny@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kmap: Create mem*_page interfaces | expand |
On Fri, Nov 27, 2020 at 03:01:56PM +0200, Joonas Lahtinen wrote: > + intel-gfx mailing list > > Quoting ira.weiny@intel.com (2020-11-24 08:07:41) > > From: Ira Weiny <ira.weiny@intel.com> > > > > The pattern of kmap/mem*/kunmap is repeated. Use the new mem*_page() > > calls instead. > > > > Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > > --- > > drivers/gpu/drm/gma500/gma_display.c | 7 +++---- > > drivers/gpu/drm/gma500/mmu.c | 4 ++-- > > drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 6 ++---- > > drivers/gpu/drm/i915/gt/intel_gtt.c | 9 ++------- > > drivers/gpu/drm/i915/gt/shmem_utils.c | 8 +++----- > > Are you looking to merge all these from the same tree, or first merge > the first patch and then trickle the rest through their own trees? I was thinking that they would go through Andrew's tree in bulk. But as I go through all the 'variants' including adding any kmap_atomic() variants it is getting to be a pretty big change. I'm trying to use Coccinelle but I'm not 100% confident in it working, more precisely in my skill to make it work. So I think I'm going to submit the base patch to Andrew today (with some cleanups per the comments in this thread). If Andrew could land that then I will can submit separate patches to each subsystem which would get full testing... :-( That is best. Thanks for making me think on this, Ira > Our last -next PR was already sent for i915, so I would queue this > only for 5.12. > > In any case, if you could split the i915 changes to a separate patch > (we have multiple sub-trees in drm), those are: > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > The gma500 changes also appear correct, so feel free to apply the > R-b for those, too. > > Regards, Joonas > > > 5 files changed, 12 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c > > index 3df6d6e850f5..f81114594211 100644 > > --- a/drivers/gpu/drm/gma500/gma_display.c > > +++ b/drivers/gpu/drm/gma500/gma_display.c > > @@ -9,6 +9,7 @@ > > > > #include <linux/delay.h> > > #include <linux/highmem.h> > > +#include <linux/pagemap.h> > > > > #include <drm/drm_crtc.h> > > #include <drm/drm_fourcc.h> > > @@ -334,7 +335,7 @@ int gma_crtc_cursor_set(struct drm_crtc *crtc, > > struct gtt_range *gt; > > struct gtt_range *cursor_gt = gma_crtc->cursor_gt; > > struct drm_gem_object *obj; > > - void *tmp_dst, *tmp_src; > > + void *tmp_dst; > > int ret = 0, i, cursor_pages; > > > > /* If we didn't get a handle then turn the cursor off */ > > @@ -400,9 +401,7 @@ int gma_crtc_cursor_set(struct drm_crtc *crtc, > > /* Copy the cursor to cursor mem */ > > tmp_dst = dev_priv->vram_addr + cursor_gt->offset; > > for (i = 0; i < cursor_pages; i++) { > > - tmp_src = kmap(gt->pages[i]); > > - memcpy(tmp_dst, tmp_src, PAGE_SIZE); > > - kunmap(gt->pages[i]); > > + memcpy_from_page(tmp_dst, gt->pages[i], 0, PAGE_SIZE); > > tmp_dst += PAGE_SIZE; > > } > > > > diff --git a/drivers/gpu/drm/gma500/mmu.c b/drivers/gpu/drm/gma500/mmu.c > > index 505044c9a673..8a0856c7f439 100644 > > --- a/drivers/gpu/drm/gma500/mmu.c > > +++ b/drivers/gpu/drm/gma500/mmu.c > > @@ -5,6 +5,7 @@ > > **************************************************************************/ > > > > #include <linux/highmem.h> > > +#include <linux/pagemap.h> > > > > #include "mmu.h" > > #include "psb_drv.h" > > @@ -204,8 +205,7 @@ struct psb_mmu_pd *psb_mmu_alloc_pd(struct psb_mmu_driver *driver, > > > > kunmap(pd->p); > > > > - clear_page(kmap(pd->dummy_page)); > > - kunmap(pd->dummy_page); > > + memzero_page(pd->dummy_page, 0, PAGE_SIZE); > > > > pd->tables = vmalloc_user(sizeof(struct psb_mmu_pt *) * 1024); > > if (!pd->tables) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > > index 75e8b71c18b9..8a25e08edd18 100644 > > --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > > @@ -558,7 +558,7 @@ i915_gem_object_create_shmem_from_data(struct drm_i915_private *dev_priv, > > do { > > unsigned int len = min_t(typeof(size), size, PAGE_SIZE); > > struct page *page; > > - void *pgdata, *vaddr; > > + void *pgdata; > > > > err = pagecache_write_begin(file, file->f_mapping, > > offset, len, 0, > > @@ -566,9 +566,7 @@ i915_gem_object_create_shmem_from_data(struct drm_i915_private *dev_priv, > > if (err < 0) > > goto fail; > > > > - vaddr = kmap(page); > > - memcpy(vaddr, data, len); > > - kunmap(page); > > + memcpy_to_page(page, 0, data, len); > > > > err = pagecache_write_end(file, file->f_mapping, > > offset, len, len, > > diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c > > index 3f1114b58b01..f3d7c601d362 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_gtt.c > > +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c > > @@ -153,13 +153,8 @@ static void poison_scratch_page(struct drm_i915_gem_object *scratch) > > if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)) > > val = POISON_FREE; > > > > - for_each_sgt_page(page, sgt, scratch->mm.pages) { > > - void *vaddr; > > - > > - vaddr = kmap(page); > > - memset(vaddr, val, PAGE_SIZE); > > - kunmap(page); > > - } > > + for_each_sgt_page(page, sgt, scratch->mm.pages) > > + memset_page(page, val, 0, PAGE_SIZE); > > } > > > > int setup_scratch_page(struct i915_address_space *vm) > > diff --git a/drivers/gpu/drm/i915/gt/shmem_utils.c b/drivers/gpu/drm/i915/gt/shmem_utils.c > > index f011ea42487e..2d5f1f2e803d 100644 > > --- a/drivers/gpu/drm/i915/gt/shmem_utils.c > > +++ b/drivers/gpu/drm/i915/gt/shmem_utils.c > > @@ -95,19 +95,17 @@ static int __shmem_rw(struct file *file, loff_t off, > > unsigned int this = > > min_t(size_t, PAGE_SIZE - offset_in_page(off), len); > > struct page *page; > > - void *vaddr; > > > > page = shmem_read_mapping_page_gfp(file->f_mapping, pfn, > > GFP_KERNEL); > > if (IS_ERR(page)) > > return PTR_ERR(page); > > > > - vaddr = kmap(page); > > if (write) > > - memcpy(vaddr + offset_in_page(off), ptr, this); > > + memcpy_to_page(page, offset_in_page(off), ptr, this); > > else > > - memcpy(ptr, vaddr + offset_in_page(off), this); > > - kunmap(page); > > + memcpy_from_page(ptr, page, offset_in_page(off), this); > > + > > put_page(page); > > > > len -= this; > > -- > > 2.28.0.rc0.12.gb6a658bd00c9 > >
On Fri, Dec 04 2020 at 08:05, Ira Weiny wrote: > So I think I'm going to submit the base patch to Andrew today (with some > cleanups per the comments in this thread). Could you please base that on tip core/mm where the kmap_local() muck is and use kmap_local() right away? Thanks, tglx
On Fri, Dec 04, 2020 at 11:33:08PM +0100, Thomas Gleixner wrote: > On Fri, Dec 04 2020 at 08:05, Ira Weiny wrote: > > So I think I'm going to submit the base patch to Andrew today (with some > > cleanups per the comments in this thread). > > Could you please base that on tip core/mm where the kmap_local() muck is > and use kmap_local() right away? Sure. Would that mean it should go through you and not Andrew? Ira > > Thanks, > > tglx
On Sun, Dec 06 2020 at 22:46, Ira Weiny wrote: > On Fri, Dec 04, 2020 at 11:33:08PM +0100, Thomas Gleixner wrote: >> On Fri, Dec 04 2020 at 08:05, Ira Weiny wrote: >> > So I think I'm going to submit the base patch to Andrew today (with some >> > cleanups per the comments in this thread). >> >> Could you please base that on tip core/mm where the kmap_local() muck is >> and use kmap_local() right away? > > Sure. Would that mean it should go through you and not Andrew? If Andrew has no objections of course. Thanks, tglx
diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c index 3df6d6e850f5..f81114594211 100644 --- a/drivers/gpu/drm/gma500/gma_display.c +++ b/drivers/gpu/drm/gma500/gma_display.c @@ -9,6 +9,7 @@ #include <linux/delay.h> #include <linux/highmem.h> +#include <linux/pagemap.h> #include <drm/drm_crtc.h> #include <drm/drm_fourcc.h> @@ -334,7 +335,7 @@ int gma_crtc_cursor_set(struct drm_crtc *crtc, struct gtt_range *gt; struct gtt_range *cursor_gt = gma_crtc->cursor_gt; struct drm_gem_object *obj; - void *tmp_dst, *tmp_src; + void *tmp_dst; int ret = 0, i, cursor_pages; /* If we didn't get a handle then turn the cursor off */ @@ -400,9 +401,7 @@ int gma_crtc_cursor_set(struct drm_crtc *crtc, /* Copy the cursor to cursor mem */ tmp_dst = dev_priv->vram_addr + cursor_gt->offset; for (i = 0; i < cursor_pages; i++) { - tmp_src = kmap(gt->pages[i]); - memcpy(tmp_dst, tmp_src, PAGE_SIZE); - kunmap(gt->pages[i]); + memcpy_from_page(tmp_dst, gt->pages[i], 0, PAGE_SIZE); tmp_dst += PAGE_SIZE; } diff --git a/drivers/gpu/drm/gma500/mmu.c b/drivers/gpu/drm/gma500/mmu.c index 505044c9a673..8a0856c7f439 100644 --- a/drivers/gpu/drm/gma500/mmu.c +++ b/drivers/gpu/drm/gma500/mmu.c @@ -5,6 +5,7 @@ **************************************************************************/ #include <linux/highmem.h> +#include <linux/pagemap.h> #include "mmu.h" #include "psb_drv.h" @@ -204,8 +205,7 @@ struct psb_mmu_pd *psb_mmu_alloc_pd(struct psb_mmu_driver *driver, kunmap(pd->p); - clear_page(kmap(pd->dummy_page)); - kunmap(pd->dummy_page); + memzero_page(pd->dummy_page, 0, PAGE_SIZE); pd->tables = vmalloc_user(sizeof(struct psb_mmu_pt *) * 1024); if (!pd->tables) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c index 75e8b71c18b9..8a25e08edd18 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c @@ -558,7 +558,7 @@ i915_gem_object_create_shmem_from_data(struct drm_i915_private *dev_priv, do { unsigned int len = min_t(typeof(size), size, PAGE_SIZE); struct page *page; - void *pgdata, *vaddr; + void *pgdata; err = pagecache_write_begin(file, file->f_mapping, offset, len, 0, @@ -566,9 +566,7 @@ i915_gem_object_create_shmem_from_data(struct drm_i915_private *dev_priv, if (err < 0) goto fail; - vaddr = kmap(page); - memcpy(vaddr, data, len); - kunmap(page); + memcpy_to_page(page, 0, data, len); err = pagecache_write_end(file, file->f_mapping, offset, len, len, diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c index 3f1114b58b01..f3d7c601d362 100644 --- a/drivers/gpu/drm/i915/gt/intel_gtt.c +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c @@ -153,13 +153,8 @@ static void poison_scratch_page(struct drm_i915_gem_object *scratch) if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)) val = POISON_FREE; - for_each_sgt_page(page, sgt, scratch->mm.pages) { - void *vaddr; - - vaddr = kmap(page); - memset(vaddr, val, PAGE_SIZE); - kunmap(page); - } + for_each_sgt_page(page, sgt, scratch->mm.pages) + memset_page(page, val, 0, PAGE_SIZE); } int setup_scratch_page(struct i915_address_space *vm) diff --git a/drivers/gpu/drm/i915/gt/shmem_utils.c b/drivers/gpu/drm/i915/gt/shmem_utils.c index f011ea42487e..2d5f1f2e803d 100644 --- a/drivers/gpu/drm/i915/gt/shmem_utils.c +++ b/drivers/gpu/drm/i915/gt/shmem_utils.c @@ -95,19 +95,17 @@ static int __shmem_rw(struct file *file, loff_t off, unsigned int this = min_t(size_t, PAGE_SIZE - offset_in_page(off), len); struct page *page; - void *vaddr; page = shmem_read_mapping_page_gfp(file->f_mapping, pfn, GFP_KERNEL); if (IS_ERR(page)) return PTR_ERR(page); - vaddr = kmap(page); if (write) - memcpy(vaddr + offset_in_page(off), ptr, this); + memcpy_to_page(page, offset_in_page(off), ptr, this); else - memcpy(ptr, vaddr + offset_in_page(off), this); - kunmap(page); + memcpy_from_page(ptr, page, offset_in_page(off), this); + put_page(page); len -= this;