All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: ocfs2-devel@oss.oracle.com
Subject: Re: [patch -next] ocfs2: scheduling in atomic in ocfs2_filecheck_store()
Date: Mon, 27 Jul 2015 21:04:05 +0000	[thread overview]
Message-ID: <20150727140405.c8281f48d81bd33c6ef957a8@linux-foundation.org> (raw)
In-Reply-To: <20150727082723.GA3310@mwanda>

On Mon, 27 Jul 2015 11:27:23 +0300 Dan Carpenter <dan.carpenter@oracle.com> wrote:

> We're hold "spin_lock(&ent->fs_fcheck->fc_lock)" so the allocation has
> to be GFP_ATOMIC.
> 
> I changed the sizeof() because otherwise the line goes over the 80
> character limit and also the new way is prefered kernel style.
> 
> Fixes: e467fe5da718 ('ocfs2: sysfile interfaces for online file check')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/fs/ocfs2/filecheck.c b/fs/ocfs2/filecheck.c
> index 3332af1..9613663 100644
> --- a/fs/ocfs2/filecheck.c
> +++ b/fs/ocfs2/filecheck.c
> @@ -544,7 +544,7 @@ static ssize_t ocfs2_filecheck_store(struct kobject *kobj,
>  			BUG_ON(!ocfs2_filecheck_erase_entry(ent));
>  		}
>  
> -		entry = kmalloc(sizeof(struct ocfs2_filecheck_entry), GFP_NOFS);
> +		entry = kmalloc(sizeof(*entry), GFP_ATOMIC);
>  		if (entry) {
>  			entry->fe_ino = args.fa_ino;
>  			entry->fe_type = args.fa_type;

GFP_NOFS is more reliable than GFP_ATOMIC, so we should retain that if
possible.  And it's pretty easy to do here.

Gang, please carefully review and test?


--- a/fs/ocfs2/filecheck.c~ocfs2-sysfile-interfaces-for-online-file-check-fix-2
+++ a/fs/ocfs2/filecheck.c
@@ -501,7 +501,7 @@ static ssize_t ocfs2_filecheck_store(str
 				const char *buf, size_t count)
 {
 	struct ocfs2_filecheck_args args;
-	struct ocfs2_filecheck_entry *entry = NULL;
+	struct ocfs2_filecheck_entry *entry;
 	struct ocfs2_filecheck_sysfs_entry *ent;
 	ssize_t ret = 0;
 
@@ -527,12 +527,19 @@ static ssize_t ocfs2_filecheck_store(str
 		return (!ret ? count : ret);
 	}
 
+	entry = kmalloc(sizeof(*entry), GFP_NOFS);
+	if (!entry) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
 	spin_lock(&ent->fs_fcheck->fc_lock);
 	if ((ent->fs_fcheck->fc_size >= ent->fs_fcheck->fc_max) &&
-		(ent->fs_fcheck->fc_done = 0)) {
-		mlog(ML_ERROR,
-		"Online file check queue(%u) is full\n",
-		ent->fs_fcheck->fc_max);
+			(ent->fs_fcheck->fc_done = 0)) {
+		mlog(ML_ERROR, "Online file check queue(%u) is full\n",
+				ent->fs_fcheck->fc_max);
+		kfree(entry);
+		entry = NULL;
 		ret = -EBUSY;
 	} else {
 		if ((ent->fs_fcheck->fc_size >= ent->fs_fcheck->fc_max) &&
@@ -544,26 +551,21 @@ static ssize_t ocfs2_filecheck_store(str
 			BUG_ON(!ocfs2_filecheck_erase_entry(ent));
 		}
 
-		entry = kmalloc(sizeof(struct ocfs2_filecheck_entry), GFP_NOFS);
-		if (entry) {
-			entry->fe_ino = args.fa_ino;
-			entry->fe_type = args.fa_type;
-			entry->fe_done = 0;
-			entry->fe_status = OCFS2_FILECHECK_ERR_INPROGRESS;
-			list_add_tail(&entry->fe_list,
-					&ent->fs_fcheck->fc_head);
-
-			ent->fs_fcheck->fc_size++;
-			ret = count;
-		} else {
-			ret = -ENOMEM;
-		}
+		entry->fe_ino = args.fa_ino;
+		entry->fe_type = args.fa_type;
+		entry->fe_done = 0;
+		entry->fe_status = OCFS2_FILECHECK_ERR_INPROGRESS;
+		list_add_tail(&entry->fe_list, &ent->fs_fcheck->fc_head);
+
+		ent->fs_fcheck->fc_size++;
+		ret = count;
 	}
 	spin_unlock(&ent->fs_fcheck->fc_lock);
 
 	if (entry)
 		ocfs2_filecheck_handle_entry(ent, entry);
 
+out:
 	ocfs2_filecheck_sysfs_put(ent);
 	return ret;
 }
_



WARNING: multiple messages have this Message-ID
From: Andrew Morton <akpm@linux-foundation.org>
To: ocfs2-devel@oss.oracle.com
Subject: [Ocfs2-devel] [patch -next] ocfs2: scheduling in atomic in ocfs2_filecheck_store()
Date: Mon, 27 Jul 2015 14:04:05 -0700	[thread overview]
Message-ID: <20150727140405.c8281f48d81bd33c6ef957a8@linux-foundation.org> (raw)
In-Reply-To: <20150727082723.GA3310@mwanda>

On Mon, 27 Jul 2015 11:27:23 +0300 Dan Carpenter <dan.carpenter@oracle.com> wrote:

> We're hold "spin_lock(&ent->fs_fcheck->fc_lock)" so the allocation has
> to be GFP_ATOMIC.
> 
> I changed the sizeof() because otherwise the line goes over the 80
> character limit and also the new way is prefered kernel style.
> 
> Fixes: e467fe5da718 ('ocfs2: sysfile interfaces for online file check')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/fs/ocfs2/filecheck.c b/fs/ocfs2/filecheck.c
> index 3332af1..9613663 100644
> --- a/fs/ocfs2/filecheck.c
> +++ b/fs/ocfs2/filecheck.c
> @@ -544,7 +544,7 @@ static ssize_t ocfs2_filecheck_store(struct kobject *kobj,
>  			BUG_ON(!ocfs2_filecheck_erase_entry(ent));
>  		}
>  
> -		entry = kmalloc(sizeof(struct ocfs2_filecheck_entry), GFP_NOFS);
> +		entry = kmalloc(sizeof(*entry), GFP_ATOMIC);
>  		if (entry) {
>  			entry->fe_ino = args.fa_ino;
>  			entry->fe_type = args.fa_type;

GFP_NOFS is more reliable than GFP_ATOMIC, so we should retain that if
possible.  And it's pretty easy to do here.

Gang, please carefully review and test?


--- a/fs/ocfs2/filecheck.c~ocfs2-sysfile-interfaces-for-online-file-check-fix-2
+++ a/fs/ocfs2/filecheck.c
@@ -501,7 +501,7 @@ static ssize_t ocfs2_filecheck_store(str
 				const char *buf, size_t count)
 {
 	struct ocfs2_filecheck_args args;
-	struct ocfs2_filecheck_entry *entry = NULL;
+	struct ocfs2_filecheck_entry *entry;
 	struct ocfs2_filecheck_sysfs_entry *ent;
 	ssize_t ret = 0;
 
@@ -527,12 +527,19 @@ static ssize_t ocfs2_filecheck_store(str
 		return (!ret ? count : ret);
 	}
 
+	entry = kmalloc(sizeof(*entry), GFP_NOFS);
+	if (!entry) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
 	spin_lock(&ent->fs_fcheck->fc_lock);
 	if ((ent->fs_fcheck->fc_size >= ent->fs_fcheck->fc_max) &&
-		(ent->fs_fcheck->fc_done == 0)) {
-		mlog(ML_ERROR,
-		"Online file check queue(%u) is full\n",
-		ent->fs_fcheck->fc_max);
+			(ent->fs_fcheck->fc_done == 0)) {
+		mlog(ML_ERROR, "Online file check queue(%u) is full\n",
+				ent->fs_fcheck->fc_max);
+		kfree(entry);
+		entry = NULL;
 		ret = -EBUSY;
 	} else {
 		if ((ent->fs_fcheck->fc_size >= ent->fs_fcheck->fc_max) &&
@@ -544,26 +551,21 @@ static ssize_t ocfs2_filecheck_store(str
 			BUG_ON(!ocfs2_filecheck_erase_entry(ent));
 		}
 
-		entry = kmalloc(sizeof(struct ocfs2_filecheck_entry), GFP_NOFS);
-		if (entry) {
-			entry->fe_ino = args.fa_ino;
-			entry->fe_type = args.fa_type;
-			entry->fe_done = 0;
-			entry->fe_status = OCFS2_FILECHECK_ERR_INPROGRESS;
-			list_add_tail(&entry->fe_list,
-					&ent->fs_fcheck->fc_head);
-
-			ent->fs_fcheck->fc_size++;
-			ret = count;
-		} else {
-			ret = -ENOMEM;
-		}
+		entry->fe_ino = args.fa_ino;
+		entry->fe_type = args.fa_type;
+		entry->fe_done = 0;
+		entry->fe_status = OCFS2_FILECHECK_ERR_INPROGRESS;
+		list_add_tail(&entry->fe_list, &ent->fs_fcheck->fc_head);
+
+		ent->fs_fcheck->fc_size++;
+		ret = count;
 	}
 	spin_unlock(&ent->fs_fcheck->fc_lock);
 
 	if (entry)
 		ocfs2_filecheck_handle_entry(ent, entry);
 
+out:
 	ocfs2_filecheck_sysfs_put(ent);
 	return ret;
 }
_

  reply	other threads:[~2015-07-27 21:04 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-27  8:27 Dan Carpenter
2015-07-27  8:27 ` [Ocfs2-devel] " Dan Carpenter
2015-07-27 21:04 ` Andrew Morton [this message]
2015-07-27 21:04   ` Andrew Morton
2015-07-27 21:19   ` Dan Carpenter
2015-07-27 21:19     ` [Ocfs2-devel] " Dan Carpenter
2015-07-28  3:15   ` Gang He
2015-07-28  3:15     ` [Ocfs2-devel] " Gang He

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=20150727140405.c8281f48d81bd33c6ef957a8@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=ocfs2-devel@oss.oracle.com \
    --subject='Re: [patch -next] ocfs2: scheduling in atomic in ocfs2_filecheck_store()' \
    /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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.