Hi, > -----Original Message----- > From: Burakov, Anatoly <anatoly.bura...@intel.com> > Sent: Thursday, August 26, 2021 6:15 PM > To: Richardson, Bruce <bruce.richard...@intel.com> > Cc: Yigit, Ferruh <ferruh.yi...@intel.com>; Ding, Xuan > <xuan.d...@intel.com>; dev@dpdk.org; maxime.coque...@redhat.com; Xia, > Chenbo <chenbo....@intel.com>; Hu, Jiayu <jiayu...@intel.com>; > techbo...@dpdk.org; David Marchand <david.march...@redhat.com> > Subject: Re: [PATCH] doc: announce change in dma mapping/unmapping > > On 26-Aug-21 11:09 AM, Bruce Richardson wrote: > > On Thu, Aug 26, 2021 at 10:46:07AM +0100, Burakov, Anatoly wrote: > >> On 26-Aug-21 10:29 AM, Ferruh Yigit wrote: > >>> On 8/25/2021 12:47 PM, Burakov, Anatoly wrote: > >>>> On 25-Aug-21 12:27 PM, Xuan Ding wrote: > >>>>> Currently, the VFIO subsystem will compact adjacent DMA regions for > the > >>>>> purposes of saving space in the internal list of mappings. This has a > >>>>> side effect of compacting two separate mappings that just happen to > be > >>>>> adjacent in memory. Since VFIO implementation on IA platforms also > does > >>>>> not allow partial unmapping of memory mapped for DMA, the current > DPDK > >>>>> VFIO implementation will prevent unmapping of accidentally adjacent > >>>>> maps even though it could have been unmapped [1]. > >>>>> > >>>>> The proper fix for this issue is to change the VFIO DMA mapping API > to > >>>>> also include page size, and always map memory page-by-page. > >>>>> > >>>>> [1] https://mails.dpdk.org/archives/dev/2021-July/213493.html > >>>>> > >>>>> Signed-off-by: Xuan Ding <xuan.d...@intel.com> > >>>>> --- > >>>>> doc/guides/rel_notes/deprecation.rst | 3 +++ > >>>>> 1 file changed, 3 insertions(+) > >>>>> > >>>>> diff --git a/doc/guides/rel_notes/deprecation.rst > >>>>> b/doc/guides/rel_notes/deprecation.rst > >>>>> index 76a4abfd6b..272ffa993e 100644 > >>>>> --- a/doc/guides/rel_notes/deprecation.rst > >>>>> +++ b/doc/guides/rel_notes/deprecation.rst > >>>>> @@ -287,3 +287,6 @@ Deprecation Notices > >>>>> reserved bytes to 2 (from 3), and use 1 byte to indicate warnings > and other > >>>>> information from the crypto/security operation. This field will be > used to > >>>>> communicate events such as soft expiry with IPsec in lookaside > mode. > >>>>> + > >>>>> + * vfio: the functions `rte_vfio_container_dma_map` and > >>>>> `rte_vfio_container_dma_unmap` > >>>>> + will be amended to include page size. This change is targeted for > DPDK 21.11. > >>>>> > >>>> > >>>> Acked-by: Anatoly Burakov <anatoly.bura...@intel.com> > >>>> > >>> > >>> Techboard decision was to add a new API, instead of updating existing > ones, to > >>> not break the apps using this API. > >>> > >>> @Xuan, @Anatoly, can you please confirm if this will solve your problem? > >>> > >> > >> I don't think adding a new API is a particularly good solution. The "new" > >> API will be almost exactly as the old one, but adding one parameter. I > don't > >> expect code duplication to be an issue, but having two API's that do the > >> same thing seems like it's rife for potential confusion. > >> > > Well, if one API is marked as deprecated, then there will be no confusion > > for users, since using the wrong one will give a warning pointing to the > > right one. > > > >> If we add a new API, we can then either remove the old API entirely in > >> 22.11 (effectively renaming it), or we remove the new API in 22.11 and > >> rename it back to the old function name. I don't think neither of these > >> is a good solution, as we risk introducing more users for the API that > >> will later change. > > The new API will not be renamed to the old one, since that would break > apps > > using it without proper deprecation process. Removing the old one alone > > would be the approach to be used, but it would be correctly following the > > deprecation process and giving users at least 1 year, if no 2, of notice > > about the change. > > > >> > >> I think the pain of updating current software for 21.11 (while keeping > >> compatibility with 20.11 ABI!) is going to happen regardless, and whether > we > >> decide to add a "temporary" new API or permanently rename the old one. > It's > >> (in my opinion) easier to just bite the bullet and update the function in > >> 21.11. > > I fail to see the issue with adding a new function. Whether we add a new > > function or add a parameter to the existing one, code will have to change > > either way. The advantage of the former scheme, adding the new function, > is > > that it shows that we are serious about our ABI/API compatibility process, > > and are not lax about passing exceptions when other options are available. > > > >> > >> However, if the tech board feels like adding a new API is a good solution, > >> then okay, but we need to flesh out roadmap a bit better. Do we rename > the > >> old API, or do we add a temporary new API? > > > > New API added, old API deprecated. In future old API goes away leaving > new > > API as the only option. > > > > /Bruce > > > > Okay, so it's settled then. I revoke my ack for this patch, and we need > a new deprecation notice.
A new depreciation notice was sent [1], targeting for API change in DPDK-22.02. For the unmapping issue mentioned before, we developed a compromised solution to optimize the partial unmap logic in DPDK-21.11, and it is compatible with current API. [1] https://mails.dpdk.org/archives/dev/2021-August/217802.html Thanks for your suggestion and support! Regards, Xuan > > -- > Thanks, > Anatoly
