summaryrefslogtreecommitdiffstats
path: root/fs
diff options
context:
space:
mode:
authorMichal Hocko <mhocko@suse.com>2017-09-13 16:28:29 -0700
committerLinus Torvalds <torvalds@linux-foundation.org>2017-09-13 18:53:16 -0700
commit0ee931c4e31a5efb134c76440405e9219f896e33 (patch)
treef87f98163759237da6f062bdec1cbe620f166b50 /fs
parentd0dbf771309fecd73f4ac1566c1400cb12807ee2 (diff)
downloadlinux-0ee931c4e31a5efb134c76440405e9219f896e33.tar.bz2
mm: treewide: remove GFP_TEMPORARY allocation flag
GFP_TEMPORARY was introduced by commit e12ba74d8ff3 ("Group short-lived and reclaimable kernel allocations") along with __GFP_RECLAIMABLE. It's primary motivation was to allow users to tell that an allocation is short lived and so the allocator can try to place such allocations close together and prevent long term fragmentation. As much as this sounds like a reasonable semantic it becomes much less clear when to use the highlevel GFP_TEMPORARY allocation flag. How long is temporary? Can the context holding that memory sleep? Can it take locks? It seems there is no good answer for those questions. The current implementation of GFP_TEMPORARY is basically GFP_KERNEL | __GFP_RECLAIMABLE which in itself is tricky because basically none of the existing caller provide a way to reclaim the allocated memory. So this is rather misleading and hard to evaluate for any benefits. I have checked some random users and none of them has added the flag with a specific justification. I suspect most of them just copied from other existing users and others just thought it might be a good idea to use without any measuring. This suggests that GFP_TEMPORARY just motivates for cargo cult usage without any reasoning. I believe that our gfp flags are quite complex already and especially those with highlevel semantic should be clearly defined to prevent from confusion and abuse. Therefore I propose dropping GFP_TEMPORARY and replace all existing users to simply use GFP_KERNEL. Please note that SLAB users with shrinkers will still get __GFP_RECLAIMABLE heuristic and so they will be placed properly for memory fragmentation prevention. I can see reasons we might want some gfp flag to reflect shorterm allocations but I propose starting from a clear semantic definition and only then add users with proper justification. This was been brought up before LSF this year by Matthew [1] and it turned out that GFP_TEMPORARY really doesn't have a clear semantic. It seems to be a heuristic without any measured advantage for most (if not all) its current users. The follow up discussion has revealed that opinions on what might be temporary allocation differ a lot between developers. So rather than trying to tweak existing users into a semantic which they haven't expected I propose to simply remove the flag and start from scratch if we really need a semantic for short term allocations. [1] http://lkml.kernel.org/r/20170118054945.GD18349@bombadil.infradead.org [akpm@linux-foundation.org: fix typo] [akpm@linux-foundation.org: coding-style fixes] [sfr@canb.auug.org.au: drm/i915: fix up] Link: http://lkml.kernel.org/r/20170816144703.378d4f4d@canb.auug.org.au Link: http://lkml.kernel.org/r/20170728091904.14627-1-mhocko@kernel.org Signed-off-by: Michal Hocko <mhocko@suse.com> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> Acked-by: Mel Gorman <mgorman@suse.de> Acked-by: Vlastimil Babka <vbabka@suse.cz> Cc: Matthew Wilcox <willy@infradead.org> Cc: Neil Brown <neilb@suse.de> Cc: "Theodore Ts'o" <tytso@mit.edu> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'fs')
-rw-r--r--fs/coredump.c2
-rw-r--r--fs/exec.c4
-rw-r--r--fs/overlayfs/copy_up.c2
-rw-r--r--fs/overlayfs/dir.c2
-rw-r--r--fs/overlayfs/namei.c12
-rw-r--r--fs/proc/base.c8
-rw-r--r--fs/proc/task_mmu.c2
7 files changed, 16 insertions, 16 deletions
diff --git a/fs/coredump.c b/fs/coredump.c
index 592683711c64..0eec03696707 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -161,7 +161,7 @@ static int cn_print_exe_file(struct core_name *cn)
if (!exe_file)
return cn_esc_printf(cn, "%s (path unknown)", current->comm);
- pathbuf = kmalloc(PATH_MAX, GFP_TEMPORARY);
+ pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
if (!pathbuf) {
ret = -ENOMEM;
goto put_exe_file;
diff --git a/fs/exec.c b/fs/exec.c
index 01a9fb9d8ac3..daa19d85c066 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1763,9 +1763,9 @@ static int do_execveat_common(int fd, struct filename *filename,
bprm->filename = filename->name;
} else {
if (filename->name[0] == '\0')
- pathbuf = kasprintf(GFP_TEMPORARY, "/dev/fd/%d", fd);
+ pathbuf = kasprintf(GFP_KERNEL, "/dev/fd/%d", fd);
else
- pathbuf = kasprintf(GFP_TEMPORARY, "/dev/fd/%d/%s",
+ pathbuf = kasprintf(GFP_KERNEL, "/dev/fd/%d/%s",
fd, filename->name);
if (!pathbuf) {
retval = -ENOMEM;
diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index acb6f97deb97..aad97b30d5e6 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -241,7 +241,7 @@ struct ovl_fh *ovl_encode_fh(struct dentry *lower, bool is_upper)
int buflen = MAX_HANDLE_SZ;
uuid_t *uuid = &lower->d_sb->s_uuid;
- buf = kmalloc(buflen, GFP_TEMPORARY);
+ buf = kmalloc(buflen, GFP_KERNEL);
if (!buf)
return ERR_PTR(-ENOMEM);
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 9cb0c80e5967..3309b1912241 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -833,7 +833,7 @@ static char *ovl_get_redirect(struct dentry *dentry, bool samedir)
goto out;
}
- buf = ret = kmalloc(buflen, GFP_TEMPORARY);
+ buf = ret = kmalloc(buflen, GFP_KERNEL);
if (!buf)
goto out;
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 8aef2b304b2d..c3addd1114f1 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -38,7 +38,7 @@ static int ovl_check_redirect(struct dentry *dentry, struct ovl_lookup_data *d,
return 0;
goto fail;
}
- buf = kzalloc(prelen + res + strlen(post) + 1, GFP_TEMPORARY);
+ buf = kzalloc(prelen + res + strlen(post) + 1, GFP_KERNEL);
if (!buf)
return -ENOMEM;
@@ -103,7 +103,7 @@ static struct ovl_fh *ovl_get_origin_fh(struct dentry *dentry)
if (res == 0)
return NULL;
- fh = kzalloc(res, GFP_TEMPORARY);
+ fh = kzalloc(res, GFP_KERNEL);
if (!fh)
return ERR_PTR(-ENOMEM);
@@ -309,7 +309,7 @@ static int ovl_check_origin(struct dentry *upperdentry,
BUG_ON(*ctrp);
if (!*stackp)
- *stackp = kmalloc(sizeof(struct path), GFP_TEMPORARY);
+ *stackp = kmalloc(sizeof(struct path), GFP_KERNEL);
if (!*stackp) {
dput(origin);
return -ENOMEM;
@@ -418,7 +418,7 @@ int ovl_verify_index(struct dentry *index, struct path *lowerstack,
err = -ENOMEM;
len = index->d_name.len / 2;
- fh = kzalloc(len, GFP_TEMPORARY);
+ fh = kzalloc(len, GFP_KERNEL);
if (!fh)
goto fail;
@@ -478,7 +478,7 @@ int ovl_get_index_name(struct dentry *origin, struct qstr *name)
return PTR_ERR(fh);
err = -ENOMEM;
- n = kzalloc(fh->len * 2, GFP_TEMPORARY);
+ n = kzalloc(fh->len * 2, GFP_KERNEL);
if (n) {
s = bin2hex(n, fh, fh->len);
*name = (struct qstr) QSTR_INIT(n, s - n);
@@ -646,7 +646,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
if (!d.stop && poe->numlower) {
err = -ENOMEM;
stack = kcalloc(ofs->numlower, sizeof(struct path),
- GFP_TEMPORARY);
+ GFP_KERNEL);
if (!stack)
goto out_put_upper;
}
diff --git a/fs/proc/base.c b/fs/proc/base.c
index e5d89a0d0b8a..ad3b0762cc3e 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -232,7 +232,7 @@ static ssize_t proc_pid_cmdline_read(struct file *file, char __user *buf,
goto out_mmput;
}
- page = (char *)__get_free_page(GFP_TEMPORARY);
+ page = (char *)__get_free_page(GFP_KERNEL);
if (!page) {
rv = -ENOMEM;
goto out_mmput;
@@ -813,7 +813,7 @@ static ssize_t mem_rw(struct file *file, char __user *buf,
if (!mm)
return 0;
- page = (char *)__get_free_page(GFP_TEMPORARY);
+ page = (char *)__get_free_page(GFP_KERNEL);
if (!page)
return -ENOMEM;
@@ -918,7 +918,7 @@ static ssize_t environ_read(struct file *file, char __user *buf,
if (!mm || !mm->env_end)
return 0;
- page = (char *)__get_free_page(GFP_TEMPORARY);
+ page = (char *)__get_free_page(GFP_KERNEL);
if (!page)
return -ENOMEM;
@@ -1630,7 +1630,7 @@ out:
static int do_proc_readlink(struct path *path, char __user *buffer, int buflen)
{
- char *tmp = (char*)__get_free_page(GFP_TEMPORARY);
+ char *tmp = (char *)__get_free_page(GFP_KERNEL);
char *pathname;
int len;
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 7b40e11ede9b..5589b4bd4b85 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -1474,7 +1474,7 @@ static ssize_t pagemap_read(struct file *file, char __user *buf,
pm.show_pfn = file_ns_capable(file, &init_user_ns, CAP_SYS_ADMIN);
pm.len = (PAGEMAP_WALK_SIZE >> PAGE_SHIFT);
- pm.buffer = kmalloc(pm.len * PM_ENTRY_BYTES, GFP_TEMPORARY);
+ pm.buffer = kmalloc(pm.len * PM_ENTRY_BYTES, GFP_KERNEL);
ret = -ENOMEM;
if (!pm.buffer)
goto out_mm;