On Thu, Aug 23, 2012 at 02:43:25PM +0400, Cyrill Gorcunov wrote: > This patch converts /proc/pid/fdinfo/ handling routines to seq-file which > is needed to extend seq operations and plug in auxiliary fdinfo provides > from subsystems like eventfd/eventpoll/fsnotify. > > Note the proc_fd_link no longer call for proc_fd_info, simply because > proc_fd_info is converted to seq_fdinfo_open (which is seq-file open() > prototype). Actually, now that I've looked at it a bit more... You've just introduced an ABI change here. Look: echo foo > /tmp/a exec 8</tmp/a # fd 8 reads from /tmp/a read i <&8 # read line from it exec 9</proc/self/fdinfo/8 # fd 9 is /proc/self/fdinfo/8 exec 8</tmp/a # close fd 8 and reopen it to the same /tmp/a cat <&9 # now read from fd 9 With the mainline it will print pos: 0 flags: 0100000 With that commit you will get pos: 4 flags: 0100000 since the file you've opened refers to what used to be at fd 8 at the moment of open(2), not read(2). It may or may not be harmless, but it definitely is a userland ABI change. And that way it's actually an extra PITA for yourself - think what /proc/self/fdinfo/9 should contain now! That's right, you've got hidden state there and would need to print it to be able to reconstruct the state on restart. Only it doesn't end just there - what if you've taken that one step further and got the struct file stashed in there at open(2) time also of the same kind? IMO doing that at open() time is just a headache for no good reason - resolving descriptor to struct file * at read() time as we do now is much saner. Better do that in your ->show(), since you are using a single-shot iterator anyway... -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
