| Message ID | 20240307181105.4081793-3-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | docs: improve the memory API documentation | expand |
On Thu, Mar 07, 2024 at 06:11:02PM +0000, Alex Bennée wrote: > The RAMBlock concept is fairly central to RAM-like MemoryRegions so > lets update the structure documentation and include in the docs. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > docs/devel/memory.rst | 1 + > include/exec/ramblock.h | 76 +++++++++++++++++++++++++++-------------- > 2 files changed, 52 insertions(+), 25 deletions(-) > > diff --git a/docs/devel/memory.rst b/docs/devel/memory.rst > index 69c5e3f914a..ed24708fce3 100644 > --- a/docs/devel/memory.rst > +++ b/docs/devel/memory.rst > @@ -369,4 +369,5 @@ callbacks are called: > API Reference > ------------- > > +.. kernel-doc:: include/exec/ramblock.h > .. kernel-doc:: include/exec/memory.h > diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h > index 848915ea5bf..eb2416b6f66 100644 > --- a/include/exec/ramblock.h > +++ b/include/exec/ramblock.h > @@ -24,68 +24,94 @@ > #include "qemu/rcu.h" > #include "exec/ramlist.h" > > +/** > + * struct RAMBlock - represents a chunk of RAM > + * > + * RAMBlocks can be backed by allocated RAM or a file-descriptor. See > + * @flags for the details. For the purposes of migration various book > + * keeping and dirty state tracking elements are also tracked in this > + * structure. > + */ > struct RAMBlock { > + /** @rcu: used for lazy free under RCU */ > struct rcu_head rcu; > + /** @mr: parent MemoryRegion the block belongs to */ > struct MemoryRegion *mr; > + /** @host: pointer to host address of RAM */ > uint8_t *host; > - uint8_t *colo_cache; /* For colo, VM's ram cache */ > + /** @colo_cache: For colo, VM's ram cache */ > + uint8_t *colo_cache; > + /** @offset: offset into host backing store??? or guest address space? */ I think it's the first, or to be explicit, "ram_addr_t address space"? > ram_addr_t offset; > + /** @used_length: amount of store used */ > ram_addr_t used_length; > + /** @max_length: for blocks that can be resized the max possible */ > ram_addr_t max_length; > + /** @resized: callback notifier when block resized */ > void (*resized)(const char*, uint64_t length, void *host); > + /** @flags: see RAM_* flags in memory.h */ > uint32_t flags; > - /* Protected by the BQL. */ > + /** @idstr: Protected by the BQL. */ Hmm, I think RCU should be enough to read an idstr? Maybe as simple as: @idstr: Name of the ramblock ? > char idstr[256]; > - /* RCU-enabled, writes protected by the ramlist lock */ > + /** > + * @next: next RAMBlock, RCU-enabled, writes protected by the > + * ramlist lock > + */ > QLIST_ENTRY(RAMBlock) next; > + /** @ramblock_notifiers: list of RAMBlockNotifier notifiers */ > QLIST_HEAD(, RAMBlockNotifier) ramblock_notifiers; > + /** @fd: fd of backing store if used */ Can also add: "For anonymous RAMBlocks, it's always -1". > int fd; > + /** @fd_offset: offset into the fd based backing store */ > uint64_t fd_offset; > + /** @page_size: ideal page size of backing store*/ "ideal" might be a bit ambiguous. How about "backend page size"? For anon, it's always PAGE_SIZE, for file, it's the one reported in fstatfs(). But this might be too verbose. > size_t page_size; > - /* dirty bitmap used during migration */ > + /** @bmap: dirty bitmap used during migration */ > unsigned long *bmap; > > /* > * Below fields are only used by mapped-ram migration > */ > - /* bitmap of pages present in the migration file */ > + > + /** @file_bmap: bitmap of pages present in the migration file */ Can append "(only used in mapped-ram migrations)". This may also apply to below two fields. > unsigned long *file_bmap; > - /* > - * offset in the file pages belonging to this ramblock are saved, > - * used only during migration to a file. > - */ > + /** @bitmap_offset: offset in the migration file of the bitmaps */ s/bitmaps/bitmap/, as there's only one for each rb. > off_t bitmap_offset; > + /** @pages_offset: offset in the migration file of the pages */ > uint64_t pages_offset; > > - /* bitmap of already received pages in postcopy */ > + /** @receivedmap: bitmap of already received pages in postcopy */ > unsigned long *receivedmap; > > - /* > - * bitmap to track already cleared dirty bitmap. When the bit is > - * set, it means the corresponding memory chunk needs a log-clear. > - * Set this up to non-NULL to enable the capability to postpone > - * and split clearing of dirty bitmap on the remote node (e.g., > - * KVM). The bitmap will be set only when doing global sync. > + /** > + * @clear_bmap: bitmap to track already cleared dirty bitmap. When > + * the bit is set, it means the corresponding memory chunk needs a > + * log-clear. Set this up to non-NULL to enable the capability to > + * postpone and split clearing of dirty bitmap on the remote node > + * (e.g., KVM). The bitmap will be set only when doing global > + * sync. > * > * It is only used during src side of ram migration, and it is > * protected by the global ram_state.bitmap_mutex. > * > * NOTE: this bitmap is different comparing to the other bitmaps > * in that one bit can represent multiple guest pages (which is > - * decided by the `clear_bmap_shift' variable below). On > + * decided by the @clear_bmap_shift variable below). On > * destination side, this should always be NULL, and the variable > - * `clear_bmap_shift' is meaningless. > + * @clear_bmap_shift is meaningless. > */ > unsigned long *clear_bmap; > + /** @clear_bmap_shift: number pages each @clear_bmap bit represents */ > uint8_t clear_bmap_shift; > > - /* > - * RAM block length that corresponds to the used_length on the migration > - * source (after RAM block sizes were synchronized). Especially, after > - * starting to run the guest, used_length and postcopy_length can differ. > - * Used to register/unregister uffd handlers and as the size of the received > - * bitmap. Receiving any page beyond this length will bail out, as it > - * could not have been valid on the source. > + /** > + * @postcopy_length: RAM block length that corresponds to the > + * @used_length on the migration source (after RAM block sizes > + * were synchronized). Especially, after starting to run the > + * guest, @used_length and @postcopy_length can differ. Used to > + * register/unregister uffd handlers and as the size of the > + * received bitmap. Receiving any page beyond this length will > + * bail out, as it could not have been valid on the source. > */ > ram_addr_t postcopy_length; > }; > -- > 2.39.2 >
diff --git a/docs/devel/memory.rst b/docs/devel/memory.rst index 69c5e3f914a..ed24708fce3 100644 --- a/docs/devel/memory.rst +++ b/docs/devel/memory.rst @@ -369,4 +369,5 @@ callbacks are called: API Reference ------------- +.. kernel-doc:: include/exec/ramblock.h .. kernel-doc:: include/exec/memory.h diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h index 848915ea5bf..eb2416b6f66 100644 --- a/include/exec/ramblock.h +++ b/include/exec/ramblock.h @@ -24,68 +24,94 @@ #include "qemu/rcu.h" #include "exec/ramlist.h" +/** + * struct RAMBlock - represents a chunk of RAM + * + * RAMBlocks can be backed by allocated RAM or a file-descriptor. See + * @flags for the details. For the purposes of migration various book + * keeping and dirty state tracking elements are also tracked in this + * structure. + */ struct RAMBlock { + /** @rcu: used for lazy free under RCU */ struct rcu_head rcu; + /** @mr: parent MemoryRegion the block belongs to */ struct MemoryRegion *mr; + /** @host: pointer to host address of RAM */ uint8_t *host; - uint8_t *colo_cache; /* For colo, VM's ram cache */ + /** @colo_cache: For colo, VM's ram cache */ + uint8_t *colo_cache; + /** @offset: offset into host backing store??? or guest address space? */ ram_addr_t offset; + /** @used_length: amount of store used */ ram_addr_t used_length; + /** @max_length: for blocks that can be resized the max possible */ ram_addr_t max_length; + /** @resized: callback notifier when block resized */ void (*resized)(const char*, uint64_t length, void *host); + /** @flags: see RAM_* flags in memory.h */ uint32_t flags; - /* Protected by the BQL. */ + /** @idstr: Protected by the BQL. */ char idstr[256]; - /* RCU-enabled, writes protected by the ramlist lock */ + /** + * @next: next RAMBlock, RCU-enabled, writes protected by the + * ramlist lock + */ QLIST_ENTRY(RAMBlock) next; + /** @ramblock_notifiers: list of RAMBlockNotifier notifiers */ QLIST_HEAD(, RAMBlockNotifier) ramblock_notifiers; + /** @fd: fd of backing store if used */ int fd; + /** @fd_offset: offset into the fd based backing store */ uint64_t fd_offset; + /** @page_size: ideal page size of backing store*/ size_t page_size; - /* dirty bitmap used during migration */ + /** @bmap: dirty bitmap used during migration */ unsigned long *bmap; /* * Below fields are only used by mapped-ram migration */ - /* bitmap of pages present in the migration file */ + + /** @file_bmap: bitmap of pages present in the migration file */ unsigned long *file_bmap; - /* - * offset in the file pages belonging to this ramblock are saved, - * used only during migration to a file. - */ + /** @bitmap_offset: offset in the migration file of the bitmaps */ off_t bitmap_offset; + /** @pages_offset: offset in the migration file of the pages */ uint64_t pages_offset; - /* bitmap of already received pages in postcopy */ + /** @receivedmap: bitmap of already received pages in postcopy */ unsigned long *receivedmap; - /* - * bitmap to track already cleared dirty bitmap. When the bit is - * set, it means the corresponding memory chunk needs a log-clear. - * Set this up to non-NULL to enable the capability to postpone - * and split clearing of dirty bitmap on the remote node (e.g., - * KVM). The bitmap will be set only when doing global sync. + /** + * @clear_bmap: bitmap to track already cleared dirty bitmap. When + * the bit is set, it means the corresponding memory chunk needs a + * log-clear. Set this up to non-NULL to enable the capability to + * postpone and split clearing of dirty bitmap on the remote node + * (e.g., KVM). The bitmap will be set only when doing global + * sync. * * It is only used during src side of ram migration, and it is * protected by the global ram_state.bitmap_mutex. * * NOTE: this bitmap is different comparing to the other bitmaps * in that one bit can represent multiple guest pages (which is - * decided by the `clear_bmap_shift' variable below). On + * decided by the @clear_bmap_shift variable below). On * destination side, this should always be NULL, and the variable - * `clear_bmap_shift' is meaningless. + * @clear_bmap_shift is meaningless. */ unsigned long *clear_bmap; + /** @clear_bmap_shift: number pages each @clear_bmap bit represents */ uint8_t clear_bmap_shift; - /* - * RAM block length that corresponds to the used_length on the migration - * source (after RAM block sizes were synchronized). Especially, after - * starting to run the guest, used_length and postcopy_length can differ. - * Used to register/unregister uffd handlers and as the size of the received - * bitmap. Receiving any page beyond this length will bail out, as it - * could not have been valid on the source. + /** + * @postcopy_length: RAM block length that corresponds to the + * @used_length on the migration source (after RAM block sizes + * were synchronized). Especially, after starting to run the + * guest, @used_length and @postcopy_length can differ. Used to + * register/unregister uffd handlers and as the size of the + * received bitmap. Receiving any page beyond this length will + * bail out, as it could not have been valid on the source. */ ram_addr_t postcopy_length; };
The RAMBlock concept is fairly central to RAM-like MemoryRegions so lets update the structure documentation and include in the docs. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- docs/devel/memory.rst | 1 + include/exec/ramblock.h | 76 +++++++++++++++++++++++++++-------------- 2 files changed, 52 insertions(+), 25 deletions(-)