Re: problem with recent VFS changes

Alexander Viro (viro@math.psu.edu)
Fri, 17 Dec 1999 15:58:12 -0500 (EST)


On 17 Dec 1999, Scott Henry wrote:

>
> I'm working on updating our driver to the changes in the 2.3.30+
> kernels, and one of the VFS changes has me stumped (I'm also
> changing to use the kiobuf stuff instead of our own code -- that's
> going fine so far...).
>
> In struct inode_operations, the following interfaces changed:
>
> from:
>
> int (*readpage) (struct file *, struct page *);
> int (*writepage) (struct file *, struct page *);
>
> to:
>
> int (*readpage) (struct dentry *, struct page *);
> int (*writepage) (struct dentry *, struct page *);
>
>
> But I need some info from the file structure (file->f_pos at least).

No, you don't. _Especially_ ->f_pos - otherwise results of reading from
mmap()'d area will depend on lseek() done on the same file. If you were
using it - you had real, honest bug. _If_ you need page offset - use
page->index << PAGE_CACHE_SHIFT. And keep in mind that it will work only
if you are using generic_file_mmap() as ->mmap() for your device. Which
may be not exactly what you need.

Moreover, even more changes are to come - readpage() and writepage() are
going away from inode_operatoins and become address_space methods (along
with get_block()). Think twice - do you really need generic_file_mmap()
here? _None_ of the currently existing devices in the main tree are using
it - it's pure filesystem thing. And certainly all drivers in the official
tree have both ->readpage() and ->writepage() NULL. Heck, none of them has
non-trivial inode_operations - only default_file_operations is there.

Again, ->readpage() and ->writepage() are _NOT_ normal inode methods -
they are callbacks provided by fs to _some_ implementations of read() and
mmap(). Unless you are using such implementation (generic_file_read() and
generic_file_mmap()) they don't make any sense. They don't belong to
inode_operations and they are going away - just as ->flushpage() did.

Think of that - we have address space for each regular file. And in such
situation (usage of pagecache) readpage() and friends make sense. But
files are not the only source of such beasts - swap uses one, quite
probably we may want several address spaces for one file (think of NTFS
and friends _and_ of caching metadata), we may also want to use them for
shm segments, etc. This stuff doesn't belong to file and we not always
_have_ a file, to start with. Even for filesystem sitautions - just think
of handling symlinks. And ->get_block() is even worse - it doesn't make
sense for many filesystems, to start with.

And no matter which variant of mmap() you are using, you should not use
->f_pos. Ever. Otherwise random lseek() will screw you royally.

-
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/