diff mbox series

[v3,1/5] drm/ttm: add helper structures for bulk moves on lru list

Message ID 1534154331-11810-2-git-send-email-ray.huang@amd.com (mailing list archive)
State New, archived
Headers show
Series drm/ttm,amdgpu: Introduce LRU bulk move functionality | expand

Commit Message

Huang Rui Aug. 13, 2018, 9:58 a.m. UTC 
From: Christian König <christian.koenig@amd.com>

Add bulk move pos to store the pointer of first and last buffer object.
The list in between will be bulk moved on lru list.

Signed-off-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Huang Rui <ray.huang@amd.com>
Tested-by: Mike Lothian <mike@fireburn.co.uk>
---
 include/drm/ttm/ttm_bo_driver.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

Comments

Christian König Aug. 13, 2018, 10:16 a.m. UTC | #1 
Am 13.08.2018 um 11:58 schrieb Huang Rui:
> From: Christian König <christian.koenig@amd.com>
>
> Add bulk move pos to store the pointer of first and last buffer object.
> The list in between will be bulk moved on lru list.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> Tested-by: Mike Lothian <mike@fireburn.co.uk>

If you ask me that looks like it should work now, but I'm prepossessed 
because I helped creating this.

Alex, David or Jerry can somebody else take a look as well?

Thanks,
Christian.

> ---
>   include/drm/ttm/ttm_bo_driver.h | 28 ++++++++++++++++++++++++++++
>   1 file changed, 28 insertions(+)
>
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 3234cc3..e4fee8e 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -491,6 +491,34 @@ struct ttm_bo_device {
>   };
>   
>   /**
> + * struct ttm_lru_bulk_move_pos
> + *
> + * @first: first BO in the bulk move range
> + * @last: last BO in the bulk move range
> + *
> + * Positions for a lru bulk move.
> + */
> +struct ttm_lru_bulk_move_pos {
> +	struct ttm_buffer_object *first;
> +	struct ttm_buffer_object *last;
> +};
> +
> +/**
> + * struct ttm_lru_bulk_move
> + *
> + * @tt: first/last lru entry for BOs in the TT domain
> + * @vram: first/last lru entry for BOs in the VRAM domain
> + * @swap: first/last lru entry for BOs on the swap list
> + *
> + * Helper structure for bulk moves on the LRU list.
> + */
> +struct ttm_lru_bulk_move {
> +	struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
> +	struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
> +	struct ttm_lru_bulk_move_pos swap[TTM_MAX_BO_PRIORITY];
> +};
> +
> +/**
>    * ttm_flag_masked
>    *
>    * @old: Pointer to the result and original value.
Chunming Zhou Aug. 14, 2018, 2:02 a.m. UTC | #2 
On 2018年08月13日 18:16, Christian König wrote:
> Am 13.08.2018 um 11:58 schrieb Huang Rui:
>> From: Christian König <christian.koenig@amd.com>
>>
>> Add bulk move pos to store the pointer of first and last buffer object.
>> The list in between will be bulk moved on lru list.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> Signed-off-by: Huang Rui <ray.huang@amd.com>
>> Tested-by: Mike Lothian <mike@fireburn.co.uk>
>
> If you ask me that looks like it should work now, but I'm prepossessed 
> because I helped creating this.
>
> Alex, David or Jerry can somebody else take a look as well?
remember position, list ops...
Acked-by: Chunming Zhou <david1.zhou@amd.com>

>
> Thanks,
> Christian.
>
>> ---
>>   include/drm/ttm/ttm_bo_driver.h | 28 ++++++++++++++++++++++++++++
>>   1 file changed, 28 insertions(+)
>>
>> diff --git a/include/drm/ttm/ttm_bo_driver.h 
>> b/include/drm/ttm/ttm_bo_driver.h
>> index 3234cc3..e4fee8e 100644
>> --- a/include/drm/ttm/ttm_bo_driver.h
>> +++ b/include/drm/ttm/ttm_bo_driver.h
>> @@ -491,6 +491,34 @@ struct ttm_bo_device {
>>   };
>>     /**
>> + * struct ttm_lru_bulk_move_pos
>> + *
>> + * @first: first BO in the bulk move range
>> + * @last: last BO in the bulk move range
>> + *
>> + * Positions for a lru bulk move.
>> + */
>> +struct ttm_lru_bulk_move_pos {
>> +    struct ttm_buffer_object *first;
>> +    struct ttm_buffer_object *last;
>> +};
>> +
>> +/**
>> + * struct ttm_lru_bulk_move
>> + *
>> + * @tt: first/last lru entry for BOs in the TT domain
>> + * @vram: first/last lru entry for BOs in the VRAM domain
>> + * @swap: first/last lru entry for BOs on the swap list
>> + *
>> + * Helper structure for bulk moves on the LRU list.
>> + */
>> +struct ttm_lru_bulk_move {
>> +    struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
>> +    struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
>> +    struct ttm_lru_bulk_move_pos swap[TTM_MAX_BO_PRIORITY];
>> +};
>> +
>> +/**
>>    * ttm_flag_masked
>>    *
>>    * @old: Pointer to the result and original value.
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Zhang, Jerry(Junwei) Aug. 14, 2018, 2:22 a.m. UTC | #3 
On 08/13/2018 06:16 PM, Christian König wrote:
> Am 13.08.2018 um 11:58 schrieb Huang Rui:
>> From: Christian König <christian.koenig@amd.com>
>>
>> Add bulk move pos to store the pointer of first and last buffer object.
>> The list in between will be bulk moved on lru list.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> Signed-off-by: Huang Rui <ray.huang@amd.com>
>> Tested-by: Mike Lothian <mike@fireburn.co.uk>
>
> If you ask me that looks like it should work now, but I'm prepossessed because I helped creating this.
>
> Alex, David or Jerry can somebody else take a look as well?

Patch 1, 2, 3, 5 are

Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>

Patch 4: comments inline.

BTW, a per-vm bo lru may be more efficient, but not a common way.
bulk move could improve that, while possibly there are some worse cases and better cases.

e.g. remark the bo position for target BO, like PD/PT bo and per-vm bo, in which range may include other BOs
If the target BO range include more other BOs, may cause evict or anything else low efficiency.
We hope target BO grouped together as much as possible, and be moved.
(if not, please correct me)

Regards,
Jerry

>
> Thanks,
> Christian.
>
>> ---
>>   include/drm/ttm/ttm_bo_driver.h | 28 ++++++++++++++++++++++++++++
>>   1 file changed, 28 insertions(+)
>>
>> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
>> index 3234cc3..e4fee8e 100644
>> --- a/include/drm/ttm/ttm_bo_driver.h
>> +++ b/include/drm/ttm/ttm_bo_driver.h
>> @@ -491,6 +491,34 @@ struct ttm_bo_device {
>>   };
>>   /**
>> + * struct ttm_lru_bulk_move_pos
>> + *
>> + * @first: first BO in the bulk move range
>> + * @last: last BO in the bulk move range
>> + *
>> + * Positions for a lru bulk move.
>> + */
>> +struct ttm_lru_bulk_move_pos {
>> +    struct ttm_buffer_object *first;
>> +    struct ttm_buffer_object *last;
>> +};
>> +
>> +/**
>> + * struct ttm_lru_bulk_move
>> + *
>> + * @tt: first/last lru entry for BOs in the TT domain
>> + * @vram: first/last lru entry for BOs in the VRAM domain
>> + * @swap: first/last lru entry for BOs on the swap list
>> + *
>> + * Helper structure for bulk moves on the LRU list.
>> + */
>> +struct ttm_lru_bulk_move {
>> +    struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
>> +    struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
>> +    struct ttm_lru_bulk_move_pos swap[TTM_MAX_BO_PRIORITY];
>> +};
>> +
>> +/**
>>    * ttm_flag_masked
>>    *
>>    * @old: Pointer to the result and original value.
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Huang Rui Aug. 14, 2018, 2:25 a.m. UTC | #4 
On Tue, Aug 14, 2018 at 10:02:00AM +0800, Zhou, David(ChunMing) wrote:
> 
> 
> On 2018年08月13日 18:16, Christian König wrote:
> > Am 13.08.2018 um 11:58 schrieb Huang Rui:
> >> From: Christian König <christian.koenig@amd.com>
> >>
> >> Add bulk move pos to store the pointer of first and last buffer object.
> >> The list in between will be bulk moved on lru list.
> >>
> >> Signed-off-by: Christian König <christian.koenig@amd.com>
> >> Signed-off-by: Huang Rui <ray.huang@amd.com>
> >> Tested-by: Mike Lothian <mike@fireburn.co.uk>
> >
> > If you ask me that looks like it should work now, but I'm prepossessed 
> > because I helped creating this.
> >
> > Alex, David or Jerry can somebody else take a look as well?
> remember position, list ops...
> Acked-by: Chunming Zhou <david1.zhou@amd.com>
> 

Thanks David, any comments are warm for me.

Best Regards,
Ray

> >
> > Thanks,
> > Christian.
> >
> >> ---
> >>   include/drm/ttm/ttm_bo_driver.h | 28 ++++++++++++++++++++++++++++
> >>   1 file changed, 28 insertions(+)
> >>
> >> diff --git a/include/drm/ttm/ttm_bo_driver.h 
> >> b/include/drm/ttm/ttm_bo_driver.h
> >> index 3234cc3..e4fee8e 100644
> >> --- a/include/drm/ttm/ttm_bo_driver.h
> >> +++ b/include/drm/ttm/ttm_bo_driver.h
> >> @@ -491,6 +491,34 @@ struct ttm_bo_device {
> >>   };
> >>     /**
> >> + * struct ttm_lru_bulk_move_pos
> >> + *
> >> + * @first: first BO in the bulk move range
> >> + * @last: last BO in the bulk move range
> >> + *
> >> + * Positions for a lru bulk move.
> >> + */
> >> +struct ttm_lru_bulk_move_pos {
> >> +    struct ttm_buffer_object *first;
> >> +    struct ttm_buffer_object *last;
> >> +};
> >> +
> >> +/**
> >> + * struct ttm_lru_bulk_move
> >> + *
> >> + * @tt: first/last lru entry for BOs in the TT domain
> >> + * @vram: first/last lru entry for BOs in the VRAM domain
> >> + * @swap: first/last lru entry for BOs on the swap list
> >> + *
> >> + * Helper structure for bulk moves on the LRU list.
> >> + */
> >> +struct ttm_lru_bulk_move {
> >> +    struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
> >> +    struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
> >> +    struct ttm_lru_bulk_move_pos swap[TTM_MAX_BO_PRIORITY];
> >> +};
> >> +
> >> +/**
> >>    * ttm_flag_masked
> >>    *
> >>    * @old: Pointer to the result and original value.
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
Huang Rui Aug. 14, 2018, 2:49 a.m. UTC | #5 
On Tue, Aug 14, 2018 at 10:22:34AM +0800, Zhang, Jerry (Junwei) wrote:
> On 08/13/2018 06:16 PM, Christian König wrote:
> >Am 13.08.2018 um 11:58 schrieb Huang Rui:
> >>From: Christian König <christian.koenig@amd.com>
> >>
> >>Add bulk move pos to store the pointer of first and last buffer object.
> >>The list in between will be bulk moved on lru list.
> >>
> >>Signed-off-by: Christian König <christian.koenig@amd.com>
> >>Signed-off-by: Huang Rui <ray.huang@amd.com>
> >>Tested-by: Mike Lothian <mike@fireburn.co.uk>
> >
> >If you ask me that looks like it should work now, but I'm prepossessed because I helped creating this.
> >
> >Alex, David or Jerry can somebody else take a look as well?
> 
> Patch 1, 2, 3, 5 are
> 
> Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>
> 
> Patch 4: comments inline.
> 
> BTW, a per-vm bo lru may be more efficient, but not a common way.
> bulk move could improve that, while possibly there are some worse cases and better cases.
> 
> e.g. remark the bo position for target BO, like PD/PT bo and per-vm bo, in which range may include other BOs
> If the target BO range include more other BOs, may cause evict or anything else low efficiency.
> We hope target BO grouped together as much as possible, and be moved.
> (if not, please correct me)
> 

Thanks, Jerry. Actually, here we will remember both per-vm and PD/PT BOs
positions in the same VM. And move them one time to the end of LRU. And
won't move them one by one. The time complexity is from O(n) to O(1).
The performance issue is not caused by eviction, but by the many times of
moving on LRU list. So for now, we do the bulk move instead.

Thanks,
Ray

> Regards,
> Jerry
> 
> >
> >Thanks,
> >Christian.
> >
> >>---
> >>  include/drm/ttm/ttm_bo_driver.h | 28 ++++++++++++++++++++++++++++
> >>  1 file changed, 28 insertions(+)
> >>
> >>diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> >>index 3234cc3..e4fee8e 100644
> >>--- a/include/drm/ttm/ttm_bo_driver.h
> >>+++ b/include/drm/ttm/ttm_bo_driver.h
> >>@@ -491,6 +491,34 @@ struct ttm_bo_device {
> >>  };
> >>  /**
> >>+ * struct ttm_lru_bulk_move_pos
> >>+ *
> >>+ * @first: first BO in the bulk move range
> >>+ * @last: last BO in the bulk move range
> >>+ *
> >>+ * Positions for a lru bulk move.
> >>+ */
> >>+struct ttm_lru_bulk_move_pos {
> >>+    struct ttm_buffer_object *first;
> >>+    struct ttm_buffer_object *last;
> >>+};
> >>+
> >>+/**
> >>+ * struct ttm_lru_bulk_move
> >>+ *
> >>+ * @tt: first/last lru entry for BOs in the TT domain
> >>+ * @vram: first/last lru entry for BOs in the VRAM domain
> >>+ * @swap: first/last lru entry for BOs on the swap list
> >>+ *
> >>+ * Helper structure for bulk moves on the LRU list.
> >>+ */
> >>+struct ttm_lru_bulk_move {
> >>+    struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
> >>+    struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
> >>+    struct ttm_lru_bulk_move_pos swap[TTM_MAX_BO_PRIORITY];
> >>+};
> >>+
> >>+/**
> >>   * ttm_flag_masked
> >>   *
> >>   * @old: Pointer to the result and original value.
> >
> >_______________________________________________
> >amd-gfx mailing list
> >amd-gfx@lists.freedesktop.org
> >https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
diff mbox series

Patch

diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 3234cc3..e4fee8e 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -491,6 +491,34 @@  struct ttm_bo_device {
 };
 
 /**
+ * struct ttm_lru_bulk_move_pos
+ *
+ * @first: first BO in the bulk move range
+ * @last: last BO in the bulk move range
+ *
+ * Positions for a lru bulk move.
+ */
+struct ttm_lru_bulk_move_pos {
+	struct ttm_buffer_object *first;
+	struct ttm_buffer_object *last;
+};
+
+/**
+ * struct ttm_lru_bulk_move
+ *
+ * @tt: first/last lru entry for BOs in the TT domain
+ * @vram: first/last lru entry for BOs in the VRAM domain
+ * @swap: first/last lru entry for BOs on the swap list
+ *
+ * Helper structure for bulk moves on the LRU list.
+ */
+struct ttm_lru_bulk_move {
+	struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
+	struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
+	struct ttm_lru_bulk_move_pos swap[TTM_MAX_BO_PRIORITY];
+};
+
+/**
  * ttm_flag_masked
  *
  * @old: Pointer to the result and original value.