Hi Huan,
> Subject: [PATCH v5 6/7] udmabuf: remove udmabuf_folio
>
> Currently, udmabuf handles folio by creating an unpin list to record
> each folio obtained from the list and unpinning them when released. To
> maintain this approach, many data structures have been established.
>
> However, maintaining this type of data structure requires a significant
> amount of memory and traversing the list is a substantial overhead,
> which is not friendly to the CPU cache.
>
> Considering that during creation, we arranged the folio array in the
> order of pin and set the offset according to pgcnt.
>
> We actually don't need to use unpin_list to unpin during release.
> Instead, we can iterate through the folios array during release and
> unpin any folio that is different from the ones previously accessed.
>
> By this, not only saves the overhead of the udmabuf_folio data structure
> but also makes array access more cache-friendly.
>
> Signed-off-by: Huan Yang <link@xxxxxxxx>
> ---
> drivers/dma-buf/udmabuf.c | 65 +++++++++++++++++----------------------
> 1 file changed, 29 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
> index 254d9ec3d9f3..d449c1fd67a5 100644
> --- a/drivers/dma-buf/udmabuf.c
> +++ b/drivers/dma-buf/udmabuf.c
> @@ -27,15 +27,21 @@ MODULE_PARM_DESC(size_limit_mb, "Max size of a
> dmabuf, in megabytes. Default is
> struct udmabuf {
> pgoff_t pagecount;
> struct folio **folios;
> +
> + /**
> + * Unlike folios, pinned_folios is only used for unpin.
> + * So, nr_pinned is not the same to pagecount, the pinned_folios
> + * only set each folio which already pinned when udmabuf_create.
> + * Note that, since a folio may be pinned multiple times, each folio
> + * can be added to pinned_folios multiple times, depending on how
> many
> + * times the folio has been pinned when create.
> + */
> + pgoff_t nr_pinned;
> + struct folio **pinned_folios;
> +
> struct sg_table *sg;
> struct miscdevice *device;
> pgoff_t *offsets;
> - struct list_head unpin_list;
> -};
> -
> -struct udmabuf_folio {
> - struct folio *folio;
> - struct list_head list;
> };
>
> static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf)
> @@ -198,38 +204,18 @@ static void unmap_udmabuf(struct
> dma_buf_attachment *at,
> return put_sg_table(at->dev, sg, direction);
> }
>
> -static void unpin_all_folios(struct list_head *unpin_list)
> +static void unpin_all_folios(struct udmabuf *ubuf)
> {
> - struct udmabuf_folio *ubuf_folio;
> + pgoff_t i;
>
> - while (!list_empty(unpin_list)) {
> - ubuf_folio = list_first_entry(unpin_list,
> - struct udmabuf_folio, list);
> - unpin_folio(ubuf_folio->folio);
> + for (i = 0; i < ubuf->nr_pinned; ++i)
> + unpin_folio(ubuf->pinned_folios[i]);
>
> - list_del(&ubuf_folio->list);
> - kfree(ubuf_folio);
> - }
> -}
> -
> -static int add_to_unpin_list(struct list_head *unpin_list,
> - struct folio *folio)
> -{
> - struct udmabuf_folio *ubuf_folio;
> -
> - ubuf_folio = kzalloc(sizeof(*ubuf_folio), GFP_KERNEL);
> - if (!ubuf_folio)
> - return -ENOMEM;
> -
> - ubuf_folio->folio = folio;
> - list_add_tail(&ubuf_folio->list, unpin_list);
> - return 0;
> + kvfree(ubuf->pinned_folios);
> }
>
> static __always_inline int init_udmabuf(struct udmabuf *ubuf, pgoff_t
> pgcnt)
> {
> - INIT_LIST_HEAD(&ubuf->unpin_list);
> -
> ubuf->folios = kvmalloc_array(pgcnt, sizeof(*ubuf->folios),
> GFP_KERNEL);
> if (!ubuf->folios)
> return -ENOMEM;
> @@ -238,12 +224,18 @@ static __always_inline int init_udmabuf(struct
> udmabuf *ubuf, pgoff_t pgcnt)
> if (!ubuf->offsets)
> return -ENOMEM;
>
> + ubuf->pinned_folios = kvmalloc_array(pgcnt,
> + sizeof(*ubuf->pinned_folios),
> + GFP_KERNEL);
> + if (!ubuf->pinned_folios)
> + return -ENOMEM;
> +
> return 0;
> }
>
> static __always_inline void deinit_udmabuf(struct udmabuf *ubuf)
> {
> - unpin_all_folios(&ubuf->unpin_list);
> + unpin_all_folios(ubuf);
> kvfree(ubuf->offsets);
> kvfree(ubuf->folios);
> }
> @@ -353,7 +345,9 @@ static int export_udmabuf(struct udmabuf *ubuf,
> static int udmabuf_pin_folios(struct udmabuf *ubuf, struct file *memfd,
> loff_t start, loff_t size)
> {
> - pgoff_t pgoff, pgcnt, upgcnt = ubuf->pagecount;
> + pgoff_t pgoff, pgcnt;
> + pgoff_t upgcnt = ubuf->pagecount;
> + pgoff_t nr_pinned = ubuf->nr_pinned;
Reverse xmas style is preferred; please try to use that.
> u32 cur_folio, cur_pgcnt;
> struct folio **folios = NULL;
> long nr_folios;
> @@ -377,9 +371,7 @@ static int udmabuf_pin_folios(struct udmabuf *ubuf,
> struct file *memfd,
> pgoff_t subpgoff = pgoff;
> size_t fsize = folio_size(folios[cur_folio]);
>
> - ret = add_to_unpin_list(&ubuf->unpin_list, folios[cur_folio]);
> - if (ret < 0)
> - goto err;
> + ubuf->pinned_folios[nr_pinned++] = folios[cur_folio];
>
> for (; subpgoff < fsize; subpgoff += PAGE_SIZE) {
> ubuf->folios[upgcnt] = folios[cur_folio];
> @@ -399,6 +391,7 @@ static int udmabuf_pin_folios(struct udmabuf *ubuf,
> struct file *memfd,
> end:
> err:
> ubuf->pagecount = upgcnt;
> + ubuf->nr_pinned = nr_pinned;
Acked-by: Vivek Kasireddy <vivek.kasireddy@xxxxxxxxx>
> kvfree(folios);
> return ret;
> }
> --
> 2.45.2
