Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1608238

Re: [PATCH v2] KVM: kvm_io_bus_unregister_dev() should never fail

From David Hildenbrand <david@redhat.com>
Newsgroups linux.kernel
Subject Re: [PATCH v2] KVM: kvm_io_bus_unregister_dev() should never fail
Date 2017-03-24 10:00 +0100
Message-ID <toteG-7pT-31@gated-at.bofh.it> (permalink)
References <toeIG-5py-23@gated-at.bofh.it> <tohQf-7EA-35@gated-at.bofh.it> <tot51-7ku-41@gated-at.bofh.it>
Organization Red Hat GmbH

Show all headers | View raw


>>> -             return r;
>>> +     if (i == bus->dev_count)
>>> +             return;
>>>
>>>       new_bus = kmalloc(sizeof(*bus) + ((bus->dev_count - 1) *
>>>                         sizeof(struct kvm_io_range)), GFP_KERNEL);
>>> -     if (!new_bus)
>>> -             return -ENOMEM;
>>> +     if (!new_bus)  {
>>> +             pr_err("kvm: failed to shrink bus, removing it completely\n");
>>> +             goto broken;
>>
>> The guest will fail in mysterious ways, if you do this (and
>> io_bus_unregister_dev can be called during runtime): in-kernel device
>> accesses will fail with unknown behaviour in the guest.

Actually, the next access to the BUS should result in -ENOMEM. And the
error message should be enough to then figure out what went wrong.
However, to hit this scenario at all feels very unlikely. So I would
like to avoid advanced allocation schemes.

>>
>> Can't you retry a handful of times with GFP_KERNEL before switching to GFP_ATOMIC?
>> (which in case fails the machine is likely to be crashing soon).
> 
> The process can run in a cgroup, then kmalloc failure has nothing to
> do with overall memory consumption. Machine can be perfectly fine.
> Also, this very process can be chosen as an OOM kill target, then it
> needs to gracefully deal with kmalloc failure and proceed to a
> termination point.
> Generally retrying something in a loop does not look like a solid plan
> to deal with errors.
> 

I agree, looping on memory allocations never feels like the right thing
to do.

-- 

Thanks,

David

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v2] KVM: kvm_io_bus_unregister_dev() should never fail David Hildenbrand <david@redhat.com> - 2017-03-23 18:30 +0100
  Re: [PATCH v2] KVM: kvm_io_bus_unregister_dev() should never fail Cornelia Huck <cornelia.huck@de.ibm.com> - 2017-03-23 18:40 +0100
  Re: [PATCH v2] KVM: kvm_io_bus_unregister_dev() should never fail Marcelo Tosatti <mtosatti@redhat.com> - 2017-03-23 21:50 +0100
    Re: [PATCH v2] KVM: kvm_io_bus_unregister_dev() should never fail Dmitry Vyukov <dvyukov@google.com> - 2017-03-24 09:50 +0100
      Re: [PATCH v2] KVM: kvm_io_bus_unregister_dev() should never fail David Hildenbrand <david@redhat.com> - 2017-03-24 10:00 +0100
        Re: [PATCH v2] KVM: kvm_io_bus_unregister_dev() should never fail Cornelia Huck <cornelia.huck@de.ibm.com> - 2017-03-24 10:40 +0100
      Re: [PATCH v2] KVM: kvm_io_bus_unregister_dev() should never fail Marcelo Tosatti <mtosatti@redhat.com> - 2017-03-24 15:10 +0100
        Re: [PATCH v2] KVM: kvm_io_bus_unregister_dev() should never fail Cornelia Huck <cornelia.huck@de.ibm.com> - 2017-03-24 15:50 +0100

csiph-web