linux-kernel - Re: NFS: 82ms wakeup latency 4.14-rc4

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171218163559.GA11829@fieldses.org>
Date:   Mon, 18 Dec 2017 11:35:59 -0500
From:   "J. Bruce Fields" <bfields@...ldses.org>
To:     Mike Galbraith <efault@....de>
Cc:     lkml <linux-kernel@...r.kernel.org>,
        Jeff Layton <jlayton@...nel.org>, linux-nfs@...r.kernel.org
Subject: Re: NFS: 82ms wakeup latency 4.14-rc4

On Mon, Dec 18, 2017 at 04:31:52PM +0100, Mike Galbraith wrote:
> On Mon, 2017-12-18 at 16:17 +0100, Mike Galbraith wrote:
> > 
> > ....
> > kworker/-7421    0.N.. 82893us : nfs_release_request <-nfs_commit_release_pages
> > kworker/-7421    0.N.. 82893us : nfs_unlock_and_release_request <-nfs_commit_release_pages
> > kworker/-7421    0.N.. 82893us : nfs_unlock_request <-nfs_unlock_and_release_request
> > kworker/-7421    0.N.. 82893us : nfs_page_group_destroy <-nfs_commit_release_pages
> > kworker/-7421    0.N.. 82893us : nfs_page_group_sync_on_bit <-nfs_page_group_destroy
> > kworker/-7421    0.N.. 82893us : nfs_page_group_lock <-nfs_page_group_sync_on_bit
> > kworker/-7421    0.N.. 82893us : nfs_page_group_unlock <-nfs_page_group_sync_on_bit
> > kworker/-7421    0.N.. 82893us : nfs_free_request <-nfs_page_group_destroy
> > kworker/-7421    0.N.. 82893us : nfs_put_lock_context <-nfs_free_request
> > kworker/-7421    0.N.. 82893us : put_nfs_open_context <-nfs_free_request
> > kworker/-7421    0.N.. 82893us : __put_nfs_open_context <-nfs_free_request
> > kworker/-7421    0.N.. 82894us : kmem_cache_free <-nfs_page_group_destroy
> > kworker/-7421    0.N.. 82894us : __slab_free <-kmem_cache_free
> > kworker/-7421    0.N.. 82894us : clear_wb_congested <-nfs_commit_release_pages
> > kworker/-7421    0.N.. 82894us : nfs_init_cinfo <-nfs_commit_release_pages
> > kworker/-7421    0.N.. 82894us : nfs_init_cinfo_from_inode <-nfs_commit_release_pages
> > kworker/-7421    0.N.. 82894us : nfs_commit_end <-nfs_commit_release_pages
> > kworker/-7421    0.N.. 82894us : nfs_commitdata_release <-rpc_free_task
> > kworker/-7421    0.N.. 82894us : put_nfs_open_context <-nfs_commitdata_release
> > kworker/-7421    0.N.. 82894us : __put_nfs_open_context <-nfs_commitdata_release
> > kworker/-7421    0.N.. 82895us : mempool_free <-rpc_free_task
> > kworker/-7421    0.N.. 82895us : mempool_free_slab <-rpc_free_task
> > kworker/-7421    0.N.. 82895us : kmem_cache_free <-rpc_free_task
> > kworker/-7421    0.N.. 82895us : ___might_sleep <-process_one_work
> > kworker/-7421    0.N.. 82895us : _cond_resched <-process_one_work
> > kworker/-7421    0dN.1 82895us : rcu_note_context_switch <-__schedule
> 
> diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
> index d0543e19098a..b42bf3b21e05 100644
> --- a/fs/nfs/pagelist.c
> +++ b/fs/nfs/pagelist.c
> @@ -428,6 +428,7 @@ void nfs_free_request(struct nfs_page *req)
>  	/* Release struct file and open context */
>  	nfs_clear_request(req);
>  	nfs_page_free(req);
> +	cond_resched();
>  }
>  
>  void nfs_release_request(struct nfs_page *req)

This probably just shows I don't understand the issues, but: isn't this
the job of preemption?  If we're not holding any locks that would
prevent scheduling here, shouldn't latency-sensitive users be building
preemptible kernels and letting the scheduler take care of this?  It
seems unfortunate to require explicit cond_resched()s allovers.

Like I say, I don't really understand the issues here, so it's more a
question than an objection....  (I don't know any reason a
cond_resched() would be bad there.)

--b.