All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Parav Pandit <parav@nvidia.com>
Cc: Cornelia Huck <cohuck@redhat.com>,
	"virtio-dev@lists.oasis-open.org"
	<virtio-dev@lists.oasis-open.org>,
	Max Gurtovoy <mgurtovoy@nvidia.com>,
	Shahaf Shuler <shahafs@nvidia.com>, Oren Duer <oren@nvidia.com>
Subject: Re: [PATCH v2] Add device reset timeout field
Date: Fri, 8 Oct 2021 06:12:11 -0400	[thread overview]
Message-ID: <20211008060329-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <PH0PR12MB5481CEA27B18421BB8432AD6DCB19@PH0PR12MB5481.namprd12.prod.outlook.com>

On Thu, Oct 07, 2021 at 05:58:09PM +0000, Parav Pandit wrote:
> > >> > > Third how about making e.g. 0 a special value meaning no limit
> > >> > > wait
> > >> forever?
> > >> > The whole idea is to have some finite/deterministic behavior,
> > >>
> > >> I guess I'm being dense, I just don't yet understand the motivation.
> > >> The cost is difficulty in migration since each and every piece of
> > >> hardware will have a different timeout.
> > >>
> > > Such timeout is already there. Advertising timeout doesn't change the LM
> > > flow.
> > > A migrating device at the destination is anyway out of the reset when
> > > migration occurs.
> > 
> > But it introduces values that may be different between different devices of the
> > same type, no? I guess the destination would need to re-read the value to get
> > the current one. Not an insurmountable problem, but still needs some care.
> >
> When a device migrates to destination, it starts from where the device
> left off on the source side.  So yes, destination side, device must be
> usable (out of reset), and after that its current state will be
> overwritten by the migrating device.

I get what you are trying to say here but it's a hack. Nothing
prevents a reset for driver's internal reasons at any point,
and in particular reset is used e.g. for driver removal.

>  If you ask, does migration
> overwrite the reset timeout register value? I would say no, because
> how long device would take to reset is decided by the destination side
> implementation.

Problem is, driver can cache the value on source. Then it's migrated
and used on destination when driver wants to reset the device.
This can lead to a timeout if the destination does not
finish within the source timeout value.

That's why I ask: why do we bother? What's wrong with just waiting
forever or until user gets tired of this and cancels with CTRL-C?
Is there a use-case where that's not good enough?


> And this is probably yet another good reason to define migratable bits
> of a virtio device in the live migration spec extension.

"migratable bits" being what? non-guest visible device state? Sure, would
be great to have.  Don't think it will help in this instance.

-- 
MST


  parent reply	other threads:[~2021-10-08 10:12 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-06 14:10 [PATCH v2] Add device reset timeout field Parav Pandit
2021-10-06 15:22 ` Michael S. Tsirkin
2021-10-06 16:11   ` Parav Pandit
2021-10-06 20:53     ` Michael S. Tsirkin
2021-10-07  3:42       ` Parav Pandit
2021-10-07 16:10         ` [virtio-dev] " Cornelia Huck
2021-10-07 17:58           ` Parav Pandit
2021-10-08 10:00             ` [virtio-dev] " Cornelia Huck
2021-10-08 10:19               ` Parav Pandit
2021-10-08 10:12             ` Michael S. Tsirkin [this message]
2021-10-08 10:51               ` Parav Pandit
2021-10-08 11:18                 ` [virtio-dev] " Michael S. Tsirkin
2021-10-08 12:55                   ` Parav Pandit
2021-10-08 10:44         ` Michael S. Tsirkin
2021-10-08 10:59           ` Parav Pandit
2021-10-08 11:21             ` Michael S. Tsirkin
2021-10-08 11:45               ` Parav Pandit
2021-10-08 11:47               ` [virtio-dev] " Cornelia Huck
2021-10-08 12:12                 ` Parav Pandit
2021-10-08 12:57                   ` Michael S. Tsirkin
2021-10-08 13:23                     ` Parav Pandit
2021-10-08 23:09                       ` Michael S. Tsirkin
2021-10-11 14:29                         ` Parav Pandit
2021-10-11 14:59                           ` [virtio-dev] " Cornelia Huck
2021-10-11 15:44                             ` Parav Pandit
2021-10-11 16:00                               ` Michael S. Tsirkin
2021-10-12  8:51                                 ` Parav Pandit
2021-10-12  9:01                                   ` Michael S. Tsirkin
2021-10-12  9:12                                     ` Parav Pandit
2021-10-14 17:35                                       ` Parav Pandit
2021-10-14 22:28                                         ` Michael S. Tsirkin
2021-10-15  4:36                                           ` Parav Pandit
2021-10-15  5:15                                             ` [virtio-dev] " Jason Wang
2021-10-15  5:20                                               ` Parav Pandit
2021-10-15  6:40                                                 ` Jason Wang
2021-10-15  6:42                                                 ` Jason Wang
2021-10-15  6:48                                                   ` Parav Pandit
2021-10-15  7:02                                                     ` Jason Wang
2021-10-15  8:21                                                       ` Parav Pandit
2021-10-15  8:42                                                         ` Jason Wang
2021-10-22  7:20                                                           ` Parav Pandit
2021-10-25  5:41                                                             ` Jason Wang
2021-10-25  6:11                                                               ` Parav Pandit
2021-10-26  4:03                                                                 ` Jason Wang
2021-10-27  8:04                                                                   ` Parav Pandit
2021-10-27  8:26                                                                     ` Michael S. Tsirkin
2021-10-28  4:01                                                                       ` Parav Pandit
2021-10-28  5:50                                                                         ` Michael S. Tsirkin
2021-10-28  6:06                                                                           ` Parav Pandit
2021-10-15  6:51                                             ` Cornelia Huck
2021-10-15  8:09                                               ` Parav Pandit
2021-10-15  9:25                                                 ` [virtio-dev] " Cornelia Huck
2021-10-22  6:29                                                   ` Parav Pandit
2021-10-11 16:22                               ` [virtio-dev] " Cornelia Huck
2021-10-12 10:35                                 ` Parav Pandit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211008060329-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=mgurtovoy@nvidia.com \
    --cc=oren@nvidia.com \
    --cc=parav@nvidia.com \
    --cc=shahafs@nvidia.com \
    --cc=virtio-dev@lists.oasis-open.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.