All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@nvidia.com>
To: Nicolin Chen <nicolinc@nvidia.com>
Cc: kevin.tian@intel.com, iommu@lists.linux.dev,
	linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH rc v1 0/4] iommufd: Fix veventq_depth boundary
Date: Thu, 21 May 2026 20:27:13 -0300	[thread overview]
Message-ID: <20260521232713.GK3602937@nvidia.com> (raw)
In-Reply-To: <ag9IjD98g2pHiGlE@Asurada-Nvidia>

On Thu, May 21, 2026 at 11:01:48AM -0700, Nicolin Chen wrote:

> FWIW, it was added to address a Sashiko review also:
> 
>   By moving the allocation outside the spinlock, the precondition check that
>   skipped the allocation when the queue was full is bypassed.
> 
>   When the queue is full, which can be common during a hardware fault storm
>   if userspace cannot keep up, the code now unconditionally allocates memory,
>   copies data, acquires the lock, and then immediately frees the memory and
>   drops the event.
> 
>   Can this tight loop of wasteful slab allocations, memory copies, and
>   deallocations exacerbate IOMMU fault storms by adding unnecessary CPU
>   overhead?
> 
>   Would it be possible to add an optimistic lockless check, such as
>   READ_ONCE(veventq->num_events) < veventq->depth, to bypass the allocation
>   when the queue appears full?

That seems like nonsense to me.

> > 6) What do you think about the "but done has
> >    already been incremented by sizeof(*hdr)" ? unrelated issue? If it
> >    is simple lets add a patch here to fix it
> 
> I added a patch but didn't include in the series -- Sashiko would
> raise more questions against that patch...
> 
> I think it's a separate bug; Sashiko pointed out another in fault
> queue as well. Both bugs are at failure (corner cases?) path.
> 
> I'd like to address them separately.

Ok

Jason

      reply	other threads:[~2026-05-21 23:27 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-18  2:28 [PATCH rc v1 0/4] iommufd: Fix veventq_depth boundary Nicolin Chen
2026-05-18  2:28 ` [PATCH rc v1 1/4] iommufd: Move vevent memory allocation outside spinlock Nicolin Chen
2026-05-18  2:28 ` [PATCH rc v1 2/4] iommufd: Set veventq_depth upper bound Nicolin Chen
2026-05-18  2:28 ` [PATCH rc v1 3/4] iommufd: Fix data_len byte-count vs element-count mismatch Nicolin Chen
2026-05-18  2:28 ` [PATCH rc v1 4/4] iommufd/selftest: Add boundary tests for veventq_depth Nicolin Chen
2026-05-18 18:11 ` [PATCH rc v1 0/4] iommufd: Fix veventq_depth boundary Jason Gunthorpe
2026-05-21 14:30 ` Jason Gunthorpe
2026-05-21 18:01   ` Nicolin Chen
2026-05-21 23:27     ` Jason Gunthorpe [this message]

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=20260521232713.GK3602937@nvidia.com \
    --to=jgg@nvidia.com \
    --cc=iommu@lists.linux.dev \
    --cc=kevin.tian@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=nicolinc@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 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.