nfs3_proc_write()
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
Quoted 11 lines
By using objdump, the exact instruction which fails in the
xdr_kmap()as the followinglen = 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 thenfs_writepage_sync()assumespage_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
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
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
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