Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752914AbZCVGEZ (ORCPT ); Sun, 22 Mar 2009 02:04:25 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754148AbZCVGEI (ORCPT ); Sun, 22 Mar 2009 02:04:08 -0400 Received: from cn.fujitsu.com ([222.73.24.84]:49269 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1753488AbZCVGEG (ORCPT ); Sun, 22 Mar 2009 02:04:06 -0400 Message-ID: <49C5D4DE.30002@cn.fujitsu.com> Date: Sun, 22 Mar 2009 14:04:14 +0800 From: Li Zefan User-Agent: Thunderbird 2.0.0.9 (X11/20071115) MIME-Version: 1.0 To: Arnaldo Carvalho de Melo CC: Ingo Molnar , Jens Axboe , Steven Rostedt , Frederic Weisbecker , LKML Subject: Re: [PATCH 3/7] blktrace: remove blk_probe_mutex References: <49C2F599.3060306@cn.fujitsu.com> <49C2F5EA.8060606@cn.fujitsu.com> <20090320130309.GJ30407@ghostprotocols.net> In-Reply-To: <20090320130309.GJ30407@ghostprotocols.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4263 Lines: 115 Arnaldo Carvalho de Melo wrote: > Em Fri, Mar 20, 2009 at 09:48:26AM +0800, Li Zefan escreveu: >> blk_register_tracepoints() always returns 0, so make it return void, thus >> we don't need to use blk_probe_mutex to protect blk_probes_ref. >> >> Signed-off-by: Li Zefan > > Historic reasons, when I first worked on this I found all those WARN_ONs > in blk_register_tracepoints a bogosity that should be later fixed, but > to reduce the patch size I never got around to submit the proper patch, > so I don't think that the fix is what you suggests, here it is what I > had before I removed the non-essential parts of the patch: > I though register_trace_xxx() will fail only when there is a name collision, in this case WARN_ON() and return 0 should be reasonable. But I just dig into the code and found it may allocate memory and thus might return ENOMEM, so I guess it's better to check the return value, though it will hardly happen in real-life.. > +static void blk_tracer_error(const char *name) > +{ > + pr_info("blk trace: Couldn't activate tracepoint " > + "probe to block_%s\n", name); > +} > + > +#define register_trace_block(tpoint) \ > + ret = register_trace_block_##tpoint(blk_add_trace_##tpoint); \ > + if (ret) { \ > + blk_tracer_error(#tpoint); \ > + goto *exit_point; \ > + } else \ > + exit_point = &&fail_deprobe_##tpoint; > + > + > +#define fail_trace_block(tpoint) \ > + fail_deprobe_##tpoint: \ > + unregister_trace_block_##tpoint(blk_add_trace_##tpoint) > + > +static int tracing_blk_register(void) > +{ > + int ret; > + void *exit_point = &&error; > + > + register_trace_block(rq_abort); > + register_trace_block(rq_insert); > + register_trace_block(rq_issue); > + register_trace_block(rq_requeue); > + register_trace_block(rq_complete); > + register_trace_block(bio_bounce); > + register_trace_block(bio_backmerge); > + register_trace_block(bio_frontmerge); > + register_trace_block(bio_queue); > + register_trace_block(getrq); > + register_trace_block(sleeprq); > + register_trace_block(plug); > + register_trace_block(unplug_timer); > + register_trace_block(unplug_io); > + register_trace_block(split); > + register_trace_block(remap); > + > + return 0; > + > + fail_trace_block(remap); > + fail_trace_block(split); > + fail_trace_block(unplug_io); > + fail_trace_block(unplug_timer); > + fail_trace_block(plug); > + fail_trace_block(sleeprq); > + fail_trace_block(getrq); > + fail_trace_block(bio_queue); > + fail_trace_block(bio_frontmerge); > + fail_trace_block(bio_backmerge); > + fail_trace_block(bio_bounce); > + fail_trace_block(rq_complete); > + fail_trace_block(rq_requeue); > + fail_trace_block(rq_issue); > + fail_trace_block(rq_insert); > + fail_trace_block(rq_abort); > +error: > + return ret; > +} > + > +static void tracing_blk_unregister(void) > +{ > + unregister_trace_block_remap(blk_add_trace_remap); > + unregister_trace_block_split(blk_add_trace_split); > + unregister_trace_block_unplug_io(blk_add_trace_unplug_io); > + unregister_trace_block_unplug_timer(blk_add_trace_unplug_timer); > + unregister_trace_block_plug(blk_add_trace_plug); > + unregister_trace_block_sleeprq(blk_add_trace_sleeprq); > + unregister_trace_block_getrq(blk_add_trace_getrq); > + unregister_trace_block_bio_queue(blk_add_trace_bio_queue); > + unregister_trace_block_bio_frontmerge(blk_add_trace_bio_frontmerge); > + unregister_trace_block_bio_backmerge(blk_add_trace_bio_backmerge); > + unregister_trace_block_bio_complete(blk_add_trace_bio_complete); > + unregister_trace_block_bio_bounce(blk_add_trace_bio_bounce); > + unregister_trace_block_rq_complete(blk_add_trace_rq_complete); > + unregister_trace_block_rq_requeue(blk_add_trace_rq_requeue); > + unregister_trace_block_rq_issue(blk_add_trace_rq_issue); > + unregister_trace_block_rq_insert(blk_add_trace_rq_insert); > + unregister_trace_block_rq_abort(blk_add_trace_rq_abort); > + tracepoint_synchronize_unregister(); > +} > > Regards, > > - Arnaldo > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/