On Thu, Jun 01, 2023 at 12:23:39PM +0000, Zhang, Qi Z wrote:
>
>
> > -----Original Message-----
> > From: David Marchand <david.march...@redhat.com>
> > Sent: Thursday, June 1, 2023 4:50 PM
> > To: Richardson, Bruce <bruce.richard...@intel.com>; Tyler Retzlaff
> > <roret...@linux.microsoft.com>
> > Cc: dev@dpdk.org; tho...@monjalon.net; Zhang, Qi Z
> > <qi.z.zh...@intel.com>
> > Subject: Re: [PATCH 0/6] windows: remove most pthread lifetime shim
> > functions
> >
> > On Tue, Apr 18, 2023 at 11:21 AM Bruce Richardson
> > <bruce.richard...@intel.com> wrote:
> > >
> > > On Sun, Apr 02, 2023 at 10:34:12PM -0700, Tyler Retzlaff wrote:
> > > > early review if possible please, would like to have this in from the
> > > > start of 23.07 to work against.
> > > >
> > > > thanks!
> > > >
> > >
> > > Don't see any problems with this set.
> >
> > Drivers maintainers were not copied (Tyler, git send-email has options --to-
> > cmd or --cc-cmd to which you can pass ./devtools/get-maintainers.sh).
> > I pinged Qi during the maintainers call today.
>
> Hi Tyler & David:
>
> The patchset looks good to me.
>
> I have just one question regarding the patch set targets, which include PMD
> iavf, ice, and ixgbe. However, I noticed that some other Intel PMDs like
> ipn3ke still use rte_ctrl_thread_create and have not been replaced.
The series really isn't about rte_ctrl_thread_create, it just happens
that for code built on Windows that code needs to stop using
rte_ctrl_thread_create because it references the pthread shim that is
being removed.
At some point in the future (it's lower priority right now) I will
submit a series that converts all rte_ctrl_thread_create ->
rte_control_thread_create since rte_ctrl_thread_create is being
deprecated.
>
> I assume that this replacement is specifically intended for PMDs that support
> Windows, as PMDs with the "Windows" feature should be covered. However, I
> didn't see the "Windows" feature enabled for iavf PMD, even though it is
> included in the patch set.
>
> Could you help me understand the criteria used for determining which PMDs
> should be included in this replacement?
Yes, you are correct the patch is intended to address PMDs / code built
on Windows specifically. I just re-verified that iavf is being built
for Windows.
If I remove the iavf patch from the series I get the following warning,
so that is why I adapted the iavf PMD code.
[249/749] Compiling C object
drivers/libtmp_rte_net_iavf.a.p/net_iavf_iavf_vchnl.c.obj
../drivers/net/iavf/iavf_vchnl.c:162:2: warning: implicit declaration of
function 'pthread_join' is invalid in C99
[-Wimplicit-function-declaration]
pthread_join(handler->tid, NULL);
>
> Thanks
> Qi
>
> >
> > The changes are straightforward and lgtm.
> > For the series,
> > Reviewed-by: David Marchand <david.march...@redhat.com>
I think with the above explained the series should be okay as is, no
changes required if Qi is okay with the above explanation.
> >
> >
> > --
> > David Marchand
>
