* TAKE 988255 - fix instant oops with tracing enabled @ 2008-10-14 1:17 Lachlan McIlroy 2008-10-14 2:05 ` Dave Chinner 0 siblings, 1 reply; 7+ messages in thread From: Lachlan McIlroy @ 2008-10-14 1:17 UTC (permalink / raw) To: sgi.bugs.xfs, xfs fix instant oops with tracing enabled We can only read inode->i_count if the inode is actually there and not a NULL pointer. This was introduced in one of the recent sync patches. Signed-off-by: Christoph Hellwig <hch@lst.de> Date: Tue Oct 14 12:17:19 EST 2008 Workarea: redback.melbourne.sgi.com:/home/lachlan/isms/2.6.x-sync Inspected by: lachlan Author: lachlan The following file(s) were checked into: longdrop.melbourne.sgi.com:/isms/linux/2.6.x-xfs-melb Modid: xfs-linux-melb:xfs-kern:32315a fs/xfs/linux-2.6/xfs_vnode.c - 1.164 - changed http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/linux-2.6/xfs_vnode.c.diff?r1=text&tr1=1.164&r2=text&tr2=1.163&f=h - fix instant oops with tracing enabled ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: TAKE 988255 - fix instant oops with tracing enabled 2008-10-14 1:17 TAKE 988255 - fix instant oops with tracing enabled Lachlan McIlroy @ 2008-10-14 2:05 ` Dave Chinner 2008-10-14 5:33 ` Lachlan McIlroy 0 siblings, 1 reply; 7+ messages in thread From: Dave Chinner @ 2008-10-14 2:05 UTC (permalink / raw) To: Lachlan McIlroy; +Cc: xfs On Tue, Oct 14, 2008 at 12:17:47PM +1100, Lachlan McIlroy wrote: > fix instant oops with tracing enabled > > We can only read inode->i_count if the inode is actually there and not > a NULL pointer. This was introduced in one of the recent sync patches. > > Signed-off-by: Christoph Hellwig <hch@lst.de> BTW, this now means that other pending patches won't apply cleanly.... Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: TAKE 988255 - fix instant oops with tracing enabled 2008-10-14 2:05 ` Dave Chinner @ 2008-10-14 5:33 ` Lachlan McIlroy 2008-10-14 5:41 ` Dave Chinner 0 siblings, 1 reply; 7+ messages in thread From: Lachlan McIlroy @ 2008-10-14 5:33 UTC (permalink / raw) To: Lachlan McIlroy, xfs Dave Chinner wrote: > On Tue, Oct 14, 2008 at 12:17:47PM +1100, Lachlan McIlroy wrote: >> fix instant oops with tracing enabled >> >> We can only read inode->i_count if the inode is actually there and not >> a NULL pointer. This was introduced in one of the recent sync patches. >> >> Signed-off-by: Christoph Hellwig <hch@lst.de> > > BTW, this now means that other pending patches won't apply cleanly.... > Yeah this patch hunk didn't apply but was easy to hand merge. It also didn't compile because of a missing comma from the 4th last line. Can you please make sure each of your patch series compile on their own? @@ -84,25 +84,12 @@ vn_ioerror( #ifdef XFS_INODE_TRACE -/* - * Reference count of Linux inode if present, -1 if the xfs_inode - * has no associated Linux inode. - */ -static inline int xfs_icount(struct xfs_inode *ip) -{ - struct inode *inode = VFS_I(ip); - - if (!inode) - return atomic_read(&inode->i_count); - return -1; -} - #define KTRACE_ENTER(ip, vk, s, line, ra) \ ktrace_enter( (ip)->i_trace, \ /* 0 */ (void *)(__psint_t)(vk), \ /* 1 */ (void *)(s), \ /* 2 */ (void *)(__psint_t) line, \ -/* 3 */ (void *)(__psint_t)xfs_icount(ip), \ +/* 3 */ (void *)(__psint_t)atomic_read(&VFS_I(ip)->i_count) \ /* 4 */ (void *)(ra), \ /* 5 */ NULL, \ /* 6 */ (void *)(__psint_t)current_cpu(), \ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: TAKE 988255 - fix instant oops with tracing enabled 2008-10-14 5:33 ` Lachlan McIlroy @ 2008-10-14 5:41 ` Dave Chinner 2008-10-14 6:12 ` Timothy Shimmin 0 siblings, 1 reply; 7+ messages in thread From: Dave Chinner @ 2008-10-14 5:41 UTC (permalink / raw) To: Lachlan McIlroy; +Cc: xfs On Tue, Oct 14, 2008 at 03:33:06PM +1000, Lachlan McIlroy wrote: > Dave Chinner wrote: >> On Tue, Oct 14, 2008 at 12:17:47PM +1100, Lachlan McIlroy wrote: >>> fix instant oops with tracing enabled >>> >>> We can only read inode->i_count if the inode is actually there and not >>> a NULL pointer. This was introduced in one of the recent sync patches. >>> >>> Signed-off-by: Christoph Hellwig <hch@lst.de> >> >> BTW, this now means that other pending patches won't apply cleanly.... >> > > Yeah this patch hunk didn't apply but was easy to hand merge. It also > didn't compile because of a missing comma from the 4th last line. Can > you please make sure each of your patch series compile on their own? I do make sure every patch compiles given the config options a mainline kernel allows. However, the CONFIG_XFS_TRACE option is not present in mainline-derived kernels so it's not obvious when problems occur inside code that isn't easily configurable on such a kernel.... As it is, I haven't done a CONFIG_XFS_TRACE build for several months because I've found that I haven't needed tracing to find problems esoteric problems. Using watchpoints, breakpoints and single stepping catches problems as they occur rather than needing tracing to try to work out what went wrong after a problem has occurred. Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: TAKE 988255 - fix instant oops with tracing enabled 2008-10-14 5:41 ` Dave Chinner @ 2008-10-14 6:12 ` Timothy Shimmin 2008-10-14 6:50 ` Dave Chinner 2008-10-14 12:56 ` Christoph Hellwig 0 siblings, 2 replies; 7+ messages in thread From: Timothy Shimmin @ 2008-10-14 6:12 UTC (permalink / raw) To: xfs Dave Chinner wrote: > On Tue, Oct 14, 2008 at 03:33:06PM +1000, Lachlan McIlroy wrote: >> Dave Chinner wrote: >>> On Tue, Oct 14, 2008 at 12:17:47PM +1100, Lachlan McIlroy wrote: >>>> fix instant oops with tracing enabled >>>> >>>> We can only read inode->i_count if the inode is actually there and not >>>> a NULL pointer. This was introduced in one of the recent sync patches. >>>> >>>> Signed-off-by: Christoph Hellwig <hch@lst.de> >>> BTW, this now means that other pending patches won't apply cleanly.... >>> >> Yeah this patch hunk didn't apply but was easy to hand merge. It also >> didn't compile because of a missing comma from the 4th last line. Can >> you please make sure each of your patch series compile on their own? > > I do make sure every patch compiles given the config options a > mainline kernel allows. However, the CONFIG_XFS_TRACE option is not > present in mainline-derived kernels so it's not obvious when > problems occur inside code that isn't easily configurable on > such a kernel.... > > As it is, I haven't done a CONFIG_XFS_TRACE build for several months > because I've found that I haven't needed tracing to find problems > esoteric problems. Using watchpoints, breakpoints and single > stepping catches problems as they occur rather than needing tracing > to try to work out what went wrong after a problem has occurred. > OOI, can anyone comment on any suggestions for other tracing mechanisms. I recall Christoph mentioning something in the past but it wasn't ready yet or something???. I'd really like to be able to access tracing output via proc instead of just within kdb, as I have used in the past with Greg Banks qtrace patches. --Tim ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: TAKE 988255 - fix instant oops with tracing enabled 2008-10-14 6:12 ` Timothy Shimmin @ 2008-10-14 6:50 ` Dave Chinner 2008-10-14 12:56 ` Christoph Hellwig 1 sibling, 0 replies; 7+ messages in thread From: Dave Chinner @ 2008-10-14 6:50 UTC (permalink / raw) To: Timothy Shimmin; +Cc: xfs On Tue, Oct 14, 2008 at 05:12:08PM +1100, Timothy Shimmin wrote: > Dave Chinner wrote: > > On Tue, Oct 14, 2008 at 03:33:06PM +1000, Lachlan McIlroy wrote: > >> Dave Chinner wrote: > >>> On Tue, Oct 14, 2008 at 12:17:47PM +1100, Lachlan McIlroy wrote: > >>>> fix instant oops with tracing enabled > >>>> > >>>> We can only read inode->i_count if the inode is actually there and not > >>>> a NULL pointer. This was introduced in one of the recent sync patches. > >>>> > >>>> Signed-off-by: Christoph Hellwig <hch@lst.de> > >>> BTW, this now means that other pending patches won't apply cleanly.... > >>> > >> Yeah this patch hunk didn't apply but was easy to hand merge. It also > >> didn't compile because of a missing comma from the 4th last line. Can > >> you please make sure each of your patch series compile on their own? > > > > I do make sure every patch compiles given the config options a > > mainline kernel allows. However, the CONFIG_XFS_TRACE option is not > > present in mainline-derived kernels so it's not obvious when > > problems occur inside code that isn't easily configurable on > > such a kernel.... > > > > As it is, I haven't done a CONFIG_XFS_TRACE build for several months > > because I've found that I haven't needed tracing to find problems > > esoteric problems. Using watchpoints, breakpoints and single > > stepping catches problems as they occur rather than needing tracing > > to try to work out what went wrong after a problem has occurred. > > > OOI, can anyone comment on any suggestions for other tracing mechanisms. > I recall Christoph mentioning something in the past but it wasn't > ready yet or something???. Well, I'd suggest that we probably should turn all the trace points into markers so that systemtap can hook them and you can write probes to extract the traces in real-time.... > I'd really like to be able to access tracing output via proc > instead of just within kdb, > as I have used in the past with Greg Banks qtrace patches. Perhaps extending something like: Documentation/ftrace.txt would be the appropriate way to go to acheive this. I think in the past Christoph has suggested switching the ktrace code over a relayfs mechanism and providing a userspace decoder would allow both kdb and userspace to access the same traces... Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: TAKE 988255 - fix instant oops with tracing enabled 2008-10-14 6:12 ` Timothy Shimmin 2008-10-14 6:50 ` Dave Chinner @ 2008-10-14 12:56 ` Christoph Hellwig 1 sibling, 0 replies; 7+ messages in thread From: Christoph Hellwig @ 2008-10-14 12:56 UTC (permalink / raw) To: Timothy Shimmin; +Cc: xfs On Tue, Oct 14, 2008 at 05:12:08PM +1100, Timothy Shimmin wrote: > OOI, can anyone comment on any suggestions for other tracing mechanisms. > I recall Christoph mentioning something in the past but it wasn't > ready yet or something???. > I'd really like to be able to access tracing output via proc > instead of just within kdb, > as I have used in the past with Greg Banks qtrace patches. Yah, I've been working on doing that and wire it up with trace_mark. I'll move it up on my TODO list a little.. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2008-10-14 12:55 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-10-14 1:17 TAKE 988255 - fix instant oops with tracing enabled Lachlan McIlroy 2008-10-14 2:05 ` Dave Chinner 2008-10-14 5:33 ` Lachlan McIlroy 2008-10-14 5:41 ` Dave Chinner 2008-10-14 6:12 ` Timothy Shimmin 2008-10-14 6:50 ` Dave Chinner 2008-10-14 12:56 ` Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox