Re: [PATCH v2 1/5] mac80211_hwsim: wait for deferred radio deletion on mod unload

[Benjamin Beichler <Benjamin.Beichler@uni-rostock.de>]

Am 11.12.2017 um 12:46 schrieb Johannes Berg:
>
>> +	spin_lock_bh(&hwsim_delete_lock);
>> +	while (!list_empty(&delete_radios)) {
>> +		pr_debug("mac80211_hwsim: wait for deferred radio remove\n");
>> +		spin_unlock_bh(&hwsim_delete_lock);
>> +		flush_work(&list_entry(&delete_radios,
>> +				       struct mac80211_hwsim_data, list)
>> +			   ->destroy_work);
> This can't possibly be right ... you're locking the list_empty which is=

> a trivial pointer comparison, but not the actual list_entry() ...
Maybe the first spin_lock is not needed, but since flush_work wait for
the completion of the task, which at the end also deletes the item from
the list, holding any spin_lock would be wrong.
>
> I'd also prefer you actually didn't leave the problem in part as you
> describe - and a new workqueue probably isn't that much overhead and
> should introduce *less* new code than this, so IMHO that's worth it.
I totally agree with you, but I don't know the actual policy for
creating workqeues. I could also simply call flush_scheduled_work,
because we may have enough time at module unload. Some modules also do
it, but the description an some mailing threads mark it like
evil/deprecated. Which solution do you prefer?

kind regards

Benjamin

--=20
M.Sc. Benjamin Beichler

Universit=C3=A4t Rostock, Fakult=C3=A4t f=C3=BCr Informatik und Elektrote=
chnik
Institut f=C3=BCr Angewandte Mikroelektronik und Datentechnik

University of Rostock, Department of CS and EE
Institute of Applied Microelectronics and CE

Richard-Wagner-Stra=C3=9Fe 31
18119 Rostock
Deutschland/Germany

phone: +49 (0) 381 498 - 7278
email: Benjamin.Beichler@uni-rostock.de
www: http://www.imd.uni-rostock.de/