[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] PCI: avoid bogus calls to get_pseg()


  • To: Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
  • Date: Wed, 10 Aug 2022 12:13:04 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=us74gs0zVWl2+1PrLlCRJesW7as0Tfs310n0PoQ/PMQ=; b=kMrLKgR1ORDaHipOS7SR6LoFGJliUv9hvx0zMFNplp85alrq6Rr/sJp68PP+O0VaKuVD6cL0+3jGpJT/00Hwl+I+U7kb36yXP+WM39N5XGYfs71+Mf7OiWprqi1coqkYJK5go/T3hpbCLxaL6xvIgPj7N9Ql/d3aofbjSmeE6Z8ffJvyMMUS/c5AnNV6hP5s65XxOQOvRv8Jk4OINAc9d7GLgkQNxOp2mRx8f8NqUCNKeR//WgWMPEVaQYqFf2+PdojfySqMtQGl5AkG3c4C4TOAhxeq7w8uTujbVa1VXD0NUDb7fUNBaG6yuzoqng6WNSxyq01D7tXzV3g+w3tDGA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=nMfAtpmth/uH/KA9iR2b0YJH0kZP68NHfCAE3jYHfCXeXE5YboCws4/T/ca/8sfhtCUzb8E70Bg/5pV6QmLNfqIOFZW91Hei5MvFyUHaOqv6GJhU7cAbvgVSLLool3i4L9xBUtqFFGyYrDBTmGsy94OfjZ2RTmCL5uGE/MZb8h58lkt9VaAOm0MiJ93opUI1lKdUKmM+e6hRemky1ZdDixwd5VIT8+hs+S9G909jLGrWXSJte4FSllr26BI+itEq7QMvPKqaHvPWmv/fGJsTQL63OyftACXOedXNqT9KlWPb0zYDbq64SfJLuj0oWzWGJ0sDTtJjWm8OqB/ysR7OLQ==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Roger Pau Monne <roger.pau@xxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>
  • Delivery-date: Wed, 10 Aug 2022 12:13:21 +0000
  • Ironport-data: A9a23:Dt/Miq02gjB2PZdiW/bD5ctwkn2cJEfYwER7XKvMYLTBsI5bpzRWx msdC2qFM6qKYTCmKYgjbIng8hxU6JOHy9U3SgFopC1hF35El5HIVI+TRqvS04J+DSFhoGZPt Zh2hgzodZhsJpPkjk7xdOKn9RGQ7InQLpLkEunIJyttcgFtTSYlmHpLlvUwx4VlmrBVOSvU0 T/Ji5CZaQTNNwJcaDpOsfrT8Us35pwehRtD1rAATaET1LPhvyF94KI3fcmZM3b+S49IKe+2L 86rIGaRpz6xE78FU7tJo56jGqE4aue60Tum0xK6b5OKkBlazhHe545gXBYqheW7vB3S9zx54 I0lWZVd0m7FNIWU8AgWe0Ew/y2TocSqUVIISJSymZX78qHIT5fj689BMUYcMI5ExqVmB1hrp dECLDEyMznW0opawJrjIgVtruIKCZGxea864TRnxzyfCus6S5feRamM/cVfwDo7msFJG7DZe tYdbj1sKh/HZnWjOH9OUM54wLju2ye5L2QwRFG9/MLb50D6ygBr3aerG93SYtGQHu1en1qCp 3KA9GP8av0fHIPPk2LUoynz7gPJtRiqG5AoEpGoz6FvhXKo4nMBK0JLWETu9JFVjWb7AbqzM Xc8+CAjsKwz/0yDVcTmUluzp3vslg4RXZ9cHvM37CmJy7HI+ECJC24cVDlDZdc68sgsSlQCz laUm/v5CDopt6eaIU9x7Z+RpDK2fCQQdmkLYHdeSRNfu4W65oYukhjIU9BvVravicH4Ei3xx DbMqzUig7IUjogA0KDTEU37vg9Ab6PhFmYdjjg7lEr8hu+lTOZJv7CV1GU=
  • Ironport-hdrordr: A9a23:xbe7Wa5JegNOdau2oAPXweCCI+orL9Y04lQ7vn2ZFiY5TiXIra qTdaogviMc0AxhI03Jmbi7Scq9qeu1z+853WBjB8bZYOCAghrlEGgC1/qp/9SEIUHDH4FmpM BdmsRFaeEYSGIK9foSgzPIXOrIouP3lpxA7N22pxgCcegpUdAY0+4TMHf4LqQCfngjOXNPLu v42iMonVqdUEVSSv7+KmgOXuDFqdGOvJX6YSQeDxpixBiSgSiu4LvaFQHd+hsFSTtAzZor7G CAymXCl+SemsD+7iWZ+37Y7pxQltek4txfBPaUgsxQBiTwhh2ubIFBXaTHmDwuuumg5Hsjjd GJiRY9OMZY7W/XYwiO0FXQ8jil9Axrx27pyFeej3emi9f+XigGB81Igp8cWgfF6mI71esMk5 5j7ia8jd56HBnAlCPy65zjTBdxjHe5pnIkjKo6k2Ffa40Dc7VcxLZvvn+9Ua1wWR4S2rpXV9 WGP/usosq+tmnqNkwxi1MfhOBEmE5DRituDHJy4fB9mAIm4UyRh3FouPD32E1wtK7VAqM0md gteM5T5c5zZ95TYqRnCOgbR8yrTmTLXBLXKWqXZU/qDacdJhv22tfKCZgOlZaXkaYzve0PsY WEVEkduX85ekroB8HL1JpX8grVSGH4WTj20MlR65Vwp7W5HdPQQGa+YUFrl9Hlr+QUA8XdVf r2MJVKA+X7JW+rHYpSxQXxV5RbNHFbWswIvdQwXU6Iv6vwW8XXn/2edOyWKKvmED4iVG+6Cn wfXCLrLMEF9UyvUm+QummkZ5osQD2LwXtdKtmrwwFI8vl9CmRliHlntX2poseWNDZFrqs6OE NjPbKPqNLImVWL
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHYrAfYR6vkCfF7n0qu0r3Ze+QmG62oDQUA
  • Thread-topic: [PATCH] PCI: avoid bogus calls to get_pseg()

On 09/08/2022 16:50, Jan Beulich wrote:
> When passed -1, the function (taking a u16) will look for segment
> 0xffff, which might exist. If it exists, we may find (return) the wrong
> device.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> An alternative would be to declare that both functions cannot be called
> with "wildcards" anymore. The last such use went away with f591755823a7
> ("IOMMU/PCI: don't let domain cleanup continue when device de-assignment
> failed") afaict.

The way wildcards were used before were always bogus IMO.

I suggest we take this opportunity to remove the ability to re-introduce
that anti-pattern.

> Each time I look at this pair of functions I wonder why we have two
> copies of almost the same code (with a curious difference of only one
> having ASSERT(pcidevs_locked())). Any opinions on deleting either one,
> subsuming its functionality into the other one by allowing the domain
> pointer to be NULL to signify "any"?

I'm in two minds about this.  Because they are the same, they ought to
be deduped.

Except they're both insane and both want changing to a less silly
datastructure, at which point they will be different.

It is a total waste to do an O(n) loop over all PCI devices in the
system checking for equality to single device (and in the domain case,
assignment to the domain).  The domain variant should loop over the pci
devices in that domain, because it is guaranteed to be a shorter list
than all devices.

The global lookup probably wants to investigate a more efficient
datastructure because I bet this is a hotpath.

~Andrew

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.