| Message ID | 20230228125054.1952657-1-andi.shyti@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/shmem-helper: Fix compile error | expand |
Hi, On 2/28/23 15:50, Andi Shyti wrote: > Commit 67b7836d4458 ("drm/shmem-helper: Switch to reservation > lock") removes the drm_gem_shmem_get_pages_locked() and > drm_gem_shmem_put_pages_locked(). > > But then commit ddddedaa0db9 ("drm/shmem-helper: Fix locking for > drm_gem_shmem_get_pages_sgt()") reintroduces it. > > Somehow these two commits got mixed up and produce the following > compile error: The 67b7836d4458 goes after ddddedaa0db9 in misc-next. It was a bad merge conflict resolution in drm-tip that was fixed yesterday, there is no problem in misc-next. Where do you see this error?
Hi Dmitry, On Tue, Feb 28, 2023 at 04:15:28PM +0300, Dmitry Osipenko wrote: > Hi, > > On 2/28/23 15:50, Andi Shyti wrote: > > Commit 67b7836d4458 ("drm/shmem-helper: Switch to reservation > > lock") removes the drm_gem_shmem_get_pages_locked() and > > drm_gem_shmem_put_pages_locked(). > > > > But then commit ddddedaa0db9 ("drm/shmem-helper: Fix locking for > > drm_gem_shmem_get_pages_sgt()") reintroduces it. > > > > Somehow these two commits got mixed up and produce the following > > compile error: > > The 67b7836d4458 goes after ddddedaa0db9 in misc-next. It was a bad > merge conflict resolution in drm-tip that was fixed yesterday, there is > no problem in misc-next. Where do you see this error? yes, indeed! I was indeed surprised to see this mismatch. I see it in the Intel's drm-tip branch[*] Cc'ing the Intel's mailing list and maintainers, as well. Tnanks, Andi [*] git.freedesktop.org/git/drm-tip
On Tue, 28 Feb 2023, Andi Shyti <andi.shyti@linux.intel.com> wrote: > Hi Dmitry, > > On Tue, Feb 28, 2023 at 04:15:28PM +0300, Dmitry Osipenko wrote: >> Hi, >> >> On 2/28/23 15:50, Andi Shyti wrote: >> > Commit 67b7836d4458 ("drm/shmem-helper: Switch to reservation >> > lock") removes the drm_gem_shmem_get_pages_locked() and >> > drm_gem_shmem_put_pages_locked(). >> > >> > But then commit ddddedaa0db9 ("drm/shmem-helper: Fix locking for >> > drm_gem_shmem_get_pages_sgt()") reintroduces it. >> > >> > Somehow these two commits got mixed up and produce the following >> > compile error: >> >> The 67b7836d4458 goes after ddddedaa0db9 in misc-next. It was a bad >> merge conflict resolution in drm-tip that was fixed yesterday, there is >> no problem in misc-next. Where do you see this error? > > yes, indeed! I was indeed surprised to see this mismatch. > > I see it in the Intel's drm-tip branch[*] To set the record straight, drm-tip isn't Intel's, it's an integration branch shared by the drm community. Looks like the same bad merge resolution has resurrected itself somehow, maybe Thomas' commit 418ce969b4c8533c7c76cc0b7adeb432ccdc137e Author: Thomas Zimmermann <tzimmermann@suse.de> Date: Tue Feb 28 10:03:24 2023 +0100 2023y-02m-28d-09h-02m-44s UTC: drm-tip rerere cache update git version 2.39.2 in drm-rerere brought it back. And the build is indeed currently broken. Moreover, when the build was fine for a while, apparently the changes in shmem broke a bunch of machines in Intel CI. And due to this, we aren't getting any CI results for incoming patches right now. BR, Jani. > > Cc'ing the Intel's mailing list and maintainers, as well. > > Tnanks, > Andi > > [*] git.freedesktop.org/git/drm-tip
Hi, > >> > Commit 67b7836d4458 ("drm/shmem-helper: Switch to reservation > >> > lock") removes the drm_gem_shmem_get_pages_locked() and > >> > drm_gem_shmem_put_pages_locked(). > >> > > >> > But then commit ddddedaa0db9 ("drm/shmem-helper: Fix locking for > >> > drm_gem_shmem_get_pages_sgt()") reintroduces it. > >> > > >> > Somehow these two commits got mixed up and produce the following > >> > compile error: > >> > >> The 67b7836d4458 goes after ddddedaa0db9 in misc-next. It was a bad > >> merge conflict resolution in drm-tip that was fixed yesterday, there is > >> no problem in misc-next. Where do you see this error? > > > > yes, indeed! I was indeed surprised to see this mismatch. > > > > I see it in the Intel's drm-tip branch[*] > > To set the record straight, drm-tip isn't Intel's, it's an integration > branch shared by the drm community. yes of course... it's a matter of fast writing :) > Looks like the same bad merge resolution has resurrected itself somehow, > maybe Thomas' > > commit 418ce969b4c8533c7c76cc0b7adeb432ccdc137e > Author: Thomas Zimmermann <tzimmermann@suse.de> > Date: Tue Feb 28 10:03:24 2023 +0100 > > 2023y-02m-28d-09h-02m-44s UTC: drm-tip rerere cache update > > git version 2.39.2 > > in drm-rerere brought it back. > > And the build is indeed currently broken. > > Moreover, when the build was fine for a while, apparently the changes in > shmem broke a bunch of machines in Intel CI. And due to this, we aren't > getting any CI results for incoming patches right now. Is there any plans for fixing it? Andi
On Tue, 28 Feb 2023, Andi Shyti <andi.shyti@linux.intel.com> wrote: > Hi, > >> >> > Commit 67b7836d4458 ("drm/shmem-helper: Switch to reservation >> >> > lock") removes the drm_gem_shmem_get_pages_locked() and >> >> > drm_gem_shmem_put_pages_locked(). >> >> > >> >> > But then commit ddddedaa0db9 ("drm/shmem-helper: Fix locking for >> >> > drm_gem_shmem_get_pages_sgt()") reintroduces it. >> >> > >> >> > Somehow these two commits got mixed up and produce the following >> >> > compile error: >> >> >> >> The 67b7836d4458 goes after ddddedaa0db9 in misc-next. It was a bad >> >> merge conflict resolution in drm-tip that was fixed yesterday, there is >> >> no problem in misc-next. Where do you see this error? >> > >> > yes, indeed! I was indeed surprised to see this mismatch. >> > >> > I see it in the Intel's drm-tip branch[*] >> >> To set the record straight, drm-tip isn't Intel's, it's an integration >> branch shared by the drm community. > > yes of course... it's a matter of fast writing :) > >> Looks like the same bad merge resolution has resurrected itself somehow, >> maybe Thomas' >> >> commit 418ce969b4c8533c7c76cc0b7adeb432ccdc137e >> Author: Thomas Zimmermann <tzimmermann@suse.de> >> Date: Tue Feb 28 10:03:24 2023 +0100 >> >> 2023y-02m-28d-09h-02m-44s UTC: drm-tip rerere cache update >> >> git version 2.39.2 >> >> in drm-rerere brought it back. >> >> And the build is indeed currently broken. >> >> Moreover, when the build was fine for a while, apparently the changes in >> shmem broke a bunch of machines in Intel CI. And due to this, we aren't >> getting any CI results for incoming patches right now. > > Is there any plans for fixing it? Someone(tm) needs to step up and do it. Personally, I'm clueless. The whole thing is made worse by the conflict and the various resolutions. At this time, I'm not certain whether the whole thing was broken to begin with, or if it's just the conflict resolution that caused the issues. I'll just note that for future reference, Cc'ing intel-gfx for anything non-trivial touching the guts of drm will be useful for running CI on our test farm pre-merge. Now, we don't know. BR, Jani.
Hi, > -----Original Message----- > From: Nikula, Jani <jani.nikula@intel.com> > Sent: tiistai 28. helmikuuta 2023 16.40 > To: Andi Shyti <andi.shyti@linux.intel.com> > Cc: Andi Shyti <andi.shyti@linux.intel.com>; Dmitry Osipenko > <dmitry.osipenko@collabora.com>; dri-devel@lists.freedesktop.org; Maarten > Lankhorst <maarten.lankhorst@linux.intel.com>; Maxime Ripard > <mripard@kernel.org>; Thomas Zimmermann <tzimmermann@suse.de>; David > Airlie <airlied@gmail.com>; Daniel Vetter <daniel@ffwll.ch>; Javier Martinez > Canillas <javierm@redhat.com>; Asahi Lina <lina@asahilina.net>; Andi Shyti > <andi@etezian.org>; Intel GFX <intel-gfx@lists.freedesktop.org>; Tvrtko Ursulin > <tvrtko.ursulin@linux.intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; Joonas > Lahtinen <joonas.lahtinen@linux.intel.com>; Saarinen, Jani > <jani.saarinen@intel.com> > Subject: Re: [PATCH] drm/shmem-helper: Fix compile error > > On Tue, 28 Feb 2023, Andi Shyti <andi.shyti@linux.intel.com> wrote: > > Hi, > > > >> >> > Commit 67b7836d4458 ("drm/shmem-helper: Switch to reservation > >> >> > lock") removes the drm_gem_shmem_get_pages_locked() and > >> >> > drm_gem_shmem_put_pages_locked(). > >> >> > > >> >> > But then commit ddddedaa0db9 ("drm/shmem-helper: Fix locking for > >> >> > drm_gem_shmem_get_pages_sgt()") reintroduces it. > >> >> > > >> >> > Somehow these two commits got mixed up and produce the following > >> >> > compile error: > >> >> > >> >> The 67b7836d4458 goes after ddddedaa0db9 in misc-next. It was a > >> >> bad merge conflict resolution in drm-tip that was fixed yesterday, > >> >> there is no problem in misc-next. Where do you see this error? > >> > > >> > yes, indeed! I was indeed surprised to see this mismatch. > >> > > >> > I see it in the Intel's drm-tip branch[*] > >> > >> To set the record straight, drm-tip isn't Intel's, it's an > >> integration branch shared by the drm community. > > > > yes of course... it's a matter of fast writing :) > > > >> Looks like the same bad merge resolution has resurrected itself > >> somehow, maybe Thomas' > >> > >> commit 418ce969b4c8533c7c76cc0b7adeb432ccdc137e > >> Author: Thomas Zimmermann <tzimmermann@suse.de> > >> Date: Tue Feb 28 10:03:24 2023 +0100 > >> > >> 2023y-02m-28d-09h-02m-44s UTC: drm-tip rerere cache update > >> > >> git version 2.39.2 > >> > >> in drm-rerere brought it back. > >> > >> And the build is indeed currently broken. > >> > >> Moreover, when the build was fine for a while, apparently the changes > >> in shmem broke a bunch of machines in Intel CI. And due to this, we > >> aren't getting any CI results for incoming patches right now. > > > > Is there any plans for fixing it? > > Someone(tm) needs to step up and do it. Personally, I'm clueless. > > The whole thing is made worse by the conflict and the various resolutions. At this > time, I'm not certain whether the whole thing was broken to begin with, or if it's > just the conflict resolution that caused the issues. > > I'll just note that for future reference, Cc'ing intel-gfx for anything non-trivial > touching the guts of drm will be useful for running CI on our test farm pre-merge. > Now, we don't know. Yeah, and sad story can be seen from https://intel-gfx-ci.01.org/tree/drm-tip/index.html? . All systems now abort on BAT run. Just to pick one: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12789/fi-tgl-1115g4/igt@gem_exec_fence@basic-busy@vecs0.html Please fix asap. Or revert from tree asap. > > > BR, > Jani. > > > -- > Jani Nikula, Intel Open Source Graphics Center
On 2/28/23 17:40, Jani Nikula wrote: ... >>> And the build is indeed currently broken. >>> >>> Moreover, when the build was fine for a while, apparently the changes in >>> shmem broke a bunch of machines in Intel CI. And due to this, we aren't >>> getting any CI results for incoming patches right now. >> >> Is there any plans for fixing it? > > Someone(tm) needs to step up and do it. Personally, I'm clueless. > > The whole thing is made worse by the conflict and the various > resolutions. At this time, I'm not certain whether the whole thing was > broken to begin with, or if it's just the conflict resolution that > caused the issues. > > I'll just note that for future reference, Cc'ing intel-gfx for anything > non-trivial touching the guts of drm will be useful for running CI on > our test farm pre-merge. Now, we don't know. Apparently, there is a missing lock for dma-buf vgem (and probably some other similar drivers) exporting code path that I missed before. I've reproduced the IGT warning locally and looking into fixing it. Will keep updating you on it. Thanks for reporting it!
Hi, > > >> >> > Commit 67b7836d4458 ("drm/shmem-helper: Switch to reservation > > >> >> > lock") removes the drm_gem_shmem_get_pages_locked() and > > >> >> > drm_gem_shmem_put_pages_locked(). > > >> >> > > > >> >> > But then commit ddddedaa0db9 ("drm/shmem-helper: Fix locking for > > >> >> > drm_gem_shmem_get_pages_sgt()") reintroduces it. > > >> >> > > > >> >> > Somehow these two commits got mixed up and produce the following > > >> >> > compile error: > > >> >> > > >> >> The 67b7836d4458 goes after ddddedaa0db9 in misc-next. It was a > > >> >> bad merge conflict resolution in drm-tip that was fixed yesterday, > > >> >> there is no problem in misc-next. Where do you see this error? > > >> > > > >> > yes, indeed! I was indeed surprised to see this mismatch. > > >> > > > >> > I see it in the Intel's drm-tip branch[*] > > >> > > >> To set the record straight, drm-tip isn't Intel's, it's an > > >> integration branch shared by the drm community. > > > > > > yes of course... it's a matter of fast writing :) > > > > > >> Looks like the same bad merge resolution has resurrected itself > > >> somehow, maybe Thomas' > > >> > > >> commit 418ce969b4c8533c7c76cc0b7adeb432ccdc137e > > >> Author: Thomas Zimmermann <tzimmermann@suse.de> > > >> Date: Tue Feb 28 10:03:24 2023 +0100 > > >> > > >> 2023y-02m-28d-09h-02m-44s UTC: drm-tip rerere cache update > > >> > > >> git version 2.39.2 > > >> > > >> in drm-rerere brought it back. > > >> > > >> And the build is indeed currently broken. > > >> > > >> Moreover, when the build was fine for a while, apparently the changes > > >> in shmem broke a bunch of machines in Intel CI. And due to this, we > > >> aren't getting any CI results for incoming patches right now. > > > > > > Is there any plans for fixing it? > > > > Someone(tm) needs to step up and do it. Personally, I'm clueless. yeah... I think we either need to fix the rerere branch (which, allow me, is like talking to an angry wife without knowing why she's angry) or just take my patch and have it fixed right away (with some bisect broken in between, I guess). I don't know what's the best approach and in any case I don't have the power to fix it (let me know, in any case, if I can help). > > The whole thing is made worse by the conflict and the various resolutions. At this > > time, I'm not certain whether the whole thing was broken to begin with, or if it's > > just the conflict resolution that caused the issues. > > > > I'll just note that for future reference, Cc'ing intel-gfx for anything non-trivial > > touching the guts of drm will be useful for running CI on our test farm pre-merge. > > Now, we don't know. Yes, fully agree! Andi > Yeah, and sad story can be seen from https://intel-gfx-ci.01.org/tree/drm-tip/index.html? . > All systems now abort on BAT run. > Just to pick one: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12789/fi-tgl-1115g4/igt@gem_exec_fence@basic-busy@vecs0.html > Please fix asap. Or revert from tree asap. > > > > > > > BR, > > Jani. > > > > > > -- > > Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index 990fff32afd69..4b725aa5ce1cd 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -643,12 +643,14 @@ static struct sg_table *drm_gem_shmem_get_pages_sgt_locked(struct drm_gem_shmem_ int ret; struct sg_table *sgt; + dma_resv_assert_held(shmem->base.resv); + if (shmem->sgt) return shmem->sgt; drm_WARN_ON(obj->dev, obj->import_attach); - ret = drm_gem_shmem_get_pages_locked(shmem); + ret = drm_gem_shmem_get_pages(shmem); if (ret) return ERR_PTR(ret); @@ -670,7 +672,7 @@ static struct sg_table *drm_gem_shmem_get_pages_sgt_locked(struct drm_gem_shmem_ sg_free_table(sgt); kfree(sgt); err_put_pages: - drm_gem_shmem_put_pages_locked(shmem); + drm_gem_shmem_put_pages(shmem); return ERR_PTR(ret); }
Commit 67b7836d4458 ("drm/shmem-helper: Switch to reservation lock") removes the drm_gem_shmem_get_pages_locked() and drm_gem_shmem_put_pages_locked(). But then commit ddddedaa0db9 ("drm/shmem-helper: Fix locking for drm_gem_shmem_get_pages_sgt()") reintroduces it. Somehow these two commits got mixed up and produce the following compile error: drivers/gpu/drm/drm_gem_shmem_helper.c: In function ‘drm_gem_shmem_get_pages_sgt_locked’: drivers/gpu/drm/drm_gem_shmem_helper.c:651:15: error: implicit declaration of function ‘drm_gem_shmem_get_pages_locked’; did you mean ‘drm_gem_shmem_get_pages_sgt_locked’? [-Werror=implicit-function-declaration] 651 | ret = drm_gem_shmem_get_pages_locked(shmem); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | drm_gem_shmem_get_pages_sgt_locked drivers/gpu/drm/drm_gem_shmem_helper.c:673:9: error: implicit declaration of function ‘drm_gem_shmem_put_pages_locked’; did you mean ‘drm_gem_shmem_get_pages_sgt_locked’? [-Werror=implicit-function-declaration] 673 | drm_gem_shmem_put_pages_locked(shmem); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | drm_gem_shmem_get_pages_sgt_locked Remove the use of the *_locked() functions as it was intended in the original patch. Fixes: ddddedaa0db9 ("drm/shmem-helper: Fix locking for drm_gem_shmem_get_pages_sgt()") Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com> Cc: Asahi Lina <lina@asahilina.net> Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com> --- Hi, I don't know if this issue has already been addressed, if so it hasn't reached yet drm-tip. Andi drivers/gpu/drm/drm_gem_shmem_helper.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)