[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [dm-devel] [PATCH] block: add missing block_bio_complete() tracepoint



Hi,

2012-01-31 7:39 PM, Tejun Heo wrote:
Hello,

On Mon, Jan 30, 2012 at 10:30 PM, Namhyung Kim<namhyung gmail com>  wrote:
Right, but the point is it could make a NULL pointer dereference during
evaluation of the argument of the TP AFAICS. I'm not sure about the TP
implementation though, I think I was wrong - T_E_C() cannot protect us from
it because it happens just before jumping to the TP, right?

So I think we need a conditional jump (with the "likely" annotation) for
this even when the TP is disabled.

Hmmm... still not following. Where the said NULL dereference happen?
TEC conditional is equivalent to "if (COND) TP;".  If you don't use
TEC, it'll be "if (COND) if (TP enabled) TP;".  With TEC, it will be
"if (TP enabled) if (COND) TP;".  There's no other difference.

Thanks.


I've made a quick investigation on TP implementation, and finally figured out what I was wrong - I thought the COND would be checkd in a probe, but it's not. Thanks for pointing it out.

However, for some reason, it seems gcc generated code that evaluates the arguments - bdev_get_queue() in this case - before checking the COND. Simple test module below caused a NULL pointer dereference when I used TRACE_EVENT_CONDITION(), but not for conditional jump:

#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/bio.h>

static int __init init_mod(void)
{
        struct bio *bio = bio_alloc(GFP_KERNEL, 0);
        bio_endio(bio, 0);
        bio_put(bio);
        return 0;
}

static void __exit exit_mod(void)
{
}

module_init(init_mod);
module_exit(exit_mod);


Thanks,
Namhyung


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]