On 12/6/24 05:55, Alexander Atanasov wrote:
From: Andrey Zhadchenko <andrey.zhadche...@virtuozzo.com>

Drop extra ploop_cluster_is_in_top_delta() as we are planning to
access BAT anyway

https://virtuozzo.atlassian.net/browse/VSTOR-91817
Signed-off-by: Andrey Zhadchenko <andrey.zhadche...@virtuozzo.com>
---
  drivers/md/dm-ploop-map.c | 28 ++++++++++++----------------
  1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/md/dm-ploop-map.c b/drivers/md/dm-ploop-map.c
index ad7ca7d43dfc..b00dd364072d 100644
--- a/drivers/md/dm-ploop-map.c
+++ b/drivers/md/dm-ploop-map.c
@@ -711,12 +711,15 @@ static void ploop_complete_cow(struct ploop_cow *cow, 
blk_status_t bi_status)
        kmem_cache_free(cow_cache, cow);
  }
-static void ploop_release_cluster(struct ploop *ploop, u32 clu)
+static void ploop_piwb_discard_completed(struct ploop *ploop,
+                                        bool success, u32 clu, u32 new_dst_clu)
  {
        u32 id, *bat_entries, dst_clu;
        struct md_page *md;
+       u8 level;
- lockdep_assert_held(&ploop->bat_rwlock);
+       if (new_dst_clu)
+               return;
id = ploop_bat_clu_to_page_nr(clu);
        md = ploop_md_page_find(ploop, id);

Is this md the same to md in caller function ploop_advance_local_after_bat_wb?

@@ -726,22 +729,15 @@ static void ploop_release_cluster(struct ploop *ploop, 
u32 clu)
bat_entries = md->kmpage;
        dst_clu = READ_ONCE(bat_entries[clu]);
-       WRITE_ONCE(bat_entries[clu], BAT_ENTRY_NONE);
-       WRITE_ONCE(md->bat_levels[clu], 0);
-
-       ploop_hole_set_bit(dst_clu, ploop);
-}
-
-static void ploop_piwb_discard_completed(struct ploop *ploop,
-                                        bool success, u32 clu, u32 new_dst_clu)
-{
-       if (new_dst_clu)
-               return;
+       level = md->bat_levels[clu];

If for previous comment the answer is no, should not we take md->md_lock here to make the use of md->bat_levels and md->kmpage atomic / consistent? In the next patch we introduce md->md_lock to "use it when accessing md->levels and md->page at the sime time to protect readers against writers".

If the answer is yes, should not we do a lockdep check for md->md_lock?

- if (ploop_cluster_is_in_top_delta(ploop, clu)) {
+       if (!(dst_clu == BAT_ENTRY_NONE || level < ploop_top_level(ploop))) {
                WARN_ON_ONCE(ploop->nr_deltas != 1);
-               if (success)
-                       ploop_release_cluster(ploop, clu);
+               if (success) {
+                       WRITE_ONCE(bat_entries[clu], BAT_ENTRY_NONE);
+                       WRITE_ONCE(md->bat_levels[clu], 0);
+                       ploop_hole_set_bit(dst_clu, ploop);
+               }
        }
  }

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