Re: [PATCH 1/3] rcu: Use static initializer for krc.lock

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Apr 17, 2020 at 02:26:41PM -0400, Joel Fernandes wrote:
> On Fri, Apr 17, 2020 at 05:04:42PM +0200, Sebastian Andrzej Siewior wrote:
> > On 2020-04-16 23:05:15 [-0400], Joel Fernandes wrote:
> > > On Thu, Apr 16, 2020 at 11:34:44PM +0200, Sebastian Andrzej Siewior wrote:
> > > > On 2020-04-16 14:00:57 [-0700], Paul E. McKenney wrote:
> > > > > 
> > > > > We might need different calling-context restrictions for the two variants
> > > > > of kfree_rcu().  And we might need to come up with some sort of lockdep
> > > > > check for "safe to use normal spinlock in -rt".
> > > > 
> > > > Oh. We do have this already, it is called CONFIG_PROVE_RAW_LOCK_NESTING.
> > > > This one will scream if you do
> > > > 	raw_spin_lock();
> > > > 	spin_lock();
> > > > 
> > > > Sadly, as of today, there is code triggering this which needs to be
> > > > addressed first (but it is one list of things to do).
> > > > 
> > > > Given the thread so far, is it okay if I repost the series with
> > > > migrate_disable() instead of accepting a possible migration before
> > > > grabbing the lock? I would prefer to avoid the extra RT case (avoiding
> > > > memory allocations in a possible atomic context) until we get there.
> > > 
> > > I prefer something like the following to make it possible to invoke
> > > kfree_rcu() from atomic context considering call_rcu() is already callable
> > > from such contexts. Thoughts?
> > 
> > So it looks like it would work. However, could we please delay this
> > until we have an actual case on RT? I just added
> > 	WARN_ON(!preemptible());
> 
> I am not sure if waiting for it to break in the future is a good idea. I'd
> rather design it in a forward thinking way. There could be folks replacing
> "call_rcu() + kfree in a callback" with kfree_rcu() for example. If they were
> in !preemptible(), we'd break on page allocation.
> 
> Also as a sidenote, the additional pre-allocation of pages that Vlad is
> planning on adding would further reduce the need for pages from the page
> allocator.
> 
> Paul, what is your opinion on this?

My experience with call_rcu(), of which kfree_rcu() is a specialization,
is that it gets invoked with preemption disabled, with interrupts
disabled, and during early boot, as in even before rcu_init() has been
invoked.  This experience does make me lean towards raw spinlocks.

But to Sebastian's point, if we are going to use raw spinlocks, we need
to keep the code paths holding those spinlocks as short as possible.
I suppose that the inability to allocate memory with raw spinlocks held
helps, but it is worth checking.

							Thanx, Paul

> thanks,
> 
>  - Joel
> 
> 
> > 
> > to kfree_call_rcu() on v5.6.4-rt4 and nothing triggered.
> > 
> > This is the list of users I had (just to figure out if this is used at
> > all):
> > - addrconf_ifdown
> > - cgroup_free
> > - cgroup_migrate_finish
> > - css_task_iter_end
> > - disk_expand_part_tbl
> > - drop_sysctl_table
> > - __hw_addr_flush
> > - inetdev_event
> > - ip6addrlbl_net_exit
> > - ip6addrlbl_net_exit
> > - ops_exit_list.isra.0
> > - rtnl_register_internal
> > - simple_set_acl
> > - swevent_hlist_put_cpu
> > - timerfd_release
> > - vfs_rename
> > - __xfs_set_acl
> > 
> > Sebastian



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux