* Regression caused by commit c54f24e3 (nfsd: fix performance-limiting session calculation)
[not found] ` <8c3e0249-b17f-4bd2-4a46-afd4d35f4763@molgen.mpg.de>
@ 2019-07-02 21:59 ` Paul Menzel
2019-07-03 9:56 ` Paul Menzel
0 siblings, 1 reply; 6+ messages in thread
From: Paul Menzel @ 2019-07-02 21:59 UTC (permalink / raw)
To: J. Bruce Fields, Jeff Layton; +Cc: Chris Tracy, linux-nfs, LKML, it+linux-nfs
Dear Bruce,
Could it be that commit c54f24e3 (nfsd: fix performance-limiting session
calculation) causes a regression on big memory machines (1 TB)?
> From c54f24e338ed2a35218f117a4a1afb5f9e2b4e64 Mon Sep 17 00:00:00 2001
> From: "J. Bruce Fields" <bfields@redhat.com>
> Date: Thu, 21 Feb 2019 10:47:00 -0500
> Subject: [PATCH] nfsd: fix performance-limiting session calculation
>
> We're unintentionally limiting the number of slots per nfsv4.1 session
> to 10. Often more than 10 simultaneous RPCs are needed for the best
> performance.
>
> This calculation was meant to prevent any one client from using up more
> than a third of the limit we set for total memory use across all clients
> and sessions. Instead, it's limiting the client to a third of the
> maximum for a single session.
>
> Fix this.
>
> Reported-by: Chris Tracy <ctracy@engr.scu.edu>
> Cc: stable@vger.kernel.org
> Fixes: de766e570413 "nfsd: give out fewer session slots as limit approaches"
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
> fs/nfsd/nfs4state.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index fb3c9844c82a..6a45fb00c5fc 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1544,16 +1544,16 @@ static u32 nfsd4_get_drc_mem(struct nfsd4_channel_attrs *ca)
> {
> u32 slotsize = slot_bytes(ca);
> u32 num = ca->maxreqs;
> - int avail;
> + unsigned long avail, total_avail;
>
> spin_lock(&nfsd_drc_lock);
> - avail = min((unsigned long)NFSD_MAX_MEM_PER_SESSION,
> - nfsd_drc_max_mem - nfsd_drc_mem_used);
> + total_avail = nfsd_drc_max_mem - nfsd_drc_mem_used;
> + avail = min((unsigned long)NFSD_MAX_MEM_PER_SESSION, total_avail);
> /*
> * Never use more than a third of the remaining memory,
> * unless it's the only way to give this client a slot:
> */
> - avail = clamp_t(int, avail, slotsize, avail/3);
> + avail = clamp_t(int, avail, slotsize, total_avail/3);
> num = min_t(int, num, avail / slotsize);
> nfsd_drc_mem_used += num * slotsize;
> spin_unlock(&nfsd_drc_lock);
Booting a 80 threads, 1 TB server with Linux 4.19.56 and Linux 5.2-rc7
causes connections problems for the clients. The problems do not happen
on servers with just 96 GB memory for example. Bisecting points to the
two commits below (and I can only continue tomorrow).
c54f24e338ed2a35218f117a4a1afb5f9e2b4e64 (nfsd: fix performance-limiting
session calculation)
8127d82705998568b52ac724e28e00941538083d (NFS: Don't recoalesce on error
in nfs_pageio_complete_mirror())
If you have things I could do to verify this besides reverting it
tomorrow, please tell. It’d be great if it could be fixed before Linux
5.2 is released.
Kind regards,
Paul
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Regression caused by commit c54f24e3 (nfsd: fix performance-limiting session calculation)
2019-07-02 21:59 ` Regression caused by commit c54f24e3 (nfsd: fix performance-limiting session calculation) Paul Menzel
@ 2019-07-03 9:56 ` Paul Menzel
2019-07-03 12:54 ` [PATCH] nfsd: Fix overflow causing non-working mounts on 1 TB machines Paul Menzel
0 siblings, 1 reply; 6+ messages in thread
From: Paul Menzel @ 2019-07-03 9:56 UTC (permalink / raw)
To: J. Bruce Fields, Jeff Layton; +Cc: Chris Tracy, linux-nfs, LKML, it+linux-nfs
[-- Attachment #1: Type: text/plain, Size: 13833 bytes --]
Dear Bruce,
On 7/2/19 11:59 PM, Paul Menzel wrote:
> Could it be that commit c54f24e3 (nfsd: fix performance-limiting
> session calculation) causes a regression on big memory machines (1
> TB)?
>
>> From c54f24e338ed2a35218f117a4a1afb5f9e2b4e64 Mon Sep 17 00:00:00 2001
>> From: "J. Bruce Fields" <bfields@redhat.com>
>> Date: Thu, 21 Feb 2019 10:47:00 -0500
>> Subject: [PATCH] nfsd: fix performance-limiting session calculation
>>
>> We're unintentionally limiting the number of slots per nfsv4.1 session
>> to 10. Often more than 10 simultaneous RPCs are needed for the best
>> performance.
>>
>> This calculation was meant to prevent any one client from using up more
>> than a third of the limit we set for total memory use across all clients
>> and sessions. Instead, it's limiting the client to a third of the
>> maximum for a single session.
>>
>> Fix this.
>>
>> Reported-by: Chris Tracy <ctracy@engr.scu.edu>
>> Cc: stable@vger.kernel.org
>> Fixes: de766e570413 "nfsd: give out fewer session slots as limit approaches"
>> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
>> ---
>> fs/nfsd/nfs4state.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index fb3c9844c82a..6a45fb00c5fc 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -1544,16 +1544,16 @@ static u32 nfsd4_get_drc_mem(struct nfsd4_channel_attrs *ca)
>> {
>> u32 slotsize = slot_bytes(ca);
>> u32 num = ca->maxreqs;
>> - int avail;
>> + unsigned long avail, total_avail;
>>
>> spin_lock(&nfsd_drc_lock);
>> - avail = min((unsigned long)NFSD_MAX_MEM_PER_SESSION,
>> - nfsd_drc_max_mem - nfsd_drc_mem_used);
>> + total_avail = nfsd_drc_max_mem - nfsd_drc_mem_used;
>> + avail = min((unsigned long)NFSD_MAX_MEM_PER_SESSION, total_avail);
>> /*
>> * Never use more than a third of the remaining memory,
>> * unless it's the only way to give this client a slot:
>> */
>> - avail = clamp_t(int, avail, slotsize, avail/3);
>> + avail = clamp_t(int, avail, slotsize, total_avail/3);
>> num = min_t(int, num, avail / slotsize);
>> nfsd_drc_mem_used += num * slotsize;
>> spin_unlock(&nfsd_drc_lock);
>
> Booting a 80 threads, 1 TB server with Linux 4.19.56 and Linux
> 5.2-rc7 causes connections problems for the clients. The problems do
> not happen on servers with just 96 GB memory for example. Bisecting
> points to the two commits below (and I can only continue tomorrow).
>
> c54f24e338ed2a35218f117a4a1afb5f9e2b4e64 (nfsd: fix performance-limiting session calculation)
> 8127d82705998568b52ac724e28e00941538083d (NFS: Don't recoalesce on error in nfs_pageio_complete_mirror())
>
> If you have things I could do to verify this besides reverting it
> tomorrow, please tell. It’d be great if it could be fixed before Linux
> 5.2 is released.
Reverting the suspected commit c54f24e3 indeed fixes the regression.
Please tell me how to continue. Should the commit be reverted or can
a fix be made shortly?
On a running system, `rpcdebug -m nfsd -s all` and
`rpcdebug -m rpc -s all` do not give a lot of information, as the
interesting values are set during module initialization.
Kind regards,
Paul
[ 765.444765] svc: socket 0000000093976859 TCP (listen) state change 10
[ 765.444784] svc: socket 0000000015376384 TCP (listen) state change 1
[ 765.444869] svc: tcp_accept 000000002ad76b70 sock 000000005eae2611
[ 765.444879] nfsd: connect from 141.14.17.51, port=785
[ 765.444881] svc: svc_setup_socket 000000003fe99ad1
[ 765.444883] setting up TCP socket for reading
[ 765.444885] svc: svc_setup_socket created 000000006952397a (inet 0000000015376384), listen 0 close 0
[ 765.444901] svc: server 00000000aad03597 waiting for data (to = 3600000)
[ 765.444904] svc: server 00000000aad03597, pool 0, transport 000000006952397a, inuse=2
[ 765.444907] svc: tcp_recv 000000006952397a data 1 conn 0 close 0
[ 765.444912] svc: socket 000000006952397a recvfrom(000000005fae69c4, 4) = 4
[ 765.444913] svc: TCP record, 40 bytes
[ 765.444919] svc: socket 000000006952397a recvfrom(00000000bce6ffde, 4096) = 40
[ 765.444920] svc: TCP final record (40 bytes)
[ 765.444926] svc: svc_authenticate (0)
[ 765.444930] svc: calling dispatcher
[ 765.444931] nfsd_dispatch: vers 4 proc 0
[ 765.444954] svc: socket 000000006952397a sendto([000000005133ae9c 28... ], 28) = 28 (addr 141.14.17.51, port=785)
[ 765.444963] svc: server 00000000aad03597 waiting for data (to = 3600000)
[ 765.444967] svc: tcp_accept 000000002ad76b70 sock 000000005eae2611
[ 765.444975] svc: server 00000000aad03597 waiting for data (to = 3600000)
[ 765.444980] svc: server 00000000be144e7a, pool 0, transport 000000006952397a, inuse=2
[ 765.444983] svc: tcp_recv 000000006952397a data 1 conn 0 close 0
[ 765.444988] svc: socket 000000006952397a recvfrom(000000005fae69c4, 4) = -11
[ 765.444989] RPC: TCP recv_record got -11
[ 765.444990] RPC: TCP recvfrom got EAGAIN
[ 765.444993] svc: server 00000000be144e7a waiting for data (to = 3600000)
[ 765.445020] svc: server 000000002b852bb6 waiting for data (to = 3600000)
[ 765.445023] svc: server 0000000011abceff waiting for data (to = 3600000)
[ 765.445177] svc: socket 000000006952397a(inet 0000000015376384), busy=0
[ 765.445401] svc: server 00000000aad03597, pool 0, transport 000000006952397a, inuse=2
[ 765.445411] svc: tcp_recv 000000006952397a data 1 conn 0 close 0
[ 765.445414] svc: socket 000000006952397a recvfrom(000000005fae69c4, 4) = 4
[ 765.445415] svc: TCP record, 252 bytes
[ 765.445418] svc: socket 000000006952397a recvfrom(00000000bce6ffde, 4096) = 252
[ 765.445419] svc: TCP final record (252 bytes)
[ 765.445427] svc: svc_authenticate (1)
[ 765.445431] svc: calling dispatcher
[ 765.445432] nfsd_dispatch: vers 4 proc 1
[ 765.445436] nfsd4_exchange_id rqstp=00000000aad03597 exid=000000000472f450 clname.len=32 clname.data=00000000ed18490f ip_addr=141.14.17.51 flags 101, spa_how 0
[ 765.445442] renewing client (clientid 5d1c72c1/ed57da3d)
[ 765.445443] nfsd4_exchange_id seqid 0 flags 10001
[ 765.445444] nfsv4 compound returned 0
[ 765.445458] svc: socket 000000006952397a sendto([00000000d62d9bb6 136... ], 136) = 136 (addr 141.14.17.51, port=785)
[ 765.445460] svc: server 00000000aad03597 waiting for data (to = 3600000)
[ 765.445462] svc: server 000000002b852bb6, pool 0, transport 000000006952397a, inuse=2
[ 765.445464] svc: tcp_recv 000000006952397a data 1 conn 0 close 0
[ 765.445467] svc: socket 000000006952397a recvfrom(000000005fae69c4, 4) = -11
[ 765.445472] RPC: TCP recv_record got -11
[ 765.445473] RPC: TCP recvfrom got EAGAIN
[ 765.445475] svc: server 000000002b852bb6 waiting for data (to = 3600000)
[ 765.445682] svc: socket 000000006952397a(inet 0000000015376384), busy=0
[ 765.445760] svc: server 00000000aad03597, pool 0, transport 000000006952397a, inuse=2
[ 765.445762] svc: tcp_recv 000000006952397a data 1 conn 0 close 0
[ 765.445765] svc: socket 000000006952397a recvfrom(000000005fae69c4, 4) = 4
[ 765.445769] svc: TCP record, 252 bytes
[ 765.445773] svc: socket 000000006952397a recvfrom(00000000bce6ffde, 4096) = 252
[ 765.445774] svc: TCP final record (252 bytes)
[ 765.445779] svc: svc_authenticate (1)
[ 765.445784] svc: calling dispatcher
[ 765.445785] nfsd_dispatch: vers 4 proc 1
[ 765.445789] nfsd4_exchange_id rqstp=00000000aad03597 exid=000000000472f450 clname.len=32 clname.data=00000000ed18490f ip_addr=141.14.17.51 flags 101, spa_how 0
[ 765.445795] renewing client (clientid 5d1c72c1/ed57da3e)
[ 765.445795] nfsd4_exchange_id seqid 0 flags 10001
[ 765.445854] svc: server 000000002b852bb6, pool 0, transport 000000006952397a, inuse=3
[ 765.445855] svc: tcp_recv 000000006952397a data 1 conn 0 close 0
[ 765.445859] svc: socket 000000006952397a recvfrom(000000005fae69c4, 4) = -11
[ 765.445860] RPC: TCP recv_record got -11
[ 765.445860] RPC: TCP recvfrom got EAGAIN
[ 765.445862] svc: server 000000002b852bb6 waiting for data (to = 3600000)
[ 765.445995] nfsv4 compound returned 0
[ 765.446013] svc: socket 000000006952397a sendto([000000006f8e68ac 136... ], 136) = 136 (addr 141.14.17.51, port=785)
[ 765.446020] svc: server 00000000aad03597 waiting for data (to = 3600000)
[ 765.446178] svc: socket 000000006952397a(inet 0000000015376384), busy=0
[ 765.446193] svc: server 00000000aad03597, pool 0, transport 000000006952397a, inuse=2
[ 765.446198] svc: tcp_recv 000000006952397a data 1 conn 0 close 0
[ 765.446200] svc: socket 000000006952397a recvfrom(000000005fae69c4, 4) = 4
[ 765.446201] svc: TCP record, 220 bytes
[ 765.446205] svc: socket 000000006952397a recvfrom(00000000bce6ffde, 4096) = 220
[ 765.446222] svc: TCP final record (220 bytes)
[ 765.446226] svc: svc_authenticate (1)
[ 765.446229] svc: calling dispatcher
[ 765.446230] nfsd_dispatch: vers 4 proc 1
[ 765.446233] nfsv4 compound returned 10008
[ 765.446243] svc: socket 000000006952397a sendto([00000000dda22c57 48... ], 48) = 48 (addr 141.14.17.51, port=785)
[ 765.446244] svc: server 00000000aad03597 waiting for data (to = 3600000)
[ 765.446250] svc: server 00000000aad03597, pool 0, transport 000000006952397a, inuse=2
[ 765.446251] svc: tcp_recv 000000006952397a data 1 conn 0 close 0
[ 765.446253] svc: socket 000000006952397a recvfrom(000000005fae69c4, 4) = -11
[ 765.446253] RPC: TCP recv_record got -11
[ 765.446254] RPC: TCP recvfrom got EAGAIN
[ 765.446257] svc: server 00000000aad03597 waiting for data (to = 3600000)
[ 765.446397] svc: server 000000002b852bb6 waiting for data (to = 3600000)
[ 766.500412] svc: socket 000000006952397a(inet 0000000015376384), busy=0
[ 766.500445] svc: server 00000000aad03597, pool 0, transport 000000006952397a, inuse=2
[ 766.500448] svc: tcp_recv 000000006952397a data 1 conn 0 close 0
[ 766.500452] svc: socket 000000006952397a recvfrom(000000005fae69c4, 4) = 4
[ 766.500454] svc: TCP record, 220 bytes
[ 766.500459] svc: socket 000000006952397a recvfrom(00000000bce6ffde, 4096) = 220
[ 766.500461] svc: TCP final record (220 bytes)
[ 766.500467] svc: svc_authenticate (1)
[ 766.500472] svc: calling dispatcher
[ 766.500474] nfsd_dispatch: vers 4 proc 1
[ 766.500480] nfsv4 compound returned 10008
[ 766.500498] svc: socket 000000006952397a sendto([000000006f8e68ac 48... ], 48) = 48 (addr 141.14.17.51, port=785)
[ 766.500505] svc: server 00000000aad03597 waiting for data (to = 3600000)
[ 766.500509] svc: server 00000000aad03597, pool 0, transport 000000006952397a, inuse=2
[ 766.500512] svc: tcp_recv 000000006952397a data 1 conn 0 close 0
[ 766.500515] svc: socket 000000006952397a recvfrom(000000005fae69c4, 4) = -11
[ 766.500516] RPC: TCP recv_record got -11
[ 766.500517] RPC: TCP recvfrom got EAGAIN
[ 766.500519] svc: server 00000000aad03597 waiting for data (to = 3600000)
[ 766.500546] svc: server 000000002b852bb6 waiting for data (to = 3600000)
[ 767.524372] svc: socket 000000006952397a(inet 0000000015376384), busy=0
[ 767.524405] svc: server 00000000aad03597, pool 0, transport 000000006952397a, inuse=2
[ 767.524408] svc: tcp_recv 000000006952397a data 1 conn 0 close 0
[ 767.524413] svc: socket 000000006952397a recvfrom(000000005fae69c4, 4) = 4
[ 767.524415] svc: TCP record, 220 bytes
[ 767.524420] svc: socket 000000006952397a recvfrom(00000000bce6ffde, 4096) = 220
[ 767.524422] svc: TCP final record (220 bytes)
[ 767.524428] svc: svc_authenticate (1)
[ 767.524434] svc: calling dispatcher
[ 767.524435] nfsd_dispatch: vers 4 proc 1
[ 767.524441] nfsv4 compound returned 10008
[ 767.524448] svc: server 000000002b852bb6, pool 0, transport 000000006952397a, inuse=3
[ 767.524451] svc: tcp_recv 000000006952397a data 1 conn 0 close 0
[ 767.524470] svc: socket 000000006952397a sendto([00000000dda22c57 48... ], 48) = 48 (addr 141.14.17.51, port=785)
[ 767.524474] svc: socket 000000006952397a recvfrom(000000005fae69c4, 4) = -11
[ 767.524476] svc: server 00000000aad03597 waiting for data (to = 3600000)
[ 767.524477] RPC: TCP recv_record got -11
[ 767.524478] RPC: TCP recvfrom got EAGAIN
[ 767.524482] svc: server 000000002b852bb6 waiting for data (to = 3600000)
[ 768.548339] svc: socket 000000006952397a(inet 0000000015376384), busy=0
[ 768.548372] svc: server 00000000aad03597, pool 0, transport 000000006952397a, inuse=2
[ 768.548375] svc: tcp_recv 000000006952397a data 1 conn 0 close 0
[ 768.548379] svc: socket 000000006952397a recvfrom(000000005fae69c4, 4) = 4
[ 768.548381] svc: TCP record, 220 bytes
[ 768.548386] svc: socket 000000006952397a recvfrom(00000000bce6ffde, 4096) = 220
[ 768.548387] svc: TCP final record (220 bytes)
[ 768.548394] svc: svc_authenticate (1)
[ 768.548400] svc: calling dispatcher
[ 768.548401] nfsd_dispatch: vers 4 proc 1
[ 768.548407] nfsv4 compound returned 10008
[ 768.548425] svc: socket 000000006952397a sendto([000000006f8e68ac 48... ], 48) = 48 (addr 141.14.17.51, port=785)
[ 768.548432] svc: server 00000000aad03597 waiting for data (to = 3600000)
[ 768.548436] svc: server 00000000aad03597, pool 0, transport 000000006952397a, inuse=2
[ 768.548439] svc: tcp_recv 000000006952397a data 1 conn 0 close 0
[ 768.548442] svc: socket 000000006952397a recvfrom(000000005fae69c4, 4) = -11
[ 768.548443] RPC: TCP recv_record got -11
[ 768.548444] RPC: TCP recvfrom got EAGAIN
[ 768.548446] svc: server 00000000aad03597 waiting for data (to = 3600000)
[ 768.548477] svc: server 000000002b852bb6 waiting for data (to = 3600000)
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5174 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] nfsd: Fix overflow causing non-working mounts on 1 TB machines
2019-07-03 9:56 ` Paul Menzel
@ 2019-07-03 12:54 ` Paul Menzel
2019-07-03 15:56 ` J. Bruce Fields
0 siblings, 1 reply; 6+ messages in thread
From: Paul Menzel @ 2019-07-03 12:54 UTC (permalink / raw)
To: J. Bruce Fields, Jeff Layton; +Cc: Chris Tracy, linux-nfs, LKML, it+linux-nfs
[-- Attachment #1: Type: text/plain, Size: 2416 bytes --]
Date: Wed, 3 Jul 2019 13:28:15 +0200
Since commit 10a68cdf10 (nfsd: fix performance-limiting session
calculation) (Linux 5.1-rc1 and 4.19.31), shares from NFS servers with
1 TB of memory cannot be mounted anymore. The mount just hangs on the
client.
The gist of commit 10a68cdf10 is the change below.
-avail = clamp_t(int, avail, slotsize, avail/3);
+avail = clamp_t(int, avail, slotsize, total_avail/3);
Here are the macros.
#define min_t(type, x, y) __careful_cmp((type)(x), (type)(y), <)
#define clamp_t(type, val, lo, hi) min_t(type, max_t(type, val, lo), hi)
`total_avail` is 8,434,659,328 on the 1 TB machine. `clamp_t()` casts
the values to `int`, which for 32-bit integers can only hold values
−2,147,483,648 (−2^31) through 2,147,483,647 (2^31 − 1).
`avail` (in the function signature) is just 65536, so that no overflow
was happening. Before the commit the assignment would result in 21845,
and `num = 4`.
When using `total_avail`, it is causing the assignment to be
18446744072226137429 (printed as %lu), and `num` is then 4164608182.
My next guess is, that `nfsd_drc_mem_used` is then exceeded, and the
server thinks there is no memory available any more for this client.
Updating the arguments of `clamp_t()` and `min_t()` to `unsigned long`
fixes the issue.
Now, `avail = 65536` (before commit 10a68cdf10 `avail = 21845`), but
`num = 4` remains the same.
Fixes: 10a68cdf10 (nfsd: fix performance-limiting session calculation)
Cc: stable@vger.kernel.org
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
1. No, idea if `min_t()` arguments also need updating.
2. Instead of `unsigned long`, should `size_t` be used?
fs/nfsd/nfs4state.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 618e66078ee5..1a0cdeb3b875 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1563,7 +1563,7 @@ static u32 nfsd4_get_drc_mem(struct nfsd4_channel_attrs *ca)
* Never use more than a third of the remaining memory,
* unless it's the only way to give this client a slot:
*/
- avail = clamp_t(int, avail, slotsize, total_avail/3);
+ avail = clamp_t(unsigned long, avail, slotsize, total_avail/3);
num = min_t(int, num, avail / slotsize);
nfsd_drc_mem_used += num * slotsize;
spin_unlock(&nfsd_drc_lock);
--
2.22.0
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5174 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nfsd: Fix overflow causing non-working mounts on 1 TB machines
2019-07-03 12:54 ` [PATCH] nfsd: Fix overflow causing non-working mounts on 1 TB machines Paul Menzel
@ 2019-07-03 15:56 ` J. Bruce Fields
2019-07-03 16:03 ` Paul Menzel
0 siblings, 1 reply; 6+ messages in thread
From: J. Bruce Fields @ 2019-07-03 15:56 UTC (permalink / raw)
To: Paul Menzel; +Cc: Jeff Layton, Chris Tracy, linux-nfs, LKML, it+linux-nfs
Good catch! And thanks for the detailed explanation. Applying for 5.2
and stable.
On Wed, Jul 03, 2019 at 02:54:43PM +0200, Paul Menzel wrote:
> Date: Wed, 3 Jul 2019 13:28:15 +0200
>
> Since commit 10a68cdf10 (nfsd: fix performance-limiting session
> calculation) (Linux 5.1-rc1 and 4.19.31), shares from NFS servers with
> 1 TB of memory cannot be mounted anymore. The mount just hangs on the
> client.
>
> The gist of commit 10a68cdf10 is the change below.
>
> -avail = clamp_t(int, avail, slotsize, avail/3);
> +avail = clamp_t(int, avail, slotsize, total_avail/3);
>
> Here are the macros.
>
> #define min_t(type, x, y) __careful_cmp((type)(x), (type)(y), <)
> #define clamp_t(type, val, lo, hi) min_t(type, max_t(type, val, lo), hi)
>
> `total_avail` is 8,434,659,328 on the 1 TB machine. `clamp_t()` casts
> the values to `int`, which for 32-bit integers can only hold values
> −2,147,483,648 (−2^31) through 2,147,483,647 (2^31 − 1).
>
> `avail` (in the function signature) is just 65536, so that no overflow
> was happening. Before the commit the assignment would result in 21845,
> and `num = 4`.
>
> When using `total_avail`, it is causing the assignment to be
> 18446744072226137429 (printed as %lu), and `num` is then 4164608182.
>
> My next guess is, that `nfsd_drc_mem_used` is then exceeded, and the
> server thinks there is no memory available any more for this client.
>
> Updating the arguments of `clamp_t()` and `min_t()` to `unsigned long`
> fixes the issue.
>
> Now, `avail = 65536` (before commit 10a68cdf10 `avail = 21845`), but
> `num = 4` remains the same.
>
> Fixes: 10a68cdf10 (nfsd: fix performance-limiting session calculation)
> Cc: stable@vger.kernel.org
> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
> ---
>
> 1. No, idea if `min_t()` arguments also need updating.
> 2. Instead of `unsigned long`, should `size_t` be used?
>
> fs/nfsd/nfs4state.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 618e66078ee5..1a0cdeb3b875 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1563,7 +1563,7 @@ static u32 nfsd4_get_drc_mem(struct nfsd4_channel_attrs *ca)
> * Never use more than a third of the remaining memory,
> * unless it's the only way to give this client a slot:
> */
> - avail = clamp_t(int, avail, slotsize, total_avail/3);
> + avail = clamp_t(unsigned long, avail, slotsize, total_avail/3);
> num = min_t(int, num, avail / slotsize);
> nfsd_drc_mem_used += num * slotsize;
> spin_unlock(&nfsd_drc_lock);
> --
> 2.22.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nfsd: Fix overflow causing non-working mounts on 1 TB machines
2019-07-03 15:56 ` J. Bruce Fields
@ 2019-07-03 16:03 ` Paul Menzel
2019-07-03 16:43 ` J. Bruce Fields
0 siblings, 1 reply; 6+ messages in thread
From: Paul Menzel @ 2019-07-03 16:03 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: Jeff Layton, Chris Tracy, linux-nfs, LKML, it+linux-nfs
[-- Attachment #1: Type: text/plain, Size: 341 bytes --]
Dear Bruce,
On 7/3/19 5:56 PM, J. Bruce Fields wrote:
> Good catch! And thanks for the detailed explanation. Applying for 5.2
> and stable.
Thanks. Please note, that in the last part are some guesses, and I am not
well versed in the terminology. So please feel free to reword the commit
messages.
Kind regards,
Paul
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5174 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nfsd: Fix overflow causing non-working mounts on 1 TB machines
2019-07-03 16:03 ` Paul Menzel
@ 2019-07-03 16:43 ` J. Bruce Fields
0 siblings, 0 replies; 6+ messages in thread
From: J. Bruce Fields @ 2019-07-03 16:43 UTC (permalink / raw)
To: Paul Menzel; +Cc: Jeff Layton, Chris Tracy, linux-nfs, LKML, it+linux-nfs
On Wed, Jul 03, 2019 at 06:03:06PM +0200, Paul Menzel wrote:
> Dear Bruce,
>
>
> On 7/3/19 5:56 PM, J. Bruce Fields wrote:
> > Good catch! And thanks for the detailed explanation. Applying for 5.2
> > and stable.
>
> Thanks. Please note, that in the last part are some guesses, and I am not
> well versed in the terminology. So please feel free to reword the commit
> messages.
I haven't checked all the arithmetic, but it sounds pretty plausible to
me, and clearly that type was wrong.
--b.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-07-03 16:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20190702165107.93C8A2067CFDD@mx.molgen.mpg.de>
[not found] ` <8c3e0249-b17f-4bd2-4a46-afd4d35f4763@molgen.mpg.de>
2019-07-02 21:59 ` Regression caused by commit c54f24e3 (nfsd: fix performance-limiting session calculation) Paul Menzel
2019-07-03 9:56 ` Paul Menzel
2019-07-03 12:54 ` [PATCH] nfsd: Fix overflow causing non-working mounts on 1 TB machines Paul Menzel
2019-07-03 15:56 ` J. Bruce Fields
2019-07-03 16:03 ` Paul Menzel
2019-07-03 16:43 ` J. Bruce Fields
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®