> diff --git a/drivers/md/bcache/nvmpg.c b/drivers/md/bcache/nvmpg.c
> index b654bbbda03e..2b70ee4a6028 100644
> --- a/drivers/md/bcache/nvmpg.c
> +++ b/drivers/md/bcache/nvmpg.c
> @@ -50,6 +50,36 @@ unsigned long bch_nvmpg_ptr_to_offset(struct bch_nvmpg_ns *ns, void *ptr)
> return BCH_NVMPG_OFFSET(ns_id, offset);
> }
>
> +static struct page *bch_nvmpg_va_to_pg(void *addr)
> +{
> + return virt_to_page(addr);
> +}
What's the purpose of this helper?
> +static inline void reserve_nvmpg_pages(struct bch_nvmpg_ns *ns,
> + pgoff_t pgoff, u64 nr)
> +{
> + while (nr > 0) {
> + unsigned int num = nr > UINT_MAX ? UINT_MAX : nr;
Surely UINT_MAX isn't anywhere near a valid limit?
> @@ -76,10 +110,73 @@ static void release_nvmpg_set(struct bch_nvmpg_set *set)
> kfree(set);
> }
>
> +static int validate_recs(int ns_id,
> + struct bch_nvmpg_head *head,
> + struct bch_nvmpg_recs *recs)
> +{
> + if (memcmp(recs->magic, bch_nvmpg_recs_magic, 16)) {
> + pr_err("Invalid bch_nvmpg_recs magic\n");
> + return -EINVAL;
> + }
> +
> + if (memcmp(recs->uuid, head->uuid, 16)) {
> + pr_err("Invalid bch_nvmpg_recs uuid\n");
> + return -EINVAL;
> + }
> +
> + if (recs->head_offset !=
> + bch_nvmpg_ptr_to_offset(global_nvmpg_set->ns_tbl[ns_id], head)) {
> + pr_err("Invalid recs head_offset\n");
> + return -EINVAL;
> + }
Same comments here on the frivilous error messaging, other places in
this file too. Check all the other patches as well, please.
> /* Namespace 0 contains all meta data of the nvmpg allocation set */
> static int init_nvmpg_set_header(struct bch_nvmpg_ns *ns)
> {
> struct bch_nvmpg_set_header *set_header;
> + struct bch_nvmpg_recs *sys_recs;
> + int i, j, used = 0, rc = 0;
>
> if (ns->ns_id != 0) {
> pr_err("unexpected ns_id %u for first nvmpg namespace.\n",
> @@ -93,9 +190,83 @@ static int init_nvmpg_set_header(struct bch_nvmpg_ns *ns)
> global_nvmpg_set->set_header = set_header;
> global_nvmpg_set->heads_size = set_header->size;
> global_nvmpg_set->heads_used = set_header->used;
> +
> + /* Reserve the used space from buddy allocator */
> + reserve_nvmpg_pages(ns, 0, div_u64(ns->pages_offset, ns->page_size));
> +
> + sys_recs = ns->base_addr + BCH_NVMPG_SYSRECS_OFFSET;
> + for (i = 0; i < set_header->size; i++) {
> + struct bch_nvmpg_head *head;
> +
> + head = &set_header->heads[i];
> + if (head->state == BCH_NVMPG_HD_STAT_FREE)
> + continue;
> +
> + used++;
> + if (used > global_nvmpg_set->heads_size) {
> + pr_err("used heads %d > heads size %d.\n",
> + used, global_nvmpg_set->heads_size);
> + goto unlock;
> + }
> +
> + for (j = 0; j < BCH_NVMPG_NS_MAX; j++) {
> + struct bch_nvmpg_recs *recs;
> +
> + recs = bch_nvmpg_offset_to_ptr(head->recs_offset[j]);
> +
> + /* Iterate the recs list */
> + while (recs) {
> + rc = validate_recs(j, head, recs);
> + if (rc < 0)
> + goto unlock;
> +
> + rc = reserve_nvmpg_recs(recs);
> + if (rc < 0)
> + goto unlock;
> +
> + bitmap_set(ns->recs_bitmap, recs - sys_recs, 1);
> + recs = bch_nvmpg_offset_to_ptr(recs->next_offset);
> + }
> + }
> + }
> +unlock:
> mutex_unlock(&global_nvmpg_set->lock);
> + return rc;
> +}
>
> - return 0;
> +static void bch_nvmpg_init_free_space(struct bch_nvmpg_ns *ns)
> +{
> + unsigned int start, end, pages;
> + int i;
> + struct page *page;
> + pgoff_t pgoff_start;
> +
> + bitmap_for_each_clear_region(ns->pages_bitmap,
> + start, end, 0, ns->pages_total) {
> + pgoff_start = start;
> + pages = end - start;
> +
> + while (pages) {
> + void *addr;
> +
> + for (i = BCH_MAX_ORDER - 1; i >= 0; i--) {
> + if ((pgoff_start % (1L << i) == 0) &&
> + (pages >= (1L << i)))
> + break;
> + }
> +
> + addr = bch_nvmpg_pgoff_to_ptr(ns, pgoff_start);
> + page = bch_nvmpg_va_to_pg(addr);
> + set_page_private(page, i);
> + page->index = pgoff_start;
> + __SetPageBuddy(page);
> + list_add((struct list_head *)&page->zone_device_data,
> + &ns->free_area[i]);
> +
> + pgoff_start += 1L << i;
> + pages -= 1L << i;
> + }
> + }
> }
>
> static int attach_nvmpg_set(struct bch_nvmpg_ns *ns)
> @@ -200,7 +371,7 @@ struct bch_nvmpg_ns *bch_register_namespace(const char *dev_path)
> char buf[BDEVNAME_SIZE];
> struct block_device *bdev;
> pgoff_t pgoff;
> - int id, err;
> + int id, i, err;
> char *path;
> long dax_ret = 0;
>
> @@ -304,13 +475,48 @@ struct bch_nvmpg_ns *bch_register_namespace(const char *dev_path)
>
> mutex_init(&ns->lock);
>
> + /*
> + * parameters of bitmap_set/clear are unsigned int.
> + * Given currently size of nvm is far from exceeding this limit,
> + * so only add a WARN_ON message.
> + */
> + WARN_ON(BITS_TO_LONGS(ns->pages_total) > UINT_MAX);
Does this really need to be a WARN_ON()? Looks more like an -EINVAL
condition.
--
Jens Axboe
