Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Thu, Apr 05, 2018 at 12:47:49AM -0700, Christoph Hellwig wrote: > Can't we just kill off spin_is_locked? It seems pretty much all uses > should simply be replaced with lockdep annotations, and there aren't > many to start with. Yeah, this is not the first time such a question has been raised

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Thu, Apr 05, 2018 at 12:47:49AM -0700, Christoph Hellwig wrote: > Can't we just kill off spin_is_locked? It seems pretty much all uses > should simply be replaced with lockdep annotations, and there aren't > many to start with. Yeah, this is not the first time such a question has been raised

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

Can't we just kill off spin_is_locked? It seems pretty much all uses should simply be replaced with lockdep annotations, and there aren't many to start with.

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

Can't we just kill off spin_is_locked? It seems pretty much all uses should simply be replaced with lockdep annotations, and there aren't many to start with.

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

Randy Dunlap wrote: > > * Note that the function only tells you that the CPU is seen to be locked, > > the CPU is locked?? > > > * not that it is locked on your CPU. Sorry, yes: "... that the lock is seen to be locked, not that it is locked on your CPU".

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

Randy Dunlap wrote: > > * Note that the function only tells you that the CPU is seen to be locked, > > the CPU is locked?? > > > * not that it is locked on your CPU. Sorry, yes: "... that the lock is seen to be locked, not that it is locked on your CPU".

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Tue, Apr 03, 2018 at 10:43:14PM +0100, David Howells wrote: > Alan Stern wrote: > > > + * Returns: 1 if @lock is locked, 0 otherwise. > > + * However, on !CONFIG_SMP builds with !CONFIG_DEBUG_SPINLOCK, > > + * the return value is always 0 (see

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Tue, Apr 03, 2018 at 10:43:14PM +0100, David Howells wrote: > Alan Stern wrote: > > > + * Returns: 1 if @lock is locked, 0 otherwise. > > + * However, on !CONFIG_SMP builds with !CONFIG_DEBUG_SPINLOCK, > > + * the return value is always 0 (see include/linux/spinlock_up.h). > > + * Therefore

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On 04/03/2018 02:43 PM, David Howells wrote: > Alan Stern wrote: > >> + * Returns: 1 if @lock is locked, 0 otherwise. >> + * However, on !CONFIG_SMP builds with !CONFIG_DEBUG_SPINLOCK, >> + * the return value is always 0 (see include/linux/spinlock_up.h). >> + *

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On 04/03/2018 02:43 PM, David Howells wrote: > Alan Stern wrote: > >> + * Returns: 1 if @lock is locked, 0 otherwise. >> + * However, on !CONFIG_SMP builds with !CONFIG_DEBUG_SPINLOCK, >> + * the return value is always 0 (see include/linux/spinlock_up.h). >> + * Therefore you should not rely

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

Alan Stern wrote: > + * Returns: 1 if @lock is locked, 0 otherwise. > + * However, on !CONFIG_SMP builds with !CONFIG_DEBUG_SPINLOCK, > + * the return value is always 0 (see include/linux/spinlock_up.h). > + * Therefore you should not rely heavily on the return value.

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

Alan Stern wrote: > + * Returns: 1 if @lock is locked, 0 otherwise. > + * However, on !CONFIG_SMP builds with !CONFIG_DEBUG_SPINLOCK, > + * the return value is always 0 (see include/linux/spinlock_up.h). > + * Therefore you should not rely heavily on the return value. Seems reasonable. It

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Tue, 3 Apr 2018, Andrea Parri wrote: > On Tue, Apr 03, 2018 at 04:23:07PM +0100, David Howells wrote: > > Andrea Parri wrote: > > > > > Sorry, but I don't understand your objection: are you suggesting to add > > > something like "Always return 0 on !SMP" to

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Tue, 3 Apr 2018, Andrea Parri wrote: > On Tue, Apr 03, 2018 at 04:23:07PM +0100, David Howells wrote: > > Andrea Parri wrote: > > > > > Sorry, but I don't understand your objection: are you suggesting to add > > > something like "Always return 0 on !SMP" to the comment? what else? > > > >

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Tue, Apr 03, 2018 at 04:23:07PM +0100, David Howells wrote: > Andrea Parri wrote: > > > Sorry, but I don't understand your objection: are you suggesting to add > > something like "Always return 0 on !SMP" to the comment? what else? > > Something like that,

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Tue, Apr 03, 2018 at 04:23:07PM +0100, David Howells wrote: > Andrea Parri wrote: > > > Sorry, but I don't understand your objection: are you suggesting to add > > something like "Always return 0 on !SMP" to the comment? what else? > > Something like that, possibly along with a warning that

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Tue, Apr 03, 2018 at 08:03:56AM -0700, Paul E. McKenney wrote: > On Tue, Apr 03, 2018 at 04:43:00PM +0200, Peter Zijlstra wrote: > > On Tue, Apr 03, 2018 at 07:17:18AM -0700, Paul E. McKenney wrote: > > > Suggestions for a fix? Clearly great care is required when using it > > > in things like

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Tue, Apr 03, 2018 at 08:03:56AM -0700, Paul E. McKenney wrote: > On Tue, Apr 03, 2018 at 04:43:00PM +0200, Peter Zijlstra wrote: > > On Tue, Apr 03, 2018 at 07:17:18AM -0700, Paul E. McKenney wrote: > > > Suggestions for a fix? Clearly great care is required when using it > > > in things like

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

Andrea Parri wrote: > Sorry, but I don't understand your objection: are you suggesting to add > something like "Always return 0 on !SMP" to the comment? what else? Something like that, possibly along with a warning that this might not be what you want. You

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

Andrea Parri wrote: > Sorry, but I don't understand your objection: are you suggesting to add > something like "Always return 0 on !SMP" to the comment? what else? Something like that, possibly along with a warning that this might not be what you want. You might actually want it to return

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Tue, Apr 03, 2018 at 04:43:00PM +0200, Peter Zijlstra wrote: > On Tue, Apr 03, 2018 at 07:17:18AM -0700, Paul E. McKenney wrote: > > Suggestions for a fix? Clearly great care is required when using it > > in things like WARN_ON()... > > Yeah, don't use it there, use lockdep_assert_held().

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Tue, Apr 03, 2018 at 04:43:00PM +0200, Peter Zijlstra wrote: > On Tue, Apr 03, 2018 at 07:17:18AM -0700, Paul E. McKenney wrote: > > Suggestions for a fix? Clearly great care is required when using it > > in things like WARN_ON()... > > Yeah, don't use it there, use lockdep_assert_held().

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Tue, Apr 03, 2018 at 07:17:18AM -0700, Paul E. McKenney wrote: > Suggestions for a fix? Clearly great care is required when using it > in things like WARN_ON()... Yeah, don't use it there, use lockdep_assert_held(). As I stated before in this thread, ideally we'd make *_is_locked() go away

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Tue, Apr 03, 2018 at 07:17:18AM -0700, Paul E. McKenney wrote: > Suggestions for a fix? Clearly great care is required when using it > in things like WARN_ON()... Yeah, don't use it there, use lockdep_assert_held(). As I stated before in this thread, ideally we'd make *_is_locked() go away

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Tue, Apr 03, 2018 at 02:52:33PM +0100, David Howells wrote: > Andrea Parri wrote: > > > > It's more complicated than that. This function is dangerous and should be > > > used with extreme care. In the case where CONFIG_SMP=n the value is > > > locked > >

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Tue, Apr 03, 2018 at 02:52:33PM +0100, David Howells wrote: > Andrea Parri wrote: > > > > It's more complicated than that. This function is dangerous and should be > > > used with extreme care. In the case where CONFIG_SMP=n the value is > > > locked > > > one way or the other and it might

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Tue, Apr 03, 2018 at 02:52:33PM +0100, David Howells wrote: > Andrea Parri wrote: > > > > It's more complicated than that. This function is dangerous and should be > > > used with extreme care. In the case where CONFIG_SMP=n the value is > > > locked > >

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Tue, Apr 03, 2018 at 02:52:33PM +0100, David Howells wrote: > Andrea Parri wrote: > > > > It's more complicated than that. This function is dangerous and should be > > > used with extreme care. In the case where CONFIG_SMP=n the value is > > > locked > > > one way or the other and it might

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

Andrea Parri wrote: > > It's more complicated than that. This function is dangerous and should be > > used with extreme care. In the case where CONFIG_SMP=n the value is locked > > one way or the other and it might be the wrong way. > > You mean "unlocked"?

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

Andrea Parri wrote: > > It's more complicated than that. This function is dangerous and should be > > used with extreme care. In the case where CONFIG_SMP=n the value is locked > > one way or the other and it might be the wrong way. > > You mean "unlocked"? (aka, return 0) No, I mean

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Tue, Apr 03, 2018 at 01:49:09PM +0100, David Howells wrote: > Andrea Parri wrote: > > > +/** > > + * spin_is_locked() - Check whether a spinlock is locked. > > + * @lock: Pointer to the spinlock. > > + * > > + * This function is NOT required to provide any

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Tue, Apr 03, 2018 at 01:49:09PM +0100, David Howells wrote: > Andrea Parri wrote: > > > +/** > > + * spin_is_locked() - Check whether a spinlock is locked. > > + * @lock: Pointer to the spinlock. > > + * > > + * This function is NOT required to provide any memory ordering > > + * guarantees;

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

Andrea Parri wrote: > +/** > + * spin_is_locked() - Check whether a spinlock is locked. > + * @lock: Pointer to the spinlock. > + * > + * This function is NOT required to provide any memory ordering > + * guarantees; it could be used for debugging purposes or,

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

Andrea Parri wrote: > +/** > + * spin_is_locked() - Check whether a spinlock is locked. > + * @lock: Pointer to the spinlock. > + * > + * This function is NOT required to provide any memory ordering > + * guarantees; it could be used for debugging purposes or, when > + * additional

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Mon, Apr 02, 2018 at 10:03:22AM -0400, Alan Stern wrote: > On Sun, 1 Apr 2018, Andrea Parri wrote: > > > There appeared to be a certain, recurrent uncertainty concerning the > > semantics of spin_is_locked(), likely a consequence of the fact that > > this semantics remains undocumented or that

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Mon, Apr 02, 2018 at 10:03:22AM -0400, Alan Stern wrote: > On Sun, 1 Apr 2018, Andrea Parri wrote: > > > There appeared to be a certain, recurrent uncertainty concerning the > > semantics of spin_is_locked(), likely a consequence of the fact that > > this semantics remains undocumented or that

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Sun, 1 Apr 2018, Andrea Parri wrote: > There appeared to be a certain, recurrent uncertainty concerning the > semantics of spin_is_locked(), likely a consequence of the fact that > this semantics remains undocumented or that it has been historically > linked to the (likewise unclear) semantics

Re: [PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

On Sun, 1 Apr 2018, Andrea Parri wrote: > There appeared to be a certain, recurrent uncertainty concerning the > semantics of spin_is_locked(), likely a consequence of the fact that > this semantics remains undocumented or that it has been historically > linked to the (likewise unclear) semantics

[PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

There appeared to be a certain, recurrent uncertainty concerning the semantics of spin_is_locked(), likely a consequence of the fact that this semantics remains undocumented or that it has been historically linked to the (likewise unclear) semantics of spin_unlock_wait(). A recent auditing [1] of

[PATCH v2 1/3] locking: Document the semantics of spin_is_locked()

There appeared to be a certain, recurrent uncertainty concerning the semantics of spin_is_locked(), likely a consequence of the fact that this semantics remains undocumented or that it has been historically linked to the (likewise unclear) semantics of spin_unlock_wait(). A recent auditing [1] of