On 2015-07-03 04:13 PM, Mel Gorman wrote:
> On Fri, Jul 03, 2015 at 02:45:02PM -0400, Theodore Ts'o wrote:
>> On Fri, Jul 03, 2015 at 12:52:06PM -0400, nick wrote:
>>> I agree with you 100 percent. The reason I can't test this is I don't have the
>>> hardware otherwise I would have tested it by now.
>>
>> Then don't send the patch out. Work on some other piece of part of
>> the kernel, or better yet, some other userspace code where testing is
>> easier. It's really quite simple.
>>
>> You don't have the technical skills, or at this point, the reputation,
>> to send patches without tesitng them first. The fact that sometimes
>> people like Linus will send out a patch labelled with "COMPLETELY
>> UNTESTED", is because he's skilled and trusted enough that he can get
>> away with it.
>
> It's not just that. In all cases I'm aware of, Linus was illustrating his
> point by means of a patch during a discussion. It was expected that the
> developer or user that started the discussion would take that patch and run
> with it if it was heading in the correct direction. In exceptional cases,
> the patch would be merged after a confirmation from that developer or user
> that the patch worked for whatever problem they faced. The only time I've
> seen a COMPLETELY UNTESTED patch merged was when it was painfully obvious it
> was correct and more importantly, it solved a specific problem. Linus is not
> the only developer that does this style of discussion through untested patch.
>
> In other cases where an untested patch has been merged, it was either due to
> it being exceptionally trivial or a major API change that affects a number
> of subsystems (like adding a new filesystem API for example). In the former
> case, it's usually self-evident and often tacked onto a larger series where
> there is a degree of trust. In the latter case, all cases they can test
> have been covered and the code for the remaining hardware was altered in
> a very similar manner. This also lends some confidence that the transform
> is ok because similar transforms were tested and known to be correct.
>
> For trivial patches that alter just return values there are a few hazards. A
> mistake can be made introducing a real bug with the upside being marginal or
> non-existent. That's a very poor tradeoff and generally why checkpatch-only
> patches fall by the wayside. Readability is a two-edged sword. Maybe the
> code is marginally easier to read but it's sometimes offset by the fact
> that git blame no longer points to the important origin of the code. If
> a real bug is being investigated then all the cleanup patches have to be
> identified and dismissed which consumes time and concentration.
>
> Cleanups in my opinion are ok in two cases. The first is if it genuinely
> makes the code much easier to follow. In cases where I've seen this, it was
> done because the code was either unreadable or it was in preparation for a
> more relevant patch series that was then easier to review and justified the
> cleanup. The second is where the affected code is being heavily modified
> anyway so the cleanup while you are there is both useful and does not
> impact git blame.
>
> This particular patch does not match any of the criteria. The DRM patch
> may or may not be correct but there is no way I'd expect something like
> it to be picked up without testing or in reference to a bug report.
>
> For this patch, NAK. Nick, from me at least consider any similar patch
> affecting mm/ that modifies return values or types without being part of
> a larger series that addresses a particular problem to be silently NAKed
> or filed under "doesn't matter" by me.
>
I am no longer going to do cleanups as they are a waste of mine and the maintainers,
thanks for pointing that out through Mel. However I did find a bad commit with the id 7202ab46f7392265c1ecbf03f600393bf32a8bdf that breaks and hangs my system on boot so
bad I can't even get into single user mode. Either one of the maintainers of that
code can revert it or we can find a proper fix to it on my broken machine with this
commit. In addition all of my patches for cleanups that don't remove unused
code have been removed, there of no use. However this patch has been on my system
for a month and is an important cleanup as it removes other 100 lines of unused
code. I also have another function removal patch for both chelsio drivers and
the mailbox api that are staying around due to them being useful in terms of
removing old/unused code.
Thanks A lot for Your and Ted's Time,
Nick
>From afa142b9826a7108825c8b0369a302a52d4be343 Mon Sep 17 00:00:00 2001
From: Nicholas Krause <xerofoify@xxxxxxxxx>
Date: Fri, 29 May 2015 23:19:43 -0400
Subject: [PATCH RESEND] wireless: Remove no longer used function definition and prototype for the function, cfg80211_can_use_iftype_chan
This removes the function, cfg80211_can_use_iftype_chan and its
prototype from the files, core.h and util.c due to having no
more users of this function. In addition due to this function
not being exported with the marco, EXPORT_SYMBOL we have no
need to worry about this function being removed now breaking
out of tree kernel code.
Signed-off-by: Nicholas Krause <xerofoify@xxxxxxxxx>
---
net/wireless/core.h | 7 ----
net/wireless/util.c | 114 ----------------------------------------------------
2 files changed, 121 deletions(-)
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 801cd49..6b68c24 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -409,13 +409,6 @@ int cfg80211_change_iface(struct cfg80211_registered_device *rdev,
void cfg80211_process_rdev_events(struct cfg80211_registered_device *rdev);
void cfg80211_process_wdev_events(struct wireless_dev *wdev);
-int cfg80211_can_use_iftype_chan(struct cfg80211_registered_device *rdev,
- struct wireless_dev *wdev,
- enum nl80211_iftype iftype,
- struct ieee80211_channel *chan,
- enum cfg80211_chan_mode chanmode,
- u8 radar_detect);
-
/**
* cfg80211_chandef_dfs_usable - checks if chandef is DFS usable
* @wiphy: the wiphy to validate against
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 70051ab..1e4f1a5 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -1619,120 +1619,6 @@ int cfg80211_check_combinations(struct wiphy *wiphy,
}
EXPORT_SYMBOL(cfg80211_check_combinations);
-int cfg80211_can_use_iftype_chan(struct cfg80211_registered_device *rdev,
- struct wireless_dev *wdev,
- enum nl80211_iftype iftype,
- struct ieee80211_channel *chan,
- enum cfg80211_chan_mode chanmode,
- u8 radar_detect)
-{
- struct wireless_dev *wdev_iter;
- int num[NUM_NL80211_IFTYPES];
- struct ieee80211_channel
- *used_channels[CFG80211_MAX_NUM_DIFFERENT_CHANNELS];
- struct ieee80211_channel *ch;
- enum cfg80211_chan_mode chmode;
- int num_different_channels = 0;
- int total = 1;
- int i;
-
- ASSERT_RTNL();
-
- if (WARN_ON(hweight32(radar_detect) > 1))
- return -EINVAL;
-
- if (WARN_ON(iftype >= NUM_NL80211_IFTYPES))
- return -EINVAL;
-
- /* Always allow software iftypes */
- if (rdev->wiphy.software_iftypes & BIT(iftype)) {
- if (radar_detect)
- return -EINVAL;
- return 0;
- }
-
- memset(num, 0, sizeof(num));
- memset(used_channels, 0, sizeof(used_channels));
-
- num[iftype] = 1;
-
- /* TODO: We'll probably not need this anymore, since this
- * should only be called with CHAN_MODE_UNDEFINED. There are
- * still a couple of pending calls where other chanmodes are
- * used, but we should get rid of them.
- */
- switch (chanmode) {
- case CHAN_MODE_UNDEFINED:
- break;
- case CHAN_MODE_SHARED:
- WARN_ON(!chan);
- used_channels[0] = chan;
- num_different_channels++;
- break;
- case CHAN_MODE_EXCLUSIVE:
- num_different_channels++;
- break;
- }
-
- list_for_each_entry(wdev_iter, &rdev->wdev_list, list) {
- if (wdev_iter == wdev)
- continue;
- if (wdev_iter->iftype == NL80211_IFTYPE_P2P_DEVICE) {
- if (!wdev_iter->p2p_started)
- continue;
- } else if (wdev_iter->netdev) {
- if (!netif_running(wdev_iter->netdev))
- continue;
- } else {
- WARN_ON(1);
- }
-
- if (rdev->wiphy.software_iftypes & BIT(wdev_iter->iftype))
- continue;
-
- /*
- * We may be holding the "wdev" mutex, but now need to lock
- * wdev_iter. This is OK because once we get here wdev_iter
- * is not wdev (tested above), but we need to use the nested
- * locking for lockdep.
- */
- mutex_lock_nested(&wdev_iter->mtx, 1);
- __acquire(wdev_iter->mtx);
- cfg80211_get_chan_state(wdev_iter, &ch, &chmode, &radar_detect);
- wdev_unlock(wdev_iter);
-
- switch (chmode) {
- case CHAN_MODE_UNDEFINED:
- break;
- case CHAN_MODE_SHARED:
- for (i = 0; i < CFG80211_MAX_NUM_DIFFERENT_CHANNELS; i++)
- if (!used_channels[i] || used_channels[i] == ch)
- break;
-
- if (i == CFG80211_MAX_NUM_DIFFERENT_CHANNELS)
- return -EBUSY;
-
- if (used_channels[i] == NULL) {
- used_channels[i] = ch;
- num_differremove rent_channels++;
- }
- break;
- case CHAN_MODE_EXCLUSIVE:
- num_different_channels++;
- break;
- }
-
- num[wdev_iter->iftype]++;
- total++;
- }
-
- if (total == 1 && !radar_detect)
- return 0;
-
- return cfg80211_check_combinations(&rdev->wiphy, num_different_channels,
- radar_detect, num);
-}
-
int ieee80211_get_ratemask(struct ieee80211_supported_band *sband,
const u8 *rates, unsigned int n_rates,
u32 *mask)
--
2.1.4
Nick
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>