* [Qemu-devel] [PATCH v2 0/2] refine memory_device_get_free_addr
@ 2019-07-30 0:37 Wei Yang
2019-07-30 0:37 ` [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to use goto for the last check Wei Yang
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Wei Yang @ 2019-07-30 0:37 UTC (permalink / raw)
To: qemu-devel; +Cc: imammedo, david, Wei Yang, mst
When we iterate the memory-device list to get the available range, it is not
necessary to iterate the whole list.
1) no more overlap for hinted range if tmp exceed it
v2:
* remove #2 as suggested by Igor and David
* add some comment to inform address assignment stay the same as before
this change
Wei Yang (2):
memory-device: not necessary to use goto for the last check
memory-device: break the loop if tmp exceed the hinted range
hw/mem/memory-device.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--
2.17.1
^ permalink raw reply [flat|nested] 18+ messages in thread* [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to use goto for the last check 2019-07-30 0:37 [Qemu-devel] [PATCH v2 0/2] refine memory_device_get_free_addr Wei Yang @ 2019-07-30 0:37 ` Wei Yang 2019-08-08 1:42 ` Zeng, Star 2019-07-30 0:37 ` [Qemu-devel] [PATCH v2 2/2] memory-device: break the loop if tmp exceed the hinted range Wei Yang 2019-09-13 23:47 ` [Qemu-devel] [PATCH v2 0/2] refine memory_device_get_free_addr Wei Yang 2 siblings, 1 reply; 18+ messages in thread From: Wei Yang @ 2019-07-30 0:37 UTC (permalink / raw) To: qemu-devel; +Cc: imammedo, david, Wei Yang, mst We are already at the last condition check. Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: David Hildenbrand <david@redhat.com> --- hw/mem/memory-device.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c index 5f2c408036..df3261b32a 100644 --- a/hw/mem/memory-device.c +++ b/hw/mem/memory-device.c @@ -186,7 +186,6 @@ static uint64_t memory_device_get_free_addr(MachineState *ms, if (!range_contains_range(&as, &new)) { error_setg(errp, "could not find position in guest address space for " "memory device - memory fragmented due to alignments"); - goto out; } out: g_slist_free(list); -- 2.17.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to use goto for the last check 2019-07-30 0:37 ` [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to use goto for the last check Wei Yang @ 2019-08-08 1:42 ` Zeng, Star 2019-08-08 2:13 ` Wei Yang 0 siblings, 1 reply; 18+ messages in thread From: Zeng, Star @ 2019-08-08 1:42 UTC (permalink / raw) To: Wei Yang, qemu-devel; +Cc: imammedo, mst, Zeng, Star, david > -----Original Message----- > From: Qemu-devel [mailto:qemu-devel- > bounces+star.zeng=intel.com@nongnu.org] On Behalf Of Wei Yang > Sent: Tuesday, July 30, 2019 8:38 AM > To: qemu-devel@nongnu.org > Cc: imammedo@redhat.com; david@redhat.com; Wei Yang > <richardw.yang@linux.intel.com>; mst@redhat.com > Subject: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to use > goto for the last check > > We are already at the last condition check. > > Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> > Reviewed-by: Igor Mammedov <imammedo@redhat.com> > Reviewed-by: David Hildenbrand <david@redhat.com> > --- > hw/mem/memory-device.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c index > 5f2c408036..df3261b32a 100644 > --- a/hw/mem/memory-device.c > +++ b/hw/mem/memory-device.c > @@ -186,7 +186,6 @@ static uint64_t > memory_device_get_free_addr(MachineState *ms, > if (!range_contains_range(&as, &new)) { > error_setg(errp, "could not find position in guest address space for " > "memory device - memory fragmented due to alignments"); > - goto out; Is it better to return 0 (or set new_addr to 0) for this error path and another remaining "goto out" path? Thanks, Star > } > out: > g_slist_free(list); > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to use goto for the last check 2019-08-08 1:42 ` Zeng, Star @ 2019-08-08 2:13 ` Wei Yang 2019-08-08 2:30 ` Zeng, Star 0 siblings, 1 reply; 18+ messages in thread From: Wei Yang @ 2019-08-08 2:13 UTC (permalink / raw) To: Zeng, Star; +Cc: imammedo, david, mst, Wei Yang, qemu-devel On Thu, Aug 08, 2019 at 01:42:14AM +0000, Zeng, Star wrote: >> -----Original Message----- >> From: Qemu-devel [mailto:qemu-devel- >> bounces+star.zeng=intel.com@nongnu.org] On Behalf Of Wei Yang >> Sent: Tuesday, July 30, 2019 8:38 AM >> To: qemu-devel@nongnu.org >> Cc: imammedo@redhat.com; david@redhat.com; Wei Yang >> <richardw.yang@linux.intel.com>; mst@redhat.com >> Subject: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to use >> goto for the last check >> >> We are already at the last condition check. >> >> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> >> Reviewed-by: Igor Mammedov <imammedo@redhat.com> >> Reviewed-by: David Hildenbrand <david@redhat.com> >> --- >> hw/mem/memory-device.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c index >> 5f2c408036..df3261b32a 100644 >> --- a/hw/mem/memory-device.c >> +++ b/hw/mem/memory-device.c >> @@ -186,7 +186,6 @@ static uint64_t >> memory_device_get_free_addr(MachineState *ms, >> if (!range_contains_range(&as, &new)) { >> error_setg(errp, "could not find position in guest address space for " >> "memory device - memory fragmented due to alignments"); >> - goto out; > >Is it better to return 0 (or set new_addr to 0) for this error path and another remaining "goto out" path? > I may not get your point. We set errp which is handled in its caller. By doing so, the error is propagated. Do I miss something? > >Thanks, >Star > >> } >> out: >> g_slist_free(list); >> -- >> 2.17.1 >> -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to use goto for the last check 2019-08-08 2:13 ` Wei Yang @ 2019-08-08 2:30 ` Zeng, Star 2019-08-08 2:38 ` Wei Yang 0 siblings, 1 reply; 18+ messages in thread From: Zeng, Star @ 2019-08-08 2:30 UTC (permalink / raw) To: Wei Yang; +Cc: imammedo, mst, qemu-devel, Zeng, Star, david > -----Original Message----- > From: Wei Yang [mailto:richardw.yang@linux.intel.com] > Sent: Thursday, August 8, 2019 10:13 AM > To: Zeng, Star <star.zeng@intel.com> > Cc: Wei Yang <richardw.yang@linux.intel.com>; qemu-devel@nongnu.org; > imammedo@redhat.com; david@redhat.com; mst@redhat.com > Subject: Re: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to > use goto for the last check > > On Thu, Aug 08, 2019 at 01:42:14AM +0000, Zeng, Star wrote: > >> -----Original Message----- > >> From: Qemu-devel [mailto:qemu-devel- > >> bounces+star.zeng=intel.com@nongnu.org] On Behalf Of Wei Yang > >> Sent: Tuesday, July 30, 2019 8:38 AM > >> To: qemu-devel@nongnu.org > >> Cc: imammedo@redhat.com; david@redhat.com; Wei Yang > >> <richardw.yang@linux.intel.com>; mst@redhat.com > >> Subject: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to > >> use goto for the last check > >> > >> We are already at the last condition check. > >> > >> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> > >> Reviewed-by: Igor Mammedov <imammedo@redhat.com> > >> Reviewed-by: David Hildenbrand <david@redhat.com> > >> --- > >> hw/mem/memory-device.c | 1 - > >> 1 file changed, 1 deletion(-) > >> > >> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c > index > >> 5f2c408036..df3261b32a 100644 > >> --- a/hw/mem/memory-device.c > >> +++ b/hw/mem/memory-device.c > >> @@ -186,7 +186,6 @@ static uint64_t > >> memory_device_get_free_addr(MachineState *ms, > >> if (!range_contains_range(&as, &new)) { > >> error_setg(errp, "could not find position in guest address space for " > >> "memory device - memory fragmented due to alignments"); > >> - goto out; > > > >Is it better to return 0 (or set new_addr to 0) for this error path and another > remaining "goto out" path? > > > > I may not get your point. > > We set errp which is handled in its caller. By doing so, the error is propagated. > > Do I miss something? Yes, you are right. Currently, the caller is checking errp, but not the returned address, so there should be no issue. But when you see other error paths, you will find they all return 0. To be aligned (return 0 when error), so just suggest also returning 0 for these two "goto out" error path. :) Thanks, Star > > > > >Thanks, > >Star > > > >> } > >> out: > >> g_slist_free(list); > >> -- > >> 2.17.1 > >> > > -- > Wei Yang > Help you, Help me ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to use goto for the last check 2019-08-08 2:30 ` Zeng, Star @ 2019-08-08 2:38 ` Wei Yang 2019-08-08 7:06 ` David Hildenbrand 0 siblings, 1 reply; 18+ messages in thread From: Wei Yang @ 2019-08-08 2:38 UTC (permalink / raw) To: Zeng, Star; +Cc: imammedo, david, mst, Wei Yang, qemu-devel On Thu, Aug 08, 2019 at 02:30:02AM +0000, Zeng, Star wrote: >> -----Original Message----- >> From: Wei Yang [mailto:richardw.yang@linux.intel.com] >> Sent: Thursday, August 8, 2019 10:13 AM >> To: Zeng, Star <star.zeng@intel.com> >> Cc: Wei Yang <richardw.yang@linux.intel.com>; qemu-devel@nongnu.org; >> imammedo@redhat.com; david@redhat.com; mst@redhat.com >> Subject: Re: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to >> use goto for the last check >> >> On Thu, Aug 08, 2019 at 01:42:14AM +0000, Zeng, Star wrote: >> >> -----Original Message----- >> >> From: Qemu-devel [mailto:qemu-devel- >> >> bounces+star.zeng=intel.com@nongnu.org] On Behalf Of Wei Yang >> >> Sent: Tuesday, July 30, 2019 8:38 AM >> >> To: qemu-devel@nongnu.org >> >> Cc: imammedo@redhat.com; david@redhat.com; Wei Yang >> >> <richardw.yang@linux.intel.com>; mst@redhat.com >> >> Subject: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to >> >> use goto for the last check >> >> >> >> We are already at the last condition check. >> >> >> >> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> >> >> Reviewed-by: Igor Mammedov <imammedo@redhat.com> >> >> Reviewed-by: David Hildenbrand <david@redhat.com> >> >> --- >> >> hw/mem/memory-device.c | 1 - >> >> 1 file changed, 1 deletion(-) >> >> >> >> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c >> index >> >> 5f2c408036..df3261b32a 100644 >> >> --- a/hw/mem/memory-device.c >> >> +++ b/hw/mem/memory-device.c >> >> @@ -186,7 +186,6 @@ static uint64_t >> >> memory_device_get_free_addr(MachineState *ms, >> >> if (!range_contains_range(&as, &new)) { >> >> error_setg(errp, "could not find position in guest address space for " >> >> "memory device - memory fragmented due to alignments"); >> >> - goto out; >> > >> >Is it better to return 0 (or set new_addr to 0) for this error path and another >> remaining "goto out" path? >> > >> >> I may not get your point. >> >> We set errp which is handled in its caller. By doing so, the error is propagated. >> >> Do I miss something? > >Yes, you are right. Currently, the caller is checking errp, but not the returned address, so there should be no issue. >But when you see other error paths, you will find they all return 0. To be aligned (return 0 when error), so just suggest also returning 0 for these two "goto out" error path. :) > You may have some point. Let's see whether others have the same taste, or we can refine it separately. >Thanks, >Star > >> >> > >> >Thanks, >> >Star >> > >> >> } >> >> out: >> >> g_slist_free(list); >> >> -- >> >> 2.17.1 >> >> >> >> -- >> Wei Yang >> Help you, Help me -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to use goto for the last check 2019-08-08 2:38 ` Wei Yang @ 2019-08-08 7:06 ` David Hildenbrand 2019-08-19 2:38 ` Wei Yang 0 siblings, 1 reply; 18+ messages in thread From: David Hildenbrand @ 2019-08-08 7:06 UTC (permalink / raw) To: Wei Yang, Zeng, Star; +Cc: imammedo, qemu-devel, mst On 08.08.19 04:38, Wei Yang wrote: > On Thu, Aug 08, 2019 at 02:30:02AM +0000, Zeng, Star wrote: >>> -----Original Message----- >>> From: Wei Yang [mailto:richardw.yang@linux.intel.com] >>> Sent: Thursday, August 8, 2019 10:13 AM >>> To: Zeng, Star <star.zeng@intel.com> >>> Cc: Wei Yang <richardw.yang@linux.intel.com>; qemu-devel@nongnu.org; >>> imammedo@redhat.com; david@redhat.com; mst@redhat.com >>> Subject: Re: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to >>> use goto for the last check >>> >>> On Thu, Aug 08, 2019 at 01:42:14AM +0000, Zeng, Star wrote: >>>>> -----Original Message----- >>>>> From: Qemu-devel [mailto:qemu-devel- >>>>> bounces+star.zeng=intel.com@nongnu.org] On Behalf Of Wei Yang >>>>> Sent: Tuesday, July 30, 2019 8:38 AM >>>>> To: qemu-devel@nongnu.org >>>>> Cc: imammedo@redhat.com; david@redhat.com; Wei Yang >>>>> <richardw.yang@linux.intel.com>; mst@redhat.com >>>>> Subject: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to >>>>> use goto for the last check >>>>> >>>>> We are already at the last condition check. >>>>> >>>>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> >>>>> Reviewed-by: Igor Mammedov <imammedo@redhat.com> >>>>> Reviewed-by: David Hildenbrand <david@redhat.com> >>>>> --- >>>>> hw/mem/memory-device.c | 1 - >>>>> 1 file changed, 1 deletion(-) >>>>> >>>>> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c >>> index >>>>> 5f2c408036..df3261b32a 100644 >>>>> --- a/hw/mem/memory-device.c >>>>> +++ b/hw/mem/memory-device.c >>>>> @@ -186,7 +186,6 @@ static uint64_t >>>>> memory_device_get_free_addr(MachineState *ms, >>>>> if (!range_contains_range(&as, &new)) { >>>>> error_setg(errp, "could not find position in guest address space for " >>>>> "memory device - memory fragmented due to alignments"); >>>>> - goto out; >>>> >>>> Is it better to return 0 (or set new_addr to 0) for this error path and another >>> remaining "goto out" path? >>>> >>> >>> I may not get your point. >>> >>> We set errp which is handled in its caller. By doing so, the error is propagated. >>> >>> Do I miss something? >> >> Yes, you are right. Currently, the caller is checking errp, but not the returned address, so there should be no issue. >> But when you see other error paths, you will find they all return 0. To be aligned (return 0 when error), so just suggest also returning 0 for these two "goto out" error path. :) >> > > You may have some point. > > Let's see whether others have the same taste, or we can refine it separately. > I don't think that's necessary (callers really should check for errors before using the return values), but I would also not object to that change. -- Thanks, David / dhildenb ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to use goto for the last check 2019-08-08 7:06 ` David Hildenbrand @ 2019-08-19 2:38 ` Wei Yang 2019-08-19 5:32 ` Zeng, Star 0 siblings, 1 reply; 18+ messages in thread From: Wei Yang @ 2019-08-19 2:38 UTC (permalink / raw) To: David Hildenbrand; +Cc: imammedo, mst, Wei Yang, Zeng, Star, qemu-devel On Thu, Aug 08, 2019 at 09:06:21AM +0200, David Hildenbrand wrote: >On 08.08.19 04:38, Wei Yang wrote: >> On Thu, Aug 08, 2019 at 02:30:02AM +0000, Zeng, Star wrote: >>>> -----Original Message----- >>>> From: Wei Yang [mailto:richardw.yang@linux.intel.com] >>>> Sent: Thursday, August 8, 2019 10:13 AM >>>> To: Zeng, Star <star.zeng@intel.com> >>>> Cc: Wei Yang <richardw.yang@linux.intel.com>; qemu-devel@nongnu.org; >>>> imammedo@redhat.com; david@redhat.com; mst@redhat.com >>>> Subject: Re: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to >>>> use goto for the last check >>>> >>>> On Thu, Aug 08, 2019 at 01:42:14AM +0000, Zeng, Star wrote: >>>>>> -----Original Message----- >>>>>> From: Qemu-devel [mailto:qemu-devel- >>>>>> bounces+star.zeng=intel.com@nongnu.org] On Behalf Of Wei Yang >>>>>> Sent: Tuesday, July 30, 2019 8:38 AM >>>>>> To: qemu-devel@nongnu.org >>>>>> Cc: imammedo@redhat.com; david@redhat.com; Wei Yang >>>>>> <richardw.yang@linux.intel.com>; mst@redhat.com >>>>>> Subject: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to >>>>>> use goto for the last check >>>>>> >>>>>> We are already at the last condition check. >>>>>> >>>>>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> >>>>>> Reviewed-by: Igor Mammedov <imammedo@redhat.com> >>>>>> Reviewed-by: David Hildenbrand <david@redhat.com> >>>>>> --- >>>>>> hw/mem/memory-device.c | 1 - >>>>>> 1 file changed, 1 deletion(-) >>>>>> >>>>>> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c >>>> index >>>>>> 5f2c408036..df3261b32a 100644 >>>>>> --- a/hw/mem/memory-device.c >>>>>> +++ b/hw/mem/memory-device.c >>>>>> @@ -186,7 +186,6 @@ static uint64_t >>>>>> memory_device_get_free_addr(MachineState *ms, >>>>>> if (!range_contains_range(&as, &new)) { >>>>>> error_setg(errp, "could not find position in guest address space for " >>>>>> "memory device - memory fragmented due to alignments"); >>>>>> - goto out; >>>>> >>>>> Is it better to return 0 (or set new_addr to 0) for this error path and another >>>> remaining "goto out" path? >>>>> >>>> >>>> I may not get your point. >>>> >>>> We set errp which is handled in its caller. By doing so, the error is propagated. >>>> >>>> Do I miss something? >>> >>> Yes, you are right. Currently, the caller is checking errp, but not the returned address, so there should be no issue. >>> But when you see other error paths, you will find they all return 0. To be aligned (return 0 when error), so just suggest also returning 0 for these two "goto out" error path. :) >>> >> >> You may have some point. >> >> Let's see whether others have the same taste, or we can refine it separately. >> > >I don't think that's necessary (callers really should check for errors >before using the return values), but I would also not object to that change. > In case there is no strong requirement to refactor the code. I would leave it here. >-- > >Thanks, > >David / dhildenb -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to use goto for the last check 2019-08-19 2:38 ` Wei Yang @ 2019-08-19 5:32 ` Zeng, Star 2019-08-19 6:36 ` Wei Yang 0 siblings, 1 reply; 18+ messages in thread From: Zeng, Star @ 2019-08-19 5:32 UTC (permalink / raw) To: Wei Yang, David Hildenbrand; +Cc: imammedo, qemu-devel, Zeng, Star, mst > -----Original Message----- > From: Wei Yang [mailto:richardw.yang@linux.intel.com] > Sent: Monday, August 19, 2019 10:39 AM > To: David Hildenbrand <david@redhat.com> > Cc: Wei Yang <richardw.yang@linux.intel.com>; Zeng, Star > <star.zeng@intel.com>; imammedo@redhat.com; qemu-devel@nongnu.org; > mst@redhat.com > Subject: Re: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to > use goto for the last check > > On Thu, Aug 08, 2019 at 09:06:21AM +0200, David Hildenbrand wrote: > >On 08.08.19 04:38, Wei Yang wrote: > >> On Thu, Aug 08, 2019 at 02:30:02AM +0000, Zeng, Star wrote: > >>>> -----Original Message----- > >>>> From: Wei Yang [mailto:richardw.yang@linux.intel.com] > >>>> Sent: Thursday, August 8, 2019 10:13 AM > >>>> To: Zeng, Star <star.zeng@intel.com> > >>>> Cc: Wei Yang <richardw.yang@linux.intel.com>; > >>>> qemu-devel@nongnu.org; imammedo@redhat.com; > david@redhat.com; > >>>> mst@redhat.com > >>>> Subject: Re: [Qemu-devel] [PATCH v2 1/2] memory-device: not > >>>> necessary to use goto for the last check > >>>> > >>>> On Thu, Aug 08, 2019 at 01:42:14AM +0000, Zeng, Star wrote: > >>>>>> -----Original Message----- > >>>>>> From: Qemu-devel [mailto:qemu-devel- > >>>>>> bounces+star.zeng=intel.com@nongnu.org] On Behalf Of Wei Yang > >>>>>> Sent: Tuesday, July 30, 2019 8:38 AM > >>>>>> To: qemu-devel@nongnu.org > >>>>>> Cc: imammedo@redhat.com; david@redhat.com; Wei Yang > >>>>>> <richardw.yang@linux.intel.com>; mst@redhat.com > >>>>>> Subject: [Qemu-devel] [PATCH v2 1/2] memory-device: not > necessary > >>>>>> to use goto for the last check > >>>>>> > >>>>>> We are already at the last condition check. > >>>>>> > >>>>>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> > >>>>>> Reviewed-by: Igor Mammedov <imammedo@redhat.com> > >>>>>> Reviewed-by: David Hildenbrand <david@redhat.com> > >>>>>> --- > >>>>>> hw/mem/memory-device.c | 1 - > >>>>>> 1 file changed, 1 deletion(-) > >>>>>> > >>>>>> diff --git a/hw/mem/memory-device.c b/hw/mem/memory- > device.c > >>>> index > >>>>>> 5f2c408036..df3261b32a 100644 > >>>>>> --- a/hw/mem/memory-device.c > >>>>>> +++ b/hw/mem/memory-device.c > >>>>>> @@ -186,7 +186,6 @@ static uint64_t > >>>>>> memory_device_get_free_addr(MachineState *ms, > >>>>>> if (!range_contains_range(&as, &new)) { > >>>>>> error_setg(errp, "could not find position in guest address space > for " > >>>>>> "memory device - memory fragmented due to > alignments"); > >>>>>> - goto out; > >>>>> > >>>>> Is it better to return 0 (or set new_addr to 0) for this error > >>>>> path and another > >>>> remaining "goto out" path? > >>>>> > >>>> > >>>> I may not get your point. > >>>> > >>>> We set errp which is handled in its caller. By doing so, the error is > propagated. > >>>> > >>>> Do I miss something? > >>> > >>> Yes, you are right. Currently, the caller is checking errp, but not the > returned address, so there should be no issue. > >>> But when you see other error paths, you will find they all return 0. > >>> To be aligned (return 0 when error), so just suggest also returning > >>> 0 for these two "goto out" error path. :) > >>> > >> > >> You may have some point. > >> > >> Let's see whether others have the same taste, or we can refine it > separately. > >> > > > >I don't think that's necessary (callers really should check for errors > >before using the return values), but I would also not object to that change. > > > > In case there is no strong requirement to refactor the code. I would leave it > here. It was just my suggestion. I am fine with any preference you and other experts have. Thanks, Star > > >-- > > > >Thanks, > > > >David / dhildenb > > -- > Wei Yang > Help you, Help me ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to use goto for the last check 2019-08-19 5:32 ` Zeng, Star @ 2019-08-19 6:36 ` Wei Yang 0 siblings, 0 replies; 18+ messages in thread From: Wei Yang @ 2019-08-19 6:36 UTC (permalink / raw) To: Zeng, Star; +Cc: qemu-devel, imammedo, mst, Wei Yang, David Hildenbrand On Mon, Aug 19, 2019 at 05:32:14AM +0000, Zeng, Star wrote: > > >> -----Original Message----- >> From: Wei Yang [mailto:richardw.yang@linux.intel.com] >> Sent: Monday, August 19, 2019 10:39 AM >> To: David Hildenbrand <david@redhat.com> >> Cc: Wei Yang <richardw.yang@linux.intel.com>; Zeng, Star >> <star.zeng@intel.com>; imammedo@redhat.com; qemu-devel@nongnu.org; >> mst@redhat.com >> Subject: Re: [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to >> use goto for the last check >> >> On Thu, Aug 08, 2019 at 09:06:21AM +0200, David Hildenbrand wrote: >> >On 08.08.19 04:38, Wei Yang wrote: >> >> On Thu, Aug 08, 2019 at 02:30:02AM +0000, Zeng, Star wrote: >> >>>> -----Original Message----- >> >>>> From: Wei Yang [mailto:richardw.yang@linux.intel.com] >> >>>> Sent: Thursday, August 8, 2019 10:13 AM >> >>>> To: Zeng, Star <star.zeng@intel.com> >> >>>> Cc: Wei Yang <richardw.yang@linux.intel.com>; >> >>>> qemu-devel@nongnu.org; imammedo@redhat.com; >> david@redhat.com; >> >>>> mst@redhat.com >> >>>> Subject: Re: [Qemu-devel] [PATCH v2 1/2] memory-device: not >> >>>> necessary to use goto for the last check >> >>>> >> >>>> On Thu, Aug 08, 2019 at 01:42:14AM +0000, Zeng, Star wrote: >> >>>>>> -----Original Message----- >> >>>>>> From: Qemu-devel [mailto:qemu-devel- >> >>>>>> bounces+star.zeng=intel.com@nongnu.org] On Behalf Of Wei Yang >> >>>>>> Sent: Tuesday, July 30, 2019 8:38 AM >> >>>>>> To: qemu-devel@nongnu.org >> >>>>>> Cc: imammedo@redhat.com; david@redhat.com; Wei Yang >> >>>>>> <richardw.yang@linux.intel.com>; mst@redhat.com >> >>>>>> Subject: [Qemu-devel] [PATCH v2 1/2] memory-device: not >> necessary >> >>>>>> to use goto for the last check >> >>>>>> >> >>>>>> We are already at the last condition check. >> >>>>>> >> >>>>>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> >> >>>>>> Reviewed-by: Igor Mammedov <imammedo@redhat.com> >> >>>>>> Reviewed-by: David Hildenbrand <david@redhat.com> >> >>>>>> --- >> >>>>>> hw/mem/memory-device.c | 1 - >> >>>>>> 1 file changed, 1 deletion(-) >> >>>>>> >> >>>>>> diff --git a/hw/mem/memory-device.c b/hw/mem/memory- >> device.c >> >>>> index >> >>>>>> 5f2c408036..df3261b32a 100644 >> >>>>>> --- a/hw/mem/memory-device.c >> >>>>>> +++ b/hw/mem/memory-device.c >> >>>>>> @@ -186,7 +186,6 @@ static uint64_t >> >>>>>> memory_device_get_free_addr(MachineState *ms, >> >>>>>> if (!range_contains_range(&as, &new)) { >> >>>>>> error_setg(errp, "could not find position in guest address space >> for " >> >>>>>> "memory device - memory fragmented due to >> alignments"); >> >>>>>> - goto out; >> >>>>> >> >>>>> Is it better to return 0 (or set new_addr to 0) for this error >> >>>>> path and another >> >>>> remaining "goto out" path? >> >>>>> >> >>>> >> >>>> I may not get your point. >> >>>> >> >>>> We set errp which is handled in its caller. By doing so, the error is >> propagated. >> >>>> >> >>>> Do I miss something? >> >>> >> >>> Yes, you are right. Currently, the caller is checking errp, but not the >> returned address, so there should be no issue. >> >>> But when you see other error paths, you will find they all return 0. >> >>> To be aligned (return 0 when error), so just suggest also returning >> >>> 0 for these two "goto out" error path. :) >> >>> >> >> >> >> You may have some point. >> >> >> >> Let's see whether others have the same taste, or we can refine it >> separately. >> >> >> > >> >I don't think that's necessary (callers really should check for errors >> >before using the return values), but I would also not object to that change. >> > >> >> In case there is no strong requirement to refactor the code. I would leave it >> here. > >It was just my suggestion. I am fine with any preference you and other experts have. > Thanks >Thanks, >Star > >> >> >-- >> > >> >Thanks, >> > >> >David / dhildenb >> >> -- >> Wei Yang >> Help you, Help me -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] memory-device: break the loop if tmp exceed the hinted range 2019-07-30 0:37 [Qemu-devel] [PATCH v2 0/2] refine memory_device_get_free_addr Wei Yang 2019-07-30 0:37 ` [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to use goto for the last check Wei Yang @ 2019-07-30 0:37 ` Wei Yang 2019-07-30 7:38 ` David Hildenbrand 2019-07-30 9:30 ` Igor Mammedov 2019-09-13 23:47 ` [Qemu-devel] [PATCH v2 0/2] refine memory_device_get_free_addr Wei Yang 2 siblings, 2 replies; 18+ messages in thread From: Wei Yang @ 2019-07-30 0:37 UTC (permalink / raw) To: qemu-devel; +Cc: imammedo, david, Wei Yang, mst The memory-device list built by memory_device_build_list is ordered by its address, this means if the tmp range exceed the hinted range, all the following range will not overlap with it. And this won't change default pc-dimm mapping and address assignment stay the same as before this change. Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> --- hw/mem/memory-device.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c index df3261b32a..df4e338b83 100644 --- a/hw/mem/memory-device.c +++ b/hw/mem/memory-device.c @@ -180,6 +180,8 @@ static uint64_t memory_device_get_free_addr(MachineState *ms, range_make_empty(&new); break; } + } else if (range_lob(&tmp) > range_upb(&new)) { + break; } } -- 2.17.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] memory-device: break the loop if tmp exceed the hinted range 2019-07-30 0:37 ` [Qemu-devel] [PATCH v2 2/2] memory-device: break the loop if tmp exceed the hinted range Wei Yang @ 2019-07-30 7:38 ` David Hildenbrand 2019-07-30 9:30 ` Igor Mammedov 1 sibling, 0 replies; 18+ messages in thread From: David Hildenbrand @ 2019-07-30 7:38 UTC (permalink / raw) To: Wei Yang, qemu-devel; +Cc: imammedo, mst On 30.07.19 02:37, Wei Yang wrote: > The memory-device list built by memory_device_build_list is ordered by > its address, this means if the tmp range exceed the hinted range, all > the following range will not overlap with it. > > And this won't change default pc-dimm mapping and address assignment stay > the same as before this change. > > Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> > --- > hw/mem/memory-device.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c > index df3261b32a..df4e338b83 100644 > --- a/hw/mem/memory-device.c > +++ b/hw/mem/memory-device.c > @@ -180,6 +180,8 @@ static uint64_t memory_device_get_free_addr(MachineState *ms, > range_make_empty(&new); > break; > } > + } else if (range_lob(&tmp) > range_upb(&new)) { > + break; > } > } > > Reviewed-by: David Hildenbrand <david@redhat.com> -- Thanks, David / dhildenb ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] memory-device: break the loop if tmp exceed the hinted range 2019-07-30 0:37 ` [Qemu-devel] [PATCH v2 2/2] memory-device: break the loop if tmp exceed the hinted range Wei Yang 2019-07-30 7:38 ` David Hildenbrand @ 2019-07-30 9:30 ` Igor Mammedov 1 sibling, 0 replies; 18+ messages in thread From: Igor Mammedov @ 2019-07-30 9:30 UTC (permalink / raw) To: Wei Yang; +Cc: mst, qemu-devel, david On Tue, 30 Jul 2019 08:37:40 +0800 Wei Yang <richardw.yang@linux.intel.com> wrote: > The memory-device list built by memory_device_build_list is ordered by > its address, this means if the tmp range exceed the hinted range, all > the following range will not overlap with it. > > And this won't change default pc-dimm mapping and address assignment stay > the same as before this change. > > Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> > --- > hw/mem/memory-device.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c > index df3261b32a..df4e338b83 100644 > --- a/hw/mem/memory-device.c > +++ b/hw/mem/memory-device.c > @@ -180,6 +180,8 @@ static uint64_t memory_device_get_free_addr(MachineState *ms, > range_make_empty(&new); > break; > } > + } else if (range_lob(&tmp) > range_upb(&new)) { > + break; > } > } > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] refine memory_device_get_free_addr 2019-07-30 0:37 [Qemu-devel] [PATCH v2 0/2] refine memory_device_get_free_addr Wei Yang 2019-07-30 0:37 ` [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to use goto for the last check Wei Yang 2019-07-30 0:37 ` [Qemu-devel] [PATCH v2 2/2] memory-device: break the loop if tmp exceed the hinted range Wei Yang @ 2019-09-13 23:47 ` Wei Yang 2019-09-14 19:40 ` Michael S. Tsirkin 2 siblings, 1 reply; 18+ messages in thread From: Wei Yang @ 2019-09-13 23:47 UTC (permalink / raw) To: Wei Yang; +Cc: imammedo, mst, qemu-devel, david On Tue, Jul 30, 2019 at 08:37:38AM +0800, Wei Yang wrote: >When we iterate the memory-device list to get the available range, it is not >necessary to iterate the whole list. > >1) no more overlap for hinted range if tmp exceed it > >v2: > * remove #2 as suggested by Igor and David > * add some comment to inform address assignment stay the same as before > this change > >Wei Yang (2): > memory-device: not necessary to use goto for the last check > memory-device: break the loop if tmp exceed the hinted range > > hw/mem/memory-device.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > Would someone take this patch set? >-- >2.17.1 > -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] refine memory_device_get_free_addr 2019-09-13 23:47 ` [Qemu-devel] [PATCH v2 0/2] refine memory_device_get_free_addr Wei Yang @ 2019-09-14 19:40 ` Michael S. Tsirkin 2019-10-12 9:02 ` Wei Yang 0 siblings, 1 reply; 18+ messages in thread From: Michael S. Tsirkin @ 2019-09-14 19:40 UTC (permalink / raw) To: Wei Yang; +Cc: imammedo, david, Wei Yang, ehabkost, qemu-devel On Fri, Sep 13, 2019 at 11:47:46PM +0000, Wei Yang wrote: > On Tue, Jul 30, 2019 at 08:37:38AM +0800, Wei Yang wrote: > >When we iterate the memory-device list to get the available range, it is not > >necessary to iterate the whole list. > > > >1) no more overlap for hinted range if tmp exceed it > > > >v2: > > * remove #2 as suggested by Igor and David > > * add some comment to inform address assignment stay the same as before > > this change > > > >Wei Yang (2): > > memory-device: not necessary to use goto for the last check > > memory-device: break the loop if tmp exceed the hinted range > > > > hw/mem/memory-device.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > Would someone take this patch set? yes looks good to me too. Eduardo? > >-- > >2.17.1 > > > > -- > Wei Yang > Help you, Help me ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] refine memory_device_get_free_addr 2019-09-14 19:40 ` Michael S. Tsirkin @ 2019-10-12 9:02 ` Wei Yang 2019-10-14 15:05 ` Eduardo Habkost 0 siblings, 1 reply; 18+ messages in thread From: Wei Yang @ 2019-10-12 9:02 UTC (permalink / raw) To: Michael S. Tsirkin Cc: ehabkost, david, qemu-devel, Wei Yang, Wei Yang, imammedo On Sat, Sep 14, 2019 at 03:40:41PM -0400, Michael S. Tsirkin wrote: >On Fri, Sep 13, 2019 at 11:47:46PM +0000, Wei Yang wrote: >> On Tue, Jul 30, 2019 at 08:37:38AM +0800, Wei Yang wrote: >> >When we iterate the memory-device list to get the available range, it is not >> >necessary to iterate the whole list. >> > >> >1) no more overlap for hinted range if tmp exceed it >> > >> >v2: >> > * remove #2 as suggested by Igor and David >> > * add some comment to inform address assignment stay the same as before >> > this change >> > >> >Wei Yang (2): >> > memory-device: not necessary to use goto for the last check >> > memory-device: break the loop if tmp exceed the hinted range >> > >> > hw/mem/memory-device.c | 3 ++- >> > 1 file changed, 2 insertions(+), 1 deletion(-) >> > >> >> Would someone take this patch set? > >yes looks good to me too. >Eduardo? > Hmm... I don't see this any where. May I ask the status? >> >-- >> >2.17.1 >> > >> >> -- >> Wei Yang >> Help you, Help me -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] refine memory_device_get_free_addr 2019-10-12 9:02 ` Wei Yang @ 2019-10-14 15:05 ` Eduardo Habkost 2019-10-14 22:00 ` Wei Yang 0 siblings, 1 reply; 18+ messages in thread From: Eduardo Habkost @ 2019-10-14 15:05 UTC (permalink / raw) To: Wei Yang; +Cc: imammedo, Wei Yang, david, qemu-devel, Michael S. Tsirkin On Sat, Oct 12, 2019 at 05:02:09PM +0800, Wei Yang wrote: > On Sat, Sep 14, 2019 at 03:40:41PM -0400, Michael S. Tsirkin wrote: > >On Fri, Sep 13, 2019 at 11:47:46PM +0000, Wei Yang wrote: > >> On Tue, Jul 30, 2019 at 08:37:38AM +0800, Wei Yang wrote: > >> >When we iterate the memory-device list to get the available range, it is not > >> >necessary to iterate the whole list. > >> > > >> >1) no more overlap for hinted range if tmp exceed it > >> > > >> >v2: > >> > * remove #2 as suggested by Igor and David > >> > * add some comment to inform address assignment stay the same as before > >> > this change > >> > > >> >Wei Yang (2): > >> > memory-device: not necessary to use goto for the last check > >> > memory-device: break the loop if tmp exceed the hinted range > >> > > >> > hw/mem/memory-device.c | 3 ++- > >> > 1 file changed, 2 insertions(+), 1 deletion(-) > >> > > >> > >> Would someone take this patch set? > > > >yes looks good to me too. > >Eduardo? > > > > Hmm... I don't see this any where. May I ask the status? Sorry, I hadn't seen Michael's message. Queued on machine-next. Thanks! -- Eduardo ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] refine memory_device_get_free_addr 2019-10-14 15:05 ` Eduardo Habkost @ 2019-10-14 22:00 ` Wei Yang 0 siblings, 0 replies; 18+ messages in thread From: Wei Yang @ 2019-10-14 22:00 UTC (permalink / raw) To: Eduardo Habkost Cc: david, Michael S. Tsirkin, qemu-devel, Wei Yang, Wei Yang, imammedo On Mon, Oct 14, 2019 at 12:05:47PM -0300, Eduardo Habkost wrote: >On Sat, Oct 12, 2019 at 05:02:09PM +0800, Wei Yang wrote: >> On Sat, Sep 14, 2019 at 03:40:41PM -0400, Michael S. Tsirkin wrote: >> >On Fri, Sep 13, 2019 at 11:47:46PM +0000, Wei Yang wrote: >> >> On Tue, Jul 30, 2019 at 08:37:38AM +0800, Wei Yang wrote: >> >> >When we iterate the memory-device list to get the available range, it is not >> >> >necessary to iterate the whole list. >> >> > >> >> >1) no more overlap for hinted range if tmp exceed it >> >> > >> >> >v2: >> >> > * remove #2 as suggested by Igor and David >> >> > * add some comment to inform address assignment stay the same as before >> >> > this change >> >> > >> >> >Wei Yang (2): >> >> > memory-device: not necessary to use goto for the last check >> >> > memory-device: break the loop if tmp exceed the hinted range >> >> > >> >> > hw/mem/memory-device.c | 3 ++- >> >> > 1 file changed, 2 insertions(+), 1 deletion(-) >> >> > >> >> >> >> Would someone take this patch set? >> > >> >yes looks good to me too. >> >Eduardo? >> > >> >> Hmm... I don't see this any where. May I ask the status? > >Sorry, I hadn't seen Michael's message. Queued on machine-next. >Thanks! > Thanks~ have a nice day. >-- >Eduardo -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2019-10-14 22:02 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-30 0:37 [Qemu-devel] [PATCH v2 0/2] refine memory_device_get_free_addr Wei Yang 2019-07-30 0:37 ` [Qemu-devel] [PATCH v2 1/2] memory-device: not necessary to use goto for the last check Wei Yang 2019-08-08 1:42 ` Zeng, Star 2019-08-08 2:13 ` Wei Yang 2019-08-08 2:30 ` Zeng, Star 2019-08-08 2:38 ` Wei Yang 2019-08-08 7:06 ` David Hildenbrand 2019-08-19 2:38 ` Wei Yang 2019-08-19 5:32 ` Zeng, Star 2019-08-19 6:36 ` Wei Yang 2019-07-30 0:37 ` [Qemu-devel] [PATCH v2 2/2] memory-device: break the loop if tmp exceed the hinted range Wei Yang 2019-07-30 7:38 ` David Hildenbrand 2019-07-30 9:30 ` Igor Mammedov 2019-09-13 23:47 ` [Qemu-devel] [PATCH v2 0/2] refine memory_device_get_free_addr Wei Yang 2019-09-14 19:40 ` Michael S. Tsirkin 2019-10-12 9:02 ` Wei Yang 2019-10-14 15:05 ` Eduardo Habkost 2019-10-14 22:00 ` Wei Yang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).