Hi Am 04.02.22 um 18:44 schrieb Lucas De Marchi:
Add a variant of shmem_read() that takes a iosys_map pointer rather than a plain pointer as argument. It's mostly a copy __shmem_rw() but adapting the api and removing the write support since there's currently only need to use iosys_map as destination. Reworking __shmem_rw() to share the implementation was tempting, but finding a good balance between reuse and clarity pushed towards a little code duplication. Since the function is small, just add the similar function with a copy/paste/adapt approach. Cc: Matt Roper <matthew.d.roper@xxxxxxxxx> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> Cc: David Airlie <airlied@xxxxxxxx> Cc: Daniel Vetter <daniel@xxxxxxxx> Cc: Matthew Auld <matthew.auld@xxxxxxxxx> Cc: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx> --- drivers/gpu/drm/i915/gt/shmem_utils.c | 33 +++++++++++++++++++++++++++ drivers/gpu/drm/i915/gt/shmem_utils.h | 3 +++ 2 files changed, 36 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/shmem_utils.c b/drivers/gpu/drm/i915/gt/shmem_utils.c index 0683b27a3890..764adefdb4be 100644 --- a/drivers/gpu/drm/i915/gt/shmem_utils.c +++ b/drivers/gpu/drm/i915/gt/shmem_utils.c @@ -3,6 +3,7 @@ * Copyright © 2020 Intel Corporation */+#include <linux/iosys-map.h>#include <linux/mm.h> #include <linux/pagemap.h> #include <linux/shmem_fs.h> @@ -123,6 +124,38 @@ static int __shmem_rw(struct file *file, loff_t off, return 0; }
Here's a good example of how to avoid iosys_map_incr() and use the memcpy offset:
+int shmem_read_to_iosys_map(struct file *file, loff_t off,
+ struct iosys_map *map, size_t len)
+{
+ struct iosys_map map_iter = *map;
Rather replace map_iter with something like unsigned long map_off = 0;
+ unsigned long pfn;
+
+ for (pfn = off >> PAGE_SHIFT; len; pfn++) {
+ 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);
+ iosys_map_memcpy_to(&map_iter, 0, vaddr + offset_in_page(off),
+ this);
Use map_off to index into map directly.
+ mark_page_accessed(page); + kunmap(page); + put_page(page); + + len -= this; + iosys_map_incr(&map_iter, this);
Raplace iosys_map_incr() with map_off += this;
+ off = 0;
Maybe off += this ?I think this pattern should be applied to all similar code. As you already noted, iosys_map_incr() is problematic.
Best regards Thomas
+ } + + return 0; +} + int shmem_read(struct file *file, loff_t off, void *dst, size_t len) { return __shmem_rw(file, off, dst, len, false); diff --git a/drivers/gpu/drm/i915/gt/shmem_utils.h b/drivers/gpu/drm/i915/gt/shmem_utils.h index c1669170c351..e1784999faee 100644 --- a/drivers/gpu/drm/i915/gt/shmem_utils.h +++ b/drivers/gpu/drm/i915/gt/shmem_utils.h @@ -8,6 +8,7 @@#include <linux/types.h> +struct iosys_map;struct drm_i915_gem_object; struct file;@@ -17,6 +18,8 @@ struct file *shmem_create_from_object(struct drm_i915_gem_object *obj);void *shmem_pin_map(struct file *file); void shmem_unpin_map(struct file *file, void *ptr);+int shmem_read_to_iosys_map(struct file *file, loff_t off,+ struct iosys_map *map, size_t len); int shmem_read(struct file *file, loff_t off, void *dst, size_t len); int shmem_write(struct file *file, loff_t off, void *src, size_t len);
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
