From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757572AbcBADCq (ORCPT ); Sun, 31 Jan 2016 22:02:46 -0500 Received: from mx2.suse.de ([195.135.220.15]:46316 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751177AbcBADCo (ORCPT ); Sun, 31 Jan 2016 22:02:44 -0500 Date: Sun, 31 Jan 2016 19:02:35 -0800 From: Davidlohr Bueso To: Kefeng Wang Cc: "Paul E. McKenney" , linux-kernel@vger.kernel.org, peterz@infradead.org, mingo@redhat.com, Josh Triplett , "Guohanjun (Hanjun Guo)" Subject: Re: [PATCH v2] locktorture: Fix NULL pointer when torture_type is invalid Message-ID: <20160201030235.GC16147@linux-uzut.site> References: <1453955159-23216-1-git-send-email-wangkefeng.wang@huawei.com> <56AC2421.7020006@huawei.com> <20160131002721.GI6719@linux.vnet.ibm.com> <20160131221736.GB16147@linux-uzut.site> <56AEC21A.5010107@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <56AEC21A.5010107@huawei.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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