On Mon, 27 Jul 2015 11:27:23 +0300 Dan Carpenter <dan.carpen...@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.carpen...@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;
 }
_



_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel

Reply via email to