On 5/2/25 15:00, Thomas Hellström wrote:
> The abstraction was previously added to support separate
> ttm_backup implementations.
>
> However with the current implementation casting from a
> struct file to a struct ttm_backup, we run into trouble since
> struct file may have randomized the layout and gcc complains.
>
> Remove the struct ttm_backup abstraction
>
> Cc: dri-devel@lists.freedesktop.org
> Cc: Matthew Brost <matthew.br...@intel.com>
> Cc: Dave Airlie <airl...@redhat.com>
> Cc: Christian König <christian.koe...@amd.com>
> Cc: Matthew Auld <matthew.a...@intel.com>
> Cc: Al Viro <v...@zeniv.linux.org.uk>
> Reported-by: Kees Cook <k...@kernel.org>
> Closes:
> https://lore.kernel.org/dri-devel/9c8dbbafdaf9f3f089da2cde5a772d69579b3795.ca...@linux.intel.com/T/#mb153ab9216cb813b92bdeb36f391ad4808c2ba29
> Suggested-by: Christian König <christian.koe...@amd.com>
> Fixes: 70d645deac98 ("drm/ttm: Add helpers for shrinking")
> Signed-off-by: Thomas Hellström <thomas.hellst...@linux.intel.com>
Reviewed-by: Christian König <christian.koe...@amd.com>
> ---
> drivers/gpu/drm/ttm/ttm_backup.c | 42 +++++++++-----------------------
> drivers/gpu/drm/ttm/ttm_pool.c | 6 ++---
> drivers/gpu/drm/ttm/ttm_tt.c | 2 +-
> include/drm/ttm/ttm_backup.h | 12 ++++-----
> include/drm/ttm/ttm_tt.h | 2 +-
> 5 files changed, 21 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_backup.c
> b/drivers/gpu/drm/ttm/ttm_backup.c
> index 93c007f18855..9b3b5624c3eb 100644
> --- a/drivers/gpu/drm/ttm/ttm_backup.c
> +++ b/drivers/gpu/drm/ttm/ttm_backup.c
> @@ -7,20 +7,6 @@
> #include <linux/page-flags.h>
> #include <linux/swap.h>
>
> -/*
> - * Casting from randomized struct file * to struct ttm_backup * is fine since
> - * struct ttm_backup is never defined nor dereferenced.
> - */
> -static struct file *ttm_backup_to_file(struct ttm_backup *backup)
> -{
> - return (void *)backup;
> -}
> -
> -static struct ttm_backup *ttm_file_to_backup(struct file *file)
> -{
> - return (void *)file;
> -}
> -
> /*
> * Need to map shmem indices to handle since a handle value
> * of 0 means error, following the swp_entry_t convention.
> @@ -40,12 +26,12 @@ static pgoff_t ttm_backup_handle_to_shmem_idx(pgoff_t
> handle)
> * @backup: The struct backup pointer used to obtain the handle
> * @handle: The handle obtained from the @backup_page function.
> */
> -void ttm_backup_drop(struct ttm_backup *backup, pgoff_t handle)
> +void ttm_backup_drop(struct file *backup, pgoff_t handle)
> {
> loff_t start = ttm_backup_handle_to_shmem_idx(handle);
>
> start <<= PAGE_SHIFT;
> - shmem_truncate_range(file_inode(ttm_backup_to_file(backup)), start,
> + shmem_truncate_range(file_inode(backup), start,
> start + PAGE_SIZE - 1);
> }
>
> @@ -60,11 +46,10 @@ void ttm_backup_drop(struct ttm_backup *backup, pgoff_t
> handle)
> * Return: 0 on success, Negative error code on failure, notably
> * -EINTR if @intr was set to true and a signal is pending.
> */
> -int ttm_backup_copy_page(struct ttm_backup *backup, struct page *dst,
> +int ttm_backup_copy_page(struct file *backup, struct page *dst,
> pgoff_t handle, bool intr)
> {
> - struct file *filp = ttm_backup_to_file(backup);
> - struct address_space *mapping = filp->f_mapping;
> + struct address_space *mapping = backup->f_mapping;
> struct folio *from_folio;
> pgoff_t idx = ttm_backup_handle_to_shmem_idx(handle);
>
> @@ -106,12 +91,11 @@ int ttm_backup_copy_page(struct ttm_backup *backup,
> struct page *dst,
> * the folio size- and usage.
> */
> s64
> -ttm_backup_backup_page(struct ttm_backup *backup, struct page *page,
> +ttm_backup_backup_page(struct file *backup, struct page *page,
> bool writeback, pgoff_t idx, gfp_t page_gfp,
> gfp_t alloc_gfp)
> {
> - struct file *filp = ttm_backup_to_file(backup);
> - struct address_space *mapping = filp->f_mapping;
> + struct address_space *mapping = backup->f_mapping;
> unsigned long handle = 0;
> struct folio *to_folio;
> int ret;
> @@ -161,9 +145,9 @@ ttm_backup_backup_page(struct ttm_backup *backup, struct
> page *page,
> *
> * After a call to this function, it's illegal to use the @backup pointer.
> */
> -void ttm_backup_fini(struct ttm_backup *backup)
> +void ttm_backup_fini(struct file *backup)
> {
> - fput(ttm_backup_to_file(backup));
> + fput(backup);
> }
>
> /**
> @@ -194,14 +178,10 @@ EXPORT_SYMBOL_GPL(ttm_backup_bytes_avail);
> *
> * Create a backup utilizing shmem objects.
> *
> - * Return: A pointer to a struct ttm_backup on success,
> + * Return: A pointer to a struct file on success,
> * an error pointer on error.
> */
> -struct ttm_backup *ttm_backup_shmem_create(loff_t size)
> +struct file *ttm_backup_shmem_create(loff_t size)
> {
> - struct file *filp;
> -
> - filp = shmem_file_setup("ttm shmem backup", size, 0);
> -
> - return ttm_file_to_backup(filp);
> + return shmem_file_setup("ttm shmem backup", size, 0);
> }
> diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
> index 83b10706ba89..c2ea865be657 100644
> --- a/drivers/gpu/drm/ttm/ttm_pool.c
> +++ b/drivers/gpu/drm/ttm/ttm_pool.c
> @@ -506,7 +506,7 @@ static void ttm_pool_allocated_page_commit(struct page
> *allocated,
> * if successful, populate the page-table and dma-address arrays.
> */
> static int ttm_pool_restore_commit(struct ttm_pool_tt_restore *restore,
> - struct ttm_backup *backup,
> + struct file *backup,
> const struct ttm_operation_ctx *ctx,
> struct ttm_pool_alloc_state *alloc)
>
> @@ -655,7 +655,7 @@ static void ttm_pool_free_range(struct ttm_pool *pool,
> struct ttm_tt *tt,
> pgoff_t start_page, pgoff_t end_page)
> {
> struct page **pages = &tt->pages[start_page];
> - struct ttm_backup *backup = tt->backup;
> + struct file *backup = tt->backup;
> pgoff_t i, nr;
>
> for (i = start_page; i < end_page; i += nr, pages += nr) {
> @@ -963,7 +963,7 @@ void ttm_pool_drop_backed_up(struct ttm_tt *tt)
> long ttm_pool_backup(struct ttm_pool *pool, struct ttm_tt *tt,
> const struct ttm_backup_flags *flags)
> {
> - struct ttm_backup *backup = tt->backup;
> + struct file *backup = tt->backup;
> struct page *page;
> unsigned long handle;
> gfp_t alloc_gfp;
> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> index df0aa6c4b8b8..698cd4bf5e46 100644
> --- a/drivers/gpu/drm/ttm/ttm_tt.c
> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
> @@ -544,7 +544,7 @@ EXPORT_SYMBOL(ttm_tt_pages_limit);
> */
> int ttm_tt_setup_backup(struct ttm_tt *tt)
> {
> - struct ttm_backup *backup =
> + struct file *backup =
> ttm_backup_shmem_create(((loff_t)tt->num_pages) << PAGE_SHIFT);
>
> if (WARN_ON_ONCE(!(tt->page_flags & TTM_TT_FLAG_EXTERNAL_MAPPABLE)))
> diff --git a/include/drm/ttm/ttm_backup.h b/include/drm/ttm/ttm_backup.h
> index 24ad120b8827..4da7c825088f 100644
> --- a/include/drm/ttm/ttm_backup.h
> +++ b/include/drm/ttm/ttm_backup.h
> @@ -9,8 +9,6 @@
> #include <linux/mm_types.h>
> #include <linux/shmem_fs.h>
>
> -struct ttm_backup;
> -
> /**
> * ttm_backup_handle_to_page_ptr() - Convert handle to struct page pointer
> * @handle: The handle to convert.
> @@ -55,20 +53,20 @@ ttm_backup_page_ptr_to_handle(const struct page *page)
> return (unsigned long)page >> 1;
> }
>
> -void ttm_backup_drop(struct ttm_backup *backup, pgoff_t handle);
> +void ttm_backup_drop(struct file *backup, pgoff_t handle);
>
> -int ttm_backup_copy_page(struct ttm_backup *backup, struct page *dst,
> +int ttm_backup_copy_page(struct file *backup, struct page *dst,
> pgoff_t handle, bool intr);
>
> s64
> -ttm_backup_backup_page(struct ttm_backup *backup, struct page *page,
> +ttm_backup_backup_page(struct file *backup, struct page *page,
> bool writeback, pgoff_t idx, gfp_t page_gfp,
> gfp_t alloc_gfp);
>
> -void ttm_backup_fini(struct ttm_backup *backup);
> +void ttm_backup_fini(struct file *backup);
>
> u64 ttm_backup_bytes_avail(void);
>
> -struct ttm_backup *ttm_backup_shmem_create(loff_t size);
> +struct file *ttm_backup_shmem_create(loff_t size);
>
> #endif
> diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
> index 13cf47f3322f..406437ad674b 100644
> --- a/include/drm/ttm/ttm_tt.h
> +++ b/include/drm/ttm/ttm_tt.h
> @@ -118,7 +118,7 @@ struct ttm_tt {
> * ttm_tt_create() callback is responsible for assigning
> * this field.
> */
> - struct ttm_backup *backup;
> + struct file *backup;
> /**
> * @caching: The current caching state of the pages, see enum
> * ttm_caching.
