ocfs2: submit another bio if current bio is full
PATCH 1/1 ocfs2: submit another bio if current bio is full
If cluster scale exceeds 16 nodes, bio will be full and bio_add_page() returns 0 when adding pages to bio. Returning -EIO to o2hb_read_slots() from o2hb_setup_one_bio() will lead to losing chance to allocate more bios to present all heartbeat region.
So o2hb_read_slots() fails.
In my test, making fs fails in starting o2cb service.
Attach error log:
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 0, vec_len = 4096, vec_start = 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 1, vec_len = 4096, vec_start = 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 2, vec_len = 4096, vec_start = 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 3, vec_len = 4096, vec_start = 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 4, vec_len = 4096, vec_start = 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 5, vec_len = 4096, vec_start = 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 6, vec_len = 4096, vec_start = 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 7, vec_len = 4096, vec_start = 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 8, vec_len = 4096, vec_start = 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 9, vec_len = 4096, vec_start = 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 10, vec_len = 4096, vec_start = 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 11, vec_len = 4096, vec_start = 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 12, vec_len = 4096, vec_start = 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 13, vec_len = 4096, vec_start = 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 14, vec_len = 4096, vec_start = 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 15, vec_len = 4096, vec_start = 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 16, vec_len = 4096, vec_start = 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:471 ERROR: Adding page[16] to bio failed, page ffffea0002d7ed40, len 0, vec_len 4096, vec_start 0, bi_sector 8192 (mkfs.ocfs2,27479,2):o2hb_read_slots:500 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_populate_slot_data:1911 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_region_dev_write:2012 ERROR: status = -5
--- fs/ocfs2/cluster/heartbeat.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
fs/ocfs2/cluster/heartbeat.c +6 -2
@@ -540,11 +540,12 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg,struct bio *bio;struct page *page; +#define O2HB_BIO_VECS 16/* Testing has shown this allocation to take long enough under * GFP_KERNEL that the local node can get fenced. It would be * nicest if we could pre-allocate these bios and avoid this * all together. */ - bio =bio_alloc (GFP_ATOMIC ,16 ); + bio =bio_alloc (GFP_ATOMIC ,O2HB_BIO_VECS );if (!bio) {mlog (ML_ERROR ,"Could not alloc slots BIO!\n" ); bio =ERR_PTR (-ENOMEM );
@@ -570,7 +571,10 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, current_page, vec_len, vec_start); len =bio_add_page (bio, page, vec_len, vec_start); -if (len != vec_len) { +if (len ==0 && current_page ==O2HB_BIO_VECS ) { +/* bio is full now. */ + goto bail; + }else if (len != vec_len) { mlog(ML_ERROR, "Adding page[%d] to bio failed, " "page %p, len %d, vec_len %u, vec_start %u, " "bi_sector %llu\n", current_page, page, len,
Signature · 1 line
2.7.4
PATCH 1/1 ocfs2: submit another bio if current bio is full
Hi Changwei,
Do you mean that if the slotnum exceed 16 like 'mkfs.ocfs2 -N 17', you still let it go rather than reture error?
thanks,
Jun
Quoted 70 lines · Changwei Ge
If cluster scale exceeds 16 nodes, bio will be full and
bio_add_page()returns 0 when adding pages to bio. Returning-EIOtoo2hb_read_slots()fromo2hb_setup_one_bio()will lead to losing chance to allocate more bios to present all heartbeat region.So
o2hb_read_slots()fails.In my test, making fs fails in starting o2cb service.
Attach error log:
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 0,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 1,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 2,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 3,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 4,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 5,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 6,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 7,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 8,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 9,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 10,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 11,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 12,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 13,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 14,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 15,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 16,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:471 ERROR: Adding page[16] to bio failed, pageffffea0002d7ed40, len 0,vec_len4096,vec_start0,bi_sector8192 (mkfs.ocfs2,27479,2):o2hb_read_slots:500 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_populate_slot_data:1911 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_region_dev_write:2012 ERROR: status = -5Fixes ba16ddfbeb9d ("ocfs2/o2hb: check len for bio_add_page() to avoid getting incorrect bio"Signed-off-by Changwei Ge <[email protected]>--- fs/ocfs2/cluster/heartbeat.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)fs/ocfs2/cluster/heartbeat.c +6
-2@@ -540,11 +540,12 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg,struct bio *bio;struct page *page; +#define O2HB_BIO_VECS 16/* Testing has shown this allocation to take long enough under * GFP_KERNEL that the local node can get fenced. It would be * nicest if we could pre-allocate these bios and avoid this * all together. */ - bio =bio_alloc (GFP_ATOMIC ,16 ); + bio =bio_alloc (GFP_ATOMIC ,O2HB_BIO_VECS );if (!bio) {mlog (ML_ERROR ,"Could not alloc slots BIO!\n" ); bio =ERR_PTR (-ENOMEM );@@ -570,7 +571,10 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, current_page, vec_len, vec_start); len =bio_add_page (bio, page, vec_len, vec_start); -if (len != vec_len) { +if (len ==0 && current_page ==O2HB_BIO_VECS ) { +/* bio is full now. */ + goto bail; + }else if (len != vec_len) { mlog(ML_ERROR, "Adding page[%d] to bio failed, " "page %p, len %d, vec_len %u, vec_start %u, " "bi_sector %llu\n", current_page, page, len,
PATCH 1/1 ocfs2: submit another bio if current bio is full
Hi Jun,
Sorry for this so late reply since I was very busy those days.
Quoted 4 lines · piaojun
Hi Changwei,
Do you mean that if the slotnum exceed 16 like 'mkfs.ocfs2 -N 17', you still let it go rather than reture error?
If your assumption is right, do you mean that ocfs2 slots can't exceed 16?
If you return error once slots exceed 16, mkfs will never succeed.
So if we can ensure that bio is full in current iteration, we should run into next iteration and allocate a
new bio adding pages and continue.
And your patch makes my ocfs2-test fail.
Thanks,
Changwei
Quoted 79 lines
thanks,
JunQuoted 60 lines · Changwei Ge
If cluster scale exceeds 16 nodes, bio will be full and
bio_add_page()returns 0 when adding pages to bio. Returning-EIOtoo2hb_read_slots()fromo2hb_setup_one_bio()will lead to losing chance to allocate more bios to present all heartbeat region.So
o2hb_read_slots()fails.In my test, making fs fails in starting o2cb service.
Attach error log:
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 0,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 1,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 2,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 3,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 4,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 5,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 6,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 7,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 8,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 9,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 10,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 11,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 12,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 13,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 14,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 15,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 16,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:471 ERROR: Adding page[16] to bio failed, pageffffea0002d7ed40, len 0,vec_len4096,vec_start0,bi_sector8192 (mkfs.ocfs2,27479,2):o2hb_read_slots:500 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_populate_slot_data:1911 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_region_dev_write:2012 ERROR: status = -5Fixes ba16ddfbeb9d ("ocfs2/o2hb: check len for bio_add_page() to avoid getting incorrect bio"Signed-off-by Changwei Ge <[email protected]>--- fs/ocfs2/cluster/heartbeat.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)fs/ocfs2/cluster/heartbeat.c +2
-1@@ -540,11 +540,12 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg,struct bio *bio;struct page *page; +#define O2HB_BIO_VECS 16/* Testing has shown this allocation to take long enough under * GFP_KERNEL that the local node can get fenced. It would be * nicest if we could pre-allocate these bios and avoid this * all together. */ - bio =bio_alloc (GFP_ATOMIC ,16 ); + bio =bio_alloc (GFP_ATOMIC ,O2HB_BIO_VECS );if (!bio) {mlog (ML_ERROR ,"Could not alloc slots BIO!\n" ); bio =ERR_PTR (-ENOMEM );@@ -570,7 +571,10 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, current_page, vec_len, vec_start);fs/ocfs2/cluster/heartbeat.c static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, · line 580
Quoted 10 lines
fs/ocfs2/cluster/heartbeat.c +4
-1len =bio_add_page (bio, page, vec_len, vec_start); -if (len != vec_len) { +if (len ==0 && current_page ==O2HB_BIO_VECS ) { +/* bio is full now. */ + goto bail; + }else if (len != vec_len) { mlog(ML_ERROR, "Adding page[%d] to bio failed, " "page %p, len %d, vec_len %u, vec_start %u, " "bi_sector %llu\n", current_page, page, len,_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
PATCH 1/1 ocfs2: submit another bio if current bio is full
Hi Changwei,
Quoted 19 lines · Changwei Ge
Hi Jun,
Sorry for this so late reply since I was very busy those days.
Quoted 4 lines · piaojun
Hi Changwei,
Do you mean that if the slotnum exceed 16 like 'mkfs.ocfs2 -N 17', you still let it go rather than reture error?
If your assumption is right, do you mean that ocfs2 slots can't exceed 16?
If you return error once slots exceed 16, mkfs will never succeed.
So if we can ensure that bio is full in current iteration, we should run into next iteration and allocate a
new bio adding pages and continue.
Quoted 12 lines
And your patch makes my ocfs2-test fail.
Thanks,
ChangweiQuoted 6 lines
thanks,
JunQuoted 1 line · Changwei Ge
If cluster scale exceeds 16 nodes, bio will be full and
bio_add_page()
Sorry for misunderstanding your fix, and do you mean that the node num is a little big which could not be covered by 16 pages, such as 129?
"one page could cover 8 node's slots"
thanks,
Jun
Quoted 74 lines
Quoted 73 lines
Quoted 59 lines
returns 0 when adding pages to bio. Returning
-EIOtoo2hb_read_slots()fromo2hb_setup_one_bio()will lead to losing chance to allocate more bios to present all heartbeat region.So
o2hb_read_slots()fails.In my test, making fs fails in starting o2cb service.
Attach error log:
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 0,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 1,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 2,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 3,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 4,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 5,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 6,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 7,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 8,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 9,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 10,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 11,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 12,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 13,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 14,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 15,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 16,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:471 ERROR: Adding page[16] to bio failed, pageffffea0002d7ed40, len 0,vec_len4096,vec_start0,bi_sector8192 (mkfs.ocfs2,27479,2):o2hb_read_slots:500 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_populate_slot_data:1911 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_region_dev_write:2012 ERROR: status = -5Fixes ba16ddfbeb9d ("ocfs2/o2hb: check len for bio_add_page() to avoid getting incorrect bio"Signed-off-by Changwei Ge <[email protected]>--- fs/ocfs2/cluster/heartbeat.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)fs/ocfs2/cluster/heartbeat.c +2
-1@@ -540,11 +540,12 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg,struct bio *bio;struct page *page; +#define O2HB_BIO_VECS 16/* Testing has shown this allocation to take long enough under * GFP_KERNEL that the local node can get fenced. It would be * nicest if we could pre-allocate these bios and avoid this * all together. */ - bio =bio_alloc (GFP_ATOMIC ,16 ); + bio =bio_alloc (GFP_ATOMIC ,O2HB_BIO_VECS );if (!bio) {mlog (ML_ERROR ,"Could not alloc slots BIO!\n" ); bio =ERR_PTR (-ENOMEM );@@ -570,7 +571,10 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, current_page, vec_len, vec_start);fs/ocfs2/cluster/heartbeat.c static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, · line 580
Quoted 10 lines
fs/ocfs2/cluster/heartbeat.c +4
-1len =bio_add_page (bio, page, vec_len, vec_start); -if (len != vec_len) { +if (len ==0 && current_page ==O2HB_BIO_VECS ) { +/* bio is full now. */ + goto bail; + }else if (len != vec_len) { mlog(ML_ERROR, "Adding page[%d] to bio failed, " "page %p, len %d, vec_len %u, vec_start %u, " "bi_sector %llu\n", current_page, page, len,_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
PATCH 1/1 ocfs2: submit another bio if current bio is full
Hi Jun,
Quoted 30 lines · piaojun
Hi Changwei,
Quoted 27 lines · Changwei Ge
Hi Jun,
Sorry for this so late reply since I was very busy those days.
Quoted 4 lines · piaojun
Hi Changwei,
Do you mean that if the slotnum exceed 16 like 'mkfs.ocfs2 -N 17', you still let it go rather than reture error?
If your assumption is right, do you mean that ocfs2 slots can't exceed 16?
If you return error once slots exceed 16, mkfs will never succeed.
So if we can ensure that bio is full in current iteration, we should run into next iteration and allocate a
new bio adding pages and continue.And your patch makes my ocfs2-test fail.
Thanks,
ChangweiQuoted 3 lines
thanks,
Jun
Quoted 6 lines
Quoted 2 lines
Quoted 2 lines · Changwei Ge
Quoted 1 line
If cluster scale exceeds 16 nodes, bio will be full and
bio_add_page()Sorry for misunderstanding your fix, and do you mean that the node num is a little big which could not be covered by 16 pages, such as 129?
"one page could cover 8 node's slots"
It has nothing to do with the capacity of page holding slots. It's about how many vecs a bio can have.
For your reference, bio_alloc() has set the maximum vec to 16 in o2hb_setup_one_bio() as precondition.
Thanks,
Changwei
Quoted 77 lines
thanks,
JunQuoted 73 lines
Quoted 73 lines
Quoted 59 lines
returns 0 when adding pages to bio. Returning
-EIOtoo2hb_read_slots()fromo2hb_setup_one_bio()will lead to losing chance to allocate more bios to present all heartbeat region.So
o2hb_read_slots()fails.In my test, making fs fails in starting o2cb service.
Attach error log:
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 0,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 1,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 2,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 3,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 4,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 5,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 6,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 7,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 8,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 9,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 10,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 11,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 12,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 13,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 14,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 15,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 16,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:471 ERROR: Adding page[16] to bio failed, pageffffea0002d7ed40, len 0,vec_len4096,vec_start0,bi_sector8192 (mkfs.ocfs2,27479,2):o2hb_read_slots:500 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_populate_slot_data:1911 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_region_dev_write:2012 ERROR: status = -5Fixes ba16ddfbeb9d ("ocfs2/o2hb: check len for bio_add_page() to avoid getting incorrect bio"Signed-off-by Changwei Ge <[email protected]>--- fs/ocfs2/cluster/heartbeat.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)fs/ocfs2/cluster/heartbeat.c +2
-1@@ -540,11 +540,12 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg,struct bio *bio;struct page *page; +#define O2HB_BIO_VECS 16/* Testing has shown this allocation to take long enough under * GFP_KERNEL that the local node can get fenced. It would be * nicest if we could pre-allocate these bios and avoid this * all together. */ - bio =bio_alloc (GFP_ATOMIC ,16 ); + bio =bio_alloc (GFP_ATOMIC ,O2HB_BIO_VECS );if (!bio) {mlog (ML_ERROR ,"Could not alloc slots BIO!\n" ); bio =ERR_PTR (-ENOMEM );@@ -570,7 +571,10 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, current_page, vec_len, vec_start);fs/ocfs2/cluster/heartbeat.c static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, · line 580
Quoted 10 lines
fs/ocfs2/cluster/heartbeat.c +4
-1len =bio_add_page (bio, page, vec_len, vec_start); -if (len != vec_len) { +if (len ==0 && current_page ==O2HB_BIO_VECS ) { +/* bio is full now. */ + goto bail; + }else if (len != vec_len) { mlog(ML_ERROR, "Adding page[%d] to bio failed, " "page %p, len %d, vec_len %u, vec_start %u, " "bi_sector %llu\n", current_page, page, len,_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
PATCH 1/1 ocfs2: submit another bio if current bio is full
Hi Changwei,
I understand your fix already, but I'm still confused by the comments "If cluster scale exceeds 16 nodes, ...".
Do you mean that this problem will happen if nodes' count exceeds 16.
thanks,
Jun
Quoted 127 lines · Changwei Ge
Hi Jun,
Quoted 30 lines · piaojun
Hi Changwei,
Quoted 27 lines · Changwei Ge
Hi Jun,
Sorry for this so late reply since I was very busy those days.
Quoted 4 lines · piaojun
Hi Changwei,
Do you mean that if the slotnum exceed 16 like 'mkfs.ocfs2 -N 17', you still let it go rather than reture error?
If your assumption is right, do you mean that ocfs2 slots can't exceed 16?
If you return error once slots exceed 16, mkfs will never succeed.
So if we can ensure that bio is full in current iteration, we should run into next iteration and allocate a
new bio adding pages and continue.And your patch makes my ocfs2-test fail.
Thanks,
ChangweiQuoted 3 lines
thanks,
JunQuoted 6 lines
Quoted 2 lines
Quoted 2 lines · Changwei Ge
Quoted 1 line
If cluster scale exceeds 16 nodes, bio will be full and
bio_add_page()Sorry for misunderstanding your fix, and do you mean that the node num is a little big which could not be covered by 16 pages, such as 129?
"one page could cover 8 node's slots"
It has nothing to do with the capacity of page holding slots. It's about how many vecs a bio can have.
For your reference,
bio_alloc()has set the maximum vec to 16 ino2hb_setup_one_bio()as precondition.Thanks,
ChangweiQuoted 77 lines
thanks,
JunQuoted 73 lines
Quoted 73 lines
Quoted 59 lines
returns 0 when adding pages to bio. Returning
-EIOtoo2hb_read_slots()fromo2hb_setup_one_bio()will lead to losing chance to allocate more bios to present all heartbeat region.So
o2hb_read_slots()fails.In my test, making fs fails in starting o2cb service.
Attach error log:
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 0,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 1,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 2,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 3,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 4,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 5,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 6,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 7,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 8,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 9,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 10,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 11,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 12,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 13,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 14,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 15,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 16,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:471 ERROR: Adding page[16] to bio failed, pageffffea0002d7ed40, len 0,vec_len4096,vec_start0,bi_sector8192 (mkfs.ocfs2,27479,2):o2hb_read_slots:500 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_populate_slot_data:1911 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_region_dev_write:2012 ERROR: status = -5Fixes ba16ddfbeb9d ("ocfs2/o2hb: check len for bio_add_page() to avoid getting incorrect bio"Signed-off-by Changwei Ge <[email protected]>--- fs/ocfs2/cluster/heartbeat.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)fs/ocfs2/cluster/heartbeat.c +2
-1@@ -540,11 +540,12 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg,struct bio *bio;struct page *page; +#define O2HB_BIO_VECS 16/* Testing has shown this allocation to take long enough under * GFP_KERNEL that the local node can get fenced. It would be * nicest if we could pre-allocate these bios and avoid this * all together. */ - bio =bio_alloc (GFP_ATOMIC ,16 ); + bio =bio_alloc (GFP_ATOMIC ,O2HB_BIO_VECS );if (!bio) {mlog (ML_ERROR ,"Could not alloc slots BIO!\n" ); bio =ERR_PTR (-ENOMEM );@@ -570,7 +571,10 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, current_page, vec_len, vec_start);fs/ocfs2/cluster/heartbeat.c static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, · line 580
Quoted 10 lines
fs/ocfs2/cluster/heartbeat.c +4
-1len =bio_add_page (bio, page, vec_len, vec_start); -if (len != vec_len) { +if (len ==0 && current_page ==O2HB_BIO_VECS ) { +/* bio is full now. */ + goto bail; + }else if (len != vec_len) { mlog(ML_ERROR, "Adding page[%d] to bio failed, " "page %p, len %d, vec_len %u, vec_start %u, " "bi_sector %llu\n", current_page, page, len,_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
PATCH 1/1 ocfs2: submit another bio if current bio is full
Hi Jun,
Quoted 5 lines · piaojun
Hi Changwei,
I understand your fix already, but I'm still confused by the comments "If cluster scale exceeds 16 nodes, ...".
Do you mean that this problem will happen if nodes' count exceeds 16.
True. Or we can say that if slot number exceeds 16.
Thanks,
Changwei
Quoted 130 lines
thanks,
JunQuoted 125 lines · Changwei Ge
Hi Jun,
Quoted 30 lines · piaojun
Hi Changwei,
Quoted 27 lines · Changwei Ge
Hi Jun,
Sorry for this so late reply since I was very busy those days.
Quoted 4 lines · piaojun
Hi Changwei,
Do you mean that if the slotnum exceed 16 like 'mkfs.ocfs2 -N 17', you still let it go rather than reture error?
If your assumption is right, do you mean that ocfs2 slots can't exceed 16?
If you return error once slots exceed 16, mkfs will never succeed.
So if we can ensure that bio is full in current iteration, we should run into next iteration and allocate a
new bio adding pages and continue.And your patch makes my ocfs2-test fail.
Thanks,
ChangweiQuoted 3 lines
thanks,
JunQuoted 6 lines
Quoted 2 lines
Quoted 2 lines · Changwei Ge
Quoted 1 line
If cluster scale exceeds 16 nodes, bio will be full and
bio_add_page()Sorry for misunderstanding your fix, and do you mean that the node num is a little big which could not be covered by 16 pages, such as 129?
"one page could cover 8 node's slots"
It has nothing to do with the capacity of page holding slots. It's about how many vecs a bio can have.
For your reference,
bio_alloc()has set the maximum vec to 16 ino2hb_setup_one_bio()as precondition.Thanks,
ChangweiQuoted 76 lines
thanks,
JunQuoted 73 lines
Quoted 73 lines
Quoted 59 lines
returns 0 when adding pages to bio. Returning
-EIOtoo2hb_read_slots()fromo2hb_setup_one_bio()will lead to losing chance to allocate more bios to present all heartbeat region.So
o2hb_read_slots()fails.In my test, making fs fails in starting o2cb service.
Attach error log:
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 0,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 1,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 2,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 3,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 4,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 5,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 6,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 7,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 8,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 9,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 10,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 11,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 12,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 13,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 14,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 15,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 16,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:471 ERROR: Adding page[16] to bio failed, pageffffea0002d7ed40, len 0,vec_len4096,vec_start0,bi_sector8192 (mkfs.ocfs2,27479,2):o2hb_read_slots:500 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_populate_slot_data:1911 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_region_dev_write:2012 ERROR: status = -5Fixes ba16ddfbeb9d ("ocfs2/o2hb: check len for bio_add_page() to avoid getting incorrect bio"Signed-off-by Changwei Ge <[email protected]>--- fs/ocfs2/cluster/heartbeat.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)fs/ocfs2/cluster/heartbeat.c +2
-1@@ -540,11 +540,12 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg,struct bio *bio;struct page *page; +#define O2HB_BIO_VECS 16/* Testing has shown this allocation to take long enough under * GFP_KERNEL that the local node can get fenced. It would be * nicest if we could pre-allocate these bios and avoid this * all together. */ - bio =bio_alloc (GFP_ATOMIC ,16 ); + bio =bio_alloc (GFP_ATOMIC ,O2HB_BIO_VECS );if (!bio) {mlog (ML_ERROR ,"Could not alloc slots BIO!\n" ); bio =ERR_PTR (-ENOMEM );@@ -570,7 +571,10 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, current_page, vec_len, vec_start);fs/ocfs2/cluster/heartbeat.c static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, · line 580
Quoted 10 lines
fs/ocfs2/cluster/heartbeat.c +4
-1len =bio_add_page (bio, page, vec_len, vec_start); -if (len != vec_len) { +if (len ==0 && current_page ==O2HB_BIO_VECS ) { +/* bio is full now. */ + goto bail; + }else if (len != vec_len) { mlog(ML_ERROR, "Adding page[%d] to bio failed, " "page %p, len %d, vec_len %u, vec_start %u, " "bi_sector %llu\n", current_page, page, len,_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
PATCH 1/1 ocfs2: submit another bio if current bio is full
In reply to Changwei GeHi Changwei,
Quoted 58 lines · Changwei Ge
If cluster scale exceeds 16 nodes, bio will be full and
bio_add_page()returns 0 when adding pages to bio. Returning-EIOtoo2hb_read_slots()fromo2hb_setup_one_bio()will lead to losing chance to allocate more bios to present all heartbeat region.So
o2hb_read_slots()fails.In my test, making fs fails in starting o2cb service.
Attach error log:
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 0,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 1,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 2,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 3,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 4,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 5,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 6,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 7,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 8,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 9,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 10,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 11,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 12,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 13,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 14,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 15,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 16,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:471 ERROR: Adding page[16] to bio failed, pageffffea0002d7ed40, len 0,vec_len4096,vec_start0,bi_sector8192 (mkfs.ocfs2,27479,2):o2hb_read_slots:500 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_populate_slot_data:1911 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_region_dev_write:2012 ERROR: status = -5Fixes ba16ddfbeb9d ("ocfs2/o2hb: check len for bio_add_page() to avoid getting incorrect bio"Signed-off-by Changwei Ge <[email protected]>--- fs/ocfs2/cluster/heartbeat.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)fs/ocfs2/cluster/heartbeat.c +2
-1@@ -540,11 +540,12 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg,struct bio *bio;struct page *page; +#define O2HB_BIO_VECS 16/* Testing has shown this allocation to take long enough under * GFP_KERNEL that the local node can get fenced. It would be * nicest if we could pre-allocate these bios and avoid this * all together. */ - bio =bio_alloc (GFP_ATOMIC ,16 ); + bio =bio_alloc (GFP_ATOMIC ,O2HB_BIO_VECS );if (!bio) {mlog (ML_ERROR ,"Could not alloc slots BIO!\n" ); bio =ERR_PTR (-ENOMEM );
fs/ocfs2/cluster/heartbeat.c static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, · line 572
Quoted 2 lines
fs/ocfs2/cluster/heartbeat.c +0
-0@@ -570,7 +571,10 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, current_page, vec_len, vec_start);
Should we check the validity of 'current_page' before bio_add_page()? And that will prevent error happen. Others looks OK.
thanks,
Jun
Quoted 10 lines
fs/ocfs2/cluster/heartbeat.c +4
-1len =bio_add_page (bio, page, vec_len, vec_start); -if (len != vec_len) { +if (len ==0 && current_page ==O2HB_BIO_VECS ) { +/* bio is full now. */ + goto bail; + }else if (len != vec_len) { mlog(ML_ERROR, "Adding page[%d] to bio failed, " "page %p, len %d, vec_len %u, vec_start %u, " "bi_sector %llu\n", current_page, page, len,
PATCH 1/1 ocfs2: submit another bio if current bio is full
Hi Jun,
Quoted 50 lines · piaojun
Hi Changwei,
Quoted 47 lines · Changwei Ge
If cluster scale exceeds 16 nodes, bio will be full and
bio_add_page()returns 0 when adding pages to bio. Returning-EIOtoo2hb_read_slots()fromo2hb_setup_one_bio()will lead to losing chance to allocate more bios to present all heartbeat region.So
o2hb_read_slots()fails.In my test, making fs fails in starting o2cb service.
Attach error log:
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 0,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 1,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 2,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 3,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 4,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 5,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 6,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 7,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 8,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 9,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 10,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 11,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 12,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 13,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 14,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 15,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 16,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:471 ERROR: Adding page[16] to bio failed, pageffffea0002d7ed40, len 0,vec_len4096,vec_start0,bi_sector8192 (mkfs.ocfs2,27479,2):o2hb_read_slots:500 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_populate_slot_data:1911 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_region_dev_write:2012 ERROR: status = -5Fixes ba16ddfbeb9d ("ocfs2/o2hb: check len for bio_add_page() to avoid getting incorrect bio"Signed-off-by Changwei Ge <[email protected]>--- fs/ocfs2/cluster/heartbeat.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)fs/ocfs2/cluster/heartbeat.c +0
-0@@ -540,11 +540,12 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg,struct bio *bio;struct page *page;
Quoted 15 lines
fs/ocfs2/cluster/heartbeat.c static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, · line 572
Quoted 13 lines
fs/ocfs2/cluster/heartbeat.c +2
-1+#define O2HB_BIO_VECS 16/* Testing has shown this allocation to take long enough under * GFP_KERNEL that the local node can get fenced. It would be * nicest if we could pre-allocate these bios and avoid this * all together. */ - bio =bio_alloc (GFP_ATOMIC ,16 ); + bio =bio_alloc (GFP_ATOMIC ,O2HB_BIO_VECS );if (!bio) {mlog (ML_ERROR ,"Could not alloc slots BIO!\n" ); bio =ERR_PTR (-ENOMEM );@@ -570,7 +571,10 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, current_page, vec_len, vec_start);Should we check the validity of '
current_page' beforebio_add_page()? And that will prevent error happen. Others looks OK.
If I understand correctly, you mean we should check current page? is NULL or not?
If so I think there is no need since o2hb should guarantee that it has already reserved enough pages for disk heartbeat read/write behalf.
Thanks,
Changwei
Quoted 17 lines
thanks,
Junfs/ocfs2/cluster/heartbeat.c static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, · line 580
Quoted 10 lines
fs/ocfs2/cluster/heartbeat.c +4
-1len =bio_add_page (bio, page, vec_len, vec_start); -if (len != vec_len) { +if (len ==0 && current_page ==O2HB_BIO_VECS ) { +/* bio is full now. */ + goto bail; + }else if (len != vec_len) { mlog(ML_ERROR, "Adding page[%d] to bio failed, " "page %p, len %d, vec_len %u, vec_start %u, " "bi_sector %llu\n", current_page, page, len,_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
PATCH 1/1 ocfs2: submit another bio if current bio is full
Quoted 70 lines · Changwei Ge
Hi Jun,
Quoted 50 lines · piaojun
Hi Changwei,
Quoted 47 lines · Changwei Ge
If cluster scale exceeds 16 nodes, bio will be full and
bio_add_page()returns 0 when adding pages to bio. Returning-EIOtoo2hb_read_slots()fromo2hb_setup_one_bio()will lead to losing chance to allocate more bios to present all heartbeat region.So
o2hb_read_slots()fails.In my test, making fs fails in starting o2cb service.
Attach error log:
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 0,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 1,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 2,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 3,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 4,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 5,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 6,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 7,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 8,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 9,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 10,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 11,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 12,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 13,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 14,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 15,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 16,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:471 ERROR: Adding page[16] to bio failed, pageffffea0002d7ed40, len 0,vec_len4096,vec_start0,bi_sector8192 (mkfs.ocfs2,27479,2):o2hb_read_slots:500 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_populate_slot_data:1911 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_region_dev_write:2012 ERROR: status = -5Fixes ba16ddfbeb9d ("ocfs2/o2hb: check len for bio_add_page() to avoid getting incorrect bio"Signed-off-by Changwei Ge <[email protected]>--- fs/ocfs2/cluster/heartbeat.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)fs/ocfs2/cluster/heartbeat.c +0
-0@@ -540,11 +540,12 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg,struct bio *bio;struct page *page;Quoted 15 lines
fs/ocfs2/cluster/heartbeat.c static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, · line 572
Quoted 13 lines
fs/ocfs2/cluster/heartbeat.c +2
-1+#define O2HB_BIO_VECS 16/* Testing has shown this allocation to take long enough under * GFP_KERNEL that the local node can get fenced. It would be * nicest if we could pre-allocate these bios and avoid this * all together. */ - bio =bio_alloc (GFP_ATOMIC ,16 ); + bio =bio_alloc (GFP_ATOMIC ,O2HB_BIO_VECS );if (!bio) {mlog (ML_ERROR ,"Could not alloc slots BIO!\n" ); bio =ERR_PTR (-ENOMEM );@@ -570,7 +571,10 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, current_page, vec_len, vec_start);Should we check the validity of '
current_page' beforebio_add_page()? And that will prevent error happen. Others looks OK.
Quoted 4 lines
If I understand correctly, you mean we should check current page is NULL or not?
If so I think there is no need since o2hb should guarantee that it has already reserved enough pages for disk heartbeat read/write behalf.
I mean we could check if 'current_page' equals O2HB_BIO_VECS before bio_add_page() to avoid NULL pointer referrence.
thanks,
Jun
Quoted 21 lines
Thanks,
ChangweiQuoted 17 lines
thanks,
Junfs/ocfs2/cluster/heartbeat.c static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, · line 580
Quoted 10 lines
fs/ocfs2/cluster/heartbeat.c +4
-1len =bio_add_page (bio, page, vec_len, vec_start); -if (len != vec_len) { +if (len ==0 && current_page ==O2HB_BIO_VECS ) { +/* bio is full now. */ + goto bail; + }else if (len != vec_len) { mlog(ML_ERROR, "Adding page[%d] to bio failed, " "page %p, len %d, vec_len %u, vec_start %u, " "bi_sector %llu\n", current_page, page, len,_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
PATCH 1/1 ocfs2: submit another bio if current bio is full
Quoted 71 lines · piaojun
Quoted 69 lines · Changwei Ge
Hi Jun,
Quoted 65 lines · piaojun
Hi Changwei,
Quoted 58 lines · Changwei Ge
If cluster scale exceeds 16 nodes, bio will be full and
bio_add_page()returns 0 when adding pages to bio. Returning-EIOtoo2hb_read_slots()fromo2hb_setup_one_bio()will lead to losing chance to allocate more bios to present all heartbeat region.So
o2hb_read_slots()fails.In my test, making fs fails in starting o2cb service.
Attach error log:
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 0,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 1,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 2,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 3,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 4,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 5,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 6,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 7,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 8,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 9,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 10,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 11,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 12,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 13,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 14,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 15,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 16,vec_len= 4096,vec_start= 0
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:471 ERROR: Adding page[16] to bio failed, pageffffea0002d7ed40, len 0,vec_len4096,vec_start0,bi_sector8192 (mkfs.ocfs2,27479,2):o2hb_read_slots:500 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_populate_slot_data:1911 ERROR: status = -5
(mkfs.ocfs2,27479,2):o2hb_region_dev_write:2012 ERROR: status = -5Fixes ba16ddfbeb9d ("ocfs2/o2hb: check len for bio_add_page() to avoid getting incorrect bio"Signed-off-by Changwei Ge <[email protected]>--- fs/ocfs2/cluster/heartbeat.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)fs/ocfs2/cluster/heartbeat.c +2
-1@@ -540,11 +540,12 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg,struct bio *bio;struct page *page; +#define O2HB_BIO_VECS 16/* Testing has shown this allocation to take long enough under * GFP_KERNEL that the local node can get fenced. It would be * nicest if we could pre-allocate these bios and avoid this * all together. */ - bio =bio_alloc (GFP_ATOMIC ,16 ); + bio =bio_alloc (GFP_ATOMIC ,O2HB_BIO_VECS );if (!bio) {mlog (ML_ERROR ,"Could not alloc slots BIO!\n" ); bio =ERR_PTR (-ENOMEM );fs/ocfs2/cluster/heartbeat.c static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, · line 572
Quoted 2 lines
fs/ocfs2/cluster/heartbeat.c +0
-0@@ -570,7 +571,10 @@ static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, current_page, vec_len, vec_start);Should we check the validity of '
current_page' beforebio_add_page()? And that will prevent error happen. Others looks OK.
Quoted 6 lines
Quoted 4 lines
If I understand correctly, you mean we should check current page is NULL or not?
If so I think there is no need since o2hb should guarantee that it has already reserved enough pages for disk heartbeat read/write behalf.I mean we could check if '
current_page' equalsO2HB_BIO_VECSbeforebio_add_page()to avoid NULL pointer referrence.
Yes, that might work.
I find another problem within this patch.
I will post v2 patch later to fix them all with consideration about your suggestion.
Thanks,
Changwei
Quoted 22 lines
thanks,
JunQuoted 18 lines
Thanks,
ChangweiQuoted 16 lines
thanks,
Junfs/ocfs2/cluster/heartbeat.c static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, · line 580
Quoted 10 lines
fs/ocfs2/cluster/heartbeat.c +4
-1len =bio_add_page (bio, page, vec_len, vec_start); -if (len != vec_len) { +if (len ==0 && current_page ==O2HB_BIO_VECS ) { +/* bio is full now. */ + goto bail; + }else if (len != vec_len) { mlog(ML_ERROR, "Adding page[%d] to bio failed, " "page %p, len %d, vec_len %u, vec_start %u, " "bi_sector %llu\n", current_page, page, len,_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/1 ocfs2: submit another bio if current bio is full
Hi Jun,
Right now, I am afraid that the easiest and fasted way to fix this issue is to revert your patch.
From comments before function bio_add_page(), we can see that it only
fails if either ::bi_vcnt == ::bi_max_vecs or it's a cloned bio.
So we can judge if bio is full from its return value is zero or not.
Thanks,
Changwei
Quoted 135 lines · Changwei Ge
Quoted 95 lines · piaojun
Quoted 93 lines · Changwei Ge
Hi Jun,
Quoted 89 lines · piaojun
Hi Changwei,
Quoted 81 lines · Changwei Ge
If cluster scale exceeds 16 nodes, bio will be full and
bio_add_page()
returns 0 when adding pages to bio. Returning-EIOtoo2hb_read_slots()
fromo2hb_setup_one_bio()will lead to losing chance to allocate more bios to present all heartbeat region.So
o2hb_read_slots()fails.In my test, making fs fails in starting o2cb service.
Attach error log:
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 0,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 1,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 2,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 3,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 4,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 5,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 6,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 7,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 8,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 9,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 10,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 11,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 12,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 13,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 14,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 15,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 16,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:471 ERROR: Adding page[16] to bio failed, pageffffea0002d7ed40, len 0,vec_len4096,vec_start0,bi_sector8192 (mkfs.ocfs2,27479,2):o2hb_read_slots:500 ERROR: status = -5 (mkfs.ocfs2,27479,2):o2hb_populate_slot_data:1911 ERROR: status = -5 (mkfs.ocfs2,27479,2):o2hb_region_dev_write:2012 ERROR: status = -5Fixes ba16ddfbeb9d ("ocfs2/o2hb: check len for bio_add_page() toavoid getting incorrect bio"
Signed-off-by Changwei Ge <[email protected]>---
??fs/ocfs2/cluster/heartbeat.c| 8 ++++++-- ?? 1 file changed, 6 insertions(+), 2 deletions(-)fs/ocfs2/cluster/heartbeat.c +0
-0@@ -540,11 +540,12 @@ static struct bio *o2hb_setup_one_bio(structo2hb_region *reg, ?????? struct bio *bio; ?????? struct page *page;?? +#define
O2HB_BIO_VECS16
?????? /* Testing has shown this allocation to take long enough under ??????? *GFP_KERNELthat the local node can get fenced. It would be ??????? * nicest if we could pre-allocate these bios and avoid this ??????? * all together. */
-??? bio =bio_alloc(GFP_ATOMIC, 16);
+??? bio =bio_alloc(GFP_ATOMIC, O2HB_BIO_VECS);
?????? if (!bio) {
??????????mlog(ML_ERROR, "Could not alloc slots BIO!\n"); ?????????? bio =ERR_PTR(-ENOMEM);fs/ocfs2/cluster/heartbeat.c static struct bio *o2hb_setup_one_bio(struct · line 572
Quoted 2 lines
fs/ocfs2/cluster/heartbeat.c +0
-0@@ -570,7 +571,10 @@ static struct bio *o2hb_setup_one_bio(structo2hb_region *reg, ??????????????? current_page, vec_len, vec_start);Should we check the validity of '
current_page' beforebio_add_page()? And
that will prevent error happen. Others looks OK.Quoted 6 lines
Quoted 4 lines
If I understand correctly, you mean we should check current page? is NULL or not?
If so I think there is no need since o2hb should guarantee that it has already reserved enough pages for disk heartbeat read/write behalf.I mean we could check if '
current_page' equalsO2HB_BIO_VECSbeforebio_add_page()to avoid NULL pointer referrence.Yes, that might work.
I find another problem within this patch.
I will post v2 patch later to fix them all with consideration about your suggestion.Thanks,
ChangweiQuoted 22 lines
thanks,
JunQuoted 18 lines
Thanks,
ChangweiQuoted 16 lines
thanks,
JunQuoted 10 lines
fs/ocfs2/cluster/heartbeat.c +4
-1?????????? len = bio_add_page(bio, page, vec_len, vec_start); -??????? if (len != vec_len) { +??????? if (len == 0 && current_page == O2HB_BIO_VECS) { +??????????? /* bio is full now. */ +??????????? goto bail; +??????? } else if (len != vec_len) {?????????????? mlog(ML_ERROR, "Adding page[%d] to bio failed, " ??????????????????? "page %p, len %d,
vec_len%u,vec_start%u, " ??????????????????? "bi_sector%llu\n",current_page, page, len,_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
PATCH 1/1 ocfs2: submit another bio if current bio is full
Hi Changwei,
I got your point, we should let the caller retry if bio is not enough, right? But some caller like o2hb_issue_node_write() won't retry once error happens, though the bio will always be enough. I think if we could calculate the number of bio we need before calling bio_add_page()?
Thanks
Jun
Quoted 154 lines · Changwei Ge
Hi Jun,
Right now, I am afraid that the easiest and fasted way to fix this issue is to revert your patch.
From comments before function
bio_add_page(), we can see that it only
fails if either ::bi_vcnt == ::bi_max_vecs or it's a cloned bio.So we can judge if bio is full from its return value is zero or not.
Thanks,
Changwei
Quoted 135 lines · Changwei Ge
Quoted 95 lines · piaojun
Quoted 93 lines · Changwei Ge
Hi Jun,
Quoted 89 lines · piaojun
Hi Changwei,
Quoted 81 lines · Changwei Ge
If cluster scale exceeds 16 nodes, bio will be full and
bio_add_page()
returns 0 when adding pages to bio. Returning-EIOtoo2hb_read_slots()
fromo2hb_setup_one_bio()will lead to losing chance to allocate more bios to present all heartbeat region.So
o2hb_read_slots()fails.In my test, making fs fails in starting o2cb service.
Attach error log:
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 0,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 1,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 2,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 3,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 4,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 5,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 6,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 7,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 8,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 9,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 10,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 11,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 12,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 13,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 14,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 15,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 16,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:471 ERROR: Adding page[16] to bio failed, pageffffea0002d7ed40, len 0,vec_len4096,vec_start0,bi_sector8192 (mkfs.ocfs2,27479,2):o2hb_read_slots:500 ERROR: status = -5 (mkfs.ocfs2,27479,2):o2hb_populate_slot_data:1911 ERROR: status = -5 (mkfs.ocfs2,27479,2):o2hb_region_dev_write:2012 ERROR: status = -5Fixes ba16ddfbeb9d ("ocfs2/o2hb: check len for bio_add_page() toavoid getting incorrect bio"
Signed-off-by Changwei Ge <[email protected]>--- fs/ocfs2/cluster/heartbeat.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)fs/ocfs2/cluster/heartbeat.c +1
-1@@ -540,11 +540,12 @@ static struct bio *o2hb_setup_one_bio(structo2hb_region *reg, struct bio *bio;struct page *page; +#define O2HB_BIO_VECS 16/* Testing has shown this allocation to take long enough under * GFP_KERNEL that the local node can get fenced. It would be * nicest if we could pre-allocate these bios and avoid this * all together. */ - bio =bio_alloc (GFP_ATOMIC ,16 ); + bio =bio_alloc (GFP_ATOMIC ,O2HB_BIO_VECS );if (!bio) {mlog (ML_ERROR ,"Could not alloc slots BIO!\n" ); bio =ERR_PTR (-ENOMEM );fs/ocfs2/cluster/heartbeat.c static struct bio *o2hb_setup_one_bio(struct · line 572
Quoted 2 lines
fs/ocfs2/cluster/heartbeat.c +0
-0@@ -570,7 +571,10 @@ static struct bio *o2hb_setup_one_bio(structo2hb_region *reg, current_page, vec_len, vec_start);Should we check the validity of '
current_page' beforebio_add_page()? And
that will prevent error happen. Others looks OK.Quoted 6 lines
Quoted 4 lines
If I understand correctly, you mean we should check current page is NULL or not?
If so I think there is no need since o2hb should guarantee that it has already reserved enough pages for disk heartbeat read/write behalf.I mean we could check if '
current_page' equalsO2HB_BIO_VECSbeforebio_add_page()to avoid NULL pointer referrence.Yes, that might work.
I find another problem within this patch.
I will post v2 patch later to fix them all with consideration about your suggestion.Thanks,
ChangweiQuoted 22 lines
thanks,
JunQuoted 18 lines
Thanks,
ChangweiQuoted 16 lines
thanks,
Junfs/ocfs2/cluster/heartbeat.c static struct bio *o2hb_setup_one_bio(struct o2hb_region *reg, · line 580
Quoted 10 lines
fs/ocfs2/cluster/heartbeat.c +4
-1len =bio_add_page (bio, page, vec_len, vec_start); -if (len != vec_len) { +if (len ==0 && current_page ==O2HB_BIO_VECS ) { +/* bio is full now. */ + goto bail; + }else if (len != vec_len) { mlog(ML_ERROR, "Adding page[%d] to bio failed, " "page %p, len %d, vec_len %u, vec_start %u, " "bi_sector %llu\n", current_page, page, len,_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
PATCH 1/1 ocfs2: submit another bio if current bio is full
Hi Jun,
IMO, o2hb_issue_node_write() just needs _one_ vec so there is no problem in it.
Calculating bio number might be get related code more complicated which I think is not necessary.
It doesn't solve any existed problem or improve performance or clean code. :(
Thanks,
Changwei
Quoted 160 lines · piaojun
Hi Changwei,
I got your point, we should let the caller retry if bio is not enough, right? But some caller like
o2hb_issue_node_write()won't retry once error happens, though the bio will always be enough. I think if we could calculate the number of bio we need before callingbio_add_page()?Thanks
JunQuoted 149 lines · Changwei Ge
Hi Jun,
Right now, I am afraid that the easiest and fasted way to fix this issue is to revert your patch.
From comments before function
bio_add_page(), we can see that it only
fails if either ::bi_vcnt == ::bi_max_vecs or it's a cloned bio.So we can judge if bio is full from its return value is zero or not.
Thanks,
Changwei
Quoted 131 lines · Changwei Ge
Quoted 94 lines · piaojun
Quoted 93 lines · Changwei Ge
Hi Jun,
Quoted 89 lines · piaojun
Hi Changwei,
Quoted 81 lines · Changwei Ge
If cluster scale exceeds 16 nodes, bio will be full and
bio_add_page()
returns 0 when adding pages to bio. Returning-EIOtoo2hb_read_slots()
fromo2hb_setup_one_bio()will lead to losing chance to allocate more bios to present all heartbeat region.So
o2hb_read_slots()fails.In my test, making fs fails in starting o2cb service.
Attach error log:
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 0,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 1,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 2,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 3,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 4,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 5,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 6,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 7,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 8,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 9,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 10,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 11,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 12,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 13,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 14,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 15,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 16,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:471 ERROR: Adding page[16] to bio failed, pageffffea0002d7ed40, len 0,vec_len4096,vec_start0,bi_sector8192 (mkfs.ocfs2,27479,2):o2hb_read_slots:500 ERROR: status = -5 (mkfs.ocfs2,27479,2):o2hb_populate_slot_data:1911 ERROR: status = -5 (mkfs.ocfs2,27479,2):o2hb_region_dev_write:2012 ERROR: status = -5Fixes ba16ddfbeb9d ("ocfs2/o2hb: check len for bio_add_page() toavoid getting incorrect bio"
Signed-off-by Changwei Ge <[email protected]>--- fs/ocfs2/cluster/heartbeat.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)fs/ocfs2/cluster/heartbeat.c +1
-1@@ -540,11 +540,12 @@ static struct bio *o2hb_setup_one_bio(structo2hb_region *reg, struct bio *bio;struct page *page; +#define O2HB_BIO_VECS 16/* Testing has shown this allocation to take long enough under * GFP_KERNEL that the local node can get fenced. It would be * nicest if we could pre-allocate these bios and avoid this * all together. */ - bio =bio_alloc (GFP_ATOMIC ,16 ); + bio =bio_alloc (GFP_ATOMIC ,O2HB_BIO_VECS );if (!bio) {mlog (ML_ERROR ,"Could not alloc slots BIO!\n" ); bio =ERR_PTR (-ENOMEM );fs/ocfs2/cluster/heartbeat.c static struct bio *o2hb_setup_one_bio(struct · line 572
Quoted 2 lines
fs/ocfs2/cluster/heartbeat.c +0
-0@@ -570,7 +571,10 @@ static struct bio *o2hb_setup_one_bio(structo2hb_region *reg, current_page, vec_len, vec_start);Should we check the validity of '
current_page' beforebio_add_page()? And
that will prevent error happen. Others looks OK.Quoted 6 lines
Quoted 4 lines
If I understand correctly, you mean we should check current page is NULL or not?
If so I think there is no need since o2hb should guarantee that it has already reserved enough pages for disk heartbeat read/write behalf.I mean we could check if '
current_page' equalsO2HB_BIO_VECSbeforebio_add_page()to avoid NULL pointer referrence.Yes, that might work.
I find another problem within this patch.
I will post v2 patch later to fix them all with consideration about your suggestion.Thanks,
ChangweiQuoted 21 lines
thanks,
JunQuoted 18 lines
Thanks,
ChangweiQuoted 16 lines
thanks,
Junfs/ocfs2/cluster/heartbeat.c static struct bio *o2hb_setup_one_bio(struct · line 580
Quoted 10 lines
fs/ocfs2/cluster/heartbeat.c +4
-1len =bio_add_page (bio, page, vec_len, vec_start); -if (len != vec_len) { +if (len ==0 && current_page ==O2HB_BIO_VECS ) { +/* bio is full now. */ + goto bail; + }else if (len != vec_len) { mlog(ML_ERROR, "Adding page[%d] to bio failed, " "page %p, len %d, vec_len %u, vec_start %u, " "bi_sector %llu\n", current_page, page, len,_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
PATCH 1/1 ocfs2: submit another bio if current bio is full
Quoted 10 lines · Changwei Ge
Hi Jun,
IMO,
o2hb_issue_node_write()just needs _one_ vec so there is no problem in it.Calculating bio number might be get related code more complicated which I think is not necessary.
It doesn't solve any existed problem or improve performance or clean code. :(
OK, it sounds reasonable, thanks for your correction.
Thanks,
Jun
Quoted 166 lines
Thanks,
ChangweiQuoted 160 lines · piaojun
Hi Changwei,
I got your point, we should let the caller retry if bio is not enough, right? But some caller like
o2hb_issue_node_write()won't retry once error happens, though the bio will always be enough. I think if we could calculate the number of bio we need before callingbio_add_page()?Thanks
JunQuoted 149 lines · Changwei Ge
Hi Jun,
Right now, I am afraid that the easiest and fasted way to fix this issue is to revert your patch.
From comments before function
bio_add_page(), we can see that it only
fails if either ::bi_vcnt == ::bi_max_vecs or it's a cloned bio.So we can judge if bio is full from its return value is zero or not.
Thanks,
Changwei
Quoted 131 lines · Changwei Ge
Quoted 94 lines · piaojun
Quoted 93 lines · Changwei Ge
Hi Jun,
Quoted 89 lines · piaojun
Hi Changwei,
Quoted 81 lines · Changwei Ge
If cluster scale exceeds 16 nodes, bio will be full and
bio_add_page()
returns 0 when adding pages to bio. Returning-EIOtoo2hb_read_slots()
fromo2hb_setup_one_bio()will lead to losing chance to allocate more bios to present all heartbeat region.So
o2hb_read_slots()fails.In my test, making fs fails in starting o2cb service.
Attach error log:
(mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 0,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 1,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 2,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 3,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 4,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 5,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 6,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 7,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 8,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 9,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 10,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 11,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 12,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 13,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 14,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 15,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:463 page 16,vec_len= 4096,vec_start= 0 (mkfs.ocfs2,27479,2):o2hb_setup_one_bio:471 ERROR: Adding page[16] to bio failed, pageffffea0002d7ed40, len 0,vec_len4096,vec_start0,bi_sector8192 (mkfs.ocfs2,27479,2):o2hb_read_slots:500 ERROR: status = -5 (mkfs.ocfs2,27479,2):o2hb_populate_slot_data:1911 ERROR: status = -5 (mkfs.ocfs2,27479,2):o2hb_region_dev_write:2012 ERROR: status = -5Fixes ba16ddfbeb9d ("ocfs2/o2hb: check len for bio_add_page() toavoid getting incorrect bio"
Signed-off-by Changwei Ge <[email protected]>--- fs/ocfs2/cluster/heartbeat.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)fs/ocfs2/cluster/heartbeat.c +1
-1@@ -540,11 +540,12 @@ static struct bio *o2hb_setup_one_bio(structo2hb_region *reg, struct bio *bio;struct page *page; +#define O2HB_BIO_VECS 16/* Testing has shown this allocation to take long enough under * GFP_KERNEL that the local node can get fenced. It would be * nicest if we could pre-allocate these bios and avoid this * all together. */ - bio =bio_alloc (GFP_ATOMIC ,16 ); + bio =bio_alloc (GFP_ATOMIC ,O2HB_BIO_VECS );if (!bio) {mlog (ML_ERROR ,"Could not alloc slots BIO!\n" ); bio =ERR_PTR (-ENOMEM );fs/ocfs2/cluster/heartbeat.c static struct bio *o2hb_setup_one_bio(struct · line 572
Quoted 2 lines
fs/ocfs2/cluster/heartbeat.c +0
-0@@ -570,7 +571,10 @@ static struct bio *o2hb_setup_one_bio(structo2hb_region *reg, current_page, vec_len, vec_start);Should we check the validity of '
current_page' beforebio_add_page()? And
that will prevent error happen. Others looks OK.Quoted 6 lines
Quoted 4 lines
If I understand correctly, you mean we should check current page is NULL or not?
If so I think there is no need since o2hb should guarantee that it has already reserved enough pages for disk heartbeat read/write behalf.I mean we could check if '
current_page' equalsO2HB_BIO_VECSbeforebio_add_page()to avoid NULL pointer referrence.Yes, that might work.
I find another problem within this patch.
I will post v2 patch later to fix them all with consideration about your suggestion.Thanks,
ChangweiQuoted 21 lines
thanks,
JunQuoted 18 lines
Thanks,
ChangweiQuoted 16 lines
thanks,
Junfs/ocfs2/cluster/heartbeat.c static struct bio *o2hb_setup_one_bio(struct · line 580
Quoted 10 lines
fs/ocfs2/cluster/heartbeat.c +4
-1len =bio_add_page (bio, page, vec_len, vec_start); -if (len != vec_len) { +if (len ==0 && current_page ==O2HB_BIO_VECS ) { +/* bio is full now. */ + goto bail; + }else if (len != vec_len) { mlog(ML_ERROR, "Adding page[%d] to bio failed, " "page %p, len %d, vec_len %u, vec_start %u, " "bi_sector %llu\n", current_page, page, len,_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel