Messages in this thread | | | From | David Howells <> | Subject | Re: [PATCH] netfs: Fix gcc-12 warning by embedding vfs inode in netfs_i_context | Date | Fri, 10 Jun 2022 19:06:34 +0100 |
| |
Linus Torvalds <torvalds@linux-foundation.org> 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
| |