Re: [PATCH] int (*readpage)(struct file *, struct page *);

From: Roman V. Shaposhnick (vugluskr@unicorn.math.spbu.ru)
Date: Fri May 12 2000 - 04:37:01 EST


Jeff,

I see no point in your arguments. The situation is pretty simple:

    1. if it's general -- use it with extreme care and check
       every new unstable kernel for modifications.
    2. if it's yours -- just track changes in relevant
       areas.

what else do you need ?

Thanks,
Roman.

On Thu, May 11, 2000 at 12:55:03PM -0600, Jeff V. Merkey wrote:
>
>
> Linus Torvalds wrote:
> >
> > On Thu, 11 May 2000, Deven T. Corzine wrote:
> > >
> > > If it doesn't seem inherently obvious that the parameter could be NULL (and
> > > I'm not disputing that),
> >
> > Note that as far as the VFS proper is concerned, at this point in time the
> > parameter really never _is_ NULL. For all the native downcalls the VFS
> > layerwill actually supply a "struct file" to the filesystem.
> >
> > The only case for a NULL "file" pointer is actually brought on by the
> > filesystem itself: the symlink helper functions will create a "readpage()"
> > (or prepare_write/commit_write) call with a NULL file pointer.
> >
> > But I want to point out that this does not happen on the behalf of the
> > real VFS layer. It's really only just a few helper functions (that a
> > filesystem doesn't _have_ to use at all) that "misuse" the read/write
> > callbacks in situations where there is no "struct file" available.
> >
> > So a filesystem can simply choose to not use the
> > "page_symlink_inode_operations" at all. There are filesystems that do this
> > (autofs comes to mind, as does /proc), because the "symlink data in the
> > page cache" approach simply doesn't make sense for them.
>
> (snip)
>
> >
> > Linus
> >
>
>
> We implement a symlink_inode_operations structure so this will apply to
> us, and lots of other folks as well. i.e.
>
> struct inode_operations nwfs_symlink_inode_operations =
> {
> readlink: page_readlink,
> follow_link: page_follow_link,
> };
>
> static int nwfs_symlink_readpage(struct dentry *dentry, struct page
> *page)
> {
> register int length, count;
> BYTE *buf = (BYTE *) kmap(page);
>
> length = dentry->d_inode->i_size; // I assume I should be using
> page->mapping->host here to
> // obtain the inode * to get the
> size?
>
> count = nwfs_readlink(dentry, buf, length);
> if (count != length)
> {
> SetPageError(page);
> kunmap(page);
> UnlockPage(page);
> return -EIO;
> }
> buf[length] = '\0';
> SetPageUptodate(page);
> kunmap(page);
> UnlockPage(page);
> return 0;
>
> }
>
> struct address_space_operations nwfs_symlink_aops =
> {
> readpage: nwfs_symlink_readpage,
> };
>
> int nwfs_readlink(struct dentry *dentry, char *buffer, int bufsiz)
> {
> size_t size = dentry->d_inode->i_size;
> loff_t loffs = 0;
> ssize_t ret;
> struct file filp;
>
> NWFSSet(&filp, 0, sizeof(struct file));
> filp.f_reada = 1;
> filp.f_dentry = dentry;
> filp.f_reada = 0;
> filp.f_flags = O_RDONLY;
>
> if (size > bufsiz)
> size = bufsiz;
>
> ret = nwfs_file_read_kernel(&filp, buffer, size, &loffs);
> if (ret != size)
> ret = -EIO;
>
> return ret;
> }
>
> Looks like NCPFS, SMPFS, NWFS, and several other FS's will be affected
> and need to instrument this change.
>
> :-)
>
> Jeff

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



This archive was generated by hypermail 2b29 : Mon May 15 2000 - 21:00:20 EST