Re: [PATCH 5/9] lib/ref_tracker: improve allocation flags

On Thu, Feb 17, 2022 at 6:05 AM Andrzej Hajda andrzej.hajda@intel.com wrote:

Library can be called in non-sleeping context, so it should not use __GFP_NOFAIL. Instead it should calmly handle allocation fails, for this __GFP_NOWARN has been added as well.

Your commit changelog is misleading .

The GFP_NOFAIL issue has been fixed already in commit c12837d1bb31032bead9060dec99ef310d5b9fb7 ("ref_tracker: use __GFP_NOFAIL more carefully")

Signed-off-by: Andrzej Hajda andrzej.hajda@intel.com

lib/ref_tracker.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/lib/ref_tracker.c b/lib/ref_tracker.c index 7b00bca300043..c8441ffbb058a 100644 --- a/lib/ref_tracker.c +++ b/lib/ref_tracker.c @@ -59,7 +59,7 @@ __ref_tracker_dir_pr_ostream(struct ref_tracker_dir *dir, if (list_empty(&dir->list)) return;

•   sbuf = kmalloc(STACK_BUF_SIZE, GFP_NOWAIT);
  

•   sbuf = kmalloc(STACK_BUF_SIZE, GFP_NOWAIT | __GFP_NOWARN);
  

This belongs to patch 3 in your series.

    list_for_each_entry(tracker, &dir->list, head)
            ++total;

@@ -154,11 +154,11 @@ int ref_tracker_alloc(struct ref_tracker_dir *dir, unsigned long entries[REF_TRACKER_STACK_ENTRIES]; struct ref_tracker *tracker; unsigned int nr_entries;

•   gfp_t gfp_mask = gfp;
  

Simply change this line to : gfp_t gfp_mask = gfp | __GFP_NOFAIL;

•   gfp_t gfp_mask;
    unsigned long flags;
  
  

Then leave all this code as is ? I find current code more readable.

•   if (gfp & __GFP_DIRECT_RECLAIM)
  

•           gfp_mask |= __GFP_NOFAIL;
  

•   gfp |= __GFP_NOWARN;
  

•   gfp_mask = (gfp & __GFP_DIRECT_RECLAIM) ? (gfp | __GFP_NOFAIL) : gfp;
    *trackerp = tracker = kzalloc(sizeof(*tracker), gfp_mask);
    if (unlikely(!tracker)) {
            pr_err_once("memory allocation failure, unreliable refcount tracker.\n");
  

@@ -191,7 +191,8 @@ int ref_tracker_free(struct ref_tracker_dir *dir, } nr_entries = stack_trace_save(entries, ARRAY_SIZE(entries), 1); nr_entries = filter_irq_stacks(entries, nr_entries);

lib/ref_tracker.c got patches in net-next, your patch series is going to add conflicts.

git log --oneline 5740d0689096..4d449bdc5b26 --no-merges -- lib/ref_tracker.c c2d1e3df4af59261777b39c2e47476acd4d1cbeb ref_tracker: remove filter_irq_stacks() call 8fd5522f44dcd7f05454ddc4f16d0f821b676cd9 ref_tracker: add a count of untracked references e3ececfe668facd87d920b608349a32607060e66 ref_tracker: implement use-after-free detection

•   stack_handle = stack_depot_save(entries, nr_entries, GFP_ATOMIC);
  

•   stack_handle = stack_depot_save(entries, nr_entries,
  

•                                   GFP_NOWAIT | __GFP_NOWARN);
  

This is fine.

    spin_lock_irqsave(&dir->lock, flags);
    if (tracker->dead) {

-- 2.25.1