RE: [PATCH 2/3] rwsem: Implement down_read_interruptible

From: David Laight <David.Laight@ACULAB.COM>
To: 'Waiman Long' <longman@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Ingo Molnar <mingo@redhat.com>, Will Deacon <will@kernel.org>,
	Jann Horn <jannh@google.com>,
	Vasiliy Kulikov <segoon@openwall.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Bernd Edlinger <bernd.edlinger@hotmail.de>,
	Oleg Nesterov <oleg@redhat.com>,
	"Christopher Yeoh" <cyeoh@au1.ibm.com>,
	Cyrill Gorcunov <gorcunov@gmail.com>,
	"Sargun Dhillon" <sargun@sargun.me>,
	Christian Brauner <christian.brauner@ubuntu.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Arnaldo Carvalho de Melo <acme@kernel.org>
Subject: RE: [PATCH 2/3] rwsem: Implement down_read_interruptible
Date: Tue, 8 Dec 2020 16:23:50 +0000	[thread overview]
Message-ID: <1be90b66dfe84a4c8a1e65bd40692c57@AcuMS.aculab.com> (raw)
In-Reply-To: <edfd0eb8-192e-60be-f0ca-2a72a26caa07@redhat.com>

From: Waiman Long
> Sent: 08 December 2020 15:34
> 
> On 12/8/20 4:12 AM, David Laight wrote:
> > From: Waiman Long
> >> Sent: 07 December 2020 19:02
> > ...
> >>> How much more difficult would it be to also add a timeout option?
> >>> I looked at adding one to the mutex code - and fell into a big pile
> >>> of replicated code.
> >>>
> >>> ISTM that one the initial locked exchange (and spin) fails a few
> >>> extra instructions when heading for the sleep don't really matter
> >>>
> >> Actually, I had tried that before. See
> >>
> >> https://lore.kernel.org/lkml/20190911150537.19527-1-longman@redhat.com/
> >>
> >> That is for rwsem, but the same can be done for mutex. However, Peter
> >> didn't seem to like the idea of a timeout parameter. Anyway, it is
> >> certainly doable if there is a good use case for it.
> > 'Unfortunately' my use-case if for an out-of-tree driver.
> >
> > The problem I was solving is a status call blocking because
> > some other code is 'stuck' (probably an oops) with a mutex held.
> >
> > The code used to use down_timeout() (it was written for 2.4).
> > When I changed to mutex_(to get optimistic spinning) I lost
> > the ability to do the timeouts.
> 
> The primary reason for sending out that patchset was to work around some
> circular locking problem in existing code even though these circular
> locking scenarios are not likely to happen. Your case is certainly
> another potential circular locking problem as well.

If you've got lock-ordering problems they need fixing.
Neither signals nor timeouts are real solutions.
Either may help diagnose the problem, but they aren't fixes.

OTOH if it reasonable to have a request interrupted by a signal
it must also be reasonable to implement a timeout.
Of course, one might wonder whether 'correct' code should ever
be waiting on a mutex for any length of time.
So is there even a justification for interruptible waits for mutex.

FWIW I could implement my timeouts using SIGALARM - but it is a lot
of work :-)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)