On 12/6/24 05:55, Alexander Atanasov wrote:
md_page is always present in memory. In that case
md_page->page could be always be mapped and we would not need to perform
kmap_atomic/kunmap_atomic during each lookup

https://virtuozzo.atlassian.net/browse/VSTOR-91659
Suggested-by: Denis V. Lunev <d...@openvz.org>
Signed-off-by: Alexander Atanasov <alexander.atana...@virtuozzo.com>
---
  drivers/md/dm-ploop-bat.c | 21 ++++++++-------------
  drivers/md/dm-ploop-cmd.c | 33 ++++++++++++---------------------
  drivers/md/dm-ploop-map.c | 35 +++++++++++++----------------------
  drivers/md/dm-ploop.h     | 14 +++++++-------
  4 files changed, 40 insertions(+), 63 deletions(-)

...
@@ -265,9 +263,9 @@ static int ploop_write_zero_cluster_sync(struct ploop 
*ploop,
        int i;
for (i = 0; i < pio->bi_vcnt; i++) {
-               data = kmap_atomic(pio->bi_io_vec[i].bv_page);
+               data = kmap(pio->bi_io_vec[i].bv_page);

This hunk seems unrelated to original md->kmpage idea and just does kmap_atomic -> kmap replacement. Should it go to separate patch? Should we prove in commit message that it is safe to call kmap here (non-atomic context)?

                memset(data, 0, PAGE_SIZE);
-               kunmap_atomic(data);
+               kunmap(data);
        }
return ploop_write_cluster_sync(ploop, pio, clu);
...
@@ -909,9 +908,10 @@ static int ploop_prepare_bat_update(struct ploop *ploop, 
struct md_page *md,
        piwb->pio = pio = ploop_alloc_pio(ploop, GFP_NOIO);
        if (!page || !pio)
                goto err;
+       piwb->kmpage = kmap(piwb->bat_page);

Same question why it is safe to switch kmap_atomic to kmap?

        ploop_init_pio(ploop, REQ_OP_WRITE, pio);
- bat_entries = kmap_atomic(md->page);
+       bat_entries = md->kmpage;
write_lock_irq(&ploop->bat_rwlock);
        md->piwb = piwb;
...

@@ -759,14 +754,12 @@ static void notify_delta_merged(struct ploop *ploop, u8 
level,
                       * 1)next delta (which became renumbered) or
                       * 2)prev delta (if !@forward).
                       */
-                     bat_entries[i] = d_bat_entries[i];
+                     WRITE_ONCE(bat_entries[i], READ_ONCE(d_bat_entries[i]));
                      if (!forward)
                              md->bat_levels[i] = level - 1;
                      else
                              md->bat_levels[i] = level;
              }
-             kunmap_atomic(bat_entries);
-             kunmap_atomic(d_bat_entries);
              if (stop)
                      break;
              d_md = ploop_md_next_entry(d_md);
...
@@ -463,11 +464,10 @@ static inline bool 
ploop_md_page_cluster_is_in_top_delta(struct ploop *ploop,
                return false;
        }
- bat_entries = kmap_atomic(md->page);
-       if (bat_entries[clu] == BAT_ENTRY_NONE ||
-           md->bat_levels[clu] < ploop_top_level(ploop))
+       bat_entries = md->kmpage;
+       if (READ_ONCE(bat_entries[clu]) == BAT_ENTRY_NONE ||
+           READ_ONCE(md->bat_levels[clu]) < ploop_top_level(ploop))

Should we explain in commit message why we need to use READ_ONCE/WRITE_ONCE above?

                ret = false;
-       kunmap_atomic(bat_entries);
        return ret;
  }

--
Best regards, Tikhomirov Pavel
Senior Software Developer, Virtuozzo.

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

Reply via email to