Uwe Kleine-König
2023-Jul-13 13:03 UTC
[Nouveau] [Freedreno] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
hello Sean, On Wed, Jul 12, 2023 at 02:31:02PM -0400, Sean Paul wrote:> I'd really prefer this patch (series or single) is not accepted. This > will cause problems for everyone cherry-picking patches to a > downstream kernel (LTS or distro tree). I usually wouldn't expect > sympathy here, but the questionable benefit does not outweigh the cost > IM[biased]O.I agree that for backports this isn't so nice. However with the split approach (that was argumented against here) it's not soo bad. Patch #1 (and similar changes for the other affected structures) could be trivially backported and with that it doesn't matter if you write dev or drm (or whatever name is chosen in the end); both work in the same way. But even with the one-patch-per-rename approach I'd consider the renaming a net win, because ease of understanding code has a big value. It's value is not so easy measurable as "conflicts when backporting", but it also matters in say two years from now, while backporting shouldn't be an issue then any more. Thanks for your input, best regards Uwe -- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | https://www.pengutronix.de/ | -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 488 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20230713/c890c45b/attachment-0001.sig>
Sean Paul
2023-Jul-13 14:41 UTC
[Freedreno] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
On Thu, Jul 13, 2023 at 9:04?AM Uwe Kleine-K?nig <u.kleine-koenig at pengutronix.de> wrote:> > hello Sean, > > On Wed, Jul 12, 2023 at 02:31:02PM -0400, Sean Paul wrote: > > I'd really prefer this patch (series or single) is not accepted. This > > will cause problems for everyone cherry-picking patches to a > > downstream kernel (LTS or distro tree). I usually wouldn't expect > > sympathy here, but the questionable benefit does not outweigh the cost > > IM[biased]O. > > I agree that for backports this isn't so nice. However with the split > approach (that was argumented against here) it's not soo bad. Patch #1 > (and similar changes for the other affected structures) could be > trivially backported and with that it doesn't matter if you write dev or > drm (or whatever name is chosen in the end); both work in the same way.Patch #1 avoids the need to backport the entire set, however every change occuring after the rename patches will cause conflicts on future cherry-picks. Downstream kernels will have to backport the whole set. Backporting the entire set will create an epoch in downstream kernels where cherry-picking patches preceding this set will need to undergo conflict resolution as well. As mentioned in my previous email, I don't expect sympathy here, it's part of maintaining a downstream kernel, but there is a real cost to kernel consumers.> > But even with the one-patch-per-rename approach I'd consider the > renaming a net win, because ease of understanding code has a big value. > It's value is not so easy measurable as "conflicts when backporting", > but it also matters in say two years from now, while backporting > shouldn't be an issue then any more.You've rightly identified the conjecture in your statement. I've been on both sides of the argument, having written/maintained drm code upstream and cherry-picked changes to a downstream kernel. Perhaps it's because drm's definition of dev is ingrained in my muscle memory, or maybe it's because I don't do a lot of upstream development these days, but I just have a hard time seeing the benefit here. I appreciate your engagement on the topic, thank you! Sean> > Thanks for your input, best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-K?nig | > Industrial Linux Solutions | https://www.pengutronix.de/ |
Thomas Zimmermann
2023-Jul-13 15:09 UTC
[Freedreno] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
Hi Am 13.07.23 um 16:41 schrieb Sean Paul:> On Thu, Jul 13, 2023 at 9:04?AM Uwe Kleine-K?nig > <u.kleine-koenig at pengutronix.de> wrote: >> >> hello Sean, >> >> On Wed, Jul 12, 2023 at 02:31:02PM -0400, Sean Paul wrote: >>> I'd really prefer this patch (series or single) is not accepted. This >>> will cause problems for everyone cherry-picking patches to a >>> downstream kernel (LTS or distro tree). I usually wouldn't expect >>> sympathy here, but the questionable benefit does not outweigh the cost >>> IM[biased]O. >> >> I agree that for backports this isn't so nice. However with the split >> approach (that was argumented against here) it's not soo bad. Patch #1 >> (and similar changes for the other affected structures) could be >> trivially backported and with that it doesn't matter if you write dev or >> drm (or whatever name is chosen in the end); both work in the same way. > > Patch #1 avoids the need to backport the entire set, however every > change occuring after the rename patches will cause conflicts on > future cherry-picks. Downstream kernels will have to backport the > whole set. Backporting the entire set will create an epoch in > downstream kernels where cherry-picking patches preceding this set > will need to undergo conflict resolution as well. As mentioned in my > previous email, I don't expect sympathy here, it's part of maintaining > a downstream kernel, but there is a real cost to kernel consumers. > >> >> But even with the one-patch-per-rename approach I'd consider the >> renaming a net win, because ease of understanding code has a big value. >> It's value is not so easy measurable as "conflicts when backporting", >> but it also matters in say two years from now, while backporting >> shouldn't be an issue then any more. > > You've rightly identified the conjecture in your statement. I've been > on both sides of the argument, having written/maintained drm code > upstream and cherry-picked changes to a downstream kernel. Perhaps > it's because drm's definition of dev is ingrained in my muscle memory, > or maybe it's because I don't do a lot of upstream development these > days, but I just have a hard time seeing the benefit here.I can only second what Sean writes. I've done quite a bit of backporting of DRM code. It's hard already. And this kind of change is going to to affect almost every backported DRM patch in the coming years. Not just for distribution kernels, but also for upstream's stable series. It's really only possible to do this change over many releases while keeping compatible with the old name. So the more I think about it, the less I like this change. Best regards Thomas> > I appreciate your engagement on the topic, thank you! > > Sean > >> >> Thanks for your input, best regards >> Uwe >> >> -- >> Pengutronix e.K. | Uwe Kleine-K?nig | >> Industrial Linux Solutions | https://www.pengutronix.de/ |-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg) -------------- next part -------------- A non-text attachment was scrubbed... Name: OpenPGP_signature Type: application/pgp-signature Size: 840 bytes Desc: OpenPGP digital signature URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20230713/daef68c8/attachment-0001.sig>
Uwe Kleine-König
2023-Jul-13 15:39 UTC
[Nouveau] [Freedreno] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
On Thu, Jul 13, 2023 at 10:41:45AM -0400, Sean Paul wrote:> On Thu, Jul 13, 2023 at 9:04?AM Uwe Kleine-K?nig > > But even with the one-patch-per-rename approach I'd consider the > > renaming a net win, because ease of understanding code has a big value. > > It's value is not so easy measurable as "conflicts when backporting", > > but it also matters in say two years from now, while backporting > > shouldn't be an issue then any more. > > You've rightly identified the conjecture in your statement. I've been > on both sides of the argument, having written/maintained drm code > upstream and cherry-picked changes to a downstream kernel. Perhaps > it's because drm's definition of dev is ingrained in my muscle memory, > or maybe it's because I don't do a lot of upstream development these > days, but I just have a hard time seeing the benefit here.I see that my change doesn't immediately benefit those who are already at home in drivers/gpu/drm. So it's quite understandable that someone in a senior role (long time contributor or maintainer) doesn't see a big upside. In contrast I think my change (with its indisputable cost) lowers the bar for new contributors to drm. IMHO that's something a maintainer has to have on their radar, too, and that's easily undervalued in my experience. Of course in the end it's about weighting the ups and downs. Thanks Uwe -- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | https://www.pengutronix.de/ | -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 488 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20230713/e537c498/attachment-0001.sig>
Reasonably Related Threads
- [Freedreno] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
- [Freedreno] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
- [Freedreno] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
- [Freedreno] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev
- [Freedreno] [PATCH RFC v1 00/52] drm/crtc: Rename struct drm_crtc::dev to drm_dev