netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@nvidia.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: Yishai Hadas <yishaih@nvidia.com>,
	"Tian, Kevin" <kevin.tian@intel.com>,
	"saeedm@nvidia.com" <saeedm@nvidia.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"kuba@kernel.org" <kuba@kernel.org>,
	"Martins, Joao" <joao.m.martins@oracle.com>,
	"leonro@nvidia.com" <leonro@nvidia.com>,
	"maorg@nvidia.com" <maorg@nvidia.com>,
	"cohuck@redhat.com" <cohuck@redhat.com>
Subject: Re: [PATCH V2 vfio 03/11] vfio: Introduce DMA logging uAPIs
Date: Tue, 26 Jul 2022 12:04:52 -0300	[thread overview]
Message-ID: <20220726150452.GE4438@nvidia.com> (raw)
In-Reply-To: <20220726080320.798129d5.alex.williamson@redhat.com>

On Tue, Jul 26, 2022 at 08:03:20AM -0600, Alex Williamson wrote:

> I raised the same concern myself, the reason for having a limit is
> clear, but focusing on a single use case and creating an arbitrary
> "good enough" limit that isn't exposed to userspace makes this an
> implementation detail that can subtly break userspace.  For instance,
> what if userspace comes to expect the limit is 1000 and we decide to be
> even more strict?  If only a few 10s of entries are used, why isn't 100
> more than sufficient?  

So lets use the number of elements that will fit in PAGE_SIZE as the
guideline. It means the kernel can memdup the userspace array into a
single kernel page of memory to process it, which seems reasonably
future proof in that we won't need to make it lower. Thus we can
promise we won't make it smaller.

However, remember, this isn't even the real device limit - this is
just the limit that the core kernel code will accept to marshal the
data to pass internally the driver.

I fully expect that the driver will still refuse ranges in certain
configurations even if they can be marshaled.

This is primarily why I don't think it make sense to expose some
internal limit that is not even the real "will the call succeed"
parameters.

The API is specifically designed as 'try and fail' to allow the
drivers flexibility it how they map the requested ranges to their
internal operations.

> We change it, we break userspace.  OTOH, if we simply make use of
> that reserved field to expose the limit, now we have a contract with
> userspace and we can change our implementation because that detail
> of the implementation is visible to userspace.  Thanks,

I think this is not correct, just because we made it discoverable does
not absolve the kernel of compatibility. If we change the limit, eg to
1, and a real userspace stops working then we still broke userspace.

Complaining that userspace does not check the discoverable limit
doesn't help matters - I seem to remember Linus has written about this
in recent times even.

So, it is ultimately not different from 'try and fail', unless we
implement some algorithm in qemu - an algorithm that would duplicate
the one we already have in the kernel :\

Jason

  reply	other threads:[~2022-07-26 15:04 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-14  8:12 [PATCH V2 vfio 00/11] Add device DMA logging support for mlx5 driver Yishai Hadas
2022-07-14  8:12 ` [PATCH V2 vfio 01/11] net/mlx5: Introduce ifc bits for page tracker Yishai Hadas
2022-07-21  8:28   ` Tian, Kevin
2022-07-21  8:43     ` Yishai Hadas
2022-07-14  8:12 ` [PATCH V2 vfio 02/11] net/mlx5: Query ADV_VIRTUALIZATION capabilities Yishai Hadas
2022-07-14  8:12 ` [PATCH V2 vfio 03/11] vfio: Introduce DMA logging uAPIs Yishai Hadas
2022-07-18 22:29   ` Alex Williamson
2022-07-19  1:39     ` Tian, Kevin
2022-07-19  5:40       ` Kirti Wankhede
2022-07-19  7:49     ` Yishai Hadas
2022-07-19 19:57       ` Alex Williamson
2022-07-19 20:18         ` Jason Gunthorpe
2022-07-21  8:45   ` Tian, Kevin
2022-07-21 12:05     ` Jason Gunthorpe
2022-07-25  7:20       ` Tian, Kevin
2022-07-25 14:33         ` Jason Gunthorpe
2022-07-26  7:07           ` Tian, Kevin
     [not found]     ` <56bd06d3-944c-18da-86ed-ae14ce5940b7@nvidia.com>
2022-07-25  7:30       ` Tian, Kevin
2022-07-26  8:37         ` Yishai Hadas
2022-07-26 14:03           ` Alex Williamson
2022-07-26 15:04             ` Jason Gunthorpe [this message]
2022-07-28  4:05               ` Tian, Kevin
2022-07-28 12:06                 ` Jason Gunthorpe
2022-07-29  3:01                   ` Tian, Kevin
2022-07-29 14:11                     ` Jason Gunthorpe
2022-07-14  8:12 ` [PATCH V2 vfio 04/11] vfio: Move vfio.c to vfio_main.c Yishai Hadas
2022-07-14  8:12 ` [PATCH V2 vfio 05/11] vfio: Add an IOVA bitmap support Yishai Hadas
2022-07-18 22:30   ` Alex Williamson
2022-07-18 22:46     ` Jason Gunthorpe
2022-07-19 19:01   ` Alex Williamson
2022-07-20  1:57     ` Joao Martins
2022-07-20 16:47       ` Alex Williamson
2022-07-20 17:27         ` Jason Gunthorpe
2022-07-20 18:16         ` Joao Martins
2022-07-14  8:12 ` [PATCH V2 vfio 06/11] vfio: Introduce the DMA logging feature support Yishai Hadas
2022-07-18 22:30   ` Alex Williamson
2022-07-19  9:19     ` Yishai Hadas
2022-07-19 19:25       ` Alex Williamson
2022-07-19 20:08         ` Jason Gunthorpe
2022-07-21  8:54           ` Tian, Kevin
2022-07-21 11:50             ` Jason Gunthorpe
2022-07-25  7:38               ` Tian, Kevin
2022-07-25 14:37                 ` Jason Gunthorpe
2022-07-26  7:34                   ` Tian, Kevin
2022-07-26 15:12                     ` Jason Gunthorpe
2022-07-14  8:12 ` [PATCH V2 vfio 07/11] vfio/mlx5: Init QP based resources for dirty tracking Yishai Hadas
2022-07-14  8:12 ` [PATCH V2 vfio 08/11] vfio/mlx5: Create and destroy page tracker object Yishai Hadas
2022-07-14  8:12 ` [PATCH V2 vfio 09/11] vfio/mlx5: Report dirty pages from tracker Yishai Hadas
2022-07-14  8:12 ` [PATCH V2 vfio 10/11] vfio/mlx5: Manage error scenarios on tracker Yishai Hadas
2022-07-14  8:12 ` [PATCH V2 vfio 11/11] vfio/mlx5: Set the driver DMA logging callbacks Yishai Hadas
2022-07-21  8:26 ` [PATCH V2 vfio 00/11] Add device DMA logging support for mlx5 driver Tian, Kevin
2022-07-21  8:55   ` Yishai Hadas

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=20220726150452.GE4438@nvidia.com \
    --to=jgg@nvidia.com \
    --cc=alex.williamson@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=joao.m.martins@oracle.com \
    --cc=kevin.tian@intel.com \
    --cc=kuba@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=leonro@nvidia.com \
    --cc=maorg@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=saeedm@nvidia.com \
    --cc=yishaih@nvidia.com \
    /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 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).