From: Leon Romanovsky <leon@kernel.org>
To: Yongxin Liu <yongxin.liu@windriver.com>
Cc: david.m.ertman@intel.com, shiraz.saleem@intel.com,
anthony.l.nguyen@intel.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, davem@davemloft.net,
jesse.brandeburg@intel.com, intel-wired-lan@lists.osuosl.org,
kuba@kernel.org
Subject: Re: [PATCH net] ice: check whether AUX devices/drivers are supported in ice_rebuild
Date: Sun, 5 Sep 2021 10:23:36 +0300 [thread overview]
Message-ID: <YTRweH4JMbzUtxLf@unreal> (raw)
In-Reply-To: <20210903012500.39407-1-yongxin.liu@windriver.com>
On Fri, Sep 03, 2021 at 09:25:00AM +0800, Yongxin Liu wrote:
> In ice_rebuild(), check whether AUX devices/drivers are supported or not
> before calling ice_plug_aux_dev().
>
> Fix the following call trace, if RDMA functionality is not available.
>
> auxiliary ice.roce.0: adding auxiliary device failed!: -17
> sysfs: cannot create duplicate filename '/bus/auxiliary/devices/ice.roce.0'
> Workqueue: ice ice_service_task [ice]
> Call Trace:
> dump_stack_lvl+0x38/0x49
> dump_stack+0x10/0x12
> sysfs_warn_dup+0x5b/0x70
> sysfs_do_create_link_sd.isra.2+0xc8/0xd0
> sysfs_create_link+0x25/0x40
> bus_add_device+0x6d/0x110
> device_add+0x49d/0x940
> ? _printk+0x52/0x6e
> ? _printk+0x52/0x6e
> __auxiliary_device_add+0x60/0xc0
> ice_plug_aux_dev+0xd3/0xf0 [ice]
> ice_rebuild+0x27d/0x510 [ice]
> ice_do_reset+0x51/0xe0 [ice]
> ice_service_task+0x108/0xe70 [ice]
> ? __switch_to+0x13b/0x510
> process_one_work+0x1de/0x420
> ? apply_wqattrs_cleanup+0xc0/0xc0
> worker_thread+0x34/0x400
> ? apply_wqattrs_cleanup+0xc0/0xc0
> kthread+0x14d/0x180
> ? set_kthread_struct+0x40/0x40
> ret_from_fork+0x1f/0x30
>
> Fixes: f9f5301e7e2d ("ice: Register auxiliary device to provide RDMA")
> Signed-off-by: Yongxin Liu <yongxin.liu@windriver.com>
> ---
> drivers/net/ethernet/intel/ice/ice_main.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index 0d6c143f6653..98cc708e9517 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -6466,7 +6466,9 @@ static void ice_rebuild(struct ice_pf *pf, enum ice_reset_req reset_type)
> /* if we get here, reset flow is successful */
> clear_bit(ICE_RESET_FAILED, pf->state);
>
> - ice_plug_aux_dev(pf);
> + if (ice_is_aux_ena(pf))
> + ice_plug_aux_dev(pf);
> +
The change is ok, but it hints that auxiliary bus is used horribly wrong
in this driver. In proper implementation, which should rely on driver/core,
every subdriver like ice.eth, ice.roce e.t.c is supposed to be retriggered
by the code and shouldn't ave "if (ice_is_aux_ena(pf))" checks.
Thanks
> return;
>
> err_vsi_rebuild:
> --
> 2.14.5
>
WARNING: multiple messages have this Message-ID (diff)
From: Leon Romanovsky <leon@kernel.org>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [PATCH net] ice: check whether AUX devices/drivers are supported in ice_rebuild
Date: Sun, 5 Sep 2021 10:23:36 +0300 [thread overview]
Message-ID: <YTRweH4JMbzUtxLf@unreal> (raw)
In-Reply-To: <20210903012500.39407-1-yongxin.liu@windriver.com>
On Fri, Sep 03, 2021 at 09:25:00AM +0800, Yongxin Liu wrote:
> In ice_rebuild(), check whether AUX devices/drivers are supported or not
> before calling ice_plug_aux_dev().
>
> Fix the following call trace, if RDMA functionality is not available.
>
> auxiliary ice.roce.0: adding auxiliary device failed!: -17
> sysfs: cannot create duplicate filename '/bus/auxiliary/devices/ice.roce.0'
> Workqueue: ice ice_service_task [ice]
> Call Trace:
> dump_stack_lvl+0x38/0x49
> dump_stack+0x10/0x12
> sysfs_warn_dup+0x5b/0x70
> sysfs_do_create_link_sd.isra.2+0xc8/0xd0
> sysfs_create_link+0x25/0x40
> bus_add_device+0x6d/0x110
> device_add+0x49d/0x940
> ? _printk+0x52/0x6e
> ? _printk+0x52/0x6e
> __auxiliary_device_add+0x60/0xc0
> ice_plug_aux_dev+0xd3/0xf0 [ice]
> ice_rebuild+0x27d/0x510 [ice]
> ice_do_reset+0x51/0xe0 [ice]
> ice_service_task+0x108/0xe70 [ice]
> ? __switch_to+0x13b/0x510
> process_one_work+0x1de/0x420
> ? apply_wqattrs_cleanup+0xc0/0xc0
> worker_thread+0x34/0x400
> ? apply_wqattrs_cleanup+0xc0/0xc0
> kthread+0x14d/0x180
> ? set_kthread_struct+0x40/0x40
> ret_from_fork+0x1f/0x30
>
> Fixes: f9f5301e7e2d ("ice: Register auxiliary device to provide RDMA")
> Signed-off-by: Yongxin Liu <yongxin.liu@windriver.com>
> ---
> drivers/net/ethernet/intel/ice/ice_main.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index 0d6c143f6653..98cc708e9517 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -6466,7 +6466,9 @@ static void ice_rebuild(struct ice_pf *pf, enum ice_reset_req reset_type)
> /* if we get here, reset flow is successful */
> clear_bit(ICE_RESET_FAILED, pf->state);
>
> - ice_plug_aux_dev(pf);
> + if (ice_is_aux_ena(pf))
> + ice_plug_aux_dev(pf);
> +
The change is ok, but it hints that auxiliary bus is used horribly wrong
in this driver. In proper implementation, which should rely on driver/core,
every subdriver like ice.eth, ice.roce e.t.c is supposed to be retriggered
by the code and shouldn't ave "if (ice_is_aux_ena(pf))" checks.
Thanks
> return;
>
> err_vsi_rebuild:
> --
> 2.14.5
>
next prev parent reply other threads:[~2021-09-05 7:23 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-03 1:25 [PATCH net] ice: check whether AUX devices/drivers are supported in ice_rebuild Yongxin Liu
2021-09-03 1:25 ` [Intel-wired-lan] " Yongxin Liu
2021-09-05 7:23 ` Leon Romanovsky [this message]
2021-09-05 7:23 ` Leon Romanovsky
2021-09-08 17:30 ` Ertman, David M
2021-09-08 17:30 ` [Intel-wired-lan] " Ertman, David M
2021-09-08 19:14 ` Liu, Yongxin
2021-09-08 19:14 ` [Intel-wired-lan] " Liu, Yongxin
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=YTRweH4JMbzUtxLf@unreal \
--to=leon@kernel.org \
--cc=anthony.l.nguyen@intel.com \
--cc=davem@davemloft.net \
--cc=david.m.ertman@intel.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=jesse.brandeburg@intel.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=shiraz.saleem@intel.com \
--cc=yongxin.liu@windriver.com \
/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: link
Be 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.