| Message ID | 20211125120711.dgbsienyrsxfzpoi@linutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [REPOST] fs/namespace: Boost the mount_lock.lock owner instead of spinning on PREEMPT_RT. | expand |
On Thu, Nov 25, 2021 at 01:07:11PM +0100, Sebastian Andrzej Siewior wrote: > The MNT_WRITE_HOLD flag is used to hold back any new writers while the > mount point is about to be made read-only. __mnt_want_write() then loops > with disabled preemption until this flag disappears. Callers of > mnt_hold_writers() (which sets the flag) hold the spinlock_t of > mount_lock (seqlock_t) which disables preemption on !PREEMPT_RT and > ensures the task is not scheduled away so that the spinning side spins > for a long time. > > On PREEMPT_RT the spinlock_t does not disable preemption and so it is > possible that the task setting MNT_WRITE_HOLD is preempted by task with > higher priority which then spins infinitely waiting for MNT_WRITE_HOLD > to get removed. > > Acquire mount_lock::lock which is held by setter of MNT_WRITE_HOLD. This > will PI-boost the owner and wait until the lock is dropped and which > means that MNT_WRITE_HOLD is cleared again. > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > Acked-by: Christian Brauner <christian.brauner@ubuntu.com> > Link: https://lore.kernel.org/r/20211025152218.opvcqfku2lhqvp4o@linutronix.de > --- I thought you'd carry this in -rt, Sebastian and Thomas. So I've picked this up and moved this into -next as we want it there soon so it can sit there for as long as possible. I'll drop it if Al objects to the patch or prefers to carry it. Christian > fs/namespace.c | 20 ++++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) > > diff --git a/fs/namespace.c b/fs/namespace.c > index 659a8f39c61af..3ab45b47b2860 100644 > --- a/fs/namespace.c > +++ b/fs/namespace.c > @@ -343,8 +343,24 @@ int __mnt_want_write(struct vfsmount *m) > * incremented count after it has set MNT_WRITE_HOLD. > */ > smp_mb(); > - while (READ_ONCE(mnt->mnt.mnt_flags) & MNT_WRITE_HOLD) > - cpu_relax(); > + might_lock(&mount_lock.lock); > + while (READ_ONCE(mnt->mnt.mnt_flags) & MNT_WRITE_HOLD) { > + if (!IS_ENABLED(CONFIG_PREEMPT_RT)) { > + cpu_relax(); > + } else { > + /* > + * This prevents priority inversion, if the task > + * setting MNT_WRITE_HOLD got preempted on a remote > + * CPU, and it prevents life lock if the task setting > + * MNT_WRITE_HOLD has a lower priority and is bound to > + * the same CPU as the task that is spinning here. > + */ > + preempt_enable(); > + lock_mount_hash(); > + unlock_mount_hash(); > + preempt_disable(); > + } > + } > /* > * After the slowpath clears MNT_WRITE_HOLD, mnt_is_readonly will > * be set to match its requirements. So we must not load that until > -- > 2.34.0 >
On 2021-11-26 14:24:14 [+0100], Christian Brauner wrote: > I thought you'd carry this in -rt, Sebastian and Thomas. So I've picked > this up and moved this into -next as we want it there soon so it can sit > there for as long as possible. I'll drop it if Al objects to the patch > or prefers to carry it. Thanks. > Christian Sebastian
On 2021-11-26 14:24:14 [+0100], Christian Brauner wrote: > I thought you'd carry this in -rt, Sebastian and Thomas. So I've picked > this up and moved this into -next as we want it there soon so it can sit > there for as long as possible. I'll drop it if Al objects to the patch > or prefers to carry it. It appears it missed -rc1. Did Al object to it or is this -rc2 material? > Christian Sebastian
On Tue, Jan 25, 2022 at 03:40:18PM +0100, Sebastian Andrzej Siewior wrote: > On 2021-11-26 14:24:14 [+0100], Christian Brauner wrote: > > I thought you'd carry this in -rt, Sebastian and Thomas. So I've picked > > this up and moved this into -next as we want it there soon so it can sit > > there for as long as possible. I'll drop it if Al objects to the patch > > or prefers to carry it. > > It appears it missed -rc1. Did Al object to it or is this -rc2 material? I didn't hear him object. I have it sitting in a separate tree [1] ready to be sent. If I don't hear anything by the end of this week I'll send it! [1]: https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=fs.fixes Christian
On 2022-01-25 16:49:37 [+0100], Christian Brauner wrote: > I didn't hear him object. I have it sitting in a separate tree [1] ready > to be sent. If I don't hear anything by the end of this week I'll send it! Thank you. > Christian Sebastian
On 2022-01-25 16:49:37 [+0100], Christian Brauner wrote: > I didn't hear him object. I have it sitting in a separate tree [1] ready > to be sent. If I don't hear anything by the end of this week I'll send it! No complains so far, I guess? ;) > Christian Sebastian
diff --git a/fs/namespace.c b/fs/namespace.c index 659a8f39c61af..3ab45b47b2860 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -343,8 +343,24 @@ int __mnt_want_write(struct vfsmount *m) * incremented count after it has set MNT_WRITE_HOLD. */ smp_mb(); - while (READ_ONCE(mnt->mnt.mnt_flags) & MNT_WRITE_HOLD) - cpu_relax(); + might_lock(&mount_lock.lock); + while (READ_ONCE(mnt->mnt.mnt_flags) & MNT_WRITE_HOLD) { + if (!IS_ENABLED(CONFIG_PREEMPT_RT)) { + cpu_relax(); + } else { + /* + * This prevents priority inversion, if the task + * setting MNT_WRITE_HOLD got preempted on a remote + * CPU, and it prevents life lock if the task setting + * MNT_WRITE_HOLD has a lower priority and is bound to + * the same CPU as the task that is spinning here. + */ + preempt_enable(); + lock_mount_hash(); + unlock_mount_hash(); + preempt_disable(); + } + } /* * After the slowpath clears MNT_WRITE_HOLD, mnt_is_readonly will * be set to match its requirements. So we must not load that until