[Cluster-devel] [PATCH 24/32] dlm: dlm workqueues aren't used in memory reclaim path

[Cluster-devel] [PATCH 24/32] dlm: dlm workqueues aren't used in memory reclaim path

[Steven Whitehouse]

Hi,

You already acked a patch to do this, which is in the DLM git tree,
waiting for the next merge window:

http://git.kernel.org/?p=linux/kernel/git/teigland/dlm.git;a=commitdiff;h=dcce240ead802d42b1e45ad2fcb2ed4a399cb255

Steve.

On Mon, 2011-01-03 at 14:49 +0100, Tejun Heo wrote:
> dlm isn't depended upon during memory reclaim and so are its two
> workqueues.  Convert to alloc[_ordered]_workqueue() without
> WQ_MEM_RECLAIM.  While at it, fix workqueue allocation failure
> handling.  They return %NULL on failure not ERR_PTR() value.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Christine Caulfield <ccaulfie@redhat.com>
> Cc: David Teigland <teigland@redhat.com>
> Cc: cluster-devel at redhat.com
> ---
> Only compile tested.  Please feel free to take it into the subsystem
> tree or simply ack - I'll route it through the wq tree.
> 
> Thanks.
> 
>  fs/dlm/lowcomms.c |   19 ++++++++-----------
>  1 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
> index 37a34c2..9aad7c0 100644
> --- a/fs/dlm/lowcomms.c
> +++ b/fs/dlm/lowcomms.c
> @@ -1430,20 +1430,17 @@ static void work_stop(void)
>  
>  static int work_start(void)
>  {
> -	int error;
> -	recv_workqueue = create_workqueue("dlm_recv");
> -	error = IS_ERR(recv_workqueue);
> -	if (error) {
> -		log_print("can't start dlm_recv %d", error);
> -		return error;
> +	recv_workqueue = alloc_workqueue("dlm_recv", 0, 1);
> +	if (!recv_workqueue) {
> +		log_print("can't start dlm_recv");
> +		return -ENOMEM;
>  	}
>  
> -	send_workqueue = create_singlethread_workqueue("dlm_send");
> -	error = IS_ERR(send_workqueue);
> -	if (error) {
> -		log_print("can't start dlm_send %d", error);
> +	send_workqueue = alloc_ordered_workqueue("dlm_send", 0);
> +	if (!send_workqueue) {
> +		log_print("can't start dlm_send");
>  		destroy_workqueue(recv_workqueue);
> -		return error;
> +		return -ENOMEM;
>  	}
>  
>  	return 0;

[Cluster-devel] [PATCH 24/32] dlm: dlm workqueues aren't used in memory reclaim path

[Steven Whitehouse]

Hi,

On Mon, 2011-01-03 at 15:01 +0100, Tejun Heo wrote:
> On Mon, Jan 03, 2011 at 01:58:27PM +0000, Steven Whitehouse wrote:
> > Hi,
> > 
> > You already acked a patch to do this, which is in the DLM git tree,
> > waiting for the next merge window:
> > 
> > http://git.kernel.org/?p=linux/kernel/git/teigland/dlm.git;a=commitdiff;h=dcce240ead802d42b1e45ad2fcb2ed4a399cb255
> 
> Ooh, I see.  Hmmm... maybe I've asked this before but do you really
> need WQ_MEM_RECLAIM there?  Is dlm used during memory reclaim?
> 
> Thanks.
> 

Yes, it is in case of deallocation of inodes, so we have to be careful
about that,

Steve.

[Cluster-devel] [PATCH 24/32] dlm: dlm workqueues aren't used in memory reclaim path

[Steven Whitehouse]

Hi,

On Mon, 2011-01-03 at 15:27 +0100, Tejun Heo wrote:
> Hello again,
> 
> On Mon, Jan 03, 2011 at 02:21:29PM +0000, Steven Whitehouse wrote:
> > > Ooh, I see.  Hmmm... maybe I've asked this before but do you really
> > > need WQ_MEM_RECLAIM there?  Is dlm used during memory reclaim?
> > 
> > Yes, it is in case of deallocation of inodes, so we have to be careful
> > about that,
> 
> I see.  I'm still slightly unsure about the WQ_MEM_RECLAIM rules when
> it comes to distributed file systems and stuff related to them, but I
> think it basically comes down to whether the code can be in the swap
> path or not.  e.g. for nfs backed write shared mapped files, nfs
> definitely can be used during memory reclaim _but_ it's not guaranteed
> to make forward progress under memory pressure and thus can't be
> depended upon and shouldn't be used for swap.  IOW, it may free memory
> but it's not in the critical path which should guarantee forward
> progress.  Isn't dlm the same case?
> 
> Thank you.
> 

No, the issue arises in case an inode is being ejected from cache with a
0 link count (and this need deallocation). At this point the dlm is
required in order to coordinate which node will perform the
deallocation, but if it blocks on memory reclaim (via the fs, so
GFP_NOFS is ok, but GFP_KERNEL is not) then it can land up calling back
into the fs via a shrink of the dcache which can potentially then also
call back into the dlm.

Since there is no workqueue option to indicate that GFP_NOFS is ok, but
GFP_KERNEL is not, I think in the workqueue case we must mark it as
WQ_MEM_RECLAIM. So far as I can tell thats a limitation of task creation
which was the reason for the pre-allocated rescuer thread. I'd rather be
safe than sorry in this case,

Steve.