Received-SPF: pass (google.com: best guess record for domain of linux-ext4-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Uladzislau Rezki <urezki@gmail.com>
Date:   Mon, 17 Feb 2020 17:08:27 +0100
To:     "Paul E. McKenney" <paulmck@kernel.org>,
        Joel Fernandes <joel@joelfernandes.org>,
        "Theodore Y. Ts'o" <tytso@mit.edu>
Cc:     "Theodore Y. Ts'o" <tytso@mit.edu>,
        Ext4 Developers List <linux-ext4@vger.kernel.org>,
        Suraj Jitindar Singh <surajjs@amazon.com>,
        Uladzislau Rezki <urezki@gmail.com>,
        Joel Fernandes <joel@joelfernandes.org>
Subject: Re: [PATCH RFC] ext4: fix potential race between online resizing and
 write operations
Message-ID: <20200217160827.GA5685@pc636>
References: <20200215233817.GA670792@mit.edu>
 <20200216121246.GG2935@paulmck-ThinkPad-P72>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200216121246.GG2935@paulmck-ThinkPad-P72>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk

Hello, Joel, Paul, Ted. 

> 
> Good point!
> 
> Now that kfree_rcu() is on its way to being less of a hack deeply
> entangled into the bowels of RCU, this might be fairly easy to implement.
> It might well be simply a matter of a function pointer and a kvfree_rcu()
> API.  Adding Uladzislau Rezki and Joel Fernandez on CC for their thoughts.
> 
I think it makes sense. For example i see there is a similar demand in
the mm/list_lru.c too. As for implementation, it will not be hard, i think. 

The easiest way is to inject kvfree() support directly into existing kfree_call_rcu()
logic(probably will need to rename that function), i.e. to free vmalloc() allocations
only in "emergency path" by just calling kvfree(). So that function in its turn will
figure out if it is _vmalloc_ address or not and trigger corresponding "free" path.

Just an example:

<snip>
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 75a2eded7aa2..0c4af5d0e3f8 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -806,6 +806,11 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
                kfree_call_rcu(head, (rcu_callback_t)(unsigned long)(offset)); \
        } while (0)

+#define __kvfree_rcu(head, offset) \
+       do { \
+               kfree_call_rcu(head, (rcu_callback_t)(unsigned long)(offset)); \
+       } while (0)
+
 /**
  * kfree_rcu() - kfree an object after a grace period.
  * @ptr:       pointer to kfree
@@ -840,6 +845,14 @@ do {                                                                       \
                __kfree_rcu(&((___p)->rhf), offsetof(typeof(*(ptr)), rhf)); \
 } while (0)

+#define kvfree_rcu_my(ptr, rhf)                                                \
+do {                                                                   \
+       typeof (ptr) ___p = (ptr);                                      \
+                                                                       \
+       if (___p)                                                       \
+               __kvfree_rcu(&((___p)->rhf), offsetof(typeof(*(ptr)), rhf)); \
+} while (0)
+
 /*
  * Place this after a lock-acquisition primitive to guarantee that
  * an UNLOCK+LOCK pair acts as a full barrier.  This guarantee applies
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 394a83bd7ff4..1a05bb44951b 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2783,11 +2783,10 @@ static void kfree_rcu_work(struct work_struct *work)
                rcu_lock_acquire(&rcu_callback_map);
                trace_rcu_invoke_kfree_callback(rcu_state.name, head, offset);
 
-               if (!WARN_ON_ONCE(!__is_kfree_rcu_offset(offset)))
-                       kfree((void *)head - offset);
+               kvfree((void *)head - offset);
 
-                rcu_lock_release(&rcu_callback_map);
-                cond_resched_tasks_rcu_qs();
+               rcu_lock_release(&rcu_callback_map);
+               cond_resched_tasks_rcu_qs();
        }
 }
 
@@ -2964,7 +2963,8 @@ void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
         * Under high memory pressure GFP_NOWAIT can fail,
         * in that case the emergency path is maintained.
         */
-       if (unlikely(!kfree_call_rcu_add_ptr_to_bulk(krcp, head, func))) {
+       if (is_vmalloc_addr((void *) head - (unsigned long) func) ||
+                       unlikely(!kfree_call_rcu_add_ptr_to_bulk(krcp, head, func))) {
                head->func = func;
                head->next = krcp->head;
                krcp->head = head;
<snip>

How to use it:

<snip>
struct test_kvfree_rcu {
       unsigned char array[PAGE_SIZE * 2];
       struct rcu_head rcu;
};

struct test_kvfree_rcu *p;

p = vmalloc(sizeof(struct test_kvfree_rcu));
kvfree_rcu_my((struct test_kvfree_rcu *) p, rcu);
<snip>

Any thoughts?

Thank you!

--
Vlad Rezki