On Tue, 27 Jul 2021, Matthew Wilcox wrote: > On Tue, Jul 27, 2021 at 08:06:21AM +1000, NeilBrown wrote: > > You seem to be assuming that inode->i_mapping->host is always 'inode'. > > That is not the case. > > Weeeelllll ... technically, outside of the filesystems that are > changed here, the only assumption in common code that is made is that > inode_to_bdi(inode->i_mapping->host->i_mapping->host) == > inode_to_bdi(inode) Individual filesystems doing their own thing is fine. Passing just an inode to inode_to_bdi is fine. But the patch changes do_dentry_open() > > Looking at inode_to_bdi, that just means that they have the same i_sb. > Which is ... not true for character raw devices? > if (++raw_devices[minor].inuse == 1) > file_inode(filp)->i_mapping = > bdev->bd_inode->i_mapping; > but then, who's using readahead on a character raw device? They > force O_DIRECT. But maybe this should pass inode->i_mapping->host > instead of inode. Also not true in coda. coda (for those who don't know) is a network filesystem which fetches whole files (and often multiple files) at a time (like the Andrew filesystem). The files are stored in a local filesystem which acts as a cache. So an inode in a 'coda' filesystem access page-cache pages from a file in e.g. an 'ext4' filesystem. This is done via the ->i_mapping link. For (nearly?) all other filesystems, ->i_mapping is a link to ->i_data in the same inode. > > > In particular, fs/coda/file.c contains > > > > if (coda_inode->i_mapping == &coda_inode->i_data) > > coda_inode->i_mapping = host_inode->i_mapping; > > > > So a "coda_inode" shares the mapping with a "host_inode". > > > > This is why an inode has both i_data and i_mapping. > > > > So I'm not really sure this patch is safe. It might break codafs. > > > > But it is more likely that codafs isn't used, doesn't work, should be > > removed, and i_data should be renamed to i_mapping. > > I think there's also something unusual going on with either ocfs2 > or gfs2. But yes, I don't understand the rules for when I need to > go from inode->i_mapping->host. > Simple. Whenever you want to work with the page-cache pages, you cannot assume anything in the original inode is relevant except i_mapping (and maybe i_size I guess). NeilBrown
