Re: Patch "drm/etnaviv: Drop the offset in page manipulation" has been added to the 6.12-stable tree

[Lucas Stach <l.stach@pengutronix.de>]

Hi Sui,

Am Montag, dem 03.02.2025 um 18:53 +0800 schrieb Sui Jingfeng:
> Hi,
> 
> On 2025/2/3 16:59, Lucas Stach wrote:
> > Hi Sasha,
> > 
> > Am Samstag, dem 01.02.2025 um 23:33 -0500 schrieb Sasha Levin:
> > > This is a note to let you know that I've just added the patch titled
> > > 
> > >      drm/etnaviv: Drop the offset in page manipulation
> > > 
> > > to the 6.12-stable tree which can be found at:
> > >      http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > > 
> > > The filename of the patch is:
> > >       drm-etnaviv-drop-the-offset-in-page-manipulation.patch
> > > and it can be found in the queue-6.12 subdirectory.
> > > 
> > > If you, or anyone else, feels it should not be added to the stable tree,
> > > please let <stable@vger.kernel.org> know about it.
> > > 
> > please drop this patch and all its dependencies from all stable queues.
> > 
> > While the code makes certain assumptions that are corrected in this
> > patch, those assumptions are always true in all use-cases today.
> 
> Those patches are harmless even we apply them, and after apply my pitch,
> it requires less CPU computation, right?
> 
> 
> > I don't see a reason
> 
> I think, if 'sg->offset != 0' could happen  or not is really matters here.
> My argument was that the real data is stored at 'sg_dma_address(sg)', NOT
> the 'sg_dma_address(sg) - sg->offset'.
> 
> 
> As we can create a test that we store some kind of data at the middle of
> a BO by the CPU, then map this BO with the MMU and ask the GPU fetch the
> data.  Do we really have a way tell the GPU to skip the leading garbage
> data?
> 
> 
> > to introduce this kind of churn to the stable trees
> 
> 
> If I'm wrong or miss something, we can get them back, possibly with new
> features, additional description, and comments for use-cases. My argument
> just that we don't have good reasons to take the'sg->offset' into account
> for now.
>   
> 
> > to fix a theoretical issue.
> 
> 
> The start PA of a buffer segment has been altered, but the corresponding
> VA is not.
> 
> Maybe a approach has to guarantee correct in the theory first.

I'm aware that one could construct cases where things fall over with
the previous code. However, there is no such case in practice. I fully
agree that this issue should be fixed, which is obviously why I merged
the patch.

I do not agree to introduce churn into the stable trees and burden
myself and others to check the correctness of the backports (just
because patches do apply to the stable tree does not mean all their
prerequisites and underlying assumptions are met), to fix a theoretical
issue.

Regards,
Lucas