Re: [PATCH RFC -next v2 00/41] md/md-bitmap: introduce bitmap_operations and make structure internel

[Su Yue]

On Wed 14 Aug 2024 at 15:10, Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> 
wrote:

> From: Yu Kuai <yukuai3@xxxxxxxxxx>
>
> Changes in v2:
>  - add patch 1-8 to prevent dereference bitmap directly, and the 
>  last
>  patch to make bitmap structure internel.
>  - use plain function alls "bitmap_ops->xxx()" directly;
>
> The background is that currently bitmap is using a global 
> spin_lock,
> cauing lock contention and huge IO performance degration for all 
> raid
> levels.
>
> However, it's impossible to implement a new lock free bitmap 
> with
> current situation that md-bitmap exposes the internal 
> implementation
> with lots of exported apis. Hence bitmap_operations is invented, 
> to
> describe bitmap core implementation, and a new bitmap can be 
> introduced
> with a new bitmap_operations, we only need to switch to the new 
> one
> during initialization.
Is the new bitmap in plan?
> And with this we can build bitmap as kernel module, but that's 
> not
> our concern for now.
>
> Noted I just compile this patchset, not tested yet.

I've looked through the patchset almostly. The changes are quite
straightforward. IMO, it's good timing to  test it and drop RFC in 
next
version.

> Yu Kuai (41):
>   md/raid1: use md_bitmap_wait_behind_writes() in 
>   raid1_read_request()
>   md/md-bitmap: replace md_bitmap_status() with a new helper
>     md_bitmap_get_stats()
>   md: use new helper md_bitmap_get_stats() in 
>   update_array_info()
>   md/md-bitmap: add 'events_cleared' into struct md_bitmap_stats
>   md/md-bitmap: add 'sync_size' into struct md_bitmap_stats
>   md/md-bitmap: add 'file_pages' into struct md_bitmap_stats
>   md/md-bitmap: add 'behind_writes' and 'behind_wait' into 
>   struct
>     md_bitmap_stats
>   md/md-cluster: use helper md_bitmap_get_stats() to get pages 
>   in
>     resize_bitmaps()
>   md/md-bitmap: add a new helper md_bitmap_set_pages()
>   md/md-bitmap: introduce struct bitmap_operations
>   md/md-bitmap: simplify md_bitmap_create() + md_bitmap_load()
>   md/md-bitmap: merge md_bitmap_create() into bitmap_operations
>   md/md-bitmap: merge md_bitmap_load() into bitmap_operations
>   md/md-bitmap: merge md_bitmap_destroy() into bitmap_operations
>   md/md-bitmap: merge md_bitmap_flush() into bitmap_operations
>   md/md-bitmap: make md_bitmap_print_sb() internal
>   md/md-bitmap: merge md_bitmap_update_sb() into 
>   bitmap_operations
>   md/md-bitmap: merge md_bitmap_status() into bitmap_operations
>   md/md-bitmap: remove md_bitmap_setallbits()
>   md/md-bitmap: merge bitmap_write_all() into bitmap_operations
>   md/md-bitmap: merge md_bitmap_dirty_bits() into 
>   bitmap_operations
>   md/md-bitmap: merge md_bitmap_startwrite() into 
>   bitmap_operations
>   md/md-bitmap: merge md_bitmap_endwrite() into 
>   bitmap_operations
>   md/md-bitmap: merge md_bitmap_start_sync() into 
>   bitmap_operations
>   md/md-bitmap: remove the parameter 'aborted' for 
>   md_bitmap_end_sync()
>   md/md-bitmap: merge md_bitmap_end_sync() into 
>   bitmap_operations
>   md/md-bitmap: merge md_bitmap_close_sync() into 
>   bitmap_operations
>   md/md-bitmap: mrege md_bitmap_cond_end_sync() into 
>   bitmap_operations
>   md/md-bitmap: merge md_bitmap_sync_with_cluster() into
>     bitmap_operations
>   md/md-bitmap: merge md_bitmap_unplug_async() into 
>   md_bitmap_unplug()
>   md/md-bitmap: merge bitmap_unplug() into bitmap_operations
>   md/md-bitmap: merge md_bitmap_daemon_work() into 
>   bitmap_operations
>   md/md-bitmap: pass in mddev directly for md_bitmap_resize()
>   md/md-bitmap: merge md_bitmap_resize() into bitmap_operations
>   md/md-bitmap: merge get_bitmap_from_slot() into 
>   bitmap_operations
>   md/md-bitmap: merge md_bitmap_copy_from_slot() into struct
>     bitmap_operation.
>   md/md-bitmap: merge md_bitmap_set_pages() into struct
>     bitmap_operations
>   md/md-bitmap: merge md_bitmap_free() into bitmap_operations
>   md/md-bitmap: merge md_bitmap_wait_behind_writes() into
>     bitmap_operations
>   md/md-bitmap: merge md_bitmap_enabled() into bitmap_operations
>   md/md-bitmap: make in memory structure internal
>
>  drivers/md/dm-raid.c     |   7 +-
>  drivers/md/md-bitmap.c   | 561 
>  +++++++++++++++++++++++++++++----------
>  drivers/md/md-bitmap.h   | 272 ++++---------------
>  drivers/md/md-cluster.c  |  79 +++---
>  drivers/md/md.c          | 133 ++++++----
>  drivers/md/md.h          |   3 +-
>  drivers/md/raid1-10.c    |   9 +-
>  drivers/md/raid1.c       |  78 +++---
>  drivers/md/raid10.c      |  73 ++---
>  drivers/md/raid5-cache.c |   8 +-
>  drivers/md/raid5.c       |  62 ++---
>  11 files changed, 731 insertions(+), 554 deletions(-)