On 2026-06-11 16:07, Yongqiang Sun wrote:
With the KFD_MMAP_TYPE_EVENTS mmap path gone, a kernel-allocated signal
page can no longer be exposed to user space, so allocate_signal_page()
and the related bookkeeping are dead code. The only remaining way to set
up a signal page is kfd_kmap_event_page()/kfd_event_page_set(), where
user space allocates the events page as a BO and passes it via the
event_page_offset of the create event IOCTL.

Remove allocate_signal_page() and require the signal page to be provided
by user space. Drop the now unused kfd_signal_page user mapping
bookkeeping (user_address/need_to_free_pages) and
kfd_event::user_signal_address.

Signed-off-by: Yongqiang Sun <[email protected]>
---
  drivers/gpu/drm/amd/amdkfd/kfd_events.c | 59 +++++--------------------
  drivers/gpu/drm/amd/amdkfd/kfd_events.h |  3 --
  2 files changed, 10 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
index e44f1d1bc516..85e4ad10d619 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
@@ -55,8 +55,6 @@ struct kfd_event_waiter {
   */
  struct kfd_signal_page {
        uint64_t *kernel_address;
-       uint64_t __user *user_address;
-       bool need_to_free_pages;
  };

This patch looks good to me. But I think you could even go one step further, maybe in a third patch. struct kfd_signal_page now has just a single member. There is really no reason why this structure needs to be allocated dynamically any more. You could just replace p->signal_page with a uint64_t * that holds the kernel_address of kmapped the signal BO directly.

That said, patches 1 and 2 are

Reviewed-by: Felix Kuehling <[email protected]>


static uint64_t *page_slots(struct kfd_signal_page *page)
@@ -64,49 +62,19 @@ static uint64_t *page_slots(struct kfd_signal_page *page)
        return page->kernel_address;
  }
-static struct kfd_signal_page *allocate_signal_page(struct kfd_process *p)
-{
-       void *backing_store;
-       struct kfd_signal_page *page;
-
-       page = kzalloc(sizeof(*page), GFP_KERNEL);
-       if (!page)
-               return NULL;
-
-       backing_store = (void *) __get_free_pages(GFP_KERNEL,
-                                       get_order(KFD_SIGNAL_EVENT_LIMIT * 8));
-       if (!backing_store)
-               goto fail_alloc_signal_store;
-
-       /* Initialize all events to unsignaled */
-       memset(backing_store, (uint8_t) UNSIGNALED_EVENT_SLOT,
-              KFD_SIGNAL_EVENT_LIMIT * 8);
-
-       page->kernel_address = backing_store;
-       page->need_to_free_pages = true;
-       pr_debug("Allocated new event signal page at %p, for process %p\n",
-                       page, p);
-
-       return page;
-
-fail_alloc_signal_store:
-       kfree(page);
-       return NULL;
-}
-
  static int allocate_event_notification_slot(struct kfd_process *p,
                                            struct kfd_event *ev,
                                            const int *restore_id)
  {
        int id;
- if (!p->signal_page) {
-               p->signal_page = allocate_signal_page(p);
-               if (!p->signal_page)
-                       return -ENOMEM;
-               /* Oldest user mode expects 256 event slots */
-               p->signal_mapped_size = 256*8;
-       }
+       /*
+        * The signal page is allocated in user mode and mapped to the kernel
+        * via the event_page_offset of the create event IOCTL. Without it no
+        * signal events can be created.
+        */
+       if (!p->signal_page)
+               return -ENOMEM;
if (restore_id) {
                id = idr_alloc(&p->event_idr, ev, *restore_id, *restore_id + 1,
@@ -212,10 +180,8 @@ static int create_signal_event(struct file *devkfd, struct 
kfd_process *p,
p->signal_event_count++; - ev->user_signal_address = &p->signal_page->user_address[ev->event_id];
-       pr_debug("Signal event number %zu created with id %d, address %p\n",
-                       p->signal_event_count, ev->event_id,
-                       ev->user_signal_address);
+       pr_debug("Signal event number %zu created with id %d\n",
+                       p->signal_event_count, ev->event_id);
return 0;
  }
@@ -303,12 +269,7 @@ static void shutdown_signal_page(struct kfd_process *p)
  {
        struct kfd_signal_page *page = p->signal_page;
- if (page) {
-               if (page->need_to_free_pages)
-                       free_pages((unsigned long)page->kernel_address,
-                                  get_order(KFD_SIGNAL_EVENT_LIMIT * 8));
-               kfree(page);
-       }
+       kfree(page);
  }
void kfd_event_free_process(struct kfd_process *p)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_events.h
index 1dc21c13833b..88e3797bfc42 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_events.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.h
@@ -63,9 +63,6 @@ struct kfd_event {
        spinlock_t lock;
        wait_queue_head_t wq; /* List of event waiters. */
- /* Only for signal events. */
-       uint64_t __user *user_signal_address;
-
        /* type specific data */
        union {
                struct kfd_hsa_memory_exception_data memory_exception_data;

Reply via email to