On 21.01.25 9:15, Pavel Tikhomirov wrote:


On 1/21/25 01:34, Alexander Atanasov wrote:
On 20.01.25 12:08, Pavel Tikhomirov wrote:


On 12/6/24 05:56, Alexander Atanasov wrote:
+static inline int ploop_runners_add_work(struct ploop *ploop, struct pio *pio)
+{
+    int i;
+    struct ploop_worker *wrkr;
+
+    if (++ploop->last_used_runner >= ploop->nkt_runners)
+        ploop->last_used_runner = 0;
+    wrkr = ploop->kt_runners[ploop->last_used_runner];

This can potentially lead to out of bound read, as we don't have locks around wrapping last_used_runner increment to zero (i.e. it's not atomic), here we can see ploop->last_used_runner >= nkt_runners.

Reworked - runners are circular list - READ_ONCE/WRITE_ONCE used to switch to ->next /see updated patches/

In updated patches:

+static inline int ploop_runners_add_work(struct ploop *ploop, struct pio *pio)
+{
+       struct ploop_worker *wrkr;
+
+       wrkr = READ_ONCE(ploop->last_used_runner)->next; // A
+       WRITE_ONCE(ploop->last_used_runner, wrkr); // B

No out of bound now. But note that as we have two operations thus it's not atomic, and thus this list iteration can go "backwards" sometimes:

A1 -> A2 -> B2 -> A3 -> B3 -> ... AN -> BN -> B1


Yes, we may send two requests to same runner or skip but this does not matter - we do not require strict scheduling here.






Note: I see (in final version, with all patches applied) multiple stacks which probably can run concurrently:

   +-< ploop_runners_add_work
     +-< ploop_index_wb_submit
     | +-< ploop_grow_relocate_cluster
     | | +-< ploop_process_resize_cmd
     | | | +-< ploop_resize
     | | | | +-< ploop_message
     | +-< ploop_grow_update_header
     | | +-< ploop_process_resize_cmd
     | | | +-< ploop_resize
     | | | | +-< ploop_message
     | +-< ploop_submit_metadata_writeback
     | | +-< do_ploop_run_work
     | | | +-< do_ploop_work
     | | | +-< ploop_worker
     +-< process_ploop_fsync_work
     | +-< do_ploop_run_work
     | | +-< do_ploop_work
     | | +-< ploop_worker
     +-< ploop_runners_add_work_list
     | +-< do_ploop_run_work
     | | +-< do_ploop_work
     | | +-< ploop_worker

+
+    atomic_inc(&ploop->kt_worker->inflight_pios);
+    llist_add((struct llist_node *)(&pio->list), &wrkr->work_llist);
+    wake_up_process(wrkr->task);
+
+    return 0;
+}




--
Regards,
Alexander Atanasov

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to