cluster-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
From: Steven Whitehouse <swhiteho@redhat.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [DLM] Fix up memory alloc/kmap
Date: Tue, 11 Nov 2008 11:02:57 +0000	[thread overview]
Message-ID: <1226401377.9571.2.camel@quoit> (raw)
In-Reply-To: <1224860742.25004.130.camel@quoit>


This is an updated version of the previous patch with one place that was
missed being fixed up.

I've been looking into DLM memory allocation issues and come up with the
following patch. I did a quick go/no-go test and it seems to work ok.

I spotted that DLM doesn't set sk->sk_allocation. Since the sockets are
associated with more than one lockspace, using ls->ls_allocation isn't
an option. Since, however DLM uses sendpage, the amount of memory
which is allocated by the socket code itself will be pretty small,
which is most likely why this issue hasn't been seen before. On that
basis, my suggestion is to use GFP_NOFS, which is the most restrictive
of the GFP modes of which ls->ls_allocation might be set to.

Also there are various places where ls->ls_allocation should have been
used, but had been forgotten. That is fixed up.

In addition kmap was being used incorrectly. The reason that this hasn't
been seen before is that the pages are allocated using ls->ls_allocation
which doesn't allow highmem pages to be returned. I haven't changed that
since although kmap implies a GFP_KERNEL allocation, without highmem
pages, they will be no-ops for now and thus safe. Also sendpage does its
own kmap (if required, which is only if the protocol is emulating
sendpage via sock_no_sendpage) so that the k(un)map pair around that
call can be removed.

Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
Cc: David Teigland <teigland@redhat.com>
Cc: Christine Caulfield <ccaulfie@redhat.com>

diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
index 3962262..07c6fba 100644
--- a/fs/dlm/lowcomms.c
+++ b/fs/dlm/lowcomms.c
@@ -295,6 +295,7 @@ static int add_sock(struct socket *sock, struct connection *con)
 	con->sock->sk->sk_write_space = lowcomms_write_space;
 	con->sock->sk->sk_state_change = lowcomms_state_change;
 	con->sock->sk->sk_user_data = con;
+	con->sock->sk->sk_allocation = GFP_NOFS;
 	return 0;
 }
 
@@ -839,6 +840,7 @@ static void sctp_init_assoc(struct connection *con)
 	outmessage.msg_controllen = cmsg->cmsg_len;
 
 	ret = kernel_sendmsg(base_con->sock, &outmessage, iov, 1, len);
+	kunmap(e->page);
 	if (ret < 0) {
 		log_print("Send first packet to node %d failed: %d",
 			  con->nodeid, ret);
@@ -846,15 +848,13 @@ static void sctp_init_assoc(struct connection *con)
 		/* Try again later */
 		clear_bit(CF_CONNECT_PENDING, &con->flags);
 		clear_bit(CF_INIT_PENDING, &con->flags);
-	}
-	else {
+	} else {
 		spin_lock(&con->writequeue_lock);
 		e->offset += ret;
 		e->len -= ret;
 
 		if (e->len == 0 && e->users == 0) {
 			list_del(&e->list);
-			kunmap(e->page);
 			free_entry(e);
 		}
 		spin_unlock(&con->writequeue_lock);
@@ -1272,7 +1272,6 @@ static void send_to_sock(struct connection *con)
 		offset = e->offset;
 		BUG_ON(len == 0 && e->users == 0);
 		spin_unlock(&con->writequeue_lock);
-		kmap(e->page);
 
 		ret = 0;
 		if (len) {
@@ -1294,7 +1293,6 @@ static void send_to_sock(struct connection *con)
 
 		if (e->len == 0 && e->users == 0) {
 			list_del(&e->list);
-			kunmap(e->page);
 			free_entry(e);
 			continue;
 		}
diff --git a/fs/dlm/memory.c b/fs/dlm/memory.c
index 54c14c6..c1775b8 100644
--- a/fs/dlm/memory.c
+++ b/fs/dlm/memory.c
@@ -39,7 +39,7 @@ char *dlm_allocate_lvb(struct dlm_ls *ls)
 {
 	char *p;
 
-	p = kzalloc(ls->ls_lvblen, GFP_KERNEL);
+	p = kzalloc(ls->ls_lvblen, ls->ls_allocation);
 	return p;
 }
 
@@ -57,7 +57,7 @@ struct dlm_rsb *dlm_allocate_rsb(struct dlm_ls *ls, int namelen)
 
 	DLM_ASSERT(namelen <= DLM_RESNAME_MAXLEN,);
 
-	r = kzalloc(sizeof(*r) + namelen, GFP_KERNEL);
+	r = kzalloc(sizeof(*r) + namelen, ls->ls_allocation);
 	return r;
 }
 
@@ -72,7 +72,7 @@ struct dlm_lkb *dlm_allocate_lkb(struct dlm_ls *ls)
 {
 	struct dlm_lkb *lkb;
 
-	lkb = kmem_cache_zalloc(lkb_cache, GFP_KERNEL);
+	lkb = kmem_cache_zalloc(lkb_cache, ls->ls_allocation);
 	return lkb;
 }
 
diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c
index 07ac709..f3396c6 100644
--- a/fs/dlm/midcomms.c
+++ b/fs/dlm/midcomms.c
@@ -112,7 +112,7 @@ int dlm_process_incoming_buffer(int nodeid, const void *base,
 		   ordinary messages). */
 
 		if (msglen > sizeof(__tmp) && p == &__tmp.p) {
-			p = kmalloc(dlm_config.ci_buffer_size, GFP_KERNEL);
+			p = kmalloc(dlm_config.ci_buffer_size, GFP_NOFS);
 			if (p == NULL)
 				return ret;
 		}




  reply	other threads:[~2008-11-11 11:02 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-24 15:05 [Cluster-devel] [DLM] Fix up memory alloc/kmap Steven Whitehouse
2008-11-11 11:02 ` Steven Whitehouse [this message]
2008-11-12 23:14   ` [Cluster-devel] " David Teigland
2008-11-13  9:56     ` Steven Whitehouse
2008-11-13 17:29       ` David Teigland

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1226401377.9571.2.camel@quoit \
    --to=swhiteho@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).