| Message ID | 1508926976-11467-1-git-send-email-rsahu@apm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello, On Wed, Oct 25, 2017 at 03:52:56PM +0530, Rameshwar Prasad Sahu wrote: > @@ -288,8 +289,8 @@ > ), > > TP_fast_assign( > - __entry->ata_port = dev->link->ap->print_id; > - __entry->ata_dev = dev->link->pmp + dev->devno; > + __entry->ata_port = link->ap->print_id; > + __entry->ata_dev = link->pmp + link->device->devno; The above is wrong if there are multiple devices on the link. It probably should take both link and dev and use dev iff it's not NULL. Thanks.
Hello, On Tue, Oct 31, 2017 at 08:52:44PM +0530, Rameshwar Sahu wrote: > > probably should take both link and dev and use dev iff it's not NULL. > > > > Instead of this would it be better to call trace_ata_eh_link_autopsy() if > dev is not NULL from ata error handler ?? Oh yeah, that'd work too and be probably better. Thanks.
On Wed, Nov 1, 2017 at 8:00 PM, Tejun Heo <tj@kernel.org> wrote: > Hello, > > On Tue, Oct 31, 2017 at 08:52:44PM +0530, Rameshwar Sahu wrote: >> > probably should take both link and dev and use dev iff it's not NULL. >> > >> >> Instead of this would it be better to call trace_ata_eh_link_autopsy() if >> dev is not NULL from ata error handler ?? > > Oh yeah, that'd work too and be probably better. I will post another version with this fix. > > Thanks. > > -- > tejun
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index e4effef..ab50e7d 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -2265,7 +2265,7 @@ static void ata_eh_link_autopsy(struct ata_link *link) eflags |= ATA_EFLAG_DUBIOUS_XFER; ehc->i.action |= ata_eh_speed_down(dev, eflags, all_err_mask); } - trace_ata_eh_link_autopsy(dev, ehc->i.action, all_err_mask); + trace_ata_eh_link_autopsy(link, ehc->i.action, all_err_mask); DPRINTK("EXIT\n"); } diff --git a/include/trace/events/libata.h b/include/trace/events/libata.h index 2fbbf99..eff035e 100644 --- a/include/trace/events/libata.h +++ b/include/trace/events/libata.h @@ -276,9 +276,10 @@ TRACE_EVENT(ata_eh_link_autopsy, - TP_PROTO(struct ata_device *dev, unsigned int eh_action, unsigned int eh_err_mask), + TP_PROTO(struct ata_link *link, unsigned int eh_action, + unsigned int eh_err_mask), - TP_ARGS(dev, eh_action, eh_err_mask), + TP_ARGS(link, eh_action, eh_err_mask), TP_STRUCT__entry( __field( unsigned int, ata_port ) @@ -288,8 +289,8 @@ ), TP_fast_assign( - __entry->ata_port = dev->link->ap->print_id; - __entry->ata_dev = dev->link->pmp + dev->devno; + __entry->ata_port = link->ap->print_id; + __entry->ata_dev = link->pmp + link->device->devno; __entry->eh_action = eh_action; __entry->eh_err_mask = eh_err_mask; ),
When tracing ata link error event, the kernel crashes when the disk is removed due to NULL pointer access by trace_ata_eh_link_autopsy API. This occurs as the dev is NULL when the disk disappeared. Given that the trace routine requires only the link info, pass the link info instead of passing dev pointer to fix this kernel crash. Signed-off-by: Rameshwar Prasad Sahu <rsahu@apm.com> --- drivers/ata/libata-eh.c | 2 +- include/trace/events/libata.h | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) -- 1.7.1