Skip to contentSkip to conversation

nfs3_proc_write()

DISCUSSION David Chow 2 participants 5 messages Jul 3, 2003 VFS

lore
Share & tools
lore

David Chow

author
#0 lore raw

Trond,

I am in the middle of porting the my old nfs_swap code from 2.4.19 to 2.4.21 . I notice the interface of nfs3_proc_write() has changed, the old code uses a (char *)buffer but the new one uses a (struct page *) . When I did the following change, it didn't work.

#if LINUX_VERSION_CODE < KERNEL_VERSION(2,4,20)
u8 *buffer = (char *)kmap(page);
#endif

result = NFS_PROTO(inode)->write(inode, cred,

                        &fattr,
                        NFS_RW_SWAP|NFS_RW_SYNC,
#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,20)
                        offset, wsize, page, &verf);
#else                         
                        offset, wsize, buffer, &verf);

#endif

What happens is it causes an exception in kmap() which is called by xdr_kmap() . By tracing the stack with kdb, the path is as follows

nfs3_proc_write-> nfs3_rpc_wrapper->
rpc_call_sync->rpc_execute->__rpc_execute->call_transmit->xprt_trnasmit->
do_xprt_transmit->xdr_kmap->kmap(inlined)

By using objdump, the exact instruction which fails in the xdr_kmap() as the following

       len = PAGE_CACHE_SIZE;
       if (first_kmap) {
           first_kmap = 0;
!!!!           iov->iov_base = kmap(*ppage); !!!!
       } else {

The exception is caused by the return page_address(page) . I've also tried a trival kmap(page) before triggering nfs3_proc_write() but the result is the same. My system only have 128MB RAM so I think it is trival that kmap(0 will actually do anything. Looks like the write path code of nfs has change a bit which causes this not going to work. The reason why nfs_writepage_sync() can't be used is because the nfs_writepage_sync() assumes page_offset(page) equals to file offset but in a swap page this is not true. Please give some hints what is going on or which part of code I should look for.

regards,
David Chow

#1 lore raw
Quoted 11 lines

By using objdump, the exact instruction which fails in the xdr_kmap() as the following

       len = PAGE_CACHE_SIZE;
       if (first_kmap) {
           first_kmap = 0;
!!!!           iov->iov_base = kmap(*ppage); !!!!
       } else {

The exception is caused by the return page_address(page)

You don't want to be writing wsize bytes unless wsize <= PAGE_SIZE. My guess is that this is why you are seeing the exception.

Quoted 4 lines

The reason why nfs_writepage_sync() can't be used is because the nfs_writepage_sync() assumes page_offset(page) equals to file offset but in a swap page this is not true. Please give some hints what is going on or which part of code I should look for.

This is not only an assumption in nfs_writepage_sync(), but is also true in the nfs*_proc_write() code. If you want file offsets that are not relative to page_offset(page), you'll have to add them as an extra argument...

Cheers,
Trond

#2 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

" " == Trond Myklebust <[email protected]> writes:

> You don't want to be writing wsize bytes unless wsize <= > PAGE_SIZE. My guess is that this is why you are seeing the > exception.

Actually, thinking about it one more time...

You've just thrown the file offset as the argument to ->write() instead of setting the offset relative to the start of the page, haven't you?

That will *seriously* confuse the RPC sendmsg() code (which is where you say the Oops occurs). It just wants to know at what offset within the page you specified it is supposed to start sending data...

Cheers,
Trond

author
#3 lore raw
Quoted 14 lines · Trond Myklebust
Quoted 7 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

" " == Trond Myklebust <[email protected]> writes:

You don't want to be writing wsize bytes unless wsize <= PAGE_SIZE. My guess is that this is why you are seeing the exception.

Actually, thinking about it one more time...

You've just thrown the file offset as the argument to ->write() instead of setting the offset relative to the start of the page, haven't you?

Quoted 8 lines

That will *seriously* confuse the RPC sendmsg() code (which is where you say the Oops occurs). It just wants to know at what offset within the page you specified it is supposed to start sending data...

Cheers,
Trond

Yep, I've just discovered and fix the problem by adding two extra rpc_nfs_ops called offset_read() and offset_wirte() which allow read/write from arbitrary offsets. Some preliminary test has passed.

I think using page_offset(page) to calculate the offset from PAGE_CACHE_SIZE is not a pretty good idea. As this interface doesn't look like nfs protocol 2/3/4 . Also, it will prohitbit any implementation of swap over NFS because page->index has no meaning to the actual writing offset of the swapfile. Anyway, it is only my personal recommendation, but looks like it does prohibits some functionality. Thanks for your comment.

regards,
David Chow

#4 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

" " == David Chow <[email protected]> writes:

> I think using page_offset(page) to calculate the offset from > PAGE_CACHE_SIZE is not a pretty good idea. As this interface > doesn't look like nfs protocol 2/3/4.

Huh???

> Also, it will prohitbit any implementation of swap over NFS > because page->index has no meaning to the actual writing offset > of the swapfile. Anyway, it is only my personal
> recommendation, but looks like it does prohibits some > functionality. Thanks for your comment.

Those APIs have not been designed for the benefit of NFS swap. They've been designed for reads/writes to the page cache. That is all we currently support in the standard kernel.
Why should we keep a load of redundant function arguments in order to support doing writes from arbitrary pages when there is nothing in the current kernel that needs this?

You are quite free to define your own specialized read/write interface to suit the NFS swap patch needs. Despite what you appear to be saying above, there is nothing that prohibits this...

Cheers,
Trond