Re: [PATCH] address_space_operations unification

From: Trond Myklebust (trond.myklebust@fys.uio.no)
Date: Sun May 07 2000 - 04:35:01 EST


>>>>> Linus Torvalds <torvalds@transmeta.com> writes:

> - remove "dentry" from the argument list. Fine. Not that I see _why_, as
> it just means that anybody who now needs the inode will
> instead do

> - struct inode *inode = dentry->d_inode;
> + struct inode *inode = (struct inode*)page->mapping->host;

No. Not good enough for those of us who actually need a dentry. c.f.
the 'readlink' code.

> whichin my opinion is not exactly making the code prettier.

> If "mapping->host" was of type "struct inode *", I might
> agree with it. At least it wouldn't need that terminally
> ugly cast. The code does in fact apparently enforce the
> "mapping->host is of type 'struct inode'", but doesn't make
> it explicit.

> - remove "file" from the argument list, replacing it with "void *
> cookie", which actually gets the value "file".

> struct file * file = (struct file *)cookie;

> which again is not, in my not so d*mn humble opinion, any
> improvement at all.

> In short, both of the changes resulted in (a) uglier code and
> (b) loss of typechecking.

> Note that the code did not add any new information - it
> couldn't do that. It just hid the information we _did_ have
> available, and made it uglier.

> And people then _applaud_ this move?

Not the cookie per se, but the struct file move.

My own outbreak of cookiephilia stems from the fact that we've tried
to force apples and oranges into a single one-size-fits-all
mapping->readpage() callback.

For the case of NFS:
Open files have a need for one set of arguments. In addition to the
dentry we want some form of access to the authentication info from the
struct file. (There are two ways of doing this either we pass the
struct file as an argument, or we allow ourselves to play with some
process variable such as current->fsuid/fsgid.)
On the other hand things like symlinks, which use the same readpage()
interface, aren't associated to open files, but still need an NFS file
handle, want only a dentry.

Cheers,
  Trond

-
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:20 EST