On Tue, Nov 7, 2023 at 2:01 AM <mlin@mail.ustc.edu.cn> 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 ?