On 12/12/21 10:05 AM, Coly Li wrote:
> +/* If not found, it will create if create == true */
> +static struct bch_nvmpg_head *find_nvmpg_head(const char *uuid, bool create)
> +{
> + struct bch_nvmpg_set_header *set_header = global_nvmpg_set->set_header;
> + struct bch_nvmpg_head *head = NULL;
> + int i;
> +
> + if (set_header == NULL)
> + goto out;
> +
> + for (i = 0; i < set_header->size; i++) {
> + struct bch_nvmpg_head *h = &set_header->heads[i];
> +
> + if (h->state != BCH_NVMPG_HD_STAT_ALLOC)
> + continue;
> +
> + if (!memcmp(uuid, h->uuid, 16)) {
> + head = h;
> + break;
> + }
> + }
> +
> + if (!head && create) {
> + u32 used = set_header->used;
> +
> + if (set_header->size > used) {
> + head = &set_header->heads[used];
> + memset(head, 0, sizeof(struct bch_nvmpg_head));
> + head->state = BCH_NVMPG_HD_STAT_ALLOC;
> + memcpy(head->uuid, uuid, 16);
> + global_nvmpg_set->heads_used++;
> + set_header->used++;
> + } else
> + pr_info("No free bch_nvmpg_head\n");
> + }
Use {} consistently. Again probably just some printk that should go
away.
> +static struct bch_nvmpg_recs *find_nvmpg_recs(struct bch_nvmpg_ns *ns,
> + struct bch_nvmpg_head *head,
> + bool create)
> +{
> + int ns_id = ns->sb->this_ns;
> + struct bch_nvmpg_recs *prev_recs = NULL, *recs = NULL;
> +
> + recs = bch_nvmpg_offset_to_ptr(head->recs_offset[ns_id]);
> +
> + /* If create=false, we return recs[nr] */
> + if (!create)
> + return recs;
Would this be cleaner to handle in the caller?
> +static void add_nvmpg_rec(struct bch_nvmpg_ns *ns,
> + struct bch_nvmpg_recs *recs,
> + unsigned long nvmpg_offset,
> + int order)
> +{
> + int i, ns_id;
> + unsigned long pgoff;
> +
> + pgoff = bch_nvmpg_offset_to_pgoff(nvmpg_offset);
> + ns_id = ns->sb->this_ns;
> +
> + for (i = 0; i < recs->size; i++) {
> + if (recs->recs[i].pgoff == 0) {
> + recs->recs[i].pgoff = pgoff;
> + recs->recs[i].order = order;
> + recs->recs[i].ns_id = ns_id;
> + recs->used++;
> + break;
> + }
> + }
> + BUG_ON(i == recs->size);
No BUG_ON's, please. It only truly belongs in core code for cases where
error handling isn't possible, does not apply here.
> diff --git a/drivers/md/bcache/nvmpg.h b/drivers/md/bcache/nvmpg.h
> index 55778d4db7da..d03f3241b45a 100644
> --- a/drivers/md/bcache/nvmpg.h
> +++ b/drivers/md/bcache/nvmpg.h
> @@ -76,6 +76,9 @@ struct bch_nvmpg_set {
> /* Indicate which field in bch_nvmpg_sb to be updated */
> #define BCH_NVMPG_TOTAL_NS 0 /* total_ns */
>
> +#define BCH_PGOFF_TO_KVADDR(pgoff) \
> + ((void *)((unsigned long)(pgoff) << PAGE_SHIFT))
Pretty sure we have a general kernel helper for this, better to use that
rather than duplicate it.
--
Jens Axboe
