Re: [PATCH v3 5/5] fbdev: Use pageref offset for deferred-I/O writeback

[Javier Martinez Canillas <javierm@redhat.com>]

On 4/26/22 14:03, Thomas Zimmermann wrote:
> Use pageref->offset instead of page->index for deferred-I/O writeback
> where appropriate. Distinguishes between file-mapping offset and video-
> memory offset. While at it, also remove unnecessary references to
> struct page.
> 
> Fbdev's deferred-I/O code uses the two related page->index and
> pageref->offset. The former is the page offset in the mapped file,
> the latter is the byte offset in the video memory (or fbdev screen
> buffer). It's the same value for fbdev drivers, but for DRM the values
> can be different. Because GEM buffer objects are mapped at an offset
> in the DRM device file, page->index has this offset added to it as well.
> We currently don't hit this case in DRM, because all affected mappings
> of GEM memory are performed with an internal, intermediate shadow buffer.
> 
> The value of page->index is required by page_mkclean(), which we
> call to reset the mappings during the writeback phase of the deferred
> I/O. The value of pageref->offset is for conveniently getting an offset
> into video memory in fb helpers.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Suggested-by: Javier Martinez Canillas <javierm@redhat.com>
> ---

Thanks for including this patch.

[snip]

>  	list_for_each_entry(pageref, pagereflist, list) {
> -		struct page *page = pageref->page;
> -		start = page->index << PAGE_SHIFT;
> +		start = pageref->offset;

It also has the side effect that makes the code easier to read and as you
said in the change description, eliminates the references to struct page.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat