On Mon, Sep 28, 2020 at 02:08:29PM +0000, chenjun (AM) wrote:
> On Mon, Sep 21, 2020 at 02:00:05AM +0000, Chen Jun wrote:
> > From: Wei Yongjun <weiyongj...@huawei.com>
> >> @@ -651,6 +672,19 @@ static void create_object(unsigned long ptr, size_t
> >> size, int min_count,
> >> raw_spin_unlock_irqrestore(&kmemleak_lock, flags);
> >> }
> >>
> >> +static void create_object(unsigned long ptr, size_t size, int min_count,
> >> + gfp_t gfp)
> >> +{
> >> + __create_object(ptr, size, min_count, 0, gfp);
> >> +}
> >> +
> >> +static void create_object_percpu(unsigned long ptr, size_t size, int
> >> min_count,
> >> + gfp_t gfp)
> >> +{
> >> + __create_object(ptr, size, min_count, OBJECT_PERCPU | OBJECT_NO_SCAN,
> >> + gfp);
> >> +}
> >> +
> >> /*
> >> * Mark the object as not allocated and schedule RCU freeing via
> >> put_object().
> >> */
> >> @@ -912,10 +946,12 @@ void __ref kmemleak_alloc_percpu(const void __percpu
> >> *ptr, size_t size,
> >> * Percpu allocations are only scanned and not reported as leaks
> >> * (min_count is set to 0).
> >> */
> >> - if (kmemleak_enabled && ptr && !IS_ERR(ptr))
> >> + if (kmemleak_enabled && ptr && !IS_ERR(ptr)) {
> >> for_each_possible_cpu(cpu)
> >> create_object((unsigned long)per_cpu_ptr(ptr, cpu),
> >> size, 0, gfp);
> >> + create_object_percpu((unsigned long)ptr, size, 1, gfp);
> >> + }
> >> }
> >
> > A concern I have here is that ptr may overlap with an existing object
> > and the insertion in the rb tree will fail. For example, with !SMP,
> > ptr == per_cpu_ptr(ptr, 0), so create_object() will fail and kmemleak
> > gets disabled.
> >
> > An option would to figure out how to allow overlapping ranges with rb
> > tree (or find a replacement for it if not possible).
> >
> > Another option would be to have an additional structure to track the
> > __percpu pointers since they have their own range. If size is not
> > relevant, maybe go for an xarray, otherwise another rb tree (do we have
> > any instance of pointers referring some inner member of a __percpu
> > object?). The scan_object() function will have to search two trees.
>
> I would like to use CONFIG_SMP to seprate code:
> if SMP, we will create some objects for per_cpu_ptr(ptr, cpu) and an
> object with OBJECT_NO_ACCESS for ptr.
> if !SMP, we will not create object for per_cpu_ptr(ptr,cpu), but an
> object without OBJECT_NO_ACCESS for ptr will be created.
> What do you think about this opinion.
The !SMP case was just an example. Do you have a guarantee that the
value of the __percpu ptr doesn't clash with a linear map address?
--
Catalin
