linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Mateusz Guzik <mjguzik@gmail.com>
To: David Laight <david.laight.linux@gmail.com>
Cc: ebiederm@xmission.com, oleg@redhat.com, brauner@kernel.org,
	 akpm@linux-foundation.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 6/6] pid: drop irq disablement around pidmap_lock
Date: Sat, 1 Feb 2025 19:42:32 +0100	[thread overview]
Message-ID: <CAGudoHFHzEQhkaJCB3z6qCfDtSRq+zZew3fDkAKG-AEjpMq8Nw@mail.gmail.com> (raw)
In-Reply-To: <20250201181933.07a3e7e2@pumpkin>

On Sat, Feb 1, 2025 at 7:19 PM David Laight
<david.laight.linux@gmail.com> wrote:
>
> On Sat,  1 Feb 2025 17:31:06 +0100
> Mateusz Guzik <mjguzik@gmail.com> wrote:
>
> > It no longer serves any purpose now that the tasklist_lock ->
> > pidmap_lock ordering got eliminated.
>
> Not disabling interrupts may make thing worse.
> It is a trade off between 'interrupt latency' and 'lock hold time'.
>
> If interrupts are disabled then (clearly) they can get delayed because
> the lock is held.
> Provided the lock is only held for a short time it probably doesn't matter.
> Indeed, unless it is the worst one, it probably doesn't matter at all.
> After all spin locks shouldn't really be held for significant periods.
>
> OTOH if the lock doesn't disable interrupts then an interrupt will
> increase the length of time a lock is held for.
> This can be significant - and I mean upwards of 1ms.
> Network interrupts can tale a while - and then the work that is deferred
> to 'softint' context happens as well (I don't think a spinlock stops
> the softint code).
>
> I've a feeling that unless a spin lock is held for 'far longer than one
> should ever be held for' then you really want to disable interrupts.
>

Note that taking the interrupt trip increases single-threaded overhead.

Per your own description, if the lock is contested and interrupts are
disabled, handling them also get delayed by CPUs which are busy just
waiting (and which would otherwise take care of them).

So while this is indeed a tradeoff, as I understand the sane default
is to *not* disable interrupts unless necessary.

-- 
Mateusz Guzik <mjguzik gmail.com>


  reply	other threads:[~2025-02-01 18:42 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-01 16:31 [PATCH v3 0/6] reduce tasklist_lock hold time on exit and do some Mateusz Guzik
2025-02-01 16:31 ` [PATCH v3 1/6] exit: perform add_device_randomness() without tasklist_lock Mateusz Guzik
2025-02-03 17:51   ` Oleg Nesterov
2025-02-03 17:55     ` Mateusz Guzik
2025-02-01 16:31 ` [PATCH v3 2/6] exit: hoist get_pid() in release_task() outside of tasklist_lock Mateusz Guzik
2025-02-03 19:27   ` Liam R. Howlett
2025-02-03 19:35     ` Mateusz Guzik
2025-02-03 20:13       ` Liam R. Howlett
2025-02-03 20:22         ` Mateusz Guzik
2025-02-03 20:28           ` Liam R. Howlett
2025-02-04  1:51           ` Andrew Morton
2025-02-01 16:31 ` [PATCH v3 3/6] exit: postpone tty_kref_put() until after tasklist_lock is dropped Mateusz Guzik
2025-02-03 18:06   ` Oleg Nesterov
2025-02-03 19:33     ` Mateusz Guzik
2025-02-04 11:22       ` Oleg Nesterov
2025-02-04 12:12         ` Mateusz Guzik
2025-02-01 16:31 ` [PATCH v3 4/6] pid: sprinkle tasklist_lock asserts Mateusz Guzik
2025-02-01 16:31 ` [PATCH v3 5/6] pid: perform free_pid() calls outside of tasklist_lock Mateusz Guzik
2025-02-03 18:49   ` Oleg Nesterov
2025-02-03 19:31     ` Mateusz Guzik
2025-02-03 20:02       ` Oleg Nesterov
2025-02-01 16:31 ` [PATCH v3 6/6] pid: drop irq disablement around pidmap_lock Mateusz Guzik
2025-02-01 17:42   ` Oleg Nesterov
2025-02-01 17:45     ` Oleg Nesterov
2025-02-01 18:19   ` David Laight
2025-02-01 18:42     ` Mateusz Guzik [this message]
2025-02-01 21:51       ` David Laight
2025-02-01 22:00         ` Matthew Wilcox
2025-02-02 13:55           ` David Laight
2025-02-02 19:34             ` Mateusz Guzik
2025-02-02 20:44               ` David Laight
2025-02-02 22:06                 ` Matthew Wilcox
2025-02-03 17:47 ` [PATCH v3 0/6] reduce tasklist_lock hold time on exit and do some Oleg Nesterov

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=CAGudoHFHzEQhkaJCB3z6qCfDtSRq+zZew3fDkAKG-AEjpMq8Nw@mail.gmail.com \
    --to=mjguzik@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=brauner@kernel.org \
    --cc=david.laight.linux@gmail.com \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=oleg@redhat.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