On 12:25 27/07, NeilBrown wrote: > 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() But do_dentry_open() is setting up the file pointer (f) based on inode (and it's i_mapping). Can f->f_mapping change within 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 -- Goldwyn
|