UBI: introduce volume refcounting

Add ref_count field to UBI volumes and remove weired "vol->removed"
field. This way things are better understandable and we do not have
to do whold show_attr operation under spinlock.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
index 2ff3492..84f7dc9 100644 (file)
--- a/drivers/mtd/ubi/eba.c
+++ b/drivers/mtd/ubi/eba.c
@@ -301,6 +301,8 @@ int ubi_eba_unmap_leb(struct ubi_device *ubi, struct ubi_volume *vol,
 {
        int err, pnum, vol_id = vol->vol_id;
 
+       ubi_assert(vol->ref_count > 0);
+
        if (ubi->ro_mode)
                return -EROFS;
 
@@ -349,6 +351,8 @@ int ubi_eba_read_leb(struct ubi_device *ubi, struct ubi_volume *vol, int lnum,
        struct ubi_vid_hdr *vid_hdr;
        uint32_t uninitialized_var(crc);
 
+       ubi_assert(vol->ref_count > 0);
+
        err = leb_read_lock(ubi, vol_id, lnum);
        if (err)
                return err;
@@ -572,6 +576,8 @@ int ubi_eba_write_leb(struct ubi_device *ubi, struct ubi_volume *vol, int lnum,
        int err, pnum, tries = 0, vol_id = vol->vol_id;
        struct ubi_vid_hdr *vid_hdr;
 
+       ubi_assert(vol->ref_count > 0);
+
        if (ubi->ro_mode)
                return -EROFS;
 
@@ -705,6 +711,8 @@ int ubi_eba_write_leb_st(struct ubi_device *ubi, struct ubi_volume *vol,
        struct ubi_vid_hdr *vid_hdr;
        uint32_t crc;
 
+       ubi_assert(vol->ref_count > 0);
+
        if (ubi->ro_mode)
                return -EROFS;
 
@@ -819,6 +827,8 @@ int ubi_eba_atomic_leb_change(struct ubi_device *ubi, struct ubi_volume *vol,
        struct ubi_vid_hdr *vid_hdr;
        uint32_t crc;
 
+       ubi_assert(vol->ref_count > 0);
+
        if (ubi->ro_mode)
                return -EROFS;
 

diff --git a/drivers/mtd/ubi/kapi.c b/drivers/mtd/ubi/kapi.c
index 9c28376..780c273 100644 (file)
--- a/drivers/mtd/ubi/kapi.c
+++ b/drivers/mtd/ubi/kapi.c
@@ -152,6 +152,7 @@ struct ubi_volume_desc *ubi_open_volume(int ubi_num, int vol_id, int mode)
                break;
        }
        get_device(&vol->dev);
+       vol->ref_count += 1;
        spin_unlock(&ubi->volumes_lock);
 
        desc->vol = vol;
@@ -261,10 +262,11 @@ void ubi_close_volume(struct ubi_volume_desc *desc)
        case UBI_EXCLUSIVE:
                vol->exclusive = 0;
        }
+       vol->ref_count -= 1;
        spin_unlock(&vol->ubi->volumes_lock);
 
-       kfree(desc);
        put_device(&vol->dev);
+       kfree(desc);
        module_put(THIS_MODULE);
 }
 EXPORT_SYMBOL_GPL(ubi_close_volume);

diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index 69cbee3..f782d5a 100644 (file)
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -140,10 +140,10 @@ struct ubi_volume_desc;
  * @cdev: character device object to create character device
  * @ubi: reference to the UBI device description object
  * @vol_id: volume ID
+ * @ref_count: volume reference count
  * @readers: number of users holding this volume in read-only mode
  * @writers: number of users holding this volume in read-write mode
  * @exclusive: whether somebody holds this volume in exclusive mode
- * @removed: if the volume was removed
  * @checked: if this static volume was checked
  *
  * @reserved_pebs: how many physical eraseblocks are reserved for this volume
@@ -156,7 +156,7 @@ struct ubi_volume_desc;
  * @corrupted: non-zero if the volume is corrupted (static volumes only)
  * @alignment: volume alignment
  * @data_pad: how many bytes are not used at the end of physical eraseblocks to
- * satisfy the requested alignment
+ *            satisfy the requested alignment
  * @name_len: volume name length
  * @name: volume name
  *
@@ -185,10 +185,10 @@ struct ubi_volume {
        struct cdev cdev;
        struct ubi_device *ubi;
        int vol_id;
+       int ref_count;
        int readers;
        int writers;
        int exclusive;
-       int removed;
        int checked;
 
        int reserved_pebs;
@@ -242,9 +242,9 @@ struct ubi_wl_entry;
  * @vol_count: number of volumes in this UBI device
  * @volumes: volumes of this UBI device
  * @volumes_lock: protects @volumes, @rsvd_pebs, @avail_pebs, beb_rsvd_pebs,
- * @beb_rsvd_level, @bad_peb_count, @good_peb_count, @vol_count, @vol->readers,
- * @vol->writers, @vol->exclusive, @vol->removed, @vol->mapping and
- * @vol->eba_tbl.
+ *                @beb_rsvd_level, @bad_peb_count, @good_peb_count, @vol_count,
+ *                @vol->readers, @vol->writers, @vol->exclusive,
+ *                @vol->ref_count, @vol->mapping and @vol->eba_tbl.
  *
  * @rsvd_pebs: count of reserved physical eraseblocks
  * @avail_pebs: count of available physical eraseblocks
@@ -273,11 +273,11 @@ struct ubi_wl_entry;
  * @prot.pnum: protection tree indexed by physical eraseblock numbers
  * @prot.aec: protection tree indexed by absolute erase counter value
  * @wl_lock: protects the @used, @free, @prot, @lookuptbl, @abs_ec, @move_from,
- * @move_to, @move_to_put @erase_pending, @wl_scheduled, and @works
- * fields
+ *           @move_to, @move_to_put @erase_pending, @wl_scheduled, and @works
+ *           fields
  * @wl_scheduled: non-zero if the wear-leveling was scheduled
  * @lookuptbl: a table to quickly find a &struct ubi_wl_entry object for any
- * physical eraseblock
+ *             physical eraseblock
  * @abs_ec: absolute erase counter
  * @move_from: physical eraseblock from where the data is being moved
  * @move_to: physical eraseblock where the data is being moved to
@@ -308,13 +308,13 @@ struct ubi_wl_entry;
  * @hdrs_min_io_size
  * @vid_hdr_shift: contains @vid_hdr_offset - @vid_hdr_aloffset
  * @bad_allowed: whether the MTD device admits of bad physical eraseblocks or
- * not
+ *               not
  * @mtd: MTD device descriptor
  *
  * @peb_buf1: a buffer of PEB size used for different purposes
  * @peb_buf2: another buffer of PEB size used for different purposes
  * @buf_mutex: proptects @peb_buf1 and @peb_buf2
- * @dbg_peb_buf:  buffer of PEB size used for debugging
+ * @dbg_peb_buf: buffer of PEB size used for debugging
  * @dbg_buf_mutex: proptects @dbg_peb_buf
  */
 struct ubi_device {

diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c
index 3d6ac02..18ef1e1 100644 (file)
--- a/drivers/mtd/ubi/vmt.c
+++ b/drivers/mtd/ubi/vmt.c
@@ -63,21 +63,24 @@ static struct device_attribute attr_vol_upd_marker =
  * B. process 2 removes volume Y;
  * C. process 1 starts reading the /<sysfs>/class/ubi/ubiX_Y/reserved_ebs file;
  *
- * What we want to do in a situation like that is to return error when the file
- * is read. This is done by means of the 'removed' flag and the 'vol_lock' of
- * the UBI volume description object.
+ * In this situation, this function will return %-ENODEV because it will find
+ * out that the volume was removed from the @ubi->volumes array.
  */
 static ssize_t vol_attribute_show(struct device *dev,
                                  struct device_attribute *attr, char *buf)
 {
-       int ret = -ENODEV;
+       int ret;
        struct ubi_volume *vol = container_of(dev, struct ubi_volume, dev);
+       struct ubi_device *ubi = vol->ubi;
 
-       spin_lock(&vol->ubi->volumes_lock);
-       if (vol->removed) {
-               spin_unlock(&vol->ubi->volumes_lock);
-               return ret;
+       spin_lock(&ubi->volumes_lock);
+       if (!ubi->volumes[vol->vol_id]) {
+               spin_unlock(&ubi->volumes_lock);
+               return -ENODEV;
        }
+       /* Take a reference to prevent volume removal */
+       vol->ref_count += 1;
+       spin_unlock(&ubi->volumes_lock);
 
        if (attr == &attr_vol_reserved_ebs)
                ret = sprintf(buf, "%d\n", vol->reserved_pebs);
@@ -102,8 +105,13 @@ static ssize_t vol_attribute_show(struct device *dev,
        else if (attr == &attr_vol_upd_marker)
                ret = sprintf(buf, "%d\n", vol->upd_marker);
        else
-               BUG();
-       spin_unlock(&vol->ubi->volumes_lock);
+               /* This must be a bug */
+               ret = -EINVAL;
+
+       spin_lock(&ubi->volumes_lock);
+       vol->ref_count -= 1;
+       ubi_assert(vol->ref_count >= 0);
+       spin_unlock(&ubi->volumes_lock);
        return ret;
 }
 
@@ -179,7 +187,7 @@ static void volume_sysfs_close(struct ubi_volume *vol)
  * @req: volume creation request
  *
  * This function creates volume described by @req. If @req->vol_id id
- * %UBI_VOL_NUM_AUTO, this function automatically assigne ID to the new volume
+ * %UBI_VOL_NUM_AUTO, this function automatically assign ID to the new volume
  * and saves it in @req->vol_id. Returns zero in case of success and a negative
  * error code in case of failure.
  */
@@ -261,7 +269,6 @@ int ubi_create_volume(struct ubi_device *ubi, struct ubi_mkvol_req *req)
        memcpy(vol->name, req->name, vol->name_len + 1);
        vol->exclusive = 1;
        vol->ubi = ubi;
-       ubi->volumes[vol_id] = vol;
        spin_unlock(&ubi->volumes_lock);
 
        /*
@@ -345,6 +352,7 @@ int ubi_create_volume(struct ubi_device *ubi, struct ubi_mkvol_req *req)
        spin_lock(&ubi->volumes_lock);
        ubi->vol_count += 1;
        vol->exclusive = 0;
+       ubi->volumes[vol_id] = vol;
        spin_unlock(&ubi->volumes_lock);
 
        paranoid_check_volumes(ubi);
@@ -353,7 +361,7 @@ int ubi_create_volume(struct ubi_device *ubi, struct ubi_mkvol_req *req)
 
 out_sysfs:
        /*
-        * We have degistered our device, we should not free the volume*
+        * We have registered our device, we should not free the volume*
         * description object in this function in case of an error - it is
         * freed by the release function.
         *
@@ -373,7 +381,6 @@ out_acc:
        spin_lock(&ubi->volumes_lock);
        ubi->rsvd_pebs -= vol->reserved_pebs;
        ubi->avail_pebs += vol->reserved_pebs;
-       ubi->volumes[vol_id] = NULL;
 out_unlock:
        spin_unlock(&ubi->volumes_lock);
        mutex_unlock(&ubi->volumes_mutex);
@@ -407,25 +414,32 @@ int ubi_remove_volume(struct ubi_volume_desc *desc)
                return -EROFS;
 
        mutex_lock(&ubi->volumes_mutex);
+       spin_lock(&ubi->volumes_lock);
+       if (vol->ref_count > 1) {
+               /*
+                * The volume is busy, probably someone is reading one of its
+                * sysfs files.
+                */
+               err = -EBUSY;
+               goto out_unlock;
+       }
+       ubi->volumes[vol_id] = NULL;
+       spin_unlock(&ubi->volumes_lock);
+
        err = ubi_destroy_gluebi(vol);
        if (err)
-               goto out;
+               goto out_err;
 
        err = ubi_change_vtbl_record(ubi, vol_id, NULL);
        if (err)
-               goto out;
+               goto out_err;
 
        for (i = 0; i < vol->reserved_pebs; i++) {
                err = ubi_eba_unmap_leb(ubi, vol, i);
                if (err)
-                       goto out;
+                       goto out_err;
        }
 
-       spin_lock(&ubi->volumes_lock);
-       vol->removed = 1;
-       ubi->volumes[vol_id] = NULL;
-       spin_unlock(&ubi->volumes_lock);
-
        kfree(vol->eba_tbl);
        vol->eba_tbl = NULL;
        cdev_del(&vol->cdev);
@@ -447,7 +461,15 @@ int ubi_remove_volume(struct ubi_volume_desc *desc)
        spin_unlock(&ubi->volumes_lock);
 
        paranoid_check_volumes(ubi);
-out:
+       mutex_unlock(&ubi->volumes_mutex);
+       return 0;
+
+out_err:
+       ubi_err("cannot remove volume %d, error %d", vol_id, err);
+       spin_lock(&ubi->volumes_lock);
+       ubi->volumes[vol_id] = vol;
+out_unlock:
+       spin_unlock(&ubi->volumes_lock);
        mutex_unlock(&ubi->volumes_mutex);
        return err;
 }
@@ -494,8 +516,17 @@ int ubi_resize_volume(struct ubi_volume_desc *desc, int reserved_pebs)
        for (i = 0; i < reserved_pebs; i++)
                new_mapping[i] = UBI_LEB_UNMAPPED;
 
-       /* Reserve physical eraseblocks */
        mutex_lock(&ubi->volumes_mutex);
+       spin_lock(&ubi->volumes_lock);
+       if (vol->ref_count > 1) {
+               spin_unlock(&ubi->volumes_lock);
+               err = -EBUSY;
+               goto out_free;
+       }
+       spin_unlock(&ubi->volumes_lock);
+
+
+       /* Reserve physical eraseblocks */
        pebs = reserved_pebs - vol->reserved_pebs;
        if (pebs > 0) {
                spin_lock(&ubi->volumes_lock);
@@ -577,8 +608,8 @@ out_free:
  * @ubi: UBI device description object
  * @vol: volume description object
  *
- * This function adds an existin volume and initializes all its data
- * structures. Returnes zero in case of success and a negative error code in
+ * This function adds an existing volume and initializes all its data
+ * structures. Returns zero in case of success and a negative error code in
  * case of failure.
  */
 int ubi_add_volume(struct ubi_device *ubi, struct ubi_volume *vol)
@@ -588,7 +619,6 @@ int ubi_add_volume(struct ubi_device *ubi, struct ubi_volume *vol)
 
        dbg_msg("add volume %d", vol_id);
        ubi_dbg_dump_vol_info(vol);
-       ubi_assert(vol);
 
        /* Register character device for the volume */
        cdev_init(&vol->cdev, &ubi_vol_cdev_operations);
@@ -645,11 +675,9 @@ void ubi_free_volume(struct ubi_device *ubi, struct ubi_volume *vol)
        int err;
 
        dbg_msg("free volume %d", vol->vol_id);
-       ubi_assert(vol);
 
-       vol->removed = 1;
-       err = ubi_destroy_gluebi(vol);
        ubi->volumes[vol->vol_id] = NULL;
+       err = ubi_destroy_gluebi(vol);
        cdev_del(&vol->cdev);
        volume_sysfs_close(vol);
 }

diff --git a/drivers/mtd/ubi/vtbl.c b/drivers/mtd/ubi/vtbl.c
index 5879fdb..a37dc7a 100644 (file)
--- a/drivers/mtd/ubi/vtbl.c
+++ b/drivers/mtd/ubi/vtbl.c
@@ -565,6 +565,7 @@ static int init_volumes(struct ubi_device *ubi, const struct ubi_scan_info *si,
                vol->last_eb_bytes = sv->last_data_size;
        }
 
+       /* And add the layout volume */
        vol = kzalloc(sizeof(struct ubi_volume), GFP_KERNEL);
        if (!vol)
                return -ENOMEM;
@@ -580,6 +581,7 @@ static int init_volumes(struct ubi_device *ubi, const struct ubi_scan_info *si,
        vol->used_bytes =
                (long long)vol->used_ebs * (ubi->leb_size - vol->data_pad);
        vol->vol_id = UBI_LAYOUT_VOL_ID;
+       vol->ref_count = 1;
 
        ubi_assert(!ubi->volumes[i]);
        ubi->volumes[vol_id2idx(ubi, vol->vol_id)] = vol;