[v8,03/10] drivers: relax dependency order

  From: Thomas Monjalon <thomas@monjalon.net>

Drivers dependencies are evaluated in the order defined per
their parent directory (also called class).
This strict ordering prevent from having 2 different drivers
of the same class with different dependencies ordering.
This problem occurs if drivers/common/mlx5 depends on drivers/bus/pci,
while drivers/bus/dpaa depends on drivers/common/dpaax.
Having a strict ordering between directories bus and common
is too much restrictive.

That's why it is made possible to have a more fine-grain directory list,
adding a driver sub-directory in the list.
In this case, the isolated driver must be removed from its class list,
and added directly in drivers/meson.build.
Also, the per-class variables must be duplicated in the isolated driver,
because the call "subdir(class)" is skipped in the isolated driver case.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 drivers/meson.build | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)
  

On Thu, Jul 23, 2020 at 11:09:03PM +0300, Parav Pandit wrote:
> From: Thomas Monjalon <thomas@monjalon.net>
> 
> Drivers dependencies are evaluated in the order defined per
> their parent directory (also called class).
> This strict ordering prevent from having 2 different drivers
> of the same class with different dependencies ordering.
> This problem occurs if drivers/common/mlx5 depends on drivers/bus/pci,
> while drivers/bus/dpaa depends on drivers/common/dpaax.
> Having a strict ordering between directories bus and common
> is too much restrictive.
> 
> That's why it is made possible to have a more fine-grain directory list,
> adding a driver sub-directory in the list.
> In this case, the isolated driver must be removed from its class list,
> and added directly in drivers/meson.build.
> Also, the per-class variables must be duplicated in the isolated driver,
> because the call "subdir(class)" is skipped in the isolated driver case.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

The commit log above has some strange word-wrapping, and occasionally
strange phrasing. I think it could be slightly reworded, perhaps as:

  Drivers dependencies are evaluated in the order defined per their parent
  directory (also called class). This strict ordering prevents from us
  from having pairs of drivers from two classes with different dependency
  ordering. For example, if the mlx5 common code depends on the pci bus
  driver, while the dpaax common code is itself a dependency of the dpaa
  bus driver.  Having a strict ordering between directories bus and common
  is too restrictive, as processing either common drivers or bus drivers
  first leads us to missing dependencies in this scenario.
   
  This patch makes it possible to have a more fine-grain directory list,
  adding a specific driver sub-directory in the top-level drivers
  subdirectory list. In this case, the isolated driver must also be removed
  from its class list, and the per-class variables must be duplicated in
  the isolated driver, because the call "subdir(class)" is skipped in the
  isolated driver case.
  

Apart from that, I think this is a good idea to give us some flexibility
in managing driver ordering which should help other drivers too -
perhaps QAT? Ideally, though, I'd like if we can limit the flexible
ordering to *only* common code, in which case we could move the
per-class variables for common to the top-level to prevent duplication,
and maybe even get rid of common/meson.build completely. That, however,
will depend on how much this feature gets used and by whom.

Therefore:
Review-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
  

On Thu, Jul 23, 2020 at 10:10 PM Parav Pandit <parav@mellanox.com> wrote:
>
> From: Thomas Monjalon <thomas@monjalon.net>
>
> Drivers dependencies are evaluated in the order defined per
> their parent directory (also called class).
> This strict ordering prevent from having 2 different drivers
> of the same class with different dependencies ordering.
> This problem occurs if drivers/common/mlx5 depends on drivers/bus/pci,
> while drivers/bus/dpaa depends on drivers/common/dpaax.
> Having a strict ordering between directories bus and common
> is too much restrictive.
>
> That's why it is made possible to have a more fine-grain directory list,
> adding a driver sub-directory in the list.
> In this case, the isolated driver must be removed from its class list,
> and added directly in drivers/meson.build.
> Also, the per-class variables must be duplicated in the isolated driver,
> because the call "subdir(class)" is skipped in the isolated driver case.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  drivers/meson.build | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/meson.build b/drivers/meson.build
> index e2aeba931..e6d0409aa 100644
> --- a/drivers/meson.build
> +++ b/drivers/meson.build
> @@ -1,8 +1,8 @@
>  # SPDX-License-Identifier: BSD-3-Clause
>  # Copyright(c) 2017-2019 Intel Corporation
>
> -# Defines the order in which the drivers are buit.
> -dpdk_driver_classes = [
> +# Defines the order of dependencies evaluation
> +subdirs = [
>         'common',
>         'bus',
>         'mempool', # depends on common and bus.
> @@ -27,7 +27,7 @@ if cc.has_argument('-Wno-format-truncation')
>         default_cflags += '-Wno-format-truncation'
>  endif
>
> -foreach class:dpdk_driver_classes
> +foreach subpath:subdirs
>         drivers = []
>         std_deps = []
>         config_flag_fmt = '' # format string used to set the value in dpdk_conf
> @@ -35,8 +35,22 @@ foreach class:dpdk_driver_classes
>                              # the library, the dependency and to find the
>                              # version file for linking
>
> -       subdir(class)
> -       class_drivers = []
> +       # subpath can be either "class" or "class/driver"
> +       if subpath.contains('/')
> +               driver_path = subpath.split('/')
> +               class = driver_path[0]
> +               drivers += driver_path[1]
> +       else
> +               class = subpath
> +               subdir(class)
> +       endif
> +
> +       # save class name on first occurence
> +       if not dpdk_driver_classes.contains(class)
> +               dpdk_driver_classes += class
> +       endif
> +       # get already enabled drivers of the same class
> +       enabled_drivers = get_variable(class + '_drivers', [])
>
>         foreach drv:drivers
>                 drv_path = join_paths(class, drv)
> @@ -96,7 +110,7 @@ foreach class:dpdk_driver_classes
>                                                 '_disable_reason', reason)
>                         endif
>                 else
> -                       class_drivers += name
> +                       enabled_drivers += name
>
>                         if fmt_name == ''
>                                 fmt_name = name
> @@ -203,5 +217,5 @@ foreach class:dpdk_driver_classes
>                 endif # build
>         endforeach
>
> -       set_variable(class + '_drivers', class_drivers)
> +       set_variable(class + '_drivers', enabled_drivers)
>  endforeach
> --
> 2.25.4
>

Reviewed-by: David Marchand <david.marchand@redhat.com>
  

Hi Bruce,

> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday, July 24, 2020 4:37 PM
> 
> On Thu, Jul 23, 2020 at 11:09:03PM +0300, Parav Pandit wrote:
> > From: Thomas Monjalon <thomas@monjalon.net>
> >
> > Drivers dependencies are evaluated in the order defined per their
> > parent directory (also called class).
> > This strict ordering prevent from having 2 different drivers of the
> > same class with different dependencies ordering.
> > This problem occurs if drivers/common/mlx5 depends on drivers/bus/pci,
> > while drivers/bus/dpaa depends on drivers/common/dpaax.
> > Having a strict ordering between directories bus and common is too
> > much restrictive.
> >
> > That's why it is made possible to have a more fine-grain directory
> > list, adding a driver sub-directory in the list.
> > In this case, the isolated driver must be removed from its class list,
> > and added directly in drivers/meson.build.
> > Also, the per-class variables must be duplicated in the isolated
> > driver, because the call "subdir(class)" is skipped in the isolated driver case.
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> 
> The commit log above has some strange word-wrapping, and occasionally
> strange phrasing. I think it could be slightly reworded, perhaps as:
> 
I updated the commit log as you suggested below along with RB, ack tag.
Thank you.

>   Drivers dependencies are evaluated in the order defined per their parent
>   directory (also called class). This strict ordering prevents from us
>   from having pairs of drivers from two classes with different dependency
>   ordering. For example, if the mlx5 common code depends on the pci bus
>   driver, while the dpaax common code is itself a dependency of the dpaa
>   bus driver.  Having a strict ordering between directories bus and common
>   is too restrictive, as processing either common drivers or bus drivers
>   first leads us to missing dependencies in this scenario.
> 
>   This patch makes it possible to have a more fine-grain directory list,
>   adding a specific driver sub-directory in the top-level drivers
>   subdirectory list. In this case, the isolated driver must also be removed
>   from its class list, and the per-class variables must be duplicated in
>   the isolated driver, because the call "subdir(class)" is skipped in the
>   isolated driver case.
> 
> 
> Apart from that, I think this is a good idea to give us some flexibility in
> managing driver ordering which should help other drivers too - perhaps QAT?
> Ideally, though, I'd like if we can limit the flexible ordering to *only* common
> code, in which case we could move the per-class variables for common to the
> top-level to prevent duplication, and maybe even get rid of
> common/meson.build completely. That, however, will depend on how much
> this feature gets used and by whom.
> 
> Therefore:
> Review-by: Bruce Richardson <bruce.richardson@intel.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
  

24/07/2020 15:48, Parav Pandit:
> Hi Bruce,
> 
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Friday, July 24, 2020 4:37 PM
> > 
> > On Thu, Jul 23, 2020 at 11:09:03PM +0300, Parav Pandit wrote:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > >
> > > Drivers dependencies are evaluated in the order defined per their
> > > parent directory (also called class).
> > > This strict ordering prevent from having 2 different drivers of the
> > > same class with different dependencies ordering.
> > > This problem occurs if drivers/common/mlx5 depends on drivers/bus/pci,
> > > while drivers/bus/dpaa depends on drivers/common/dpaax.
> > > Having a strict ordering between directories bus and common is too
> > > much restrictive.
> > >
> > > That's why it is made possible to have a more fine-grain directory
> > > list, adding a driver sub-directory in the list.
> > > In this case, the isolated driver must be removed from its class list,
> > > and added directly in drivers/meson.build.
> > > Also, the per-class variables must be duplicated in the isolated
> > > driver, because the call "subdir(class)" is skipped in the isolated driver case.
> > >
> > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > 
> > The commit log above has some strange word-wrapping, and occasionally
> > strange phrasing. I think it could be slightly reworded, perhaps as:
> > 
> I updated the commit log as you suggested below along with RB, ack tag.
> Thank you.
> 
> >   Drivers dependencies are evaluated in the order defined per their parent
> >   directory (also called class). This strict ordering prevents from us

Is "from us" too much?

> >   from having pairs of drivers from two classes with different dependency
> >   ordering. For example, if the mlx5 common code depends on the pci bus
> >   driver, while the dpaax common code is itself a dependency of the dpaa
> >   bus driver.  Having a strict ordering between directories bus and common
> >   is too restrictive, as processing either common drivers or bus drivers
> >   first leads us to missing dependencies in this scenario.
> > 
> >   This patch makes it possible to have a more fine-grain directory list,
> >   adding a specific driver sub-directory in the top-level drivers
> >   subdirectory list. In this case, the isolated driver must also be removed
> >   from its class list, and the per-class variables must be duplicated in
> >   the isolated driver, because the call "subdir(class)" is skipped in the
> >   isolated driver case.
> > 
> > 
> > Apart from that, I think this is a good idea to give us some flexibility in
> > managing driver ordering which should help other drivers too - perhaps QAT?
> > Ideally, though, I'd like if we can limit the flexible ordering to *only* common
> > code, in which case we could move the per-class variables for common to the
> > top-level to prevent duplication, and maybe even get rid of
> > common/meson.build completely. That, however, will depend on how much
> > this feature gets used and by whom.

For now it is used only to have common/mlx5 isolated of the rest
of common drivers, as an exception.
I think it is good to keep common/meson.build
until there are more exceptions than the rest.
  

On Fri, Jul 24, 2020 at 03:54:33PM +0200, Thomas Monjalon wrote:
> 24/07/2020 15:48, Parav Pandit:
> > Hi Bruce,
> > 
> > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > Sent: Friday, July 24, 2020 4:37 PM
> > > 
> > > On Thu, Jul 23, 2020 at 11:09:03PM +0300, Parav Pandit wrote:
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > >
> > > > Drivers dependencies are evaluated in the order defined per their
> > > > parent directory (also called class).
> > > > This strict ordering prevent from having 2 different drivers of the
> > > > same class with different dependencies ordering.
> > > > This problem occurs if drivers/common/mlx5 depends on drivers/bus/pci,
> > > > while drivers/bus/dpaa depends on drivers/common/dpaax.
> > > > Having a strict ordering between directories bus and common is too
> > > > much restrictive.
> > > >
> > > > That's why it is made possible to have a more fine-grain directory
> > > > list, adding a driver sub-directory in the list.
> > > > In this case, the isolated driver must be removed from its class list,
> > > > and added directly in drivers/meson.build.
> > > > Also, the per-class variables must be duplicated in the isolated
> > > > driver, because the call "subdir(class)" is skipped in the isolated driver case.
> > > >
> > > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > > 
> > > The commit log above has some strange word-wrapping, and occasionally
> > > strange phrasing. I think it could be slightly reworded, perhaps as:
> > > 
> > I updated the commit log as you suggested below along with RB, ack tag.
> > Thank you.
> > 
> > >   Drivers dependencies are evaluated in the order defined per their parent
> > >   directory (also called class). This strict ordering prevents from us
> 
> Is "from us" too much?

The word "from" is incorrect, anyway, it should just be "prevents us".
Evidently I didn't proofread my own text well enough. :-(

> 
> > >   from having pairs of drivers from two classes with different dependency
> > >   ordering. For example, if the mlx5 common code depends on the pci bus
> > >   driver, while the dpaax common code is itself a dependency of the dpaa
> > >   bus driver.  Having a strict ordering between directories bus and common
> > >   is too restrictive, as processing either common drivers or bus drivers
> > >   first leads us to missing dependencies in this scenario.
> > > 
> > >   This patch makes it possible to have a more fine-grain directory list,
> > >   adding a specific driver sub-directory in the top-level drivers
> > >   subdirectory list. In this case, the isolated driver must also be removed
> > >   from its class list, and the per-class variables must be duplicated in
> > >   the isolated driver, because the call "subdir(class)" is skipped in the
> > >   isolated driver case.
> > > 
> > > 
> > > Apart from that, I think this is a good idea to give us some flexibility in
> > > managing driver ordering which should help other drivers too - perhaps QAT?
> > > Ideally, though, I'd like if we can limit the flexible ordering to *only* common
> > > code, in which case we could move the per-class variables for common to the
> > > top-level to prevent duplication, and maybe even get rid of
> > > common/meson.build completely. That, however, will depend on how much
> > > this feature gets used and by whom.
> 
> For now it is used only to have common/mlx5 isolated of the rest
> of common drivers, as an exception.
> I think it is good to keep common/meson.build
> until there are more exceptions than the rest.
> 

+1 agreed. 
Just suggesting possible future changes depending on how things
go, and also seeding the idea that any out-of-order build processing should
really be limited to the common folder.

/Bruce
  

> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday, July 24, 2020 8:20 PM
> 
> On Fri, Jul 24, 2020 at 03:54:33PM +0200, Thomas Monjalon wrote:
> > 24/07/2020 15:48, Parav Pandit:
> > > Hi Bruce,
> > >
> > > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > Sent: Friday, July 24, 2020 4:37 PM
> > > >
> > > > On Thu, Jul 23, 2020 at 11:09:03PM +0300, Parav Pandit wrote:
> > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > >
> > > > > Drivers dependencies are evaluated in the order defined per
> > > > > their parent directory (also called class).
> > > > > This strict ordering prevent from having 2 different drivers of
> > > > > the same class with different dependencies ordering.
> > > > > This problem occurs if drivers/common/mlx5 depends on
> > > > > drivers/bus/pci, while drivers/bus/dpaa depends on
> drivers/common/dpaax.
> > > > > Having a strict ordering between directories bus and common is
> > > > > too much restrictive.
> > > > >
> > > > > That's why it is made possible to have a more fine-grain
> > > > > directory list, adding a driver sub-directory in the list.
> > > > > In this case, the isolated driver must be removed from its class
> > > > > list, and added directly in drivers/meson.build.
> > > > > Also, the per-class variables must be duplicated in the isolated
> > > > > driver, because the call "subdir(class)" is skipped in the isolated driver
> case.
> > > > >
> > > > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > > >
> > > > The commit log above has some strange word-wrapping, and
> > > > occasionally strange phrasing. I think it could be slightly reworded,
> perhaps as:
> > > >
> > > I updated the commit log as you suggested below along with RB, ack tag.
> > > Thank you.
> > >
> > > >   Drivers dependencies are evaluated in the order defined per their
> parent
> > > >   directory (also called class). This strict ordering prevents
> > > > from us
> >
> > Is "from us" too much?
> 
> The word "from" is incorrect, anyway, it should just be "prevents us".
> Evidently I didn't proofread my own text well enough. :-(

In v10 I rephase it as "strict ordering prevents from having pairs of drivers...".
  

On Fri, Jul 24, 2020 at 03:17:38PM +0000, Parav Pandit wrote:
> 
> 
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Friday, July 24, 2020 8:20 PM
> > 
> > On Fri, Jul 24, 2020 at 03:54:33PM +0200, Thomas Monjalon wrote:
> > > 24/07/2020 15:48, Parav Pandit:
> > > > Hi Bruce,
> > > >
> > > > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > > Sent: Friday, July 24, 2020 4:37 PM
> > > > >
> > > > > On Thu, Jul 23, 2020 at 11:09:03PM +0300, Parav Pandit wrote:
> > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > >
> > > > > > Drivers dependencies are evaluated in the order defined per
> > > > > > their parent directory (also called class).
> > > > > > This strict ordering prevent from having 2 different drivers of
> > > > > > the same class with different dependencies ordering.
> > > > > > This problem occurs if drivers/common/mlx5 depends on
> > > > > > drivers/bus/pci, while drivers/bus/dpaa depends on
> > drivers/common/dpaax.
> > > > > > Having a strict ordering between directories bus and common is
> > > > > > too much restrictive.
> > > > > >
> > > > > > That's why it is made possible to have a more fine-grain
> > > > > > directory list, adding a driver sub-directory in the list.
> > > > > > In this case, the isolated driver must be removed from its class
> > > > > > list, and added directly in drivers/meson.build.
> > > > > > Also, the per-class variables must be duplicated in the isolated
> > > > > > driver, because the call "subdir(class)" is skipped in the isolated driver
> > case.
> > > > > >
> > > > > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > > > >
> > > > > The commit log above has some strange word-wrapping, and
> > > > > occasionally strange phrasing. I think it could be slightly reworded,
> > perhaps as:
> > > > >
> > > > I updated the commit log as you suggested below along with RB, ack tag.
> > > > Thank you.
> > > >
> > > > >   Drivers dependencies are evaluated in the order defined per their
> > parent
> > > > >   directory (also called class). This strict ordering prevents
> > > > > from us
> > >
> > > Is "from us" too much?
> > 
> > The word "from" is incorrect, anyway, it should just be "prevents us".
> > Evidently I didn't proofread my own text well enough. :-(
> 
> In v10 I rephase it as "strict ordering prevents from having pairs of drivers...".

Great, thanks.