----- Original Message ----- > > Hi Dave > > I found a problem with the net -s command. It concerns line 1451 in net.c > > struct_socket = inode - SIZE(socket); > > As I understand it we have the type > > struct socket_alloc { > struct socket socket; > struct inode vfs_inode; > } > > and we have the address of the second field and want the address of > the first. The calculation, using the size of the socket struct, > used in net.c require that the second field is aligned directly > after the first field. This is unfortunately not true in cases I > have seen. By changing the line 1451 to: > > struct_socket = inode - MEMBER_OFFSET("socket_alloc", "vfs_inode"); > > things work better. > > Is this something you would like to change in Crash? I assume you > will move the offset calculation to somewhere else so it is only > performed once. Probably so... Although I'm curious -- what kernel version do you see this on? It works as expected on RHEL5, RHEL6 and a Fedora 16 3.1.7-based kernel. What do you see when you do this: crash> socket_alloc -o struct socket_alloc { [0] struct socket socket; [48] struct inode vfs_inode; } SIZE: 616 crash> socket struct socket { socket_state state; short int type; long unsigned int flags; struct socket_wq *wq; struct file *file; struct sock *sk; const struct proto_ops *ops; } SIZE: 48 crash> And just for the changelog description, what havoc does it wreak? Thanks, Dave

This problem was really found by Pavel Modilaynen (Pavel.Modilaynen(a)sonyericsson.com) working in the same group as I do. We saw the same kind of information using struct -o that you found yourself. The only reason I can think of for the 8-byte alignment is that the struct vfs_inode itself starts with an 8-byte struct. That we might not have exactly the same alignment rules on ARM and x86 worries me. The whole idea to debug an ARM kernel using an x86 machine is based on the assumption that sizes of types, alignment rules, and so on are the same. Jan

This problem effects net -s and net -S, as the pointer to the socket is 4 bytes off, which of course means that all information about the socket is garbage. Example: Before patch: crash> net -s PID: 377 TASK: cc4dc360 CPU: 0 COMMAND: "NetworkStats" FD SOCKET SOCK FAMILY:TYPE SOURCE-PORT DESTINATION-PORT .... 101 cf55b784 cc450a40 214324:23124 .... After patch: crash> net -s PID: 377 TASK: cc4dc360 CPU: 0 COMMAND: "NetworkStats" FD SOCKET SOCK FAMILY:TYPE SOURCE-PORT DESTINATION-PORT .... 101 cf55b780 cc450a40 INET6:DGRAM 0:0:0:0:0:0:0:0-50170 0:0:0:0:0:0:0:0-0 ....

This problem was really found by Pavel Modilaynen (Pavel.Modilaynen(a)sonyericsson.com) working in the same group as I do. We saw the same kind of information using struct -o that you found yourself. The only reason I can think of for the 8-byte alignment is that the struct vfs_inode itself starts with an 8-byte struct. That we might not have exactly the same alignment rules on ARM and x86 worries me. The whole idea to debug an ARM kernel using an x86 machine is based on the assumption that sizes of types, alignment rules, and so on are the same. Jan

-----Original Message----- From: crash-utility-bounces(a)redhat.com [mailto:crash-utility-bounces@redhat.com] On Behalf Of Dave Anderson Sent: torsdag den 26 januari 2012 17:36 To: Discussion list for crash utility usage, maintenance and development Cc: Fänge, Thomas Subject: Re: [Crash-utility] Problem in command net -s ----- Original Message ----- > ----- Original Message ----- > > > Hi Dave > > > I found a problem with the net -s command. It concerns line 1451 in net.c > > > struct_socket = inode - SIZE(socket); > > > As I understand it we have the type > > > struct socket_alloc { > > struct socket socket; > > struct inode vfs_inode; > > } > > > and we have the address of the second field and want the address of > > the first. The calculation, using the size of the socket struct, > > used in net.c require that the second field is aligned directly > > after the first field. This is unfortunately not true in cases I > > have seen. By changing the line 1451 to: > > > > struct_socket = inode - MEMBER_OFFSET("socket_alloc", > > "vfs_inode"); > > > things work better. > > > Is this something you would like to change in Crash? I assume you > > will move the offset calculation to somewhere else so it is only > > performed once. > Probably so... > Although I'm curious -- what kernel version do you see this on? > It works as expected on RHEL5, RHEL6 and a Fedora 16 3.1.7-based > kernel. What do you see when you do this: > crash> socket_alloc -o > struct socket_alloc { > [0] struct socket socket; > [48] struct inode vfs_inode; > } > SIZE: 616 > crash> socket > struct socket { > socket_state state; > short int type; > long unsigned int flags; > struct socket_wq *wq; > struct file *file; > struct sock *sk; > const struct proto_ops *ops; > } > SIZE: 48 > crash> > And just for the changelog description, what havoc does it wreak? > Thanks, > Dave Interesing -- I see the problem with the 3 sample ARM dumpfiles I have on hand. I would have thought the same issue would be seen with a 32-bit x86, but it looks like it's an ARM compiler issue? Check this comparison -- while the inode structure is different in these two kernels, the socket structure is the same: X86: ARM: crash> socket_alloc -o crash> socket_alloc -o struct socket_alloc { struct socket_alloc { [0] struct socket socket; [0] struct socket socket; [28] struct inode vfs_inode; [32] struct inode vfs_inode; } } SIZE: 388 SIZE: 584 crash> socket -o crash> socket -o struct socket { struct socket { [0] socket_state state; [0] socket_state state; [4] short int type; [4] short int type; [8] long unsigned int flags; [8] long unsigned int flags; [12] struct socket_wq *wq; [12] struct socket_wq *wq; [16] struct file *file; [16] struct file *file; [20] struct sock *sk; [20] struct sock *sk; [24] const struct proto_ops *ops; [24] const struct proto_ops *ops; } } SIZE: 28 SIZE: 28 crash> crash> But for whatever reason, the ARM kernel pushes the vfs_inode to offset 32 even though the preceding socket structure is 28 bytes long. Anyway, using the offset instead of the size is a better idea, so I'll make that change. Although -- my sample ARM dumpfiles don't have any tasks with open sockets, so I still am interested in seeing what the failure looks like for the changelog entry. Thanks, Dave