Re: problem with recent VFS changes

Scott Henry (scotth@sgi.com)
17 Dec 1999 15:16:42 -0800


>>>>> "A" == Alexander Viro <viro@math.psu.edu> writes:

A> 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).

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

Ah, good to know that. I'll check what we are doing elsewhere.

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

I mis-spoke slightly. What we have is a filesystem and a device
masquerading as a filesystem (kind of plan9-ish concept).

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

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

I was mostly looking at the existing standard functions and using
the ones that seem appropriate. Then implemeting the pieces that
they call.

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

Thanks for the info. I'll probably have more questions later...

-- 
 Scott Henry <scotth@sgi.com> /  Help! My disclaimer is missing!
 IRIX MTS,                   /  GIGO *really* means: Garbage in, Gospel Out
 Silicon Graphics, Inc      /  http://reality.sgi.com/scotth/

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