On 12/14/2023 2:27 AM, Eric W. Biederman wrote:

Matthew Wilcox [off-list ref] writes:

quoted

On Wed, Dec 13, 2023 at 06:17:45PM +0800, Maria Yu wrote:

quoted

+static inline void write_lock_tasklist_lock(void)
+{
+	while (1) {
+		local_irq_disable();
+		if (write_trylock(&tasklist_lock))
+			break;
+		local_irq_enable();
+		cpu_relax();

This is a bad implementation though.  You don't set the _QW_WAITING flag

Any better ideas and suggestions are welcomed. :)

quoted

so readers don't know that there's a pending writer.  Also, I've see >> cpu_relax() pessimise CPU behaviour; putting it into a low-power mode
that takes a while to wake up from.

I think the right way to fix this is to pass a boolean flag to
queued_write_lock_slowpath() to let it know whether it can re-enable
interrupts while checking whether _QW_WAITING is set.

Yes.  It seems to make sense to distinguish between write_lock_irq and
write_lock_irqsave and fix this for all of write_lock_irq.

Let me think about this.
It seems a possible because there is a special behavior from reader side 
when in interrupt it will directly get the lock regardless of the 
pending writer.

Either that or someone can put in the work to start making the
tasklist_lock go away.

Eric

-- 
Thx and BRs,
Aiqun(Maria) Yu