[12/13] media: atomisp: hmm_bo: Drop PFN code path from alloc_user_pages()
Commit Message
alloc_user_pages() is only ever called on qbuf for USERPTR buffers which
always hits the get_user_pages_fast() path, so the pin_user_pages() path
can be removed.
Getting the vma then also is no longer necessary since that is only
done to determine which path to use.
And this also removes the only users of the mem_type struct hmm_bo member,
so remove that as well.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
.../media/atomisp/include/hmm/hmm_bo.h | 3 --
.../staging/media/atomisp/pci/hmm/hmm_bo.c | 48 ++++---------------
2 files changed, 9 insertions(+), 42 deletions(-)
Comments
On Mon, Aug 22, 2022 at 12:50 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> alloc_user_pages() is only ever called on qbuf for USERPTR buffers which
> always hits the get_user_pages_fast() path, so the pin_user_pages() path
> can be removed.
>
> Getting the vma then also is no longer necessary since that is only
> done to determine which path to use.
>
> And this also removes the only users of the mem_type struct hmm_bo member,
> so remove that as well.
...
> + /*Handle frame buffer allocated in user space*/
Spaces?
> + mutex_unlock(&bo->mutex);
> + page_nr = get_user_pages_fast((unsigned long)userptr,
> + (int)(bo->pgnr), 1, bo->pages);
No need for parentheses in the first argument.
> + mutex_lock(&bo->mutex);
> + dev_dbg(atomisp_dev, "%s: %d user pages were allocated as 0x%08x\n",
> + __func__, bo->pgnr, page_nr);
Since you touch this you may remove __func__, which can be enabled via
dynamic debug. OTOH, it might be better to go and drop __func__
everywhere in the driver in the debug messages.
Hi,
On 8/22/22 15:02, Andy Shevchenko wrote:
> On Mon, Aug 22, 2022 at 12:50 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> alloc_user_pages() is only ever called on qbuf for USERPTR buffers which
>> always hits the get_user_pages_fast() path, so the pin_user_pages() path
>> can be removed.
>>
>> Getting the vma then also is no longer necessary since that is only
>> done to determine which path to use.
>>
>> And this also removes the only users of the mem_type struct hmm_bo member,
>> so remove that as well.
>
> ...
>
>> + /*Handle frame buffer allocated in user space*/
>
> Spaces?
I just changed the indentation on this, but I might just as well
add the spaces while at it, will do so for v2.
>
>> + mutex_unlock(&bo->mutex);
>
>> + page_nr = get_user_pages_fast((unsigned long)userptr,
>> + (int)(bo->pgnr), 1, bo->pages);
>
> No need for parentheses in the first argument.
Ack, will fix for v2.
>
>> + mutex_lock(&bo->mutex);
>
>> + dev_dbg(atomisp_dev, "%s: %d user pages were allocated as 0x%08x\n",
>> + __func__, bo->pgnr, page_nr);
>
> Since you touch this you may remove __func__, which can be enabled via
> dynamic debug. OTOH, it might be better to go and drop __func__
> everywhere in the driver in the debug messages.
Or even better, I'll just drop this useless debug statement entirely for v2.
Regards,
Hans
@@ -86,8 +86,6 @@ enum hmm_bo_type {
#define HMM_BO_VMAPED 0x10
#define HMM_BO_VMAPED_CACHED 0x20
#define HMM_BO_ACTIVE 0x1000
-#define HMM_BO_MEM_TYPE_USER 0x1
-#define HMM_BO_MEM_TYPE_PFN 0x2
struct hmm_bo_device {
struct isp_mmu mmu;
@@ -123,7 +121,6 @@ struct hmm_buffer_object {
enum hmm_bo_type type;
int mmap_count;
int status;
- int mem_type;
void *vmap_addr; /* kernel virtual address by vmap */
struct rb_node node;
@@ -656,12 +656,8 @@ static void free_user_pages(struct hmm_buffer_object *bo,
{
int i;
- if (bo->mem_type == HMM_BO_MEM_TYPE_PFN) {
- unpin_user_pages(bo->pages, page_nr);
- } else {
- for (i = 0; i < page_nr; i++)
- put_page(bo->pages[i]);
- }
+ for (i = 0; i < page_nr; i++)
+ put_page(bo->pages[i]);
}
/*
@@ -671,43 +667,17 @@ static int alloc_user_pages(struct hmm_buffer_object *bo,
const void __user *userptr)
{
int page_nr;
- struct vm_area_struct *vma;
-
- mutex_unlock(&bo->mutex);
- mmap_read_lock(current->mm);
- vma = find_vma(current->mm, (unsigned long)userptr);
- mmap_read_unlock(current->mm);
- if (!vma) {
- dev_err(atomisp_dev, "find_vma failed\n");
- mutex_lock(&bo->mutex);
- return -EFAULT;
- }
- mutex_lock(&bo->mutex);
- /*
- * Handle frame buffer allocated in other kerenl space driver
- * and map to user space
- */
userptr = untagged_addr(userptr);
- if (vma->vm_flags & (VM_IO | VM_PFNMAP)) {
- page_nr = pin_user_pages((unsigned long)userptr, bo->pgnr,
- FOLL_LONGTERM | FOLL_WRITE,
- bo->pages, NULL);
- bo->mem_type = HMM_BO_MEM_TYPE_PFN;
- } else {
- /*Handle frame buffer allocated in user space*/
- mutex_unlock(&bo->mutex);
- page_nr = get_user_pages_fast((unsigned long)userptr,
- (int)(bo->pgnr), 1, bo->pages);
- mutex_lock(&bo->mutex);
- bo->mem_type = HMM_BO_MEM_TYPE_USER;
- }
+ /*Handle frame buffer allocated in user space*/
+ mutex_unlock(&bo->mutex);
+ page_nr = get_user_pages_fast((unsigned long)userptr,
+ (int)(bo->pgnr), 1, bo->pages);
+ mutex_lock(&bo->mutex);
- dev_dbg(atomisp_dev, "%s: %d %s pages were allocated as 0x%08x\n",
- __func__,
- bo->pgnr,
- bo->mem_type == HMM_BO_MEM_TYPE_USER ? "user" : "pfn", page_nr);
+ dev_dbg(atomisp_dev, "%s: %d user pages were allocated as 0x%08x\n",
+ __func__, bo->pgnr, page_nr);
/* can be written by caller, not forced */
if (page_nr != bo->pgnr) {