On Sun, 7 May 2000, Linus Torvalds wrote:
> The reason I like "struct file" is that everythingis reachable from there:
> including "struct dentry", which names whatever device/file/xxx we're
> working with.
>
> I love the fact that when I'm debugging I can do debugging based on names
> (I've done printk's that say _exactly_ which file has problems, and I've
> had debugging code that only gets triggered for certain names so that I
> wouldn't drown in debugging information - extremely useful).
>
> Opaque handles don't have that kind of behaviour.
That's different. The problem with struct file * is very simple: what
it _tries_ to do is distinguishing between openers. And sometimes the
opener is just "VFS" and that's fscking it - nobody who would open the file
anywhere in sight.
Natural solution for that is to use NULL. I have no problems with struct file *
_IF_ ->readpage() on everything that doesn't need to distiguish between the
users of pagecache will be able to pick everything it needs from the page
itself. IOW, I think that if we change the first argument of ->readpage()
from struct dentry* to struct file* we must make sure that symlinks will
use the *!@#! ->host and be happy with getting NULL as the first argument.
> Using that as the basic data structure means that we can thus (for
> example) always give names to the things we're working with. Which is good
> for other things than just debugging: it is also very useful for things
> like /proc.
Fine with me.
> And I have absolute no problem with having an extra dummy "struct file"
> lying around. For example, you may remember that "struct vm_struct" used
> to have just an inode pointer: the logic being that VM mappings
> traditionally do not _have_ a file (it usually gets closed after the mmap
> is done). So the inode was the obvious thing to have there.
"Lying around" is fine. But look what happens with symlinks and directories -
you can't keep it around (reference to dentry and refcounting goes to hell)
and allocating it on every operation is the point where I become very unhappy.
[snip]
> otherwise hard to carry around. Like "who opened this file originally?"
The answer may very well be "nobody".
Linus, aside of the fact that on-stack allocation of struct file, etc. is
_huge_ PITA and it had always been a source of hard-to-debug shit (e.g.
keeping trace of what should and what shouldn't be freed - very funny when
that goes wrong), struct file * is _not_ a universal thing for IO-related
operations. Let me describe what I consider as a potential solution:
a) fsck it, let's pass struct file * explicitly. Meaning: which
of pagecache users tries to access it.
b) NULL == "kernel".
c) everything that doesn't care about pagecache sharing (e.g. all
symlinks - by necessity; there is _no_ struct file * there and creating a
phoney one just to pass ->f_dentry->d_inode that already _is_ available in
page->mapping->host just plain sucks) must be happy with NULL. Same goes
for directories, etc.
That would preserve the type-checks and would not force the sea
of kludges on the symlink handling/potential directory handling/etc.
Comments?
Cheers,
Al
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Sun May 07 2000 - 21:00:21 EST