From: Matthew Brost <matthew.brost@intel.com>
To: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: intel-xe@lists.freedesktop.org,
"Christian König" <christian.koenig@amd.com>,
"Somalapuram Amaranath" <Amaranath.Somalapuram@amd.com>,
dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v2 4/9] drm/ttm: Allow continued swapout after -ENOSPC falure
Date: Wed, 17 Apr 2024 01:44:33 +0000 [thread overview]
Message-ID: <Zh8pgadLeSpHB9Hr@DUT025-TGLU.fm.intel.com> (raw)
In-Reply-To: <20240416100730.6666-5-thomas.hellstrom@linux.intel.com>
On Tue, Apr 16, 2024 at 12:07:25PM +0200, Thomas Hellström wrote:
> The -ENOSPC failure from ttm_bo_swapout() meant that the lru_lock
> was dropped and simply restarting the iteration meant we'd likely
> hit the same error again on the same resource. Now that we can
> restart the iteration even if the lock was dropped, do that.
>
It is not clear what you describe in this commit message (-ENOSPC ==
-EBUSY + lru_lock dropped) is true (no comments in code).
It does appears to be true after examining ttm_bo_swapout() closely.
Maybe out of scope for the series but would it be possible to add some
kernel doc to ttm_device_swapout stating this?
Patch it self makes sense to me.
Matt
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Somalapuram Amaranath <Amaranath.Somalapuram@amd.com>
> Cc: <dri-devel@lists.freedesktop.org>
> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> ---
> drivers/gpu/drm/ttm/ttm_device.c | 21 +++++++++++++--------
> 1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
> index e8a6a1dab669..4a030b4bc848 100644
> --- a/drivers/gpu/drm/ttm/ttm_device.c
> +++ b/drivers/gpu/drm/ttm/ttm_device.c
> @@ -168,15 +168,20 @@ int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
>
> num_pages = PFN_UP(bo->base.size);
> ret = ttm_bo_swapout(bo, ctx, gfp_flags);
> - /* ttm_bo_swapout has dropped the lru_lock */
> - if (!ret) {
> - ttm_resource_cursor_fini(&cursor);
> - return num_pages;
> - }
> - if (ret != -EBUSY) {
> - ttm_resource_cursor_fini(&cursor);
> - return ret;
> + /* Couldn't swap out, and retained the lru_lock */
> + if (ret == -EBUSY)
> + continue;
> + /* Couldn't swap out and dropped the lru_lock */
> + if (ret == -ENOSPC) {
> + spin_lock(&bdev->lru_lock);
> + continue;
> }
> + /*
> + * Dropped the lock and either succeeded or
> + * hit an error that forces us to break.
> + */
> + ttm_resource_cursor_fini(&cursor);
> + return ret ? ret : num_pages;
> }
> }
> ttm_resource_cursor_fini_locked(&cursor);
> --
> 2.44.0
>
next prev parent reply other threads:[~2024-04-17 1:44 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-16 10:07 [PATCH v2 0/9] TTM shrinker helpers and xe buffer object shrinker Thomas Hellström
2024-04-16 10:07 ` [PATCH v2 1/9] drm/ttm: Allow TTM LRU list nodes of different types Thomas Hellström
2024-04-17 1:15 ` Matthew Brost
2024-04-17 6:09 ` Christian König
2024-05-02 11:41 ` Thomas Hellström
2024-04-16 10:07 ` [PATCH v2 2/9] drm/ttm: Use LRU hitches Thomas Hellström
2024-04-26 1:23 ` Matthew Brost
2024-04-16 10:07 ` [PATCH v2 3/9] drm/ttm, drm/amdgpu, drm/xe: Consider hitch moves within bulk sublist moves Thomas Hellström
2024-04-26 1:20 ` Matthew Brost
2024-04-16 10:07 ` [PATCH v2 4/9] drm/ttm: Allow continued swapout after -ENOSPC falure Thomas Hellström
2024-04-17 1:44 ` Matthew Brost [this message]
2024-04-16 10:07 ` [PATCH v2 5/9] drm/ttm: Add a virtual base class for graphics memory backup Thomas Hellström
2024-04-16 10:07 ` [PATCH v2 6/9] drm/ttm/pool: Provide a helper to shrink pages Thomas Hellström
2024-04-16 10:07 ` [PATCH v2 7/9] drm/ttm: Use fault-injection to test error paths Thomas Hellström
2024-04-16 10:07 ` [PATCH v2 8/9] drm/xe, drm/ttm: Provide a generic LRU walker helper Thomas Hellström
2024-04-16 10:07 ` [PATCH v2 9/9] drm/xe: Add a shrinker for xe bos Thomas Hellström
2024-04-16 21:32 ` kernel test robot
2024-04-17 13:45 ` kernel test robot
2024-04-16 11:55 ` [PATCH v2 0/9] TTM shrinker helpers and xe buffer object shrinker Christian König
2024-04-16 13:08 ` Thomas Hellström
2024-04-16 13:24 ` Christian König
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Zh8pgadLeSpHB9Hr@DUT025-TGLU.fm.intel.com \
--to=matthew.brost@intel.com \
--cc=Amaranath.Somalapuram@amd.com \
--cc=christian.koenig@amd.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-xe@lists.freedesktop.org \
--cc=thomas.hellstrom@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).