Skip to contentSkip to conversation

Memory: Trivial fixes

PATCH In review [email protected] 3 participants 7 messages Jun 3, 2013 – Jun 4, 2013

lore
Share & tools
lore
#0 lore raw

cover Memory: Trivial fixes

From: Peter Crosthwaite <[email protected]>

Some trivial fixes to memory API.

Peter Crosthwaite (3):
memory: Fix comment typo
memory: as_update_topology_pass: Improve comments
memory: render_memory_region: factor out fr constant setters

 memory.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
#1 lore raw

PATCH 1/3 memory: Fix comment typo

From: Peter Crosthwaite <[email protected]>

s/ajacent/adjacent

---

 memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
memory.c +1 -1
index 5cb8f4a..60e033b 100644
@@ -282,7 +282,7 @@ static bool can_merge(FlatRange *r1, FlatRange *r2)
         && r1->readonly == r2->readonly;
 }
 
-/* Attempt to simplify a view by merging ajacent ranges */
+/* Attempt to simplify a view by merging adjacent ranges */
 static void flatview_simplify(FlatView *view)
 {
     unsigned i, j;
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
#2 lore raw

PATCH 2/3 memory: as_update_topology_pass: Improve comments

In reply to [email protected]

From: Peter Crosthwaite <[email protected]>

These comments we're a little difficult to read. First one had incorrect parenthesis. The part about attributes changing is really applicable to the region being 'in both' rather than 'in new'

Second comment has an obscure parenthetic about 'Logging may have changed'. Made clearer, as this if is supposed to handle the case where the memory region is unchanged (with the notable exception re logging).

---

 memory.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
memory.c +2 -2
index 60e033b..7e710c4 100644
@@ -719,7 +719,7 @@ static void address_space_update_topology_pass(AddressSpace *as,
                 || int128_lt(frold->addr.start, frnew->addr.start)
                 || (int128_eq(frold->addr.start, frnew->addr.start)
                     && !flatrange_equal(frold, frnew)))) {
-            /* In old, but (not in new, or in new but attributes changed). */
+            /* In old but not in new, or in both but attributes changed. */
 
             if (!adding) {
                 MEMORY_LISTENER_UPDATE_REGION(frold, as, Reverse, region_del);
@@ -727,7 +727,7 @@ static void address_space_update_topology_pass(AddressSpace *as,
 
             ++iold;
         } else if (frold && frnew && flatrange_equal(frold, frnew)) {
-            /* In both (logging may have changed) */
+            /* In both and unchanged (except logging may have changed) */
 
             if (adding) {
                 MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, region_nop);
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
#3 lore raw

Re: [Qemu-devel] [PATCH v1 2/3] memory: as_update_topology_pass: Improve comments

Am 03.06.2013 07:32, schrieb [email protected]:

Quoted 3 lines

From: Peter Crosthwaite <[email protected]>

These comments we're a little difficult to read. First one had

While we're picky: "were" - Paolo can you fix up in your queue?

Andreas

Quoted 37 lines

incorrect parenthesis. The part about attributes changing is really applicable to the region being 'in both' rather than 'in new'

Second comment has an obscure parenthetic about 'Logging may have changed'. Made clearer, as this if is supposed to handle the case where the memory region is unchanged (with the notable exception re logging).

---

 memory.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
memory.c +2 -2
index 60e033b..7e710c4 100644
@@ -719,7 +719,7 @@ static void address_space_update_topology_pass(AddressSpace *as,
                 || int128_lt(frold->addr.start, frnew->addr.start)
                 || (int128_eq(frold->addr.start, frnew->addr.start)
                     && !flatrange_equal(frold, frnew)))) {
-            /* In old, but (not in new, or in new but attributes changed). */
+            /* In old but not in new, or in both but attributes changed. */
 
             if (!adding) {
                 MEMORY_LISTENER_UPDATE_REGION(frold, as, Reverse, region_del);
@@ -727,7 +727,7 @@ static void address_space_update_topology_pass(AddressSpace *as,
 
             ++iold;
         } else if (frold && frnew && flatrange_equal(frold, frnew)) {
-            /* In both (logging may have changed) */
+            /* In both and unchanged (except logging may have changed) */
 
             if (adding) {
                 MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, region_nop);
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
#4 lore raw

Re: [Qemu-devel] [PATCH v1 2/3] memory: as_update_topology_pass: Improve comments

Il 04/06/2013 13:43, Andreas Färber ha scritto:

Quoted 5 lines

Am 03.06.2013 07:32, schrieb [email protected]:

Quoted 3 lines
Quoted 3 lines

From: Peter Crosthwaite <[email protected]>

These comments we're a little difficult to read. First one had

While we're picky: "were" - Paolo can you fix up in your queue?

Of course.

Paolo

#5 lore raw

PATCH 3/3 memory: render_memory_region: factor out fr constant setters

In reply to [email protected]

From: Peter Crosthwaite <[email protected]>

These 4 replicated lines set properties of fr that are constant over the course of the function. Factor out their repeated setting (and also guards against them being set multiple times in the loop below).

---

 memory.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)
memory.c +5 -8
index 7e710c4..7bdfc58 100644
@@ -556,6 +556,11 @@ static void render_memory_region(FlatView *view,
     base = clip.start;
     remain = clip.size;
 
+    fr.mr = mr;
+    fr.dirty_log_mask = mr->dirty_log_mask;
+    fr.romd_mode = mr->romd_mode;
+    fr.readonly = readonly;
+
     /* Render the region itself into any gaps left by the current view. */
     for (i = 0; i < view->nr && int128_nz(remain); ++i) {
         if (int128_ge(base, addrrange_end(view->ranges[i].addr))) {
@@ -564,12 +569,8 @@ static void render_memory_region(FlatView *view,
         if (int128_lt(base, view->ranges[i].addr.start)) {
             now = int128_min(remain,
                              int128_sub(view->ranges[i].addr.start, base));
-            fr.mr = mr;
             fr.offset_in_region = offset_in_region;
             fr.addr = addrrange_make(base, now);
-            fr.dirty_log_mask = mr->dirty_log_mask;
-            fr.romd_mode = mr->romd_mode;
-            fr.readonly = readonly;
             flatview_insert(view, i, &fr);
             ++i;
             int128_addto(&base, now);
@@ -584,12 +585,8 @@ static void render_memory_region(FlatView *view,
         int128_subfrom(&remain, now);
     }
     if (int128_nz(remain)) {
-        fr.mr = mr;
         fr.offset_in_region = offset_in_region;
         fr.addr = addrrange_make(base, remain);
-        fr.dirty_log_mask = mr->dirty_log_mask;
-        fr.romd_mode = mr->romd_mode;
-        fr.readonly = readonly;
         flatview_insert(view, i, &fr);
     }
 }
Signature · 1 line
1.8.3.rc1.44.gb387c77.dirty
#6 lore raw
In reply to [email protected]

Il 03/06/2013 07:31, [email protected] ha scritto:

Quoted 5 lines

From: Peter Crosthwaite <[email protected]>

Some trivial fixes to memory API.

Quoted 8 lines

Peter Crosthwaite (3):
memory: Fix comment typo
memory: as_update_topology_pass: Improve comments
memory: render_memory_region: factor out fr constant setters

 memory.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

Thanks, applying to my iommu branch.