On 2021-10-26 23:06:21 [+0200], Christian Brauner wrote:
> I'm not an expert in real-time locking but this solution is way less
> intrusive and easier to explain and understand than the first version.
> Based on how I understand priority inheritance this solution seems good.
>
> The scenario that we seem to mostly worry is:
> Let's assume a low-priority task A is holding lock_mount_hash() and
> writes MNT_WRITE_HOLD to mnt->mnt_flags. Another high-priority task B is
> spinning waiting for MNT_WRITE_HOLD to be cleared.
> On rt kernels task A could be scheduled away causing the high-priority
> task B to spin for a long time. However, if we force the high-priority
s/for a long time/indefinitely
> task B to grab lock_mount_hash() we thereby priority boost low-priorty
> task A causing it to relinquish the lock quickly preventing task B from
> spinning for a long time.
Yes. Task B goes idle, Task A continues with B's priority until
unlock_mount_hash(). After unlock A gets its old priority back and B
continues.
Side note: Because task A is holding a spinlock_t (lock_mount_hash()) it
can't be moved to another CPU (other reasons). Therefore if task B is on
the same CPU as A with higher priority then the scheduler can't move A
away and won't move B either because it is running. So the system locks
up.
> Under the assumption I got this right:
> Acked-by: Christian Brauner <christian.brauner@xxxxxxxxxx>
Thanks.
> > --- 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();
>
> IS_ENABLED will have the same effect as using ifdef, i.e. compiling the
> irrelevant branch out, I hope.
Yes. It turns into if (0) or if (1) leading to an elimination of one of
the branches.
Sebastian
