Re: [Xen-devel] [PATCH v4 09/10] IOMMU: propagate IOMMU Device-TLB flush error up to IOMMU suspending

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.05.16 at 05:39, <quan.xu@xxxxxxxxx> wrote:
> On May 10, 2016 5:25 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>> >>> On 06.05.16 at 10:54, <quan.xu@xxxxxxxxx> wrote:
>> > --- a/xen/arch/x86/acpi/power.c
>> > +++ b/xen/arch/x86/acpi/power.c
>> >  static int device_power_down(void)
>> >  {
>> > -    console_suspend();
>> > +    if ( console_suspend() )
>> > +        return TYPE_CONSOLE;
>> 
>> This (together with the resume side) makes me guess that the use of TYPE_ as
>> a prefix confused not just me, but also you:
> 
> Yes,  this is really not a good prefix, and probably pretty bad to use 
> 'ERROR_'.
> What about 'PRIOR_'?  then I also need to adjust  device_power_up() as ...

What about SAVED_?

>> > -    time_suspend();
>> > +    if ( time_suspend() )
>> > +        return TYPE_TIME;
>> >
>> > -    i8259A_suspend();
>> > +    if ( i8259A_suspend() )
>> > +        return TYPE_I8259A;
>> >
>> > +    /* ioapic_suspend should never fail */
>> >      ioapic_suspend();
>> 
>> The comment is bogus: "should" means it can in theory. Yet the function
>> having void return type means it just cannot fail.
>> 
> 
> I'll use 'cannot', instead of 'should'.
> Another question, I check the code again, and the rest of the functions 
> (console_suspend/ time_suspend/ i8259A_suspend / ioapic_suspend / 
> lapic_suspend ), in device_power_down(), always returned '0'.
> Maybe I need to fix these functions  annotation from 'int' to 'void', and 
> then I can add a comment on the device_power_down().  

Please don't. Generally the possibility of failure exists, and hence if
functions have already been written to account for that, we shouldn't
strip that capability out.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel