Messages in this thread |  | | From | Namhyung Kim <> | Subject | Re: [PATCH/RFC 17/17] tracing/uprobes: Add @+file_offset fetch method | Date | Fri, 29 Nov 2013 09:25:10 +0900 |
| |
Hi Oleg,
On Thu, 28 Nov 2013 17:31:48 +0100, Oleg Nesterov wrote: > On 11/28, Namhyung Kim wrote: >> >> I thought we need a fetch_param anyway if we will add support for >> cross-fetch later. But I won't insist it strongly, I can delay it to >> later work and make current code simpler if you want. :) > > OK, great,
So do you want me to change to make it simpler without a fetch_param?
> >> >> static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs) >> >> { >> >> struct trace_uprobe *tu; >> >> + struct uprobe_task *utask; >> >> int ret = 0; >> >> >> >> tu = container_of(con, struct trace_uprobe, consumer); >> >> tu->nhit++; >> >> >> >> + utask = current->utask; >> >> + if (utask == NULL) >> >> + return UPROBE_HANDLER_REMOVE; >> > >> > Hmm, why? The previous change ensures ->utask is not NULL? If we hit >> > NULL we have a bug, we should not remove this uprobe. >> >> Yes, I just want to be defensive. :) >> >> So do you suggest to add BUG_ON()? > > We are going to crash with the same effect if it is NULL ;)
I see. I'll just get rid of the if (...) part.
> >> And can I convert or remove a >> similar check in uprobes.c:pre_ssout() too? > > Well, yes, we _can_ do this. But unless you have the strong opinion > I'd suggest to not do this. At least right now. > > To remind, perhaps we can revert the previous patch later if we find > a better solution (placeholder).
OK, I'll just leave it as is.
> > And. Note that we will change this code in any case. I suggested to > use ->vaddr to avoid the other (potentially conflicting) changes in > uprobes.h. Even if we use current->utask, we should add another member > into the union. But again, it would be better to do this later, and > the change will be trivial.
Got it. Thank you for the explanation.
Thanks, Namhyung
|  |