On Wed, Jan 03, 2024 at 01:46:56PM +0800, Huang, Ying wrote: > Gregory Price <gregory.price@xxxxxxxxxxxx> writes: > > I'm specifically concerned about: > > weighted_interleave_nid > > alloc_pages_bulk_array_weighted_interleave > > > > I'm unsure whether kmalloc/kfree is safe (and non-offensive) in those > > contexts. If kmalloc/kfree is safe fine, this problem is trivial. > > > > If not, there is no good solution to this without pre-allocating a > > scratch area per-task. > > You need to audit whether it's safe for all callers. I guess that you > need to allocate pages after calling, so you can use the same GFP flags > here. > After picking away i realized that this code is usually going to get called during page fault handling - duh. So kmalloc is almost never safe (or can fail), and we it's nasty to try to handle those errors. Instead of doing that, I simply chose to implement the scratch space in the mempolicy structure mempolicy->wil.scratch_weights[MAX_NUMNODES]. We eat an extra 1kb of memory in the mempolicy, but it gives us a safe scratch space we can use any time the task is allocating memory, and prevents the need for any fancy error handling. That seems like a perfectly reasonable tradeoff. > > > > Weights are collected individually onto the stack because we have to sum > > them up before we actually apply the weights. > > > > A stale weight is not offensive. RCU is not needed and doesn't help. > > When you copy weights from iw_table[] to stack, it's possible for > compiler to cache its contents in register, or merge, split the memory > operations. At the same time, iw_table[] may be changed simultaneously > via sysfs interface. So, we need a mechanism to guarantee that we read > the latest contents consistently. > Fair enough, I went ahead and added a similar interaction. ~Gregoryg
