Skip to contentSkip to conversation

staging/lustre: Always try kmalloc first for OBD_ALLOC_LARGE

PATCH In review [email protected] 2 participants 5 messages May 3, 2015 – May 4, 2015

Subsystems (2)
Staging Kernel janitors
lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 1/1 staging/lustre: Always try kmalloc first for OBD_ALLOC_LARGE

From: Oleg Drokin <[email protected]>

Create libcfs_kvzalloc and libcfs_kvzalloc_cpt are
are designed to replace OBD_ALLOC_LARGE and OBD_CPT_ALLOC_LARGE.

Not a drop-in replacement as they also take gfp flags armument for more flexibility.

Signed-off-by Oleg Drokin <[email protected]>

---
This is how I envision the OBD_ALLOC_LARGE replacement.

BTW, it appears we also have LIBCFS_ALLOC and friends that we probably dont' need either.

Thanks your your great help!

 .../staging/lustre/include/linux/libcfs/libcfs.h   |  4 ++
 .../staging/lustre/lustre/include/obd_support.h    | 24 ++-------
 drivers/staging/lustre/lustre/libcfs/Makefile      |  1 +
 .../staging/lustre/lustre/libcfs/linux/linux-mem.c | 59 ++++++++++++++++++++++
 4 files changed, 67 insertions(+), 21 deletions(-)
 create mode 100644 drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c
drivers/staging/lustre/include/linux/libcfs/libcfs.h +4 -0
index 4410d7f..947df7e 100644
@@ -184,4 +184,8 @@ static inline void *__container_of(void *ptr, unsigned long shift)
 
 #define _LIBCFS_H
 
+void *libcfs_kvzalloc(size_t size, gfp_t flags);
+void *libcfs_kvzalloc_cpt(struct cfs_cpt_table *cptab, int cpt, size_t size,
+			  gfp_t flags);
+
 #endif /* _LIBCFS_H */
drivers/staging/lustre/lustre/include/obd_support.h +3 -21
index 2991d2e..379266d 100644
@@ -676,37 +676,19 @@ do {									      \
 	 __OBD_VMALLOC_VEROBSE(ptr, cptab, cpt, size)
 
 
-/* Allocations above this size are considered too big and could not be done
- * atomically.
- *
- * Be very careful when changing this value, especially when decreasing it,
- * since vmalloc in Linux doesn't perform well on multi-cores system, calling
- * vmalloc in critical path would hurt performance badly. See LU-66.
- */
-#define OBD_ALLOC_BIG (4 * PAGE_CACHE_SIZE)
-
 #define OBD_ALLOC_LARGE(ptr, size)					    \
 do {									  \
-	if (size > OBD_ALLOC_BIG)					     \
-		OBD_VMALLOC(ptr, size);				       \
-	else								  \
-		OBD_ALLOC(ptr, size);					 \
+	ptr = libcfs_kvzalloc(size, GFP_NOFS);				  \
 } while (0)
 
 #define OBD_CPT_ALLOC_LARGE(ptr, cptab, cpt, size)			      \
 do {									      \
-	if (size > OBD_ALLOC_BIG)					      \
-		OBD_CPT_VMALLOC(ptr, cptab, cpt, size);			      \
-	else								      \
-		OBD_CPT_ALLOC(ptr, cptab, cpt, size);			      \
+	ptr = libcfs_kvzalloc_cpt(cptab, cpt, size, GFP_NOFS);		      \
 } while (0)
 
 #define OBD_FREE_LARGE(ptr, size)					     \
 do {									  \
-	if (size > OBD_ALLOC_BIG)					     \
-		OBD_VFREE(ptr, size);					 \
-	else								  \
-		OBD_FREE(ptr, size);					  \
+	kvfree(ptr);							  \
 } while (0)
drivers/staging/lustre/lustre/libcfs/Makefile +1 -0
index 2996a48..fabdd3e 100644
@@ -7,6 +7,7 @@ libcfs-linux-objs += linux-curproc.o
 libcfs-linux-objs += linux-module.o
 libcfs-linux-objs += linux-crypto.o
 libcfs-linux-objs += linux-crypto-adler.o
+libcfs-linux-objs += linux-mem.o
 
 libcfs-linux-objs := $(addprefix linux/,$(libcfs-linux-objs))
drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c new file +59 -0

59 lines not on this page · raw message

Signature · 1 line
2.1.0
#1 lore raw
Quoted 51 lines · On Sat, May 02, 2015 at 11:58:21PM -0400, [email protected] wrote:

From: Oleg Drokin <[email protected]>

Create libcfs_kvzalloc and libcfs_kvzalloc_cpt are
are designed to replace OBD_ALLOC_LARGE and OBD_CPT_ALLOC_LARGE.

Not a drop-in replacement as they also take gfp flags armument for more flexibility.

Signed-off-by Oleg Drokin <[email protected]>

---
This is how I envision the OBD_ALLOC_LARGE replacement.

BTW, it appears we also have LIBCFS_ALLOC and friends that we probably dont' need either.

Thanks your your great help!

 .../staging/lustre/include/linux/libcfs/libcfs.h   |  4 ++
 .../staging/lustre/lustre/include/obd_support.h    | 24 ++-------
 drivers/staging/lustre/lustre/libcfs/Makefile      |  1 +
 .../staging/lustre/lustre/libcfs/linux/linux-mem.c | 59 ++++++++++++++++++++++
 4 files changed, 67 insertions(+), 21 deletions(-)
 create mode 100644 drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c
drivers/staging/lustre/include/linux/libcfs/libcfs.h +4 -0
index 4410d7f..947df7e 100644
@@ -184,4 +184,8 @@ static inline void *__container_of(void *ptr, unsigned long shift)
 
 #define _LIBCFS_H
 
+void *libcfs_kvzalloc(size_t size, gfp_t flags);
+void *libcfs_kvzalloc_cpt(struct cfs_cpt_table *cptab, int cpt, size_t size,
+			  gfp_t flags);
+
 #endif /* _LIBCFS_H */
drivers/staging/lustre/lustre/include/obd_support.h +0 -7
index 2991d2e..379266d 100644
@@ -676,37 +676,19 @@ do {									      \
 	 __OBD_VMALLOC_VEROBSE(ptr, cptab, cpt, size)
 
 
-/* Allocations above this size are considered too big and could not be done
- * atomically.
- *
- * Be very careful when changing this value, especially when decreasing it,
- * since vmalloc in Linux doesn't perform well on multi-cores system, calling
- * vmalloc in critical path would hurt performance badly. See LU-66.
- */

drivers/staging/lustre/lustre/include/obd_support.h do { \ · line 682

Quoted 10 lines
drivers/staging/lustre/lustre/include/obd_support.h +1 -6
-#define OBD_ALLOC_BIG (4 * PAGE_CACHE_SIZE)
-
 #define OBD_ALLOC_LARGE(ptr, size)					    \
 do {									  \
-	if (size > OBD_ALLOC_BIG)					     \
-		OBD_VMALLOC(ptr, size);				       \
-	else								  \
-		OBD_ALLOC(ptr, size);					 \
+	ptr = libcfs_kvzalloc(size, GFP_NOFS);				  \
 } while (0)

Just fix up all callers of these functions, if there are any anymore.

drivers/staging/lustre/lustre/include/obd_support.h do { \ · line 687

Quoted 9 lines
drivers/staging/lustre/lustre/include/obd_support.h +1 -4
 #define OBD_CPT_ALLOC_LARGE(ptr, cptab, cpt, size)			      \
 do {									      \
-	if (size > OBD_ALLOC_BIG)					      \
-		OBD_CPT_VMALLOC(ptr, cptab, cpt, size);			      \
-	else								      \
-		OBD_CPT_ALLOC(ptr, cptab, cpt, size);			      \
+	ptr = libcfs_kvzalloc_cpt(cptab, cpt, size, GFP_NOFS);		      \
 } while (0)

Same here.

drivers/staging/lustre/lustre/include/obd_support.h do { \ · line 691

Quoted 8 lines
drivers/staging/lustre/lustre/include/obd_support.h +1 -4
 #define OBD_FREE_LARGE(ptr, size)					     \
 do {									  \
-	if (size > OBD_ALLOC_BIG)					     \
-		OBD_VFREE(ptr, size);					 \
-	else								  \
-		OBD_FREE(ptr, size);					  \
+	kvfree(ptr);							  \

Same here.

Quoted 21 lines
drivers/staging/lustre/lustre/include/obd_support.h +0 -0
 } while (0)
drivers/staging/lustre/lustre/libcfs/Makefile +1 -0
index 2996a48..fabdd3e 100644
@@ -7,6 +7,7 @@ libcfs-linux-objs += linux-curproc.o
 libcfs-linux-objs += linux-module.o
 libcfs-linux-objs += linux-crypto.o
 libcfs-linux-objs += linux-crypto-adler.o
+libcfs-linux-objs += linux-mem.o
 
 libcfs-linux-objs := $(addprefix linux/,$(libcfs-linux-objs))
drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c new file +0 -0
new file mode 100644
index 0000000..1b068ae

drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c line 2

Quoted 2 lines
drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c new file +2 -0
@@ -0,0 +1,59 @@
+/*
+ * GPL HEADER START

This line isn't needed.

drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c line 4

Quoted 2 lines
drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c new file +2 -0
+ *
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.

This line isn't needed either, and in fact, is a lie, you can't force anyone to not modify your file once it's in the kernel tree.

Quoted 6 lines
drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c new file +6 -0
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 only,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but

drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c line 20

Quoted 10 lines
drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c new file +10 -0
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License version 2 for more details (a copy is included
+ * in the LICENSE file that accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License
+ * version 2 along with this program; If not, see
+ * http://www.sun.com/software/products/lustre/docs/GPLv2.pdf
+ *
+ * GPL HEADER END

Again, not needed.

drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c line 23

Quoted 3 lines
drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c new file +3 -0
+ */
+/*
+ * Copyright (c) 2015, Oleg Drokin <[email protected]>

I think your employer would like a different line here...

thanks,

greg k-h

author
#2 lore raw
In reply to [email protected]

Hello!

Quoted 11 lines · Greg KH
Quoted 11 lines
drivers/staging/lustre/lustre/include/obd_support.h +0 -9
-/* Allocations above this size are considered too big and could not be done
- * atomically.
- *
- * Be very careful when changing this value, especially when decreasing it,
- * since vmalloc in Linux doesn't perform well on multi-cores system, calling
- * vmalloc in critical path would hurt performance badly. See LU-66.
- */
-#define OBD_ALLOC_BIG (4 * PAGE_CACHE_SIZE)
-

#define OBD_ALLOC_LARGE(ptr, size) \
do { \

Quoted 8 lines
Quoted 6 lines
drivers/staging/lustre/lustre/include/obd_support.h +1 -4
-	if (size > OBD_ALLOC_BIG)					     \
-		OBD_VMALLOC(ptr, size);				       \
-	else								  \
-		OBD_ALLOC(ptr, size);					 \
+	ptr = libcfs_kvzalloc(size, GFP_NOFS);				  \

} while (0)

Just fix up all callers of these functions, if there are any anymore.

This is what Julia is doing. I am providing the stub for her wonderful scripts to unwrap per her request.

Quoted 5 lines
Quoted 3 lines

+ */
+/*
+ * Copyright (c) 2015, Oleg Drokin <[email protected]>

I think your employer would like a different line here...

Only on stuff that I do at work when I am getting paid.

Stuff that I do on my own uncompensated, I own all the rights to, I hope.

Bye,
Oleg

#3 lore raw
In reply to [email protected]
Quoted 30 lines · Oleg Drokin

Hello!

Quoted 11 lines · Greg KH
Quoted 11 lines
drivers/staging/lustre/lustre/include/obd_support.h +0 -9
-/* Allocations above this size are considered too big and could not be done
- * atomically.
- *
- * Be very careful when changing this value, especially when decreasing it,
- * since vmalloc in Linux doesn't perform well on multi-cores system, calling
- * vmalloc in critical path would hurt performance badly. See LU-66.
- */
-#define OBD_ALLOC_BIG (4 * PAGE_CACHE_SIZE)
-

#define OBD_ALLOC_LARGE(ptr, size) \
do { \

Quoted 8 lines
Quoted 6 lines
drivers/staging/lustre/lustre/include/obd_support.h +1 -4
-	if (size > OBD_ALLOC_BIG)					     \
-		OBD_VMALLOC(ptr, size);				       \
-	else								  \
-		OBD_ALLOC(ptr, size);					 \
+	ptr = libcfs_kvzalloc(size, GFP_NOFS);				  \

} while (0)

Just fix up all callers of these functions, if there are any anymore.

This is what Julia is doing. I am providing the stub for her wonderful scripts to unwrap per her request.

Quoted 4 lines
Quoted 3 lines

+ */
+/*
+ * Copyright (c) 2015, Oleg Drokin <[email protected]>

Quoted 5 lines
Quoted 1 line

I think your employer would like a different line here...

Only on stuff that I do at work when I am getting paid.

Stuff that I do on my own uncompensated, I own all the rights to, I hope.

I wouldn't be so sure about this, please read your employment contract, almost no companies allow this, it is very rare.

thanks,

greg k-h

author
#4 lore raw

PATCH 1/1 staging/lustre: Always try kmalloc first for OBD_ALLOC_LARGE

In reply to [email protected]

From: Oleg Drokin <[email protected]>

Create libcfs_kvzalloc and libcfs_kvzalloc_cpt that are designed to replace OBD_ALLOC_LARGE and OBD_CPT_ALLOC_LARGE.

Not a drop-in replacement as they also take gfp flags armument for more flexibility.

Signed-off-by Oleg Drokin <[email protected]>

---

Greg, I have addressed your comments wrt the copyright header. Since I do not have the patches that replace OBD_ALLOC_LARGE with this new function and Julia does, do you think you can accept this as is and she'll follow with the replacement then or what is best to do here you think?

 .../staging/lustre/include/linux/libcfs/libcfs.h   |  4 ++
 .../staging/lustre/lustre/include/obd_support.h    | 24 ++-------
 drivers/staging/lustre/lustre/libcfs/Makefile      |  1 +
 .../staging/lustre/lustre/libcfs/linux/linux-mem.c | 59 ++++++++++++++++++++++
 4 files changed, 67 insertions(+), 21 deletions(-)
 create mode 100644 drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c
drivers/staging/lustre/include/linux/libcfs/libcfs.h +4 -0
index 4410d7f..947df7e 100644
@@ -184,4 +184,8 @@ static inline void *__container_of(void *ptr, unsigned long shift)
 
 #define _LIBCFS_H
 
+void *libcfs_kvzalloc(size_t size, gfp_t flags);
+void *libcfs_kvzalloc_cpt(struct cfs_cpt_table *cptab, int cpt, size_t size,
+			  gfp_t flags);
+
 #endif /* _LIBCFS_H */
drivers/staging/lustre/lustre/include/obd_support.h +3 -21
index 2991d2e..379266d 100644
@@ -676,37 +676,19 @@ do {									      \
 	 __OBD_VMALLOC_VEROBSE(ptr, cptab, cpt, size)
 
 
-/* Allocations above this size are considered too big and could not be done
- * atomically.
- *
- * Be very careful when changing this value, especially when decreasing it,
- * since vmalloc in Linux doesn't perform well on multi-cores system, calling
- * vmalloc in critical path would hurt performance badly. See LU-66.
- */
-#define OBD_ALLOC_BIG (4 * PAGE_CACHE_SIZE)
-
 #define OBD_ALLOC_LARGE(ptr, size)					    \
 do {									  \
-	if (size > OBD_ALLOC_BIG)					     \
-		OBD_VMALLOC(ptr, size);				       \
-	else								  \
-		OBD_ALLOC(ptr, size);					 \
+	ptr = libcfs_kvzalloc(size, GFP_NOFS);				  \
 } while (0)
 
 #define OBD_CPT_ALLOC_LARGE(ptr, cptab, cpt, size)			      \
 do {									      \
-	if (size > OBD_ALLOC_BIG)					      \
-		OBD_CPT_VMALLOC(ptr, cptab, cpt, size);			      \
-	else								      \
-		OBD_CPT_ALLOC(ptr, cptab, cpt, size);			      \
+	ptr = libcfs_kvzalloc_cpt(cptab, cpt, size, GFP_NOFS);		      \
 } while (0)
 
 #define OBD_FREE_LARGE(ptr, size)					     \
 do {									  \
-	if (size > OBD_ALLOC_BIG)					     \
-		OBD_VFREE(ptr, size);					 \
-	else								  \
-		OBD_FREE(ptr, size);					  \
+	kvfree(ptr);							  \
 } while (0)
drivers/staging/lustre/lustre/libcfs/Makefile +1 -0
index 2996a48..fabdd3e 100644
@@ -7,6 +7,7 @@ libcfs-linux-objs += linux-curproc.o
 libcfs-linux-objs += linux-module.o
 libcfs-linux-objs += linux-crypto.o
 libcfs-linux-objs += linux-crypto-adler.o
+libcfs-linux-objs += linux-mem.o
 
 libcfs-linux-objs := $(addprefix linux/,$(libcfs-linux-objs))
drivers/staging/lustre/lustre/libcfs/linux/linux-mem.c new file +59 -0

59 lines not on this page · raw message

Signature · 1 line
2.1.0