md/bitmap: don't abuse i_writecount for bitmap files.
authorNeilBrown <neilb@suse.de>
Wed, 9 Apr 2014 02:25:40 +0000 (12:25 +1000)
committerNeilBrown <neilb@suse.de>
Wed, 9 Apr 2014 02:26:59 +0000 (12:26 +1000)
md bitmap code currently tries to use i_writecount to stop any other
process from writing to out bitmap file.  But that is really an abuse
and has bit-rotted so locking is all wrong.

So discard that - root should be allowed to shoot self in foot.

Still use it in a much less intrusive way to stop the same file being
used as bitmap on two different array, and apply other checks to
ensure the file is at least vaguely usable for bitmap storage
(is regular, is open for write.  Support for ->bmap is already checked
elsewhere).

Reported-by: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: NeilBrown <neilb@suse.de>
drivers/md/bitmap.c
drivers/md/md.c
drivers/md/md.h

index 4195a01b15359bcf44950b2904c723194c4d048b..9a8e66ae04f51e95c169ea9a38f48b0d83f6ed03 100644 (file)
@@ -1988,7 +1988,6 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
                if (mddev->bitmap_info.file) {
                        struct file *f = mddev->bitmap_info.file;
                        mddev->bitmap_info.file = NULL;
-                       restore_bitmap_write_access(f);
                        fput(f);
                }
        } else {
index 4ad5cc4e63e8438ca3c32fea1f40f69ec71657fb..3fa2fc0a5dd2065f1a1e2da51f394c4ea43a1fb2 100644 (file)
@@ -5181,32 +5181,6 @@ static int restart_array(struct mddev *mddev)
        return 0;
 }
 
-/* similar to deny_write_access, but accounts for our holding a reference
- * to the file ourselves */
-static int deny_bitmap_write_access(struct file * file)
-{
-       struct inode *inode = file->f_mapping->host;
-
-       spin_lock(&inode->i_lock);
-       if (atomic_read(&inode->i_writecount) > 1) {
-               spin_unlock(&inode->i_lock);
-               return -ETXTBSY;
-       }
-       atomic_set(&inode->i_writecount, -1);
-       spin_unlock(&inode->i_lock);
-
-       return 0;
-}
-
-void restore_bitmap_write_access(struct file *file)
-{
-       struct inode *inode = file->f_mapping->host;
-
-       spin_lock(&inode->i_lock);
-       atomic_set(&inode->i_writecount, 1);
-       spin_unlock(&inode->i_lock);
-}
-
 static void md_clean(struct mddev *mddev)
 {
        mddev->array_sectors = 0;
@@ -5427,7 +5401,6 @@ static int do_md_stop(struct mddev * mddev, int mode,
 
                bitmap_destroy(mddev);
                if (mddev->bitmap_info.file) {
-                       restore_bitmap_write_access(mddev->bitmap_info.file);
                        fput(mddev->bitmap_info.file);
                        mddev->bitmap_info.file = NULL;
                }
@@ -5979,7 +5952,7 @@ abort_export:
 
 static int set_bitmap_file(struct mddev *mddev, int fd)
 {
-       int err;
+       int err = 0;
 
        if (mddev->pers) {
                if (!mddev->pers->quiesce)
@@ -5991,6 +5964,7 @@ static int set_bitmap_file(struct mddev *mddev, int fd)
 
 
        if (fd >= 0) {
+               struct inode *inode;
                if (mddev->bitmap)
                        return -EEXIST; /* cannot add when bitmap is present */
                mddev->bitmap_info.file = fget(fd);
@@ -6001,10 +5975,21 @@ static int set_bitmap_file(struct mddev *mddev, int fd)
                        return -EBADF;
                }
 
-               err = deny_bitmap_write_access(mddev->bitmap_info.file);
-               if (err) {
+               inode = mddev->bitmap_info.file->f_mapping->host;
+               if (!S_ISREG(inode->i_mode)) {
+                       printk(KERN_ERR "%s: error: bitmap file must be a regular file\n",
+                              mdname(mddev));
+                       err = -EBADF;
+               } else if (!(mddev->bitmap_info.file->f_mode & FMODE_WRITE)) {
+                       printk(KERN_ERR "%s: error: bitmap file must open for write\n",
+                              mdname(mddev));
+                       err = -EBADF;
+               } else if (atomic_read(&inode->i_writecount) != 1) {
                        printk(KERN_ERR "%s: error: bitmap file is already in use\n",
                               mdname(mddev));
+                       err = -EBUSY;
+               }
+               if (err) {
                        fput(mddev->bitmap_info.file);
                        mddev->bitmap_info.file = NULL;
                        return err;
@@ -6027,10 +6012,8 @@ static int set_bitmap_file(struct mddev *mddev, int fd)
                mddev->pers->quiesce(mddev, 0);
        }
        if (fd < 0) {
-               if (mddev->bitmap_info.file) {
-                       restore_bitmap_write_access(mddev->bitmap_info.file);
+               if (mddev->bitmap_info.file)
                        fput(mddev->bitmap_info.file);
-               }
                mddev->bitmap_info.file = NULL;
        }
 
index 07bba96de26047d3f4b868eefc78d3727f0be52d..a49d991f3fe11d4d9fa1622096c1ca2735c0572d 100644 (file)
@@ -605,7 +605,6 @@ extern int md_check_no_bitmap(struct mddev *mddev);
 extern int md_integrity_register(struct mddev *mddev);
 extern void md_integrity_add_rdev(struct md_rdev *rdev, struct mddev *mddev);
 extern int strict_strtoul_scaled(const char *cp, unsigned long *res, int scale);
-extern void restore_bitmap_write_access(struct file *file);
 
 extern void mddev_init(struct mddev *mddev);
 extern int md_run(struct mddev *mddev);