All of lore.kernel.org
 help / color / mirror / Atom feed
* SRCU question
@ 2020-11-12 20:15 Kent Overstreet
  2020-11-15 20:11 ` Paul E. McKenney
  0 siblings, 1 reply; 7+ messages in thread
From: Kent Overstreet @ 2020-11-12 20:15 UTC (permalink / raw)
  To: Paul E. McKenney, rcu

Hi to Paul & the rcu mailing list,

I've got a problem I'm trying to figure out if I can adapt SRCU for.

Within bcachefs, currently struct btree obects, and now also btree_cached_key,
aren't freed until the filesystem is torn down, because btree iterators contain
pointers to them and will drop and retake locks (iff a sequence number hasn't
changed) without holding a ref. With the btree key cache code, this is now
something I need to fix.

What I plan on doing is having struct btree_trans (container for btree
iterators) hold an srcu read lock while it's alive. But, I don't want to just
use call_srcu to free the btree/btree_cached_key objects, because btree trans
objects can at times be fairly long lived and the existing code can reuse these
objects for other btree nodes/btree cached keys immediately. Freeing them with
call_srcu() would break that; I could have my callback function check if the
object has been reused before freeing, but I'd still have a problem when the
object gets freed a second time before the first call_scru() has finished.

What I'm wondering is if the SRCU code has a well defined notion of a clock that
I could make use of. What I would like to do is, instead of doing call_srcu() to
free the object - just mark that object with the current SRCU context time, and
then when my shrinkers run they can free objects that haven't been reused and
are old enough according the the current SRCU time.

Thoughts?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: SRCU question
  2020-11-12 20:15 SRCU question Kent Overstreet
@ 2020-11-15 20:11 ` Paul E. McKenney
  2020-11-15 20:20   ` Paul E. McKenney
  0 siblings, 1 reply; 7+ messages in thread
From: Paul E. McKenney @ 2020-11-15 20:11 UTC (permalink / raw)
  To: Kent Overstreet; +Cc: rcu

On Thu, Nov 12, 2020 at 03:15:47PM -0500, Kent Overstreet wrote:
> Hi to Paul & the rcu mailing list,
> 
> I've got a problem I'm trying to figure out if I can adapt SRCU for.
> 
> Within bcachefs, currently struct btree obects, and now also btree_cached_key,
> aren't freed until the filesystem is torn down, because btree iterators contain
> pointers to them and will drop and retake locks (iff a sequence number hasn't
> changed) without holding a ref. With the btree key cache code, this is now
> something I need to fix.
> 
> What I plan on doing is having struct btree_trans (container for btree
> iterators) hold an srcu read lock while it's alive. But, I don't want to just
> use call_srcu to free the btree/btree_cached_key objects, because btree trans
> objects can at times be fairly long lived and the existing code can reuse these
> objects for other btree nodes/btree cached keys immediately. Freeing them with
> call_srcu() would break that; I could have my callback function check if the
> object has been reused before freeing, but I'd still have a problem when the
> object gets freed a second time before the first call_scru() has finished.
> 
> What I'm wondering is if the SRCU code has a well defined notion of a clock that
> I could make use of. What I would like to do is, instead of doing call_srcu() to
> free the object - just mark that object with the current SRCU context time, and
> then when my shrinkers run they can free objects that haven't been reused and
> are old enough according the the current SRCU time.
> 
> Thoughts?

An early prototype is available on -rcu [1].  The Tree SRCU version
seems to be reasonably reliable, but I do not yet trust the Tiny SRCU
implementation.  So please avoid !SMP&&!PREEMPT if you would like to
give it a try.  Unless you -really- like helping me find bugs, in which
case full speed ahead!!!

Here is the API:

unsigned long start_poll_synchronize_srcu(struct srcu_struct *ssp)

	Returns a "cookie" that can be thought of as a snapshot of your
	"clock" above.	(SRCU calls it a "grace-period sequence number".)
	Also ensures that enough future grace periods happen to eventually
	make the grace-period sequence number reach the cookie.

bool poll_state_synchronize_srcu(struct srcu_struct *ssp, unsigned long cookie)

	Given a cookie from start_poll_synchronize_srcu(), returns true if
	at least one full SRCU grace period has elapsed in the meantime.
	Given finite SRCU readers in a well-behaved kernel, the following
	code will complete in finite time:

		cookie = start_poll_synchronize_srcu(&my_srcu);
		while (!poll_state_synchronize_srcu(&my_srcu, cookie))
			schedule_timeout_uninterruptible(1);

unsigned long get_state_synchronize_srcu(struct srcu_struct *ssp)

	Like start_poll_synchronize_srcu(), except that it does not start
	any grace periods.  This means that the following code is -not-
	guaranteed to complete:

		cookie = get_state_synchronize_srcu(&my_srcu);
		while (!poll_state_synchronize_srcu(&my_srcu, cookie))
			schedule_timeout_uninterruptible(1);

	Use this if you know that something else will be starting the
	needed SRCU grace periods.  This might also be useful if you
	had items that were likely to be reused before the SRCU grace
	period elapsed, so that you avoid burning CPU on SRCU grace
	periods that prove to be unnecessary.  Or if you don't want
	to have more than (say) 100 SRCU grace periods per seconds,
	in which case you might use a timer to start the grace periods.
	Or maybe you don't bother starting the SRCU grace period until
	some sort of emergency situation has arisen.  Or...

	OK, maybe you don't need it, but I do need it for rcutorture,
	so here it is anyway.

All of these can be invoked anywhere that call_srcu() can be invoked.

Does this look like it will work for you?

							Thanx, Paul

[1] git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: SRCU question
  2020-11-15 20:11 ` Paul E. McKenney
@ 2020-11-15 20:20   ` Paul E. McKenney
  2020-11-15 20:35     ` Kent Overstreet
  0 siblings, 1 reply; 7+ messages in thread
From: Paul E. McKenney @ 2020-11-15 20:20 UTC (permalink / raw)
  To: Kent Overstreet; +Cc: rcu

On Sun, Nov 15, 2020 at 12:11:24PM -0800, Paul E. McKenney wrote:
> On Thu, Nov 12, 2020 at 03:15:47PM -0500, Kent Overstreet wrote:
> > Hi to Paul & the rcu mailing list,
> > 
> > I've got a problem I'm trying to figure out if I can adapt SRCU for.
> > 
> > Within bcachefs, currently struct btree obects, and now also btree_cached_key,
> > aren't freed until the filesystem is torn down, because btree iterators contain
> > pointers to them and will drop and retake locks (iff a sequence number hasn't
> > changed) without holding a ref. With the btree key cache code, this is now
> > something I need to fix.
> > 
> > What I plan on doing is having struct btree_trans (container for btree
> > iterators) hold an srcu read lock while it's alive. But, I don't want to just
> > use call_srcu to free the btree/btree_cached_key objects, because btree trans
> > objects can at times be fairly long lived and the existing code can reuse these
> > objects for other btree nodes/btree cached keys immediately. Freeing them with
> > call_srcu() would break that; I could have my callback function check if the
> > object has been reused before freeing, but I'd still have a problem when the
> > object gets freed a second time before the first call_scru() has finished.
> > 
> > What I'm wondering is if the SRCU code has a well defined notion of a clock that
> > I could make use of. What I would like to do is, instead of doing call_srcu() to
> > free the object - just mark that object with the current SRCU context time, and
> > then when my shrinkers run they can free objects that haven't been reused and
> > are old enough according the the current SRCU time.
> > 
> > Thoughts?
> 
> An early prototype is available on -rcu [1].  The Tree SRCU version
> seems to be reasonably reliable, but I do not yet trust the Tiny SRCU
> implementation.  So please avoid !SMP&&!PREEMPT if you would like to
> give it a try.  Unless you -really- like helping me find bugs, in which
> case full speed ahead!!!
> 
> Here is the API:
> 
> unsigned long start_poll_synchronize_srcu(struct srcu_struct *ssp)
> 
> 	Returns a "cookie" that can be thought of as a snapshot of your
> 	"clock" above.	(SRCU calls it a "grace-period sequence number".)
> 	Also ensures that enough future grace periods happen to eventually
> 	make the grace-period sequence number reach the cookie.
> 
> bool poll_state_synchronize_srcu(struct srcu_struct *ssp, unsigned long cookie)
> 
> 	Given a cookie from start_poll_synchronize_srcu(), returns true if
> 	at least one full SRCU grace period has elapsed in the meantime.
> 	Given finite SRCU readers in a well-behaved kernel, the following
> 	code will complete in finite time:
> 
> 		cookie = start_poll_synchronize_srcu(&my_srcu);
> 		while (!poll_state_synchronize_srcu(&my_srcu, cookie))
> 			schedule_timeout_uninterruptible(1);
> 
> unsigned long get_state_synchronize_srcu(struct srcu_struct *ssp)
> 
> 	Like start_poll_synchronize_srcu(), except that it does not start
> 	any grace periods.  This means that the following code is -not-
> 	guaranteed to complete:
> 
> 		cookie = get_state_synchronize_srcu(&my_srcu);
> 		while (!poll_state_synchronize_srcu(&my_srcu, cookie))
> 			schedule_timeout_uninterruptible(1);
> 
> 	Use this if you know that something else will be starting the
> 	needed SRCU grace periods.  This might also be useful if you
> 	had items that were likely to be reused before the SRCU grace
> 	period elapsed, so that you avoid burning CPU on SRCU grace
> 	periods that prove to be unnecessary.  Or if you don't want
> 	to have more than (say) 100 SRCU grace periods per seconds,
> 	in which case you might use a timer to start the grace periods.
> 	Or maybe you don't bother starting the SRCU grace period until
> 	some sort of emergency situation has arisen.  Or...
> 
> 	OK, maybe you don't need it, but I do need it for rcutorture,
> 	so here it is anyway.
> 
> All of these can be invoked anywhere that call_srcu() can be invoked.
> 
> Does this look like it will work for you?

Oh, and due to historical inertia, Tiny SRCU's grace-period sequence
number is only 16 bits.  I can change this easily, but I need to know
that it is a real problem for you before I can do so.

The potential problem for you is that if you let a given cookie lie
dormant for 16384 grace periods, it will take another 16385 grace
periods for get_state_synchronize_srcu() to say that a grace period
has elapsed.

In contrast, Tree SRCU's grace-period sequence number is either 32 bits
or 64 bits, depending on the size of unsized long.

							Thanx, Paul

> [1] git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: SRCU question
  2020-11-15 20:20   ` Paul E. McKenney
@ 2020-11-15 20:35     ` Kent Overstreet
  2020-11-15 20:53       ` Paul E. McKenney
  0 siblings, 1 reply; 7+ messages in thread
From: Kent Overstreet @ 2020-11-15 20:35 UTC (permalink / raw)
  To: Paul E. McKenney; +Cc: rcu

On Sun, Nov 15, 2020 at 12:20:17PM -0800, Paul E. McKenney wrote:
> On Sun, Nov 15, 2020 at 12:11:24PM -0800, Paul E. McKenney wrote:
> > On Thu, Nov 12, 2020 at 03:15:47PM -0500, Kent Overstreet wrote:
> > > Hi to Paul & the rcu mailing list,
> > > 
> > > I've got a problem I'm trying to figure out if I can adapt SRCU for.
> > > 
> > > Within bcachefs, currently struct btree obects, and now also btree_cached_key,
> > > aren't freed until the filesystem is torn down, because btree iterators contain
> > > pointers to them and will drop and retake locks (iff a sequence number hasn't
> > > changed) without holding a ref. With the btree key cache code, this is now
> > > something I need to fix.
> > > 
> > > What I plan on doing is having struct btree_trans (container for btree
> > > iterators) hold an srcu read lock while it's alive. But, I don't want to just
> > > use call_srcu to free the btree/btree_cached_key objects, because btree trans
> > > objects can at times be fairly long lived and the existing code can reuse these
> > > objects for other btree nodes/btree cached keys immediately. Freeing them with
> > > call_srcu() would break that; I could have my callback function check if the
> > > object has been reused before freeing, but I'd still have a problem when the
> > > object gets freed a second time before the first call_scru() has finished.
> > > 
> > > What I'm wondering is if the SRCU code has a well defined notion of a clock that
> > > I could make use of. What I would like to do is, instead of doing call_srcu() to
> > > free the object - just mark that object with the current SRCU context time, and
> > > then when my shrinkers run they can free objects that haven't been reused and
> > > are old enough according the the current SRCU time.
> > > 
> > > Thoughts?
> > 
> > An early prototype is available on -rcu [1].  The Tree SRCU version
> > seems to be reasonably reliable, but I do not yet trust the Tiny SRCU
> > implementation.  So please avoid !SMP&&!PREEMPT if you would like to
> > give it a try.  Unless you -really- like helping me find bugs, in which
> > case full speed ahead!!!
> > 
> > Here is the API:
> > 
> > unsigned long start_poll_synchronize_srcu(struct srcu_struct *ssp)
> > 
> > 	Returns a "cookie" that can be thought of as a snapshot of your
> > 	"clock" above.	(SRCU calls it a "grace-period sequence number".)
> > 	Also ensures that enough future grace periods happen to eventually
> > 	make the grace-period sequence number reach the cookie.
> > 
> > bool poll_state_synchronize_srcu(struct srcu_struct *ssp, unsigned long cookie)
> > 
> > 	Given a cookie from start_poll_synchronize_srcu(), returns true if
> > 	at least one full SRCU grace period has elapsed in the meantime.
> > 	Given finite SRCU readers in a well-behaved kernel, the following
> > 	code will complete in finite time:
> > 
> > 		cookie = start_poll_synchronize_srcu(&my_srcu);
> > 		while (!poll_state_synchronize_srcu(&my_srcu, cookie))
> > 			schedule_timeout_uninterruptible(1);
> > 
> > unsigned long get_state_synchronize_srcu(struct srcu_struct *ssp)
> > 
> > 	Like start_poll_synchronize_srcu(), except that it does not start
> > 	any grace periods.  This means that the following code is -not-
> > 	guaranteed to complete:
> > 
> > 		cookie = get_state_synchronize_srcu(&my_srcu);
> > 		while (!poll_state_synchronize_srcu(&my_srcu, cookie))
> > 			schedule_timeout_uninterruptible(1);
> > 
> > 	Use this if you know that something else will be starting the
> > 	needed SRCU grace periods.  This might also be useful if you
> > 	had items that were likely to be reused before the SRCU grace
> > 	period elapsed, so that you avoid burning CPU on SRCU grace
> > 	periods that prove to be unnecessary.  Or if you don't want
> > 	to have more than (say) 100 SRCU grace periods per seconds,
> > 	in which case you might use a timer to start the grace periods.
> > 	Or maybe you don't bother starting the SRCU grace period until
> > 	some sort of emergency situation has arisen.  Or...
> > 
> > 	OK, maybe you don't need it, but I do need it for rcutorture,
> > 	so here it is anyway.
> > 
> > All of these can be invoked anywhere that call_srcu() can be invoked.
> > 
> > Does this look like it will work for you?

Yeah. My one quibble is, instead of having to call poll_state_synchronize_srcu()
on every cookie - would it be possible to get function that returns a cookie we
can compare against (i.e. with time_after())? I'm going to be use this in the
shrinker where we have to walk and check potentially tens of thousands of
objects.

> Oh, and due to historical inertia, Tiny SRCU's grace-period sequence
> number is only 16 bits.  I can change this easily, but I need to know
> that it is a real problem for you before I can do so.
> 
> The potential problem for you is that if you let a given cookie lie
> dormant for 16384 grace periods, it will take another 16385 grace
> periods for get_state_synchronize_srcu() to say that a grace period
> has elapsed.
> 
> In contrast, Tree SRCU's grace-period sequence number is either 32 bits
> or 64 bits, depending on the size of unsized long.

It's not something I'd lose sleep over, but I think it could be. If there isn't
memory pressure, then the shrinker won't be running and we won't be freeing the
objects with the oldest cookies, but freeing them internally will be creating
new grace periods - but if I make sure we're reusing objects in LIFO order would
also work against the shrinker actually being able to free any objects, so not
sure I want to do that...

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: SRCU question
  2020-11-15 20:35     ` Kent Overstreet
@ 2020-11-15 20:53       ` Paul E. McKenney
  2020-11-16  1:25         ` Kent Overstreet
  0 siblings, 1 reply; 7+ messages in thread
From: Paul E. McKenney @ 2020-11-15 20:53 UTC (permalink / raw)
  To: Kent Overstreet; +Cc: rcu

On Sun, Nov 15, 2020 at 03:35:51PM -0500, Kent Overstreet wrote:
> On Sun, Nov 15, 2020 at 12:20:17PM -0800, Paul E. McKenney wrote:
> > On Sun, Nov 15, 2020 at 12:11:24PM -0800, Paul E. McKenney wrote:
> > > On Thu, Nov 12, 2020 at 03:15:47PM -0500, Kent Overstreet wrote:
> > > > Hi to Paul & the rcu mailing list,
> > > > 
> > > > I've got a problem I'm trying to figure out if I can adapt SRCU for.
> > > > 
> > > > Within bcachefs, currently struct btree obects, and now also btree_cached_key,
> > > > aren't freed until the filesystem is torn down, because btree iterators contain
> > > > pointers to them and will drop and retake locks (iff a sequence number hasn't
> > > > changed) without holding a ref. With the btree key cache code, this is now
> > > > something I need to fix.
> > > > 
> > > > What I plan on doing is having struct btree_trans (container for btree
> > > > iterators) hold an srcu read lock while it's alive. But, I don't want to just
> > > > use call_srcu to free the btree/btree_cached_key objects, because btree trans
> > > > objects can at times be fairly long lived and the existing code can reuse these
> > > > objects for other btree nodes/btree cached keys immediately. Freeing them with
> > > > call_srcu() would break that; I could have my callback function check if the
> > > > object has been reused before freeing, but I'd still have a problem when the
> > > > object gets freed a second time before the first call_scru() has finished.
> > > > 
> > > > What I'm wondering is if the SRCU code has a well defined notion of a clock that
> > > > I could make use of. What I would like to do is, instead of doing call_srcu() to
> > > > free the object - just mark that object with the current SRCU context time, and
> > > > then when my shrinkers run they can free objects that haven't been reused and
> > > > are old enough according the the current SRCU time.
> > > > 
> > > > Thoughts?
> > > 
> > > An early prototype is available on -rcu [1].  The Tree SRCU version
> > > seems to be reasonably reliable, but I do not yet trust the Tiny SRCU
> > > implementation.  So please avoid !SMP&&!PREEMPT if you would like to
> > > give it a try.  Unless you -really- like helping me find bugs, in which
> > > case full speed ahead!!!
> > > 
> > > Here is the API:
> > > 
> > > unsigned long start_poll_synchronize_srcu(struct srcu_struct *ssp)
> > > 
> > > 	Returns a "cookie" that can be thought of as a snapshot of your
> > > 	"clock" above.	(SRCU calls it a "grace-period sequence number".)
> > > 	Also ensures that enough future grace periods happen to eventually
> > > 	make the grace-period sequence number reach the cookie.
> > > 
> > > bool poll_state_synchronize_srcu(struct srcu_struct *ssp, unsigned long cookie)
> > > 
> > > 	Given a cookie from start_poll_synchronize_srcu(), returns true if
> > > 	at least one full SRCU grace period has elapsed in the meantime.
> > > 	Given finite SRCU readers in a well-behaved kernel, the following
> > > 	code will complete in finite time:
> > > 
> > > 		cookie = start_poll_synchronize_srcu(&my_srcu);
> > > 		while (!poll_state_synchronize_srcu(&my_srcu, cookie))
> > > 			schedule_timeout_uninterruptible(1);
> > > 
> > > unsigned long get_state_synchronize_srcu(struct srcu_struct *ssp)
> > > 
> > > 	Like start_poll_synchronize_srcu(), except that it does not start
> > > 	any grace periods.  This means that the following code is -not-
> > > 	guaranteed to complete:
> > > 
> > > 		cookie = get_state_synchronize_srcu(&my_srcu);
> > > 		while (!poll_state_synchronize_srcu(&my_srcu, cookie))
> > > 			schedule_timeout_uninterruptible(1);
> > > 
> > > 	Use this if you know that something else will be starting the
> > > 	needed SRCU grace periods.  This might also be useful if you
> > > 	had items that were likely to be reused before the SRCU grace
> > > 	period elapsed, so that you avoid burning CPU on SRCU grace
> > > 	periods that prove to be unnecessary.  Or if you don't want
> > > 	to have more than (say) 100 SRCU grace periods per seconds,
> > > 	in which case you might use a timer to start the grace periods.
> > > 	Or maybe you don't bother starting the SRCU grace period until
> > > 	some sort of emergency situation has arisen.  Or...
> > > 
> > > 	OK, maybe you don't need it, but I do need it for rcutorture,
> > > 	so here it is anyway.
> > > 
> > > All of these can be invoked anywhere that call_srcu() can be invoked.
> > > 
> > > Does this look like it will work for you?
> 
> Yeah. My one quibble is, instead of having to call poll_state_synchronize_srcu()
> on every cookie - would it be possible to get function that returns a cookie we
> can compare against (i.e. with time_after())? I'm going to be use this in the
> shrinker where we have to walk and check potentially tens of thousands of
> objects.

If the cookies compare equal, poll_state_synchronize_srcu() will treat
them the same.  If that does not help, could you please show me a code
snippet illustrating what you would like to do?

(Yes, even if equality comparison works, I probably need to give you
an API member just in case the nature of grace-period sequence numbers
changes in the future.)

> > Oh, and due to historical inertia, Tiny SRCU's grace-period sequence
> > number is only 16 bits.  I can change this easily, but I need to know
> > that it is a real problem for you before I can do so.
> > 
> > The potential problem for you is that if you let a given cookie lie
> > dormant for 16384 grace periods, it will take another 16385 grace
> > periods for get_state_synchronize_srcu() to say that a grace period
> > has elapsed.
> > 
> > In contrast, Tree SRCU's grace-period sequence number is either 32 bits
> > or 64 bits, depending on the size of unsized long.
> 
> It's not something I'd lose sleep over, but I think it could be. If there isn't
> memory pressure, then the shrinker won't be running and we won't be freeing the
> objects with the oldest cookies, but freeing them internally will be creating
> new grace periods - but if I make sure we're reusing objects in LIFO order would
> also work against the shrinker actually being able to free any objects, so not
> sure I want to do that...

OK, I will leave it, at least until you tell me otherwise.

I probably need to add a warning to the header comment, though...

							Thanx, Paul

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: SRCU question
  2020-11-15 20:53       ` Paul E. McKenney
@ 2020-11-16  1:25         ` Kent Overstreet
  2020-11-16  4:14           ` Paul E. McKenney
  0 siblings, 1 reply; 7+ messages in thread
From: Kent Overstreet @ 2020-11-16  1:25 UTC (permalink / raw)
  To: Paul E. McKenney; +Cc: rcu

On Sun, Nov 15, 2020 at 12:53:39PM -0800, Paul E. McKenney wrote:
> On Sun, Nov 15, 2020 at 03:35:51PM -0500, Kent Overstreet wrote:
> > Yeah. My one quibble is, instead of having to call poll_state_synchronize_srcu()
> > on every cookie - would it be possible to get function that returns a cookie we
> > can compare against (i.e. with time_after())? I'm going to be use this in the
> > shrinker where we have to walk and check potentially tens of thousands of
> > objects.
> 
> If the cookies compare equal, poll_state_synchronize_srcu() will treat
> them the same.  If that does not help, could you please show me a code
> snippet illustrating what you would like to do?
> 
> (Yes, even if equality comparison works, I probably need to give you
> an API member just in case the nature of grace-period sequence numbers
> changes in the future.)

Having just looked at your code - I think it's fine as is. I wouldn't complain
if it was a static inline, though.

> 
> > > Oh, and due to historical inertia, Tiny SRCU's grace-period sequence
> > > number is only 16 bits.  I can change this easily, but I need to know
> > > that it is a real problem for you before I can do so.
> > > 
> > > The potential problem for you is that if you let a given cookie lie
> > > dormant for 16384 grace periods, it will take another 16385 grace
> > > periods for get_state_synchronize_srcu() to say that a grace period
> > > has elapsed.
> > > 
> > > In contrast, Tree SRCU's grace-period sequence number is either 32 bits
> > > or 64 bits, depending on the size of unsized long.
> > 
> > It's not something I'd lose sleep over, but I think it could be. If there isn't
> > memory pressure, then the shrinker won't be running and we won't be freeing the
> > objects with the oldest cookies, but freeing them internally will be creating
> > new grace periods - but if I make sure we're reusing objects in LIFO order would
> > also work against the shrinker actually being able to free any objects, so not
> > sure I want to do that...
> 
> OK, I will leave it, at least until you tell me otherwise.
> 
> I probably need to add a warning to the header comment, though...
> 
> 							Thanx, Paul

Thanks! 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: SRCU question
  2020-11-16  1:25         ` Kent Overstreet
@ 2020-11-16  4:14           ` Paul E. McKenney
  0 siblings, 0 replies; 7+ messages in thread
From: Paul E. McKenney @ 2020-11-16  4:14 UTC (permalink / raw)
  To: Kent Overstreet; +Cc: rcu

On Sun, Nov 15, 2020 at 08:25:25PM -0500, Kent Overstreet wrote:
> On Sun, Nov 15, 2020 at 12:53:39PM -0800, Paul E. McKenney wrote:
> > On Sun, Nov 15, 2020 at 03:35:51PM -0500, Kent Overstreet wrote:
> > > Yeah. My one quibble is, instead of having to call poll_state_synchronize_srcu()
> > > on every cookie - would it be possible to get function that returns a cookie we
> > > can compare against (i.e. with time_after())? I'm going to be use this in the
> > > shrinker where we have to walk and check potentially tens of thousands of
> > > objects.
> > 
> > If the cookies compare equal, poll_state_synchronize_srcu() will treat
> > them the same.  If that does not help, could you please show me a code
> > snippet illustrating what you would like to do?
> > 
> > (Yes, even if equality comparison works, I probably need to give you
> > an API member just in case the nature of grace-period sequence numbers
> > changes in the future.)
> 
> Having just looked at your code - I think it's fine as is. I wouldn't complain
> if it was a static inline, though.

Thank you for taking a look!

My guess is that given the required memory-barrier instructions, the
overhead of the external call is way down in the noise.  But let's see
how the performance and scalability looks in your particular use case,
and if it needs help, base the changes on your actual experience.

							Thanx, Paul

> > > > Oh, and due to historical inertia, Tiny SRCU's grace-period sequence
> > > > number is only 16 bits.  I can change this easily, but I need to know
> > > > that it is a real problem for you before I can do so.
> > > > 
> > > > The potential problem for you is that if you let a given cookie lie
> > > > dormant for 16384 grace periods, it will take another 16385 grace
> > > > periods for get_state_synchronize_srcu() to say that a grace period
> > > > has elapsed.
> > > > 
> > > > In contrast, Tree SRCU's grace-period sequence number is either 32 bits
> > > > or 64 bits, depending on the size of unsized long.
> > > 
> > > It's not something I'd lose sleep over, but I think it could be. If there isn't
> > > memory pressure, then the shrinker won't be running and we won't be freeing the
> > > objects with the oldest cookies, but freeing them internally will be creating
> > > new grace periods - but if I make sure we're reusing objects in LIFO order would
> > > also work against the shrinker actually being able to free any objects, so not
> > > sure I want to do that...
> > 
> > OK, I will leave it, at least until you tell me otherwise.
> > 
> > I probably need to add a warning to the header comment, though...
> > 
> > 							Thanx, Paul
> 
> Thanks! 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-11-16  4:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-12 20:15 SRCU question Kent Overstreet
2020-11-15 20:11 ` Paul E. McKenney
2020-11-15 20:20   ` Paul E. McKenney
2020-11-15 20:35     ` Kent Overstreet
2020-11-15 20:53       ` Paul E. McKenney
2020-11-16  1:25         ` Kent Overstreet
2020-11-16  4:14           ` Paul E. McKenney

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.