Messages in this thread | | | From | Uladzislau Rezki <> | Date | Mon, 30 Mar 2020 14:48:41 +0200 | Subject | Re: [PATCH 4/7] rcu/tree: support reclaim for head-less object |
| |
> > Very nice work, Vlad! And beautifully split! Makes it easy to review! One > comment below but otherwise patches 1-4 look good to me, I will look at > others as well now. I have some patches on top of the series, mostly little > clean ups which I will send together with yours. > Thanks, Joel!
> > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com> > > --- > > kernel/rcu/tree.c | 94 +++++++++++++++++++++++++++++++++++++++++++---- > > 1 file changed, 86 insertions(+), 8 deletions(-) > > > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > > index 869a72e25d38..5a64c92feafc 100644 > > --- a/kernel/rcu/tree.c > > +++ b/kernel/rcu/tree.c > > @@ -2892,19 +2892,34 @@ static void kfree_rcu_work(struct work_struct *work) > > * when we could not allocate a bulk array. > > * > > * Under that condition an object is queued to the > > - * list instead. > > + * list instead. Please note that head-less objects > > + * have dynamically attached rcu_head, so they also > > + * contain a back-pointer that has to be freed. > > */ > > for (; head; head = next) { > > unsigned long offset = (unsigned long)head->func; > > - void *ptr = (void *)head - offset; > > + bool headless; > > + void *ptr; > > > > next = head->next; > > + > > + /* We tag the headless object, if so adjust offset. */ > > + headless = (((unsigned long) head - offset) & BIT(0)); > > + if (headless) > > + offset -= 1; > > Just to be sure, can vmalloc() ever allocate an object at an odd valued > memory address? I'm not fully sure looking at vmalloc code whether this is > the case. > No. It must be PAGE_ALIGNED(addr).
> > As per the tagging, allocated objects have to at least at a 2-byte boundary > for the pointer's BIT(0) to be available. If that's not the case, we need to > add a warning to the code at a bare minimum. > > Another approach which is better I think is to add the tag to the offset > itself. So if the offset is > LONG_MAX / 2 or something like that, then > assume it is headless, and override offset to sizeof(unsigned long *) in that > case. Then you would arrive at the correct pointer for the wrapper. That > would take care of the case where in the future, either SLAB or vmalloc() > ends up returning pointers that are only byte-aligned. > > Thoughts? > I saw your https://lkml.org/lkml/2020/3/29/480. I think it is OK, and we can go your way. There is advantage that it is tolerate to any alignment :)
Thanks!
-- Vlad Rezki
| |