Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 12 May 2023 16:02:44 +0800
From:   Yan Zhao <yan.y.zhao@intel.com>
To:     Alex Williamson <alex.williamson@redhat.com>
CC:     Jason Gunthorpe <jgg@nvidia.com>, <kvm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <kevin.tian@intel.com>,
        <yishaih@nvidia.com>, <shameerali.kolothum.thodi@huawei.com>,
        =?iso-8859-1?Q?C=E9dric?= Le Goater <clg@redhat.com>
Subject: Re: [PATCH] vfio/pci: take mmap write lock for io_remap_pfn_range
Message-ID: <ZF3ypHwBaFljso+Z@yzhao56-desk.sh.intel.com>
Reply-To: Yan Zhao <yan.y.zhao@intel.com>
References: <20230508125842.28193-1-yan.y.zhao@intel.com>
 <ZFkn3q45RUJXMS+P@nvidia.com>
 <20230508145715.630fe3ae.alex.williamson@redhat.com>
 <ZFwBYtjL1V0r5WW3@nvidia.com>
 <20230511100706.63d420db.alex.williamson@redhat.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230511100706.63d420db.alex.williamson@redhat.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?ai3abgDecYwaehupD+tV48mgxWk2RlOEnp+wAqQbtYMVqnNVpQix04jbr8rO?=
 =?us-ascii?Q?VUS9re+LpsgL7crZfduXAjjvZwrfULR5jUIiTQL8sdkUER4/RFK1mZL0jmXI?=
 =?us-ascii?Q?SIJDbB8vVVsGcPy/F1CWQl4kyMNAEI1hMXG0b0j/XjImbQfy4hgjtEvLQmW8?=
 =?us-ascii?Q?FLZ21o7enASG65nhvHyzYWYra4fj7c1xDWnjSvrD7t+GY6uhllgsgLrDUZXH?=
 =?us-ascii?Q?RjaiCbPRQrMBRrCvd2/RAO3kljaJyme3u5OgKXujqtN+hsPD7zoCf7ob2PpW?=
 =?us-ascii?Q?h5tJ30S54vl6Fwx9hNqh5qfz5WEUyU/lxwDqS7kbW5aLE/4VVFyWVtncnQuM?=
 =?us-ascii?Q?rZ7hXezmoouV0guKO1zcnEi0zRp9zkS3FGilBuDNs7XoZ6QevXe/P+U82anZ?=
 =?us-ascii?Q?xOUlCMRsAw0+i7qUydDdOCrPh9lL7P4Hx5TvnwdoITjk0K97pUYSiUDDTEro?=
 =?us-ascii?Q?NHot0z5K/8phIKCRqs+kU57x3b5brVdmyh35hOaAlJs8RN2+at1kkLIGwMFc?=
 =?us-ascii?Q?EV6AhuFkC0aQj6HsKTSrwsgsSr/LaOM1ht3TAF1KFVhxqLvb7dbWGGiB3kYx?=
 =?us-ascii?Q?efZQKhkI0wNGbJitzMTQlGKorW6w9bySFEH99FMbx5/TRFm3yxt8d/mqoaUw?=
 =?us-ascii?Q?H24+RppvB5pMVC1659M05Gs6pg1M7uVN2PyJ11sy+w/um5Kvu7TsnWngl7M6?=
 =?us-ascii?Q?BAyXoATqBvLqmFOW8qC04YDtdv6r2bNSpryM6e22bbZXltdJJDtMYBgRH+ui?=
 =?us-ascii?Q?a8kBrrxqTX4BKsb69uAsQ7XS5H3sbnLzys8xEa35rPABtEmaIySZWnt+suul?=
 =?us-ascii?Q?P16TXBkkpfuBgmQfXc+aFQbosxMQeMeJ5ylndXA35owEW0Oo1GbET6p2JqYX?=
 =?us-ascii?Q?HYv8kJl8GW7G5JGyz7LbKWg3KhG9E/+kYC2Mkb5sFpvO5+eDOmetLcE/bMxu?=
 =?us-ascii?Q?DBsDD4KZckI40iyuDpP+Wut5PDzSDVex2eyQfDb8n3o/NBV2Bqplk9gURrnh?=
 =?us-ascii?Q?is3tmXBQ2YAUix2on5+6w8W0/vNbI4k0GLYCUfmL1fLY42yc1BseoxwJ8s5f?=
 =?us-ascii?Q?FR2xZOMWYifS0aecsWbkqq9j0rEU9D/C1GgR8OdvtDAGK0V1Mop5sTZBWNqU?=
 =?us-ascii?Q?QL8NuJQOQIxn956es+YLGjNWhP+ldOfACXvJ7LpnvrcFACJbZE7VPeXlgmPF?=
 =?us-ascii?Q?aXtam2ZvtY/P1YDbTd+qNzJ4ZD0/PrbjW/rRbn7plZ1s0RK+8Dt4Cb/fb1+4?=
 =?us-ascii?Q?ZVcQ0OTjmMvLSqq91wsEjrdIO/C8eK/wtbXeYFoibqE1B0bvdDXDjKDoQ9B/?=
 =?us-ascii?Q?Q8pryZBjszMPiu9RvgSwyK6fl+gONyunrdU8Bq2Xk3J/ZiaOI9zcohM5cHhK?=
 =?us-ascii?Q?MGIlXLMq0d2EA8FosXwK6h7UazZW0oWcvKoxwc/J229hom8RyjxlU1/ie7uJ?=
 =?us-ascii?Q?FgMn0wCIVl6Uo78rV6r1K8bJe63VB8O6iTWxN0N4GRVhB9fhkfUCuxij4SPo?=
 =?us-ascii?Q?6VY/p3qlKfhXWUJGWfrQsaR9BKvKzztR+1ZBy/mNoZoeCke5ZAqDoRoX78q8?=
 =?us-ascii?Q?1TOCRDohahWaSc8xI728/jXmJZZ4gePUIxFDrQfB?=
X-MS-Exchange-CrossTenant-Network-Message-Id: bc0f659f-2e38-41df-583e-08db52c2c519
X-MS-Exchange-CrossTenant-AuthSource: DS7PR11MB5966.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 May 2023 08:27:48.8681
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Idygrtz89Z8shCHicOZdDSjpoJQK4WXF6uN8IcgJ0TD1ZRyXUuLvXnVkxLSflDkjLDTXscLUZ4h1CO4Dqxw9kw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB7246
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, May 11, 2023 at 10:07:06AM -0600, Alex Williamson wrote:
> On Wed, 10 May 2023 17:41:06 -0300
> Jason Gunthorpe <jgg@nvidia.com> wrote:
> 
> > On Mon, May 08, 2023 at 02:57:15PM -0600, Alex Williamson wrote:
> > 
> > > We already try to set the flags in advance, but there are some
> > > architectural flags like VM_PAT that make that tricky.  Cedric has been
> > > looking at inserting individual pages with vmf_insert_pfn(), but that
> > > incurs a lot more faults and therefore latency vs remapping the entire
> > > vma on fault.  I'm not convinced that we shouldn't just attempt to
> > > remove the fault handler entirely, but I haven't tried it yet to know
> > > what gotchas are down that path.  Thanks,  
> > 
> > I thought we did it like this because there were races otherwise with
> > PTE insertion and zapping? I don't remember well anymore.
> 
> TBH, I don't recall if we tried a synchronous approach previously.  The
> benefit of the faulting approach was that we could track the minimum
> set of vmas which are actually making use of the mapping and throw that
> tracking list away when zapping.  Without that, we need to add vmas
> both on mmap and in vm_ops.open, removing only in vm_ops.close, and
> acquire all the proper mm locking for each vma to re-insert the
> mappings.
> 
> > I vaugely remember the address_space conversion might help remove the
> > fault handler?
> 
> Yes, this did remove the fault handler entirely, it's (obviously)
> dropped off my radar, but perhaps in the interim we could switch to
> vmf_insert_pfn() and revive the address space series to eventually
> remove the fault handling and vma list altogether.
> 
> For reference, I think this was the last posting of the address space
> series:
> 
> https://lore.kernel.org/all/162818167535.1511194.6614962507750594786.stgit@omen/

Just took a quick look at this series.
A question is that looks it still needs to call io_remap_pfn_range() in
places like vfio_basic_config_write() for PCI_COMMAND, and device reset,
so mmap write lock is still required around vdev->memory_lock.