On Mon, 2024-04-08 at 11:29 +1000, NeilBrown wrote:
> recalculate_deny_mode() takes time that is linear in the number of
> stateids active on the file.
>
> When called from
> release_openowner -> free_ol_stateid_reaplist ->nfs4_free_ol_stateid
> -> release_all_access
>
> the number of times it is called is linear in the number of stateids.
> The net result is that time taken by release_openowner is quadratic in
> the number of stateids.
>
> When the nfsd server is shut down while there are many active stateids
> this can result in a soft lockup. ("CPU stuck for 302s" seen in one case).
>
> In many cases all the states have the same deny modes and there is no
> need to examine the entire list in recalculate_deny_mode(). In
> particular, recalculate_deny_mode() will only reduce the deny mode,
> never increase it. So if some prefix of the list causes the original
> deny mode to be required, there is no need to examine the remainder of
> the list.
>
> So we can improve recalculate_deny_mode() to usually run in constant
> time, so release_openowner will typically be only linear in the number
> of states.
>
> Signed-off-by: NeilBrown <neilb@xxxxxxx>
> ---
> fs/nfsd/nfs4state.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 1824a30e7dd4..a46f5230bc9b 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1409,11 +1409,16 @@ static void
> recalculate_deny_mode(struct nfs4_file *fp)
> {
> struct nfs4_ol_stateid *stp;
> + u32 old_deny;
>
> spin_lock(&fp->fi_lock);
> + old_deny = fp->fi_share_deny;
> fp->fi_share_deny = 0;
> - list_for_each_entry(stp, &fp->fi_stateids, st_perfile)
> + list_for_each_entry(stp, &fp->fi_stateids, st_perfile) {
> fp->fi_share_deny |= bmap_to_share_mode(stp->st_deny_bmap);
> + if (fp->fi_share_deny == old_deny)
> + break;
> + }
> spin_unlock(&fp->fi_lock);
> }
>
Clever!
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
