From: Zhangfei Gao <zhangfei.gao@linaro.org>
To: "zhangfei.gao@foxmail.com" <zhangfei.gao@foxmail.com>
Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>,
Fenghua Yu <fenghua.yu@intel.com>,
Dave Hansen <dave.hansen@intel.com>,
Joerg Roedel <joro@8bytes.org>,
Ravi V Shankar <ravi.v.shankar@intel.com>,
Tony Luck <tony.luck@intel.com>, Ashok Raj <ashok.raj@intel.com>,
Peter Zijlstra <peterz@infradead.org>,
Dave Hansen <dave.hansen@linux.intel.com>, x86 <x86@kernel.org>,
linux-kernel <linux-kernel@vger.kernel.org>,
iommu <iommu@lists.linux-foundation.org>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
Andy Lutomirski <luto@kernel.org>,
Josh Poimboeuf <jpoimboe@redhat.com>,
Thomas Gleixner <tglx@linutronix.de>,
will@kernel.org, robin.murphy@arm.com
Subject: Re: [PATCH v4 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit
Date: Sun, 24 Apr 2022 10:58:10 +0800 [thread overview]
Message-ID: <CABQgh9GzuQqsuPVyur+QurN8p+1BciMfOw0TccwiZSvvtrcowg@mail.gmail.com> (raw)
In-Reply-To: <tencent_2922DAB6F3D5789A1CD3A21A843B4007ED09@qq.com>
On Sat, 23 Apr 2022 at 19:13, zhangfei.gao@foxmail.com
<zhangfei.gao@foxmail.com> wrote:
>
> Hi, Jean
>
> On 2022/4/22 下午11:50, Jean-Philippe Brucker wrote:
> > On Fri, Apr 22, 2022 at 09:15:01PM +0800, zhangfei.gao@foxmail.com wrote:
> >>> I'm trying to piece together what happens from the kernel point of view.
> >>>
> >>> * master process with mm A opens a queue fd through uacce, which calls
> >>> iommu_sva_bind_device(dev, A) -> PASID 1
> >>>
> >>> * master forks and exits. Child (daemon) gets mm B, inherits the queue fd.
> >>> The device is still bound to mm A with PASID 1, since the queue fd is
> >>> still open.
> >>> We discussed this before, but I don't remember where we left off. The
> >>> child can't use the queue because its mappings are not copied on fork(),
> >>> and the queue is still bound to the parent mm A. The child either needs to
> >>> open a new queue or take ownership of the old one with a new uacce ioctl.
> >> Yes, currently nginx aligned with the case.
> >> Child process (worker process) reopen uacce,
> >>
> >> Master process (do init) open uacce, iommu_sva_bind_device(dev, A) -> PASID
> >> 1
> >> Master process fork Child (daemon) and exit.
> >>
> >> Child (daemon) does not use PASID 1 any more, only fork and manage worker
> >> process.
> >> Worker process reopen uacce, iommu_sva_bind_device(dev, B) PASID 2
> >>
> >> So it is expected.
> > Yes, that's fine
> >
> >>> Is that the "IMPLEMENT_DYNAMIC_BIND_FN()" you mention, something out of
> >>> tree? This operation should unbind from A before binding to B, no?
> >>> Otherwise we leak PASID 1.
> >> In 5.16 PASID 1 from master is hold until nginx service stop.
> >> nginx start
> >> master:
> >> iommu_sva_alloc_pasid mm->pasid=1 // master process
> >>
> >> lynx https start:
> >> iommu_sva_alloc_pasid mm->pasid=2 //worker process
> >>
> >> nginx stop: from fops_release
> >> iommu_sva_free_pasid mm->pasid=2 // worker process
> >> iommu_sva_free_pasid mm->pasid=1 // master process
> > That's the expected behavior (master could close its fd before forking, in
> > order to free things up earlier, but it's not mandatory)
> Currently we unbind in fops_release, so the ioasid allocated in master
> can only be freed when nginx stop,
> when all forked fd are closed.
>
> >
> >> Have one silly question.
> >>
> >> kerne driver
> >> fops_open
> >> iommu_sva_bind_device
> >>
> >> fops_release
> >> iommu_sva_unbind_device
> >>
> >> application
> >> main()
> >> fd = open
> >> return;
> >>
> >> Application exit but not close(fd), is it expected fops_release will be
> >> called automatically by system?
> > Yes, the application doesn't have to call close() explicitly, the file
> > descriptor is closed automatically on exit. Note that the fd is copied on
> > fork(), so it is only released once parent and all child processes exit.
> Yes, in case the application ended unexpected, like ctrl+c
> >
> >> On 5.17
> >> fops_release is called automatically, as well as iommu_sva_unbind_device.
> >> On 5.18-rc1.
> >> fops_release is not called, have to manually call close(fd)
> > Right that's weird
> Looks it is caused by the fix patch, via mmget, which may add refcount
> of fd.
>
> Some experiments
> 1. 5.17, everything works well.
>
> 2. 5.17 + patchset of "iommu/sva: Assign a PASID to mm on PASID
> allocation and free it on mm exit"
>
> Test application, exit without close uacce fd
> First time: fops_release can be called automatically.
>
> log:
> ioasid_alloc ioasid=1
> iommu_sva_alloc_pasid pasid=1
> iommu_sva_bind_device handle=00000000263a2ee8
> ioasid_free ioasid=1
> uacce_fops_release q=0000000055ca3cdf
> iommu_sva_unbind_device handle=00000000263a2ee8
>
> Second time: hardware reports error
>
> uacce_fops_open q=000000008e4d6f78
> ioasid_alloc ioasid=1
> iommu_sva_alloc_pasid pasid=1
> iommu_sva_bind_device handle=00000000cfd11788
> // Haredware reports error
> hisi_sec2 0000:b6:00.0: qm_acc_do_task_timeout [error status=0x20] found
> hisi_sec2 0000:b6:00.0: qm_acc_wb_not_ready_timeout [error status=0x40]
> found
> hisi_sec2 0000:b6:00.0: sec_fsm_hbeat_rint [error status=0x20] found
> hisi_sec2 0000:b6:00.0: Controller resetting...
> hisi_sec2 0000:b6:00.0: QM mailbox operation timeout!
> hisi_sec2 0000:b6:00.0: Failed to dump sqc!
> hisi_sec2 0000:b6:00.0: Failed to drain out data for stopping!
> hisi_sec2 0000:b6:00.0: Bus lock! Please reset system.
> hisi_sec2 0000:b6:00.0: Controller reset failed (-110)
> hisi_sec2 0000:b6:00.0: controller reset failed (-110)
>
> 3. Add the fix patch of using mmget in bind.
> Test application, exit without close uacce fd
>
> fops_release can NOT be called automatically, looks mmget adds refcount
> of fd.
Test application, exit without closing fd.
> >> kernel driver
> >> fops_open
> >> iommu_sva_bind_device
> >>
> >> fops_release
> >> iommu_sva_unbind_device
1.
5.17 kernel, no mmget & mmput
wd_release_queue no close
Compress bz=512000 nb=1×10, speed=139.5 MB/s (±0.0% N=1) overall=122.9
MB/s (±0.0%)
[ 16.052989] do_exit current=d380000
[ 16.053828] mmput atomic=1
[ 16.054511] __mmput atomic=0
[ 16.070382] exit_task_work
[ 16.070981] uacce_fops_release current=d380000
[ 16.071999] CPU: 0 PID: 176 Comm: test_sva_perf Not tainted
5.16.0-rc1-27342-ge5f9f3f99a88-dirty #240
[ 16.074007] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
[ 16.075530] Call trace:
[ 16.076069] dump_backtrace+0x0/0x1a0
[ 16.076887] show_stack+0x20/0x30
[ 16.077629] dump_stack_lvl+0x8c/0xb8
[ 16.078441] dump_stack+0x18/0x34
[ 16.079176] uacce_fops_release+0x44/0xdc
[ 16.080060] __fput+0x78/0x240
[ 16.080743] ____fput+0x18/0x28
[ 16.081447] task_work_run+0x88/0x160
[ 16.082259] do_exit+0x52c/0xa50
[ 16.082974] do_group_exit+0x84/0xa8
[ 16.083768] __wake_up_parent+0x0/0x38
[ 16.084597] invoke_syscall+0x4c/0x110
[ 16.085435] el0_svc_common.constprop.0+0x68/0x128
[ 16.086501] do_el0_svc+0x2c/0x90
[ 16.087243] el0_svc+0x24/0x70
[ 16.087928] el0t_64_sync_handler+0xb0/0xb8
[ 16.088854] el0t_64_sync+0x1a0/0x1a4
[ 16.089775] arm_smmu_sva_unbind
[ 16.090577] iommu_sva_free_pasid mm->pasid=1
[ 16.091763] exit_task_work done
2. Add mmget in bind and mmput in unbind,
Since application do not close fd, so no unbind,& mmput
And fops_release is not called since mm_users account.
log:
[ 101.642690] mmput atomic=3
wd_release_queue no close
Compress bz=512000 nb=1×10, speed=40.3 MB/s (±0.0% N=1) overall=38.7
MB/s (±0.0%)
[ 101.671167] do_exit current=d9daf40
[ 101.672003] mmput atomic=2
[ 101.672712] exit_task_work
[ 101.673292] exit_task_work done
Thanks
>
> So the fix method of using mmget blocks fops_release to be called once
> fd is closed without unbind.
>
> >> Since nginx may have a issue, it does not call close(fd) when nginx -s quit.
> > And you're sure that none of the processes are still alive or in zombie
> > state? Just to cover every possibility.
> It can also reproduced by a simple application exit without close(uacce_fd)
>
> Thanks
>
next prev parent reply other threads:[~2022-04-24 2:58 UTC|newest]
Thread overview: 113+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-07 23:02 [PATCH v4 00/11] Re-enable ENQCMD and PASID MSR Fenghua Yu
2022-02-07 23:02 ` [PATCH v4 01/11] iommu/sva: Rename CONFIG_IOMMU_SVA_LIB to CONFIG_IOMMU_SVA Fenghua Yu
2022-02-08 2:39 ` Lu Baolu
2022-02-15 10:54 ` [tip: x86/pasid] " tip-bot2 for Fenghua Yu
2022-02-07 23:02 ` [PATCH v4 02/11] mm: Change CONFIG option for mm->pasid field Fenghua Yu
2022-02-08 2:40 ` Lu Baolu
2022-02-15 10:54 ` [tip: x86/pasid] " tip-bot2 for Fenghua Yu
2022-02-07 23:02 ` [PATCH v4 03/11] iommu/ioasid: Introduce a helper to check for valid PASIDs Fenghua Yu
2022-02-08 2:40 ` Lu Baolu
2022-02-15 10:54 ` [tip: x86/pasid] " tip-bot2 for Fenghua Yu
2022-02-07 23:02 ` [PATCH v4 04/11] kernel/fork: Initialize mm's PASID Fenghua Yu
2022-02-14 17:23 ` Thomas Gleixner
2022-02-15 10:54 ` [tip: x86/pasid] " tip-bot2 for Fenghua Yu
2022-02-07 23:02 ` [PATCH v4 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit Fenghua Yu
2022-02-08 2:41 ` Lu Baolu
2022-02-08 15:01 ` Fenghua Yu
2022-02-10 3:16 ` Jacob Pan
2022-02-10 16:27 ` Fenghua Yu
2022-02-10 17:24 ` Luck, Tony
2022-02-10 18:31 ` Fenghua Yu
2022-02-10 23:52 ` Fenghua Yu
2022-02-10 18:49 ` Jacob Pan
2022-02-10 23:15 ` Fenghua Yu
2022-02-11 22:00 ` Dave Hansen
2022-02-14 17:24 ` Thomas Gleixner
2022-02-15 9:55 ` Joerg Roedel
[not found] ` <tencent_F6830A1196DB4C6A904D7C691F0D961D1108@qq.com>
2022-04-11 14:10 ` Dave Hansen
2022-04-11 14:20 ` zhangfei.gao
2022-04-11 14:36 ` Dave Hansen
2022-04-11 14:44 ` zhangfei.gao
2022-04-11 14:52 ` Dave Hansen
2022-04-11 15:13 ` zhangfei.gao
2022-04-12 7:04 ` zhangfei.gao
2022-04-12 13:41 ` Fenghua Yu
2022-04-12 14:39 ` Dave Hansen
2022-04-15 9:59 ` Fenghua Yu
2022-04-12 15:35 ` zhangfei.gao
2022-04-14 10:08 ` zhangfei.gao
2022-04-15 9:51 ` Fenghua Yu
[not found] ` <99bcb9f5-4776-9c40-a776-cdecfa9e1010@foxmail.com>
2022-04-15 10:14 ` zhangfei.gao
2022-04-15 10:50 ` Fenghua Yu
2022-04-15 11:52 ` zhangfei.gao
2022-04-15 12:37 ` Fenghua Yu
2022-04-16 1:30 ` zhangfei.gao
2022-04-15 19:07 ` Fenghua Yu
2022-04-15 21:00 ` Jacob Pan
2022-04-16 1:43 ` zhangfei.gao
2022-04-18 18:14 ` Jacob Pan
2022-04-19 1:02 ` zhangfei.gao
2022-04-18 6:34 ` Tian, Kevin
2022-04-18 18:11 ` Jacob Pan
2022-04-20 16:45 ` Jean-Philippe Brucker
2022-04-21 6:47 ` zhangfei.gao
2022-04-22 9:03 ` zhangfei.gao
2022-04-22 10:11 ` Jean-Philippe Brucker
2022-04-22 13:15 ` zhangfei.gao
2022-04-22 15:50 ` Jean-Philippe Brucker
2022-04-23 11:13 ` zhangfei.gao
2022-04-24 2:58 ` Zhangfei Gao [this message]
2022-04-24 9:52 ` Zhangfei Gao
2022-04-25 13:53 ` Jean-Philippe Brucker
2022-04-25 14:18 ` Dave Hansen
2022-04-25 14:26 ` Jean-Philippe Brucker
2022-04-25 15:34 ` Jacob Pan
2022-04-25 16:13 ` Jean-Philippe Brucker
2022-04-25 22:32 ` Jacob Pan
2022-04-26 4:20 ` Fenghua Yu
2022-04-26 5:04 ` Zhangfei Gao
2022-04-28 0:54 ` Fenghua Yu
2022-04-28 8:43 ` Jean-Philippe Brucker
2022-04-28 15:09 ` Dave Hansen
2022-04-28 15:28 ` Fenghua Yu
2022-04-28 15:42 ` Dave Hansen
2022-04-28 16:01 ` Jean-Philippe Brucker
2022-04-28 16:35 ` Dave Hansen
2022-04-26 4:28 ` Zhangfei Gao
2022-04-26 4:36 ` Fenghua Yu
2022-04-26 5:19 ` Zhangfei Gao
2022-04-25 15:55 ` Dave Hansen
2022-04-25 16:40 ` Jean-Philippe Brucker
2022-04-26 15:27 ` Dave Hansen
2022-04-26 16:48 ` Jean-Philippe Brucker
2022-04-26 23:31 ` Dave Hansen
2022-04-28 8:39 ` Jean-Philippe Brucker
2022-04-29 7:53 ` Baolu Lu
2022-04-29 13:51 ` Fenghua Yu
2022-04-29 14:34 ` Jean-Philippe Brucker
2022-04-29 22:19 ` Fenghua Yu
2022-04-30 7:33 ` Baolu Lu
2022-05-03 7:49 ` Jean-Philippe Brucker
2022-05-06 5:36 ` Baolu Lu
2022-04-12 14:36 ` Dave Hansen
2022-04-12 15:10 ` Jean-Philippe Brucker
2022-04-12 15:35 ` Dave Hansen
2022-04-13 11:14 ` Lu Baolu
2022-04-25 2:57 ` zhangfei.gao
2022-02-15 10:54 ` [tip: x86/pasid] " tip-bot2 for Fenghua Yu
2022-02-07 23:02 ` [PATCH v4 06/11] x86/fpu: Clear PASID when copying fpstate Fenghua Yu
2022-02-15 10:54 ` [tip: x86/pasid] " tip-bot2 for Fenghua Yu
2022-02-07 23:02 ` [PATCH v4 07/11] sched: Define and initialize a flag to identify valid PASID in the task Fenghua Yu
2022-02-15 10:54 ` [tip: x86/pasid] " tip-bot2 for Peter Zijlstra
2022-02-07 23:02 ` [PATCH v4 08/11] x86/traps: Demand-populate PASID MSR via #GP Fenghua Yu
2022-02-15 10:54 ` [tip: x86/pasid] " tip-bot2 for Fenghua Yu
2022-02-07 23:02 ` [PATCH v4 09/11] x86/cpufeatures: Re-enable ENQCMD Fenghua Yu
2022-02-15 10:54 ` [tip: x86/pasid] " tip-bot2 for Fenghua Yu
2022-02-07 23:02 ` [PATCH v4 10/11] tools/objtool: Check for use of the ENQCMD instruction in the kernel Fenghua Yu
2022-02-15 10:54 ` [tip: x86/pasid] " tip-bot2 for Fenghua Yu
2022-03-09 7:55 ` [tip: x86/core] " tip-bot2 for Fenghua Yu
2022-03-15 10:44 ` tip-bot2 for Fenghua Yu
2022-02-07 23:02 ` [PATCH v4 11/11] docs: x86: Change documentation for SVA (Shared Virtual Addressing) Fenghua Yu
2022-02-14 17:25 ` Thomas Gleixner
2022-02-15 10:54 ` [tip: x86/pasid] Documentation/x86: Update " tip-bot2 for Fenghua Yu
2022-02-11 20:04 ` [PATCH v4 00/11] Re-enable ENQCMD and PASID MSR Fenghua Yu
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=CABQgh9GzuQqsuPVyur+QurN8p+1BciMfOw0TccwiZSvvtrcowg@mail.gmail.com \
--to=zhangfei.gao@linaro.org \
--cc=ashok.raj@intel.com \
--cc=bp@alien8.de \
--cc=dave.hansen@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=fenghua.yu@intel.com \
--cc=iommu@lists.linux-foundation.org \
--cc=jean-philippe@linaro.org \
--cc=joro@8bytes.org \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=ravi.v.shankar@intel.com \
--cc=robin.murphy@arm.com \
--cc=tglx@linutronix.de \
--cc=tony.luck@intel.com \
--cc=will@kernel.org \
--cc=x86@kernel.org \
--cc=zhangfei.gao@foxmail.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).