Re: [PATCH v2] locktorture: Fix NULL pointer when torture_type is invalid

[Davidlohr Bueso]

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