Re: [PATCH] address_space_operations unification

From: Linus Torvalds (torvalds@transmeta.com)
Date: Sun May 07 2000 - 12:44:10 EST


On Sun, 7 May 2000, Alexander Viro wrote:
>
> On Sun, 7 May 2000, Linus Torvalds wrote:
>
> > Oh.
> >
> > If anybody whips out a patch to do the "struct file" move, and not have a
> > "void *", then I'll apply that. No problem. I agree 100% with the fact
> > that "struct file" is the fundamental IO entity - even if it turns out
> > that most users don't actually need either the file nor the dentry to
> > actually do the physical IO (ie all "unix-like" filesystems will always be
> > happy with just the inode - that is, to some degree, what "unix-like"
> > really means).
>
> Interesting... What, in your opinion, makes pagecache unsuitable for stuff
> like inode table (done the same way as regular files on quite a few filesystems)
> or EAs on NTFS/HPFS/whatnot or forks on HFS, etc.? If it _is_ suitable - OK,
> which struct file might be naturally associated with these objects? Before
> you say that these case are pathologies consider directories on good ol'
> ext2. No opened files around in that case.

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.

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.

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.

Saying "what the hell" and just putting the file there instead made a lot
of things much easier, and means that /proc/xxx/mappings gets the right
output etc - even though historically there shouldn't be any file
association any more at the point where you actually have a mapping.

> That's what I find ugly about struct file * - yes, void * is a cop-out
> and we might need to rethink the separation of code in mm/filemap.c.
> But IMO requiring struct file * for every pagecache is not a good
> idea.

Why not? It's not a very large structure - in fact it is a whole lot
smaller than "struct inode".

I'd rather get away from the requirement that you have to have an inode
for the mapping. Which _may_ actually work as-is (well, a lot of the code
assumes that you always have a dentry->d_inode, but that could be a common
one shared across a lot of different dentries/files).

I don't see any problem to having structure. Anybody who _really_ wants an
anonymous pointer can always use "file->f_private".

Note that having a "struct file", and having that kind of structure (and
_enforcing_ it) allows you to carry around a lot of information that is
otherwise hard to carry around. Like "who opened this file originally?"
Things that are generic, and things you might want to expose under /proc.

Let's say that you really start having meta-data in an address_space. It
can be either "per-file metadata" (ie things like the indirect block
information etc - which can be quite complex for more complex
filesystems), or it can be "global meta-data" (ie things like superblock
information etc).

I've not looked at the details, but you should be able to re-use the same
"struct file" for the per-file metadata that you already have for the
per-file actual data. So in this case you'd not actually create a new
"struct file" - you'd just use the one you have. Which you already have
hanging around, as the only time you should really need the per-file
meta-data stuff is when you're doing per-file operations (which all pass
in "struct file" - or would pass it in if "writepage()" were to pass it in
likethe NFS people want).

For global meta-data, you just create an extra "struct file" or two. No
big deal. And by doing so, you now have a common element that allows the
MM layer, for example, to have a handle on you. So that when you're
debugging the MM layer, the MM layer might be able to print out a sane
debugging message instead of just printing out "opaque handle 0xC0127762
illegal pagefault".

Some extra structure is not necessarily a bad thing.

                Linus

-
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