Re: [PATCH v13 04/12] bcache: bch_nvmpg_alloc_pages() of the buddy

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/13/21 4:14 AM, Jens Axboe wrote:
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.

Copied.

+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?

Cure, I will suggest Jianpeng and Qiaowei to change this.

+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.

It is because currently only 1 single record allocated for bcache journal, and if i == recs->size happens it means the on-NVDIMM struct bch_nvmpg_recs is corrupted.

Currently we are working on storing Btree nodes on NVDIMM, such BUG_ON() is dropped.


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.


Copied. Thank for pointing out this.

Coly Li



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux