Re: Socket related code duplication in NFS

On Thu, 21 May 2009 07:33:20 -0700, Sam Leffler <sam_at_freebsd.org> wrote:
> Doug Rabson wrote:
>> On Thu, 21 May 2009 11:36:49 +0100 (BST), Robert Watson
>> <rwatson_at_FreeBSD.org> wrote:
>>   
>>> On Wed, 20 May 2009, Robert Watson wrote:
>>>
>>>     
>>>> On Wed, 20 May 2009, Andre Oppermann wrote:
>>>>
>>>>       
>>>>> While working on an optimized soreceive_stream() function [1] and
>>>>> checking
>>>>> the code how it is used I've come across quite a bit of code
>>>>>         
>> duplication
>>   
>>>>> in
>>>>> the various NFS directories.
>>>>>
>>>>> Socket (read) operations are handled multiple times in a very similar

>>>>> manner in these places:
>>>>>         
>>>> My recommendation would be to do this analysis against the new NFS
>>>>       
>> client
>>   
>>>> and server found in sys/{kgssapi,nlm,fs/{nfs,nfsclient,nfsserver}},
>>>>       
>> which
>>   
>>>> is
>>>> the NFSv234 implementation.  Note in particular that in the new world
>>>> order
>>>> there's a centralize RPC implementation.
>>>>
>>>> The code you're looking at is a blend of the old NFSv23 client/server 
>>>> (nfsclient/nfsserver) and the old NFSv4 client (rpc/nfs4client), all
if
>>>>
>>>> which are on a gradual de-orbit burn.
>>>>       
>>> After re-reading this e-mail, I realize that I'd mislabeled src/sys/rpc
>>>     
>> as 
>>   
>>> being only used by the old code -- this is in fact not the case, it's
>>>     
>> also 
>>   
>>> used by the new code.
>>>     
>>
>> Everything in src/sys/rpc except rpcclient.[ch] is part of the new RPC
>> transport. The files rpcclient.c and rpcclient.h are part of the old
>> broken
>> NFSv4 client which is scheduled for removal.
>>
>> There is older RPC transport code in src/sys/nfsclient and
>> src/sys/nfsserver which is currently used if the user uses the
>> NFS_LEGACYRPC option. This too should be removed before 8.0 ships.
>>   
> 
> NFS_LEGACYRPC is the only code that works reliably for me on xscale (BE 
> arm).  The new code randomly causes alignment faults.  I've not had time 
> to track down this problem but gonzo_at_ also reported this problem on mips 
> and I believe sent you a line# and stack trace of a fault.  Until this 
> problem is fixed removing legacy support cannot happen.

I'm aware of this one but I'm not exactly certain where the problem is. I
did spend some time dredging through email archives trying to find out
without success. I have a good idea where the bug is but I'd like to see at
least one stacktrace or similar which confirms my suspicion before I commit
a fix. When I get home, I'll take another look and see if I can find your
original report. I'm certainly not going to be removing the NFS_LEGACYRPC
option until this is fixed.