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

On Mon, May 25, 2009 at 6:16 AM, Jeff Layton <[email protected]> wrote:
> 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.

We have debug code that can dump out to /proc/fs/cifs/DebugData the
requests that are pending (waiting on response from the server)
and I have done trivial temporary changes to dump xid there too
in the past.

If xid is not incrementing on every request then we should fix that or
use tgid (although there may be a way to have printk print the
tgid with the message - I haven't checked recently)



-- 
Thanks,

Steve