uma_zalloc_arg complaining about non-sleepable locks

[Rick Macklem]

On Mon, 1 Feb 2010, John Baldwin wrote:

>>> I'd say that your patch works.
>>
>> John, are you okay with that patch?
>> http://people.freebsd.org/~marius/fha_extract_info_realign2.diff
>>
>> It's intention is to:
>> - Move nfs_realign() from the NFS client to the shared NFS code and
>>   remove the NFS server version in order to reduce code duplication.
>>   The shared version now uses a second parameter how, which is passed
>>   on to m_get(9) and m_getcl(9) as the server used M_WAIT while the
>>   client requires M_DONTWAIT, and replaces the the previously unused
>>   parameter hsiz.

I don't think the client side can use M_WAIT/M_WAITOK if it wants to.
(I believe that it was once called from the socket upcall and that was
  why M_DONTWAIT was used, but that isn't how it is called in the client
  now.)

I mentioned this to dfr@ because I use a version shared between client
and server for the experimental one that does M_WAIT, but he didn't
think the regular client needed to be changed. Basically, I think the 
current code in the regular client can return ENOMEM for I/O system calls, 
which probably isn't what the app. expected. In the NFS client, you end up
with this "do I wait forever?" or "do I return an error to an I/O system
call which an app. doesn't expect" issue cropping up here and there. I
don't know the correct answer to this, but tend to lean in the "wait 
forever" direction.

>> - Change nfs_realign() to use nfsm_aligned() so as with other NFS code
>>   the alignment check isn't actually performed on platforms without
>>   strict alignment requirements for performance reasons because as the
>>   comment suggests only occasionally occurs with TCP.
>> - Change fha_extract_info() to use nfs_realign() with M_NOWAIT rather
>>   than M_DONTWAIT because it's called with the RPC sp_lock held.
>>

Btw, from an historical perspective, this was originally added for the
DEC PMAX port (MIPS R2000), which would trap whenever an unaligned ptr
was used. Then, there was this involved chunk of MIPS assembly code that
worked around the unaligned ptr access and the trap returned. Obviously 
this was a major performance hit and happened fairly frequently for NFS 
over ISO TP4. As you've seen, it happens infrequently enough over TCP 
(back then, I think it was only when the TCP window size ended up really 
small, although I'm way out of date on the TCP stack, so I have no idea 
now:-) that I'd only do the re-alignment on achitectures that will crash
if it isn't done?

I missed the beginning of this. Was there crashes occurring because
the alignment wasn't being done or problems because the alignment
wasn't working correctly? (I guess I'm trying to say that, if the
arch works without nfs_realign(), then you might consider just not
doing it for that arch. I suppose that isn't a good enough reason
to not fix the function, though?;-)

>> The only downside of the shared nfs_realign() are the combined
>> SYSCTL counters but the fact we incremented them non-atomically
>> so far I think already indicates that their intention only is a
>> rough indication rather than exact values for client and server.
>

I'll admit I don't see how NFSCLIENT and NFSSERVER can both be loaded
as modules without the stuff in sys/nfs/nfs_common.c coming up
multiply defined, but it seems to work, so...

> This all sounds good to me, but isn't M_NOWAIT == M_DONTWAIT?  Hmm, reading
> the code more closely, it looks like fha_extract_info() was using M_WAIT
> rather than M_DONTWAIT previously.  Also, I think you should be careful to use
> M_DONTWAIT instead of M_NOWAIT for mbufs, so I would fix that in
> fha_extract_info().
>
As noted above, I think the client can use M_WAIT safely. It was just a
design choice to do otherwise.

Have fun with it, rick