All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kalle Valo <kvalo@kernel.org>
To: z <zhaojunkui2008@126.com>
Cc: "Jakub Kicinski" <kubakici@wp.pl>,
	"David S. Miller" <davem@davemloft.net>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
	bernard@vivo.com
Subject: Re: [PATCH v2] mediatek/mt7601u: add debugfs exit function
Date: Mon, 25 Apr 2022 09:01:18 +0300	[thread overview]
Message-ID: <87h76hk8gh.fsf@kernel.org> (raw)
In-Reply-To: <15b4f.2aad.1805ece06a1.Coremail.zhaojunkui2008@126.com> (z.'s message of "Mon, 25 Apr 2022 11:40:02 +0800 (CST)")

z  <zhaojunkui2008@126.com> writes:

> At 2022-04-23 03:47:04, "Jakub Kicinski" <kubakici@wp.pl> wrote:
>>On Fri, 22 Apr 2022 01:08:54 -0700 Bernard Zhao wrote:
>>> When mt7601u loaded, there are two cases:
>>> First when mt7601u is loaded, in function mt7601u_probe, if
>>> function mt7601u_probe run into error lable err_hw,
>>> mt7601u_cleanup didn`t cleanup the debugfs node.
>>> Second when the module disconnect, in function mt7601u_disconnect,
>>> mt7601u_cleanup didn`t cleanup the debugfs node.
>>> This patch add debugfs exit function and try to cleanup debugfs
>>> node when mt7601u loaded fail or unloaded.
>>> 
>>> Signed-off-by: Bernard Zhao <zhaojunkui2008@126.com>
>>
>>Ah, missed that there was a v2. My point stands, wiphy debugfs dir
>>should do the cleanup.
>>
>>Do you encounter problems in practice or are you sending this patches
>>based on reading / static analysis of the code only.
>
> Hi Jakub Kicinski:
>
> The issue here is found by reading code.
> I read the drivers/net/wireless code and found that many modules are
> not cleanup the debugfs.
> I sorted out the modules that were not cleaned up the debugfs:
> ./ti/wl18xx
> ./ti/wl12xx
> ./intel/iwlwifi
> ./intel/iwlwifi
> ./mediatek/mt76
> I am not sure whether this part is welcome to kernel so I submitted a patch.
> If you have any suggestions, welcome to put forward for discussion, thank you！

Jakub is saying that wiphy_unregister() recursively removes the debugfs
directories:

	/*
	 * First remove the hardware from everywhere, this makes
	 * it impossible to find from userspace.
	 */
	debugfs_remove_recursive(rdev->wiphy.debugfsdir);

So AFAICS there is no bug. But if you are testing this on a real
hardware and something is wrong, please provide more info.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

WARNING: multiple messages have this Message-ID (diff)
From: Kalle Valo <kvalo@kernel.org>
To: z <zhaojunkui2008@126.com>
Cc: "Jakub Kicinski" <kubakici@wp.pl>,
	"David S. Miller" <davem@davemloft.net>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
	bernard@vivo.com
Subject: Re: [PATCH v2] mediatek/mt7601u: add debugfs exit function
Date: Mon, 25 Apr 2022 09:01:18 +0300	[thread overview]
Message-ID: <87h76hk8gh.fsf@kernel.org> (raw)
In-Reply-To: <15b4f.2aad.1805ece06a1.Coremail.zhaojunkui2008@126.com> (z.'s message of "Mon, 25 Apr 2022 11:40:02 +0800 (CST)")

z  <zhaojunkui2008@126.com> writes:

> At 2022-04-23 03:47:04, "Jakub Kicinski" <kubakici@wp.pl> wrote:
>>On Fri, 22 Apr 2022 01:08:54 -0700 Bernard Zhao wrote:
>>> When mt7601u loaded, there are two cases:
>>> First when mt7601u is loaded, in function mt7601u_probe, if
>>> function mt7601u_probe run into error lable err_hw,
>>> mt7601u_cleanup didn`t cleanup the debugfs node.
>>> Second when the module disconnect, in function mt7601u_disconnect,
>>> mt7601u_cleanup didn`t cleanup the debugfs node.
>>> This patch add debugfs exit function and try to cleanup debugfs
>>> node when mt7601u loaded fail or unloaded.
>>> 
>>> Signed-off-by: Bernard Zhao <zhaojunkui2008@126.com>
>>
>>Ah, missed that there was a v2. My point stands, wiphy debugfs dir
>>should do the cleanup.
>>
>>Do you encounter problems in practice or are you sending this patches
>>based on reading / static analysis of the code only.
>
> Hi Jakub Kicinski:
>
> The issue here is found by reading code.
> I read the drivers/net/wireless code and found that many modules are
> not cleanup the debugfs.
> I sorted out the modules that were not cleaned up the debugfs:
> ./ti/wl18xx
> ./ti/wl12xx
> ./intel/iwlwifi
> ./intel/iwlwifi
> ./mediatek/mt76
> I am not sure whether this part is welcome to kernel so I submitted a patch.
> If you have any suggestions, welcome to put forward for discussion, thank you！

Jakub is saying that wiphy_unregister() recursively removes the debugfs
directories:

	/*
	 * First remove the hardware from everywhere, this makes
	 * it impossible to find from userspace.
	 */
	debugfs_remove_recursive(rdev->wiphy.debugfsdir);

So AFAICS there is no bug. But if you are testing this on a real
hardware and something is wrong, please provide more info.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

WARNING: multiple messages have this Message-ID (diff)
From: Kalle Valo <kvalo@kernel.org>
To: z <zhaojunkui2008@126.com>
Cc: "Jakub Kicinski" <kubakici@wp.pl>,
	"David S. Miller" <davem@davemloft.net>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
	bernard@vivo.com
Subject: Re: [PATCH v2] mediatek/mt7601u: add debugfs exit function
Date: Mon, 25 Apr 2022 09:01:18 +0300	[thread overview]
Message-ID: <87h76hk8gh.fsf@kernel.org> (raw)
In-Reply-To: <15b4f.2aad.1805ece06a1.Coremail.zhaojunkui2008@126.com> (z.'s message of "Mon, 25 Apr 2022 11:40:02 +0800 (CST)")

z  <zhaojunkui2008@126.com> writes:

> At 2022-04-23 03:47:04, "Jakub Kicinski" <kubakici@wp.pl> wrote:
>>On Fri, 22 Apr 2022 01:08:54 -0700 Bernard Zhao wrote:
>>> When mt7601u loaded, there are two cases:
>>> First when mt7601u is loaded, in function mt7601u_probe, if
>>> function mt7601u_probe run into error lable err_hw,
>>> mt7601u_cleanup didn`t cleanup the debugfs node.
>>> Second when the module disconnect, in function mt7601u_disconnect,
>>> mt7601u_cleanup didn`t cleanup the debugfs node.
>>> This patch add debugfs exit function and try to cleanup debugfs
>>> node when mt7601u loaded fail or unloaded.
>>> 
>>> Signed-off-by: Bernard Zhao <zhaojunkui2008@126.com>
>>
>>Ah, missed that there was a v2. My point stands, wiphy debugfs dir
>>should do the cleanup.
>>
>>Do you encounter problems in practice or are you sending this patches
>>based on reading / static analysis of the code only.
>
> Hi Jakub Kicinski:
>
> The issue here is found by reading code.
> I read the drivers/net/wireless code and found that many modules are
> not cleanup the debugfs.
> I sorted out the modules that were not cleaned up the debugfs:
> ./ti/wl18xx
> ./ti/wl12xx
> ./intel/iwlwifi
> ./intel/iwlwifi
> ./mediatek/mt76
> I am not sure whether this part is welcome to kernel so I submitted a patch.
> If you have any suggestions, welcome to put forward for discussion, thank you！

Jakub is saying that wiphy_unregister() recursively removes the debugfs
directories:

	/*
	 * First remove the hardware from everywhere, this makes
	 * it impossible to find from userspace.
	 */
	debugfs_remove_recursive(rdev->wiphy.debugfsdir);

So AFAICS there is no bug. But if you are testing this on a real
hardware and something is wrong, please provide more info.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2022-04-25  6:01 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-22  8:08 [PATCH v2] mediatek/mt7601u: add debugfs exit function Bernard Zhao
2022-04-22  8:08 ` Bernard Zhao
2022-04-22  8:08 ` Bernard Zhao
2022-04-22 19:47 ` Jakub Kicinski
2022-04-22 19:47   ` Jakub Kicinski
2022-04-22 19:47   ` Jakub Kicinski
2022-04-25  3:40   ` z
2022-04-25  3:40     ` z
2022-04-25  3:40     ` z
2022-04-25  6:01     ` Kalle Valo [this message]
2022-04-25  6:01       ` Kalle Valo
2022-04-25  6:01       ` Kalle Valo

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=87h76hk8gh.fsf@kernel.org \
    --to=kvalo@kernel.org \
    --cc=bernard@vivo.com \
    --cc=davem@davemloft.net \
    --cc=kubakici@wp.pl \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=matthias.bgg@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=zhaojunkui2008@126.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.