On Tue, Nov 7, 2023 at 2:01 AM <mlin@xxxxxxxxxxxxxxxx> wrote:
>
> Hi,
>
> I find this function __up_writer refering to an anonymous a writer? How does it occur?
> ```
> // kernel/locking/rwsem.c
> static inline void __up_write(struct rw_semaphore *sem)
> {
> long tmp;
>
> DEBUG_RWSEMS_WARN_ON(sem->magic != sem, sem);
> /*
> * sem->owner may differ from current if the ownership is transferred
> * to an anonymous writer by setting the RWSEM_NONSPINNABLE bits.
> */
> DEBUG_RWSEMS_WARN_ON((rwsem_owner(sem) != current) &&
> !rwsem_test_oflags(sem, RWSEM_NONSPINNABLE), sem); <------------------ is it necessary?
>
> preempt_disable();
> rwsem_clear_owner(sem);
> tmp = atomic_long_fetch_add_release(-RWSEM_WRITER_LOCKED, &sem->count);
> if (unlikely(tmp & RWSEM_FLAG_WAITERS))
> rwsem_wake(sem);
> preempt_enable();
> }
> ```
>
> I do a search by using `git blame` and find it's added in this commit 02f1082b003a0cd48f48f12533d969cdbf1c2b63,
> and in this commit d7d760efad70c7a030725499bf9f342f04af24dd, it refers to below situation that can have an anonymous writer:
>
> > There are use cases where a rwsem can be acquired by one task, but
> > released by another task. In thess cases, optimistic spinning may need
> > to be disabled. One example will be the filesystem freeze/thaw code
> > where the task that freezes the filesystem will acquire a write lock
> > on a rwsem and then un-owns it before returning to userspace. Later on,
> > another task will come along, acquire the ownership, thaw the filesystem
> > and release the rwsem.
> >
> > Bit 0 of the owner field was used to designate that it is a reader
> > owned rwsem. It is now repurposed to mean that the owner of the rwsem
> > is not known. If only bit 0 is set, the rwsem is reader owned. If bit
> > 0 and other bits are set, it is writer owned with an unknown owner.
> > One such value for the latter case is (-1L). So we can set owner to 1 for
> > reader-owned, -1 for writer-owned. The owner is unknown in both cases.
> >
> > To handle transfer of rwsem ownership, the higher level code should
> > set the owner field to -1 to indicate a write-locked rwsem with unknown
> > owner. Optimistic spinning will be disabled in this case.
> >
> > Once the higher level code figures who the new owner is, it can then
> > set the owner field accordingly.
>
> As it mentions that at higher level, we set -1 to owner subjectively in order to do an owner migration, and I only find it's used in only
> one place, i.e. percpu-rwsem in this commit 5a817641f68a6399a5fac8b7d2da67a73698ffed which has been removed in commit
> 7f26482a872c36b2ee87ea95b9dcd96e3d5805df.
>
> So does it mean an anonymous writer don't exist at current kernel? should we remove the second condition in this assert check?
>
> DEBUG_RWSEMS_WARN_ON((rwsem_owner(sem) != current) &&
> !rwsem_test_oflags(sem, RWSEM_NONSPINNABLE), sem);
>
> Thanks,
> Mu
you didnt really say why youre looking at it.
I presume youve triggered an error that implied it was your prob,
and removing the 2nd condition fixed it ?
If so (and you get no further "advice" here)
I would try feeding it to lkp-robot, it will at least build-test it
on "all" the arches you dont have at hand.
FWIW, there are ~12 uses of that macro, and only 1 has the 2nd condition.
Were they all added together, or were they spread across several commits ?
_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@xxxxxxxxxxxxxxxxx
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
