Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> Side note: I think this could have been done with an unnamed union as
>
> struct my_inode {
> union {
> struct inode vfs_inode;
> struct netfs_inode netfs_inode;
> };
> [...]
>
> instead, with the rule that 'netfs_inode' always starts with a 'struct inode'.
I'm slightly wary of that, lest struct netfs_inode gets randomised. I'm not
sure how likely that would be without netfs_inode getting explicitly marked.
> But in a lot of cases you really could do so much better: you *have* a
> "struct netfs_inode" to begin with, but you converted it to just
> "struct inode *", and now you're converting it back.
>
> Look at that AFS code, for example, where we have afs_vnode_cache() doing
>
> return netfs_i_cookie(&vnode->netfs.inode);
>
> and look how it *had* a netfs structure, and it was passing it to a
> netfs function, but it explicitly passed the WRONG TYPE, so now we've
> lost the type information and it is using that cast to fake it all
> back.
Yeah, I didn't look at those as they didn't cause warnings, but you're right -
those should take struct netfs_inode pointers in some cases, rather than
struct inode.
Note that some functions, such as netfs_readpage() and netfs_readpages() do
need to take struct inode pointers as I'm trying to get the VFS ops to jump
into netfslib and get all the VM interface stuff out of the network
filesystems - the idea being that the network filesystem will provide netfslib
primarily with two functions: do a read op and do a write op.
I'll have a look at your patch in a bit.
Thanks,
David
