Re: Re: What is GetXid/FreeXid supposed to actually do?

On Mon, 25 May 2009 06:52:03 -0400
Jeff Layton <[email protected]> wrote:

> On Sun, 24 May 2009 20:36:25 -0500
> Steve French <[email protected]> wrote:
> 
> > On Sun, May 24, 2009 at 4:03 PM, Jeff Layton <[email protected]> wrote:
> > > On Sun, 24 May 2009 15:38:22 -0500
> > > Steve French <[email protected]> wrote:
> > >
> > >> On Sun, May 24, 2009 at 2:31 PM, Christoph Hellwig <[email protected]> wrote:
> > >> > On Sun, May 24, 2009 at 03:21:07PM -0400, Jeff Layton wrote:
> > >> >> Any objection to just replacing this xid value with the pid or tgid
> > >> >> instead? Does this xid value give you anything that that doesn't?
> > >> >
> > >> > Yeah, from all that I can see there is always at most one xid in
> > >> > use per thread.  So just replacing it with a tgid should work easily.
> > >> > Even better would be replacing all that ad-hoc debugging with the
> > >> > ftrace event tracer.
> > >>
> > >> That may work (using the tgid) but there are various cases where it was
> > >> helpful to distinguish log entries this way (rather than simply using
> > >> timestamps,
> > >> especially since dmesg is lossy so you don't always see the function entry
> > >> and exits) when you have multiple requests in a series from one or more threads
> > >> request from thread1, request from thread 2, request 2 from thread1,
> > >> request 2 from thread 2 etc.
> > >>
> > >
> > > dmesg is never lossy...maybe you mean syslog?
> > 
> > Yes - in particular running dbench with logging on you tend to lose
> > a lot, and when I add in debugging messages (using the xid)
> > it helps sort them.
> > 
> > > Anyway, there are never multiple requests in flight from single threads
> > > in cifs. In all cases, a thread transmits a request and then goes to
> > > sleep until the reply comes in.
> > 
> > There are a couple of rare cases, but usually this (xid) is to untangle
> > the case of lots of requests in the log.
> > 
> 
> That might be the case if it ever got printed with every message, but
> it looks like it almost never does.
> 
> A more useful scheme would be to make the tgid get printed whenever a
> cFYI/cERROR message is printed. With that we could get rid of all of
> this GetXid/FreeXid infrastructure.
> 

Ahh I stand corrected -- this xid thing does show up in a few messages
from higher levels: cifs_close, cifs_writepages, etc. Still though, I
fail to see the value of this over the tgid. The non-uniqueness of this
value would make it extremely hard to use to reliably match up these
calls.


> > > AFAICT too, this xid only ever shows up in one cERROR message:
> > >
> > >    cERROR(1, ("Frame too large received.  Length: %d  Xid: %d",
> > >                receive_len, xid));
> > >
> > > ...that just doesn't seem like it's worth the cost.
> > 
> > In the past I also have used it (xid) a lot in debugging through /proc/fs/cifs/
> > 
> 
> I'm not sure I follow this. This xid is only showing up in one or two
> error messages as far as I can tell. How does it get used
> under /proc/fs/cifs?
> 
> > >
> > >> Using the ftrace event tracer may be good enough for some of this
> > >> (except the stats gathering, counters) - any pointers to its
> > >> use in NFS?
> > >>
> > >
> > > I know Steve D. has been working on tracepoints for nfs, but I haven't
> > > given them a hard look as of yet.
> > 
> > Some guys (OLS speakers) from India were talking about fs tracepoints last
> > year at OLS but I got the feeling that not much progress had been made.
> > 
> 
> I think a lot of progress has been made in the last year on this. At
> this point, it does look like ftrace is the way of the future. LWN has
> a good article on this if you have access:
> 
>     http://lwn.net/Articles/322666/
> 
> ...we'd want to add static tracepoints:
> 
>     http://lwn.net/Articles/330402/
> 
> ...though I think the interface may have changed a little since that
> article was written.
> 


-- 
Jeff Layton <[email protected]>