From: "Christian König" <ckoenig.leichtzumerken@gmail.com> To: Russell King - ARM Linux admin <linux@armlinux.org.uk> Cc: xen-devel@lists.xenproject.org, devel@driverdev.osuosl.org, linux-tegra@vger.kernel.org, intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org, sstabellini@kernel.org, jgross@suse.com, boris.ostrovsky@oracle.com, digetx@gmail.com, gregkh@linuxfoundation.org, arnd@arndb.de, mchehab@kernel.org, kyungmin.park@samsung.com, m.szyprowski@samsung.com, pawel@osciak.com, jonathanh@nvidia.com, thierry.reding@gmail.com, tomi.valkeinen@ti.com, rodrigo.vivi@intel.com, joonas.lahtinen@linux.intel.com, jani.nikula@linux.intel.com, sean@poorly.run, maxime.ripard@bootlin.com, maarten.lankhorst@linux.intel.com, sumit.semwal@linaro.org Subject: Re: [PATCH] dma-buf: add struct dma_buf_attach_info v2 Date: Fri, 3 May 2019 14:05:47 +0200 [thread overview] Message-ID: <cbcbb076-a8b0-67b0-8c16-daf1d060fc1d@gmail.com> (raw) In-Reply-To: <20190430173127.k5ivpaz6ktbfecgo@shell.armlinux.org.uk> Am 30.04.19 um 19:31 schrieb Russell King - ARM Linux admin: > On Tue, Apr 30, 2019 at 01:10:02PM +0200, Christian König wrote: >> Add a structure for the parameters of dma_buf_attach, this makes it much easier >> to add new parameters later on. > I don't understand this reasoning. What are the "new parameters" that > are being proposed, and why do we need to put them into memory to pass > them across this interface? > > If the intention is to make it easier to change the interface, passing > parameters in this manner mean that it's easy for the interface to > change and drivers not to notice the changes, since the compiler will > not warn (unless some member of the structure that the driver is using > gets removed, in which case it will error.) > > Additions to the structure will go unnoticed by drivers - what if the > caller is expecting some different kind of behaviour, and the driver > ignores that new addition? Well, exactly that's the intention here: That the drivers using this interface should be able to ignore the new additions for now as long as they are not going to use them. The background is that we have multiple interface changes in the pipeline, and each step requires new optional parameters. > This doesn't seem to me like a good idea. Well, the obvious alternatives are: a) Change all drivers to explicitly provide NULL/0 for the new parameters. b) Use a wrapper, so that the function signature of dma_buf_attach stays the same. Key point here is that I have an invalidation callback change, a P2P patch set and some locking changes which all require adding new parameters or flags. And at each step I would then start to change all drivers, adding some more NULL pointers or flags with 0 default value. I'm actually perfectly fine going down any route, but this just seemed to me simplest and with the least risk of breaking anything. Opinions? Thanks, Christian.
WARNING: multiple messages have this Message-ID (diff)
From: "Christian König" <ckoenig.leichtzumerken@gmail.com> To: Russell King - ARM Linux admin <linux@armlinux.org.uk> Cc: maxime.ripard@bootlin.com, joonas.lahtinen@linux.intel.com, dri-devel@lists.freedesktop.org, thierry.reding@gmail.com, digetx@gmail.com, sumit.semwal@linaro.org, m.szyprowski@samsung.com, devel@driverdev.osuosl.org, sstabellini@kernel.org, arnd@arndb.de, jonathanh@nvidia.com, tomi.valkeinen@ti.com, xen-devel@lists.xenproject.org, linux-media@vger.kernel.org, pawel@osciak.com, intel-gfx@lists.freedesktop.org, maarten.lankhorst@linux.intel.com, jani.nikula@linux.intel.com, rodrigo.vivi@intel.com, linux-tegra@vger.kernel.org, boris.ostrovsky@oracle.com, mchehab@kernel.org, sean@poorly.run, jgross@suse.com, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, kyungmin.park@samsung.com Subject: Re: [Xen-devel] [PATCH] dma-buf: add struct dma_buf_attach_info v2 Date: Fri, 3 May 2019 14:05:47 +0200 [thread overview] Message-ID: <cbcbb076-a8b0-67b0-8c16-daf1d060fc1d@gmail.com> (raw) In-Reply-To: <20190430173127.k5ivpaz6ktbfecgo@shell.armlinux.org.uk> Am 30.04.19 um 19:31 schrieb Russell King - ARM Linux admin: > On Tue, Apr 30, 2019 at 01:10:02PM +0200, Christian König wrote: >> Add a structure for the parameters of dma_buf_attach, this makes it much easier >> to add new parameters later on. > I don't understand this reasoning. What are the "new parameters" that > are being proposed, and why do we need to put them into memory to pass > them across this interface? > > If the intention is to make it easier to change the interface, passing > parameters in this manner mean that it's easy for the interface to > change and drivers not to notice the changes, since the compiler will > not warn (unless some member of the structure that the driver is using > gets removed, in which case it will error.) > > Additions to the structure will go unnoticed by drivers - what if the > caller is expecting some different kind of behaviour, and the driver > ignores that new addition? Well, exactly that's the intention here: That the drivers using this interface should be able to ignore the new additions for now as long as they are not going to use them. The background is that we have multiple interface changes in the pipeline, and each step requires new optional parameters. > This doesn't seem to me like a good idea. Well, the obvious alternatives are: a) Change all drivers to explicitly provide NULL/0 for the new parameters. b) Use a wrapper, so that the function signature of dma_buf_attach stays the same. Key point here is that I have an invalidation callback change, a P2P patch set and some locking changes which all require adding new parameters or flags. And at each step I would then start to change all drivers, adding some more NULL pointers or flags with 0 default value. I'm actually perfectly fine going down any route, but this just seemed to me simplest and with the least risk of breaking anything. Opinions? Thanks, Christian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2019-05-03 12:05 UTC|newest] Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-04-30 11:10 [PATCH] dma-buf: add struct dma_buf_attach_info v2 Christian König 2019-04-30 11:10 ` [Xen-devel] " Christian König 2019-04-30 13:49 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork 2019-04-30 14:05 ` ✓ Fi.CI.BAT: success " Patchwork 2019-04-30 15:23 ` [PATCH] " kbuild test robot 2019-04-30 15:23 ` [Xen-devel] [Intel-gfx] " kbuild test robot 2019-04-30 15:23 ` kbuild test robot 2019-04-30 15:23 ` kbuild test robot 2019-04-30 16:59 ` Boris Ostrovsky 2019-04-30 16:59 ` Boris Ostrovsky 2019-04-30 16:59 ` [Xen-devel] " Boris Ostrovsky 2019-04-30 16:59 ` Boris Ostrovsky 2019-04-30 17:31 ` Russell King - ARM Linux admin 2019-04-30 17:31 ` [Xen-devel] " Russell King - ARM Linux admin 2019-05-03 12:05 ` Christian König 2019-05-03 12:05 ` Christian König [this message] 2019-05-03 12:05 ` [Xen-devel] " Christian König 2019-05-03 12:09 ` [Intel-gfx] " Daniel Vetter 2019-05-03 12:09 ` [Xen-devel] " Daniel Vetter 2019-05-03 12:09 ` Daniel Vetter 2019-05-03 12:15 ` Koenig, Christian 2019-05-03 12:15 ` [Xen-devel] " Koenig, Christian 2019-05-03 12:15 ` Koenig, Christian 2019-05-03 12:15 ` Koenig, Christian 2019-05-03 12:09 ` Daniel Vetter 2019-04-30 17:31 ` Russell King - ARM Linux admin 2019-04-30 23:06 ` [Intel-gfx] " kbuild test robot 2019-04-30 23:06 ` kbuild test robot 2019-04-30 23:06 ` [Xen-devel] [Intel-gfx] " kbuild test robot 2019-05-01 4:44 ` ✗ Fi.CI.IGT: failure for " Patchwork 2019-05-02 7:59 ` Patchwork 2019-05-02 8:54 ` Patchwork -- strict thread matches above, loose matches on Subject: below -- 2019-04-30 11:10 [PATCH] " 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=cbcbb076-a8b0-67b0-8c16-daf1d060fc1d@gmail.com \ --to=ckoenig.leichtzumerken@gmail.com \ --cc=arnd@arndb.de \ --cc=boris.ostrovsky@oracle.com \ --cc=christian.koenig@amd.com \ --cc=devel@driverdev.osuosl.org \ --cc=digetx@gmail.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=gregkh@linuxfoundation.org \ --cc=intel-gfx@lists.freedesktop.org \ --cc=jani.nikula@linux.intel.com \ --cc=jgross@suse.com \ --cc=jonathanh@nvidia.com \ --cc=joonas.lahtinen@linux.intel.com \ --cc=kyungmin.park@samsung.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=linux-tegra@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=m.szyprowski@samsung.com \ --cc=maarten.lankhorst@linux.intel.com \ --cc=maxime.ripard@bootlin.com \ --cc=mchehab@kernel.org \ --cc=pawel@osciak.com \ --cc=rodrigo.vivi@intel.com \ --cc=sean@poorly.run \ --cc=sstabellini@kernel.org \ --cc=sumit.semwal@linaro.org \ --cc=thierry.reding@gmail.com \ --cc=tomi.valkeinen@ti.com \ --cc=xen-devel@lists.xenproject.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.