Re: [Devel] [PATCH rh7] ploop: io_direct: delay f_op->fsync() until index_update for reloc requests
Maxim Patlasov writes: > Dima, > > > I have not heard from you since 07/06/2016. Do you agree with that > reasoning I provided in last email? What's your objection against the > patch now? Max, this patch looks ugly because it mix many things in one place. In order to do this in right way let's introduce fsync-pended eng-state where we can queue our requests and let fsync_thread will handle this. Thre are three places where we need such functionality. ENTRY: for req with FUA and IO_FSYNC_PENDED PLOOP_E_DATA_WBI: for reqs with FUA PLOOP_E_NULLIFY: PLOOP_E_COMPLETE: for reqs with FUA Let's do it one once and it will works fine for all cases. > > > Thanks, > > Maxim > > > On 07/06/2016 11:10 AM, Maxim Patlasov wrote: >> Dima, >> >> On 07/06/2016 04:58 AM, Dmitry Monakhov wrote: >> >>> Maxim Patlasov writes: >>> Commit 9f860e606 introduced an engine to delay fsync: doing fallocate(FALLOC_FL_CONVERT_UNWRITTEN) dio_post_submit marks io as PLOOP_IO_FSYNC_DELAYED to ensure that fsync happens later, when incoming FLUSH|FUA comes. That was deemed as important because (PSBM-47026): > This optimization becomes more important due to the fact that > customers tend to use pcompact heavily => ploop images grow each day. Now, we can easily re-use the engine to delay fsync for reloc requests as well. As explained in the description of commit 5aa3fe09: > 1->read_data_from_old_post > 2->write_to_new_pos >->sumbit_alloc > ->submit_pad > ->post_submit->convert_unwritten > 3->update_index ->write_page with FLUSH|FUA > 4->nullify_old_pos > 5->issue_flush by the time of step 3 extent coversion is not yet stable because belongs to uncommitted transaction. But instead of doing fsync inside ->post_submit, we can fsync later, as the very first step of write_page for index_update. >>> NAK from me. What is advantage of this patch? >> >> The advantage is the following: in case of BAT multi-updates, instead >> of doing many fsync-s (one per dio_post_submit), we'll do only one >> (when final ->write_page is called). >> >>> Does it makes code more optimal? No >> >> Yes, it does. In the same sense as 9f860e606: saving some fsync-s. >> >>> Does it makes main ploop more asynchronous? No. >> >> Correct, the patch optimizes ploop in the other way. It's not about >> making ploop more asynchronous. >> >> >>> >>> If you want to make optimization then it is reasonable to >>> queue preq with PLOOP_IO_FSYNC_DELAYED to top_io->fsync_queue >>> before processing PLOOP_E_DATA_WBI state for preq with FUA >>> So sequence will looks like follows: >>> ->sumbit_alloc >>>->submit_pad >>>->post_submit->convert_unwritten-> tag PLOOP_IO_FSYNC_DELAYED >>> ->ploop_req_state_process >>>case PLOOP_E_DATA_WBI: >>>if (preq->start & PLOOP_IO_FSYNC_DELAYED_FL) { >>>preq->start &= ~PLOOP_IO_FSYNC_DELAYED_FL >>>list_add_tail(&preq->list, &top_io->fsync_queue) >>>return; >>> } >>> ##Let fsync_thread do it's work >>> ->ploop_req_state_process >>> case LOOP_E_DATA_WBI: >>> update_index->write_page with FUA (FLUSH is not required because >>> we already done fsync) >> >> That's another type of optimization: making ploop more asynchronous. I >> thought about it, but didn't come to conclusion whether it's worthy >> w.r.t. adding more complexity to ploop-state-machine and possible bugs >> introduced with that. >> >> Thanks, >> Maxim >> >>> https://jira.sw.ru/browse/PSBM-47026 Signed-off-by: Maxim Patlasov --- drivers/block/ploop/dev.c |4 ++-- drivers/block/ploop/io_direct.c | 25 - drivers/block/ploop/io_kaio.c |3 ++- drivers/block/ploop/map.c | 17 - include/linux/ploop/ploop.h |3 ++- 5 files changed, 42 insertions(+), 10 deletions(-) diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c index e5f010b..40768b6 100644 --- a/drivers/block/ploop/dev.c +++ b/drivers/block/ploop/dev.c @@ -4097,7 +4097,7 @@ static void ploop_relocate(struct ploop_device * plo) preq->bl.tail = preq->bl.head = NULL; preq->req_cluster = 0; preq->req_size = 0; -preq->req_rw = WRITE_SYNC|REQ_FUA; +preq->req_rw = WRITE_SYNC; preq->eng_state = PLOOP_E_ENTRY; preq->state = (1 << PLOOP_REQ_SYNC) | (1 << PLOOP_REQ_RELOC_A); preq->error = 0; @@ -4401,7 +4401,7 @@ static void ploop_relocblks_process(struct ploop_device *plo) preq->bl.tail = preq->bl.head = NULL; preq->req_cluster = ~0U; /* uninitialized */ preq->req_size = 0; -preq->req_rw = WRITE_SYNC|REQ_FUA; +preq->req_rw = WRITE_SYNC; preq->eng_state = PLOOP_E_ENTRY;
[Devel] [PATCH] ext4: fix broken fsync for dirs/symlink
bad commit: 6a63db16da84fe xfstests: generic/321 generic/335 generic/348 Signed-off-by: Dmitry Monakhov --- fs/ext4/super.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/ext4/super.c b/fs/ext4/super.c index c0e7acd..7e44850 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -4919,8 +4919,8 @@ int ext4_force_commit(struct super_block *sb) smp_rmb(); if (EXT4_SB(sb)->s_mount_flags & EXT4_MF_FS_ABORTED) return -EROFS; - } return 0; + } journal = EXT4_SB(sb)->s_journal; return ext4_journal_force_commit(journal); -- 1.8.3.1 ___ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel
[Devel] [PATCH rh7 1/3] ploop: factor ->write_page() out
Simple re-work. No logic changed. Will be useful for the next patch. Signed-off-by: Maxim Patlasov --- drivers/block/ploop/map.c | 39 +-- include/linux/ploop/ploop.h |2 ++ 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/drivers/block/ploop/map.c b/drivers/block/ploop/map.c index 1883674..5f7fd66 100644 --- a/drivers/block/ploop/map.c +++ b/drivers/block/ploop/map.c @@ -896,6 +896,25 @@ static void copy_index_for_wb(struct page * page, struct map_node * m, int level } } + +void ploop_index_wb_proceed(struct ploop_request * preq) +{ + struct map_node * m = preq->map; + struct ploop_delta * top_delta = map_top_delta(m->parent); + struct page * page = preq->sinfo.wi.tpage; + unsigned long rw = preq->req_index_update_rw; + sector_t sec; + + preq->eng_state = PLOOP_E_INDEX_WB; + + top_delta->ops->map_index(top_delta, m->mn_start, &sec); + + __TRACE("wbi-proceed %p %u %p\n", preq, preq->req_cluster, m); + top_delta->io.ops->write_page(&top_delta->io, preq, page, sec, rw); + + put_page(page); +} + /* Data write is commited. Now we need to update index. */ void ploop_index_update(struct ploop_request * preq) @@ -907,8 +926,6 @@ void ploop_index_update(struct ploop_request * preq) map_index_t blk; int old_level; struct page * page; - sector_t sec; - unsigned long rw; unsigned long state = READ_ONCE(preq->state); /* No way back, we are going to initiate index write. */ @@ -955,15 +972,13 @@ void ploop_index_update(struct ploop_request * preq) ((map_index_t*)page_address(page))[idx] = preq->iblock << ploop_map_log(plo); - preq->eng_state = PLOOP_E_INDEX_WB; get_page(page); preq->sinfo.wi.tpage = page; __TRACE("wbi %p %u %p\n", preq, preq->req_cluster, m); plo->st.map_single_writes++; - top_delta->ops->map_index(top_delta, m->mn_start, &sec); - rw = (preq->req_rw & (REQ_FUA | REQ_FLUSH)); + preq->req_index_update_rw = (preq->req_rw & (REQ_FUA | REQ_FLUSH)); /* We've just set REQ_FLUSH in rw, ->write_page() below will do the FLUSH */ @@ -971,11 +986,9 @@ void ploop_index_update(struct ploop_request * preq) /* Relocate requires consistent index update */ if (state & (PLOOP_REQ_RELOC_A_FL|PLOOP_REQ_RELOC_S_FL)) - rw |= (REQ_FLUSH | REQ_FUA); + preq->req_index_update_rw |= (REQ_FLUSH | REQ_FUA); - top_delta->io.ops->write_page(&top_delta->io, preq, page, sec, rw); - - put_page(page); + ploop_index_wb_proceed(preq); return; enomem: @@ -991,6 +1004,7 @@ out: } EXPORT_SYMBOL(ploop_index_update); + int map_index(struct ploop_delta * delta, struct ploop_request * preq, unsigned long *sec) { return delta->ops->map_index(delta, preq->map->mn_start, sec); @@ -1094,7 +1108,6 @@ static void map_wb_complete(struct map_node * m, int err) struct page * page = NULL; int delayed = 0; unsigned int idx; - sector_t sec; unsigned long rw; /* First, complete processing of written back indices, @@ -1219,11 +1232,9 @@ static void map_wb_complete(struct map_node * m, int err) __TRACE("wbi2 %p %u %p\n", main_preq, main_preq->req_cluster, m); plo->st.map_multi_writes++; - top_delta->ops->map_index(top_delta, m->mn_start, &sec); - top_delta->io.ops->write_page(&top_delta->io, main_preq, page, sec, - rw); - put_page(page); + main_preq->req_index_update_rw = rw; + ploop_index_wb_proceed(main_preq); } void diff --git a/include/linux/ploop/ploop.h b/include/linux/ploop/ploop.h index 7864edf..3d52f28 100644 --- a/include/linux/ploop/ploop.h +++ b/include/linux/ploop/ploop.h @@ -534,6 +534,7 @@ struct ploop_request sector_treq_sector; unsigned intreq_size; unsigned intreq_rw; + unsigned intreq_index_update_rw; unsigned long tstamp; struct io_context *ioc; @@ -803,6 +804,7 @@ void map_init(struct ploop_device *, struct ploop_map * map); void ploop_map_start(struct ploop_map * map, u64 bd_size); void ploop_map_destroy(struct ploop_map * map); void ploop_map_remove_delta(struct ploop_map * map, int level); +void ploop_index_wb_proceed(struct ploop_request * preq); void ploop_index_update(struct ploop_request * preq); void ploop_index_wb_complete(struct ploop_request * preq); int __init ploop_map_init(void); ___ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel
[Devel] [PATCH rh7 3/3] ploop: io_direct: delay f_op->fsync() until index_update for reloc requests (v2)
Commit 9f860e606 introduced an engine to delay fsync: doing fallocate(FALLOC_FL_CONVERT_UNWRITTEN) dio_post_submit marks io as PLOOP_IO_FSYNC_DELAYED to ensure that fsync happens later, when incoming FLUSH|FUA comes. That was deemed as important because (PSBM-47026): > This optimization becomes more important due to the fact that customers tend > to use pcompact heavily => ploop images grow each day. Now, we can easily re-use the engine to delay fsync for reloc requests as well. As explained in the description of commit 5aa3fe09: > 1->read_data_from_old_post > 2->write_to_new_pos > ->sumbit_alloc > ->submit_pad > ->post_submit->convert_unwritten > 3->update_index ->write_page with FLUSH|FUA > 4->nullify_old_pos >5->issue_flush by the time of step 3 extent coversion is not yet stable because belongs to uncommitted transaction. But instead of doing fsync inside ->post_submit, we can fsync later, as the very first step of write_page for index_update. Changed in v2: - process delayed fsync asynchronously, via PLOOP_E_FSYNC_PENDED eng_state https://jira.sw.ru/browse/PSBM-47026 Signed-off-by: Maxim Patlasov --- drivers/block/ploop/dev.c |9 +++-- drivers/block/ploop/map.c | 33 + include/linux/ploop/ploop.h |2 ++ 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c index df3eec9..ed60b1f 100644 --- a/drivers/block/ploop/dev.c +++ b/drivers/block/ploop/dev.c @@ -2720,6 +2720,11 @@ restart: ploop_index_wb_complete(preq); break; + case PLOOP_E_FSYNC_PENDED: + /* fsync done */ + ploop_index_wb_proceed(preq); + break; + default: BUG(); } @@ -4106,7 +4111,7 @@ static void ploop_relocate(struct ploop_device * plo) preq->bl.tail = preq->bl.head = NULL; preq->req_cluster = 0; preq->req_size = 0; - preq->req_rw = WRITE_SYNC|REQ_FUA; + preq->req_rw = WRITE_SYNC; preq->eng_state = PLOOP_E_ENTRY; preq->state = (1 << PLOOP_REQ_SYNC) | (1 << PLOOP_REQ_RELOC_A); preq->error = 0; @@ -4410,7 +4415,7 @@ static void ploop_relocblks_process(struct ploop_device *plo) preq->bl.tail = preq->bl.head = NULL; preq->req_cluster = ~0U; /* uninitialized */ preq->req_size = 0; - preq->req_rw = WRITE_SYNC|REQ_FUA; + preq->req_rw = WRITE_SYNC; preq->eng_state = PLOOP_E_ENTRY; preq->state = (1 << PLOOP_REQ_SYNC) | (1 << PLOOP_REQ_RELOC_S); preq->error = 0; diff --git a/drivers/block/ploop/map.c b/drivers/block/ploop/map.c index 5f7fd66..01e1064 100644 --- a/drivers/block/ploop/map.c +++ b/drivers/block/ploop/map.c @@ -915,6 +915,23 @@ void ploop_index_wb_proceed(struct ploop_request * preq) put_page(page); } +static void ploop_index_wb_proceed_or_delay(struct ploop_request * preq) +{ + if (test_and_clear_bit(PLOOP_REQ_FSYNC_IF_DELAYED, &preq->state)) { + struct map_node * m = preq->map; + struct ploop_delta * top_delta = map_top_delta(m->parent); + struct ploop_io * top_io = &top_delta->io; + + if (test_bit(PLOOP_IO_FSYNC_DELAYED, &top_io->io_state)) { + preq->eng_state = PLOOP_E_FSYNC_PENDED; + ploop_add_req_to_fsync_queue(preq); + return; + } + } + + ploop_index_wb_proceed(preq); +} + /* Data write is commited. Now we need to update index. */ void ploop_index_update(struct ploop_request * preq) @@ -985,10 +1002,12 @@ void ploop_index_update(struct ploop_request * preq) preq->req_rw &= ~REQ_FLUSH; /* Relocate requires consistent index update */ - if (state & (PLOOP_REQ_RELOC_A_FL|PLOOP_REQ_RELOC_S_FL)) + if (state & (PLOOP_REQ_RELOC_A_FL|PLOOP_REQ_RELOC_S_FL)) { preq->req_index_update_rw |= (REQ_FLUSH | REQ_FUA); + set_bit(PLOOP_REQ_FSYNC_IF_DELAYED, &preq->state); + } - ploop_index_wb_proceed(preq); + ploop_index_wb_proceed_or_delay(preq); return; enomem: @@ -1109,6 +1128,7 @@ static void map_wb_complete(struct map_node * m, int err) int delayed = 0; unsigned int idx; unsigned long rw; + int do_fsync_if_delayed = 0; /* First, complete processing of written back indices, * finally instantiate indices in mapping cache. @@ -1206,8 +1226,10 @@ static void map_wb_complete(struct map_node * m, int err) state = READ_ONCE(preq->state); /* Relocate requires consistent index update */ - if (state & (PLOOP_REQ_RELOC_A_FL|PLOOP_REQ_RELOC_S_FL)) + if (state & (PLOOP_REQ_RELOC_A_FL|PLOOP_REQ_RELOC_S_FL)
[Devel] [PATCH rh7 2/3] ploop: factor out add preq to fsync_queue
Simple re-work. No logic changed. Will be useful for the next patch. Signed-off-by: Maxim Patlasov --- drivers/block/ploop/dev.c | 19 ++- include/linux/ploop/ploop.h |1 + 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c index 3dc94ca..df3eec9 100644 --- a/drivers/block/ploop/dev.c +++ b/drivers/block/ploop/dev.c @@ -2028,6 +2028,19 @@ static inline bool preq_is_special(struct ploop_request * preq) PLOOP_REQ_ZERO_FL); } +void ploop_add_req_to_fsync_queue(struct ploop_request * preq) +{ + struct ploop_device * plo = preq->plo; + struct ploop_delta * top_delta = ploop_top_delta(plo); + struct ploop_io * top_io= &top_delta->io; + + spin_lock_irq(&plo->lock); + list_add_tail(&preq->list, &top_io->fsync_queue); + if (waitqueue_active(&top_io->fsync_waitq)) + wake_up_interruptible(&top_io->fsync_waitq); + spin_unlock_irq(&plo->lock); +} + static void ploop_entry_request(struct ploop_request * preq) { @@ -2053,11 +2066,7 @@ ploop_entry_request(struct ploop_request * preq) if ((preq->req_rw & REQ_FLUSH) && test_bit(PLOOP_IO_FSYNC_DELAYED, &top_io->io_state) && !test_bit(PLOOP_REQ_FSYNC_DONE, &preq->state)) { - spin_lock_irq(&plo->lock); - list_add_tail(&preq->list, &top_io->fsync_queue); - if (waitqueue_active(&top_io->fsync_waitq)) - wake_up_interruptible(&top_io->fsync_waitq); - spin_unlock_irq(&plo->lock); + ploop_add_req_to_fsync_queue(preq); return; } diff --git a/include/linux/ploop/ploop.h b/include/linux/ploop/ploop.h index 3d52f28..d8e01b6 100644 --- a/include/linux/ploop/ploop.h +++ b/include/linux/ploop/ploop.h @@ -809,6 +809,7 @@ void ploop_index_update(struct ploop_request * preq); void ploop_index_wb_complete(struct ploop_request * preq); int __init ploop_map_init(void); void ploop_map_exit(void); +void ploop_add_req_to_fsync_queue(struct ploop_request * preq); void ploop_quiesce(struct ploop_device * plo); ___ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel
[Devel] [PATCH rh7 3/3] ploop: io_direct: delay f_op->fsync() until index_update for reloc requests (v3)
Commit 9f860e606 introduced an engine to delay fsync: doing fallocate(FALLOC_FL_CONVERT_UNWRITTEN) dio_post_submit marks io as PLOOP_IO_FSYNC_DELAYED to ensure that fsync happens later, when incoming FLUSH|FUA comes. That was deemed as important because (PSBM-47026): > This optimization becomes more important due to the fact that customers tend > to use pcompact heavily => ploop images grow each day. Now, we can easily re-use the engine to delay fsync for reloc requests as well. As explained in the description of commit 5aa3fe09: > 1->read_data_from_old_post > 2->write_to_new_pos > ->sumbit_alloc > ->submit_pad > ->post_submit->convert_unwritten > 3->update_index ->write_page with FLUSH|FUA > 4->nullify_old_pos >5->issue_flush by the time of step 3 extent coversion is not yet stable because belongs to uncommitted transaction. But instead of doing fsync inside ->post_submit, we can fsync later, as the very first step of write_page for index_update. Changed in v2: - process delayed fsync asynchronously, via PLOOP_E_FSYNC_PENDED eng_state Changed in v3: - use extra arg for ploop_index_wb_proceed_or_delay() instead of ad-hoc PLOOP_REQ_FSYNC_IF_DELAYED https://jira.sw.ru/browse/PSBM-47026 Signed-off-by: Maxim Patlasov --- drivers/block/ploop/dev.c |9 +++-- drivers/block/ploop/map.c | 32 include/linux/ploop/ploop.h |1 + 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c index df3eec9..ed60b1f 100644 --- a/drivers/block/ploop/dev.c +++ b/drivers/block/ploop/dev.c @@ -2720,6 +2720,11 @@ restart: ploop_index_wb_complete(preq); break; + case PLOOP_E_FSYNC_PENDED: + /* fsync done */ + ploop_index_wb_proceed(preq); + break; + default: BUG(); } @@ -4106,7 +4111,7 @@ static void ploop_relocate(struct ploop_device * plo) preq->bl.tail = preq->bl.head = NULL; preq->req_cluster = 0; preq->req_size = 0; - preq->req_rw = WRITE_SYNC|REQ_FUA; + preq->req_rw = WRITE_SYNC; preq->eng_state = PLOOP_E_ENTRY; preq->state = (1 << PLOOP_REQ_SYNC) | (1 << PLOOP_REQ_RELOC_A); preq->error = 0; @@ -4410,7 +4415,7 @@ static void ploop_relocblks_process(struct ploop_device *plo) preq->bl.tail = preq->bl.head = NULL; preq->req_cluster = ~0U; /* uninitialized */ preq->req_size = 0; - preq->req_rw = WRITE_SYNC|REQ_FUA; + preq->req_rw = WRITE_SYNC; preq->eng_state = PLOOP_E_ENTRY; preq->state = (1 << PLOOP_REQ_SYNC) | (1 << PLOOP_REQ_RELOC_S); preq->error = 0; diff --git a/drivers/block/ploop/map.c b/drivers/block/ploop/map.c index 5f7fd66..715dc15 100644 --- a/drivers/block/ploop/map.c +++ b/drivers/block/ploop/map.c @@ -915,6 +915,24 @@ void ploop_index_wb_proceed(struct ploop_request * preq) put_page(page); } +static void ploop_index_wb_proceed_or_delay(struct ploop_request * preq, + int do_fsync_if_delayed) +{ + if (do_fsync_if_delayed) { + struct map_node * m = preq->map; + struct ploop_delta * top_delta = map_top_delta(m->parent); + struct ploop_io * top_io = &top_delta->io; + + if (test_bit(PLOOP_IO_FSYNC_DELAYED, &top_io->io_state)) { + preq->eng_state = PLOOP_E_FSYNC_PENDED; + ploop_add_req_to_fsync_queue(preq); + return; + } + } + + ploop_index_wb_proceed(preq); +} + /* Data write is commited. Now we need to update index. */ void ploop_index_update(struct ploop_request * preq) @@ -927,6 +945,7 @@ void ploop_index_update(struct ploop_request * preq) int old_level; struct page * page; unsigned long state = READ_ONCE(preq->state); + int do_fsync_if_delayed = 0; /* No way back, we are going to initiate index write. */ @@ -985,10 +1004,12 @@ void ploop_index_update(struct ploop_request * preq) preq->req_rw &= ~REQ_FLUSH; /* Relocate requires consistent index update */ - if (state & (PLOOP_REQ_RELOC_A_FL|PLOOP_REQ_RELOC_S_FL)) + if (state & (PLOOP_REQ_RELOC_A_FL|PLOOP_REQ_RELOC_S_FL)) { preq->req_index_update_rw |= (REQ_FLUSH | REQ_FUA); + do_fsync_if_delayed = 1; + } - ploop_index_wb_proceed(preq); + ploop_index_wb_proceed_or_delay(preq, do_fsync_if_delayed); return; enomem: @@ -1109,6 +1130,7 @@ static void map_wb_complete(struct map_node * m, int err) int delayed = 0; unsigned int idx; unsigned long rw; + int do_fsync_if_delayed = 0; /* First, complete processing of written back indices,