Messages in this thread | | | Date | Sun, 31 Jan 2016 19:02:35 -0800 | From | Davidlohr Bueso <> | Subject | Re: [PATCH v2] locktorture: Fix NULL pointer when torture_type is invalid |
| |
On Mon, 01 Feb 2016, Kefeng Wang wrote:
>Hi Davidlohr, > >On 2016/2/1 6:17, Davidlohr Bueso wrote: >> On Sat, 30 Jan 2016, Paul E. McKenney wrote: >> >>>> On 2016/1/28 12:25, Kefeng Wang wrote: >>>> > Insmod locktorture with torture_type=mutex will lead to crash, >> >> You actually want mutex_lock here, we always use the _lock suffix, mainly >> because it all started out with spin_lock. And you just showed how fragile >> this is -- I'd say most of use use this module in a scripted setup, which >> is why it was not seen before. >> >[snip...] >> >> So we shouldn't be doing anything with statistics here in the first place, as >> it was a bogus argument. Instead, we should just exit with EINVAL. Something >> like the below does the trick for me. >> > >Yes, it works, but what you are doing is to revert commit a36a99618b1adb2d6ca0b7e08e3a656a04e477fe
Oh, I see. I was definitely not aware of that one.
[...]
>And what Paul wanted is something that would print the full statistics >at the end regardless of the periodic statistics. > >I prefer my version 2, here is some log with my patch v2, it is keep consistent >with rcutorture. >------------------------------------------------------- >-bash-4.3# insmod locktorture.ko torture_type=mutex >[ 190.845067] lock-torture: invalid torture type: "mutex" >[ 190.845748] lock-torture types: >[ 190.846099] lock_busted spin_lock >[ 190.863211] spin_lock_irq rw_lock >[ 190.863668] rw_lock_irq mutex_lock >[ 190.864049] rtmutex_lock rwsem_lock >[ 190.864390] percpu_rwsem_lock[ 190.864686] >[ 190.865662] Writes: Total: 0 Max/Min: 0/0 Fail: 0 >[ 190.866218] Reads : Total: 0 Max/Min: 0/0 Fail: 0 >[ 190.875071] mutex-torture:--- End of test: SUCCESS: nwriters_stress=0 nreaders_stress=0 stat_interval=60 verbose=1 shuffle_interval=3 stutter=5 shutdown_secs=0 onoff_interval=0 onoff_holdoff=0
How can the above be a successful run (SUCCESS string) if we didn't pass a valid torture_type? iow, there is no test without it. Just think of passing the wrong param in a userland application, 99.999% of the tools simply error out complaining about the bogus input.
I think the right approach would be to decouple the statistics from the cleanup, that way we can still do the required cleanups and avoid any stats.
Thanks, Davidlohr
| |