From: Rao Shoaib <rao.shoaib@oracle.com> To: Boqun Feng <boqun.feng@gmail.com> Cc: Matthew Wilcox <willy@infradead.org>, linux-kernel@vger.kernel.org, paulmck@linux.vnet.ibm.com, brouer@redhat.com, linux-mm@kvack.org Subject: Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c Date: Thu, 4 Jan 2018 12:35:55 -0800 [thread overview] Message-ID: <be1abd24-56c8-45bc-fecc-3f0c5b978678@oracle.com> (raw) In-Reply-To: <20180104013807.GA31392@tardis> Hi Boqun, Thanks a lot for all your guidance and for catching the cut and paster error. Please see inline. On 01/03/2018 05:38 PM, Boqun Feng wrote: > > But you introduced a bug here, you should use rcu_state_p instead of > &rcu_sched_state as the third parameter for __call_rcu(). > > Please re-read: > > https://marc.info/?l=linux-mm&m=151390529209639 > > , and there are other comments, which you still haven't resolved in this > version. You may want to write a better commit log to explain the > reasons of each modifcation and fix bugs or typos in your previous > version. That's how review process works ;-) > > Regards, > Boqun > This is definitely a serious error. Thanks for catching this. As far as your previous comments are concerned, only the following one has not been addressed. Can you please elaborate as I do not understand the comment. The code was expanded because the new macro expansion check fails. Based on Matthew Wilcox's comment I have reverted rcu_head_name back to rcu_head. > +#define kfree_rcu(ptr, rcu_head_name) \ > + do { \ > + typeof(ptr) __ptr = ptr; \ > + unsigned long __off = offsetof(typeof(*(__ptr)), \ > + rcu_head_name); \ > + struct rcu_head *__rptr = (void *)__ptr + __off; \ > + __kfree_rcu(__rptr, __off); \ > + } while (0) why do you want to open code this? Does the following text for the commit log looks better. kfree_rcu() should use the new kfree_bulk() interface for freeing rcu structures The newly implemented kfree_bulk() interfaces are more efficient, using the interfaces for freeing rcu structures has shown performance improvements in synthetic benchmarks that allocate and free rcu structures at a high rate. Shoaib
WARNING: multiple messages have this Message-ID (diff)
From: Rao Shoaib <rao.shoaib@oracle.com> To: Boqun Feng <boqun.feng@gmail.com> Cc: Matthew Wilcox <willy@infradead.org>, linux-kernel@vger.kernel.org, paulmck@linux.vnet.ibm.com, brouer@redhat.com, linux-mm@kvack.org Subject: Re: [PATCH 1/2] Move kfree_call_rcu() to slab_common.c Date: Thu, 4 Jan 2018 12:35:55 -0800 [thread overview] Message-ID: <be1abd24-56c8-45bc-fecc-3f0c5b978678@oracle.com> (raw) In-Reply-To: <20180104013807.GA31392@tardis> Hi Boqun, Thanks a lot for all your guidance and for catching the cut and paster error. Please see inline. On 01/03/2018 05:38 PM, Boqun Feng wrote: > > But you introduced a bug here, you should use rcu_state_p instead of > &rcu_sched_state as the third parameter for __call_rcu(). > > Please re-read: > > https://marc.info/?l=linux-mm&m=151390529209639 > > , and there are other comments, which you still haven't resolved in this > version. You may want to write a better commit log to explain the > reasons of each modifcation and fix bugs or typos in your previous > version. That's how review process works ;-) > > Regards, > Boqun > This is definitely a serious error. Thanks for catching this. As far as your previous comments are concerned, only the following one has not been addressed. Can you please elaborate as I do not understand the comment. The code was expanded because the new macro expansion check fails. Based on Matthew Wilcox's comment I have reverted rcu_head_name back to rcu_head. > +#define kfree_rcu(ptr, rcu_head_name) \ > + do { \ > + typeof(ptr) __ptr = ptr; \ > + unsigned long __off = offsetof(typeof(*(__ptr)), \ > + rcu_head_name); \ > + struct rcu_head *__rptr = (void *)__ptr + __off; \ > + __kfree_rcu(__rptr, __off); \ > + } while (0) why do you want to open code this? Does the following text for the commit log looks better. kfree_rcu() should use the new kfree_bulk() interface for freeing rcu structures The newly implemented kfree_bulk() interfaces are more efficient, using the interfaces for freeing rcu structures has shown performance improvements in synthetic benchmarks that allocate and free rcu structures at a high rate. Shoaib -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2018-01-04 20:36 UTC|newest] Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-01-02 20:11 [PATCH 1/2] Move kfree_call_rcu() to slab_common.c rao.shoaib 2018-01-02 20:11 ` rao.shoaib 2018-01-02 20:11 ` [PATCH 2/2] kfree_rcu() should use the new kfree_bulk() interface for freeing rcu structures rao.shoaib 2018-01-02 20:11 ` rao.shoaib 2018-01-02 22:23 ` [PATCH 1/2] Move kfree_call_rcu() to slab_common.c Matthew Wilcox 2018-01-02 22:23 ` Matthew Wilcox 2018-01-02 22:49 ` Rao Shoaib 2018-01-02 22:49 ` Rao Shoaib 2018-01-04 1:38 ` Boqun Feng 2018-01-04 20:35 ` Rao Shoaib [this message] 2018-01-04 20:35 ` Rao Shoaib 2018-01-04 21:27 ` Rao Shoaib 2018-01-04 21:27 ` Rao Shoaib 2018-01-04 21:46 ` Matthew Wilcox 2018-01-04 21:46 ` Matthew Wilcox 2018-01-04 22:18 ` Rao Shoaib 2018-01-04 22:18 ` Rao Shoaib 2018-01-04 23:13 ` Matthew Wilcox 2018-01-04 23:13 ` Matthew Wilcox 2018-01-04 23:47 ` Paul E. McKenney 2018-01-04 23:47 ` Paul E. McKenney 2018-01-05 0:07 ` Matthew Wilcox 2018-01-05 0:07 ` Matthew Wilcox 2018-01-05 2:14 ` Rao Shoaib 2018-01-05 2:14 ` Rao Shoaib 2018-01-05 6:46 ` Joe Perches 2018-01-05 6:46 ` Joe Perches 2018-03-27 1:56 ` Rao Shoaib 2018-03-27 2:06 ` Joe Perches 2018-04-02 5:31 [PATCH 0/2] Move kfree_rcu out of rcu code and use kfree_bulk rao.shoaib 2018-04-02 5:31 ` [PATCH 1/2] Move kfree_call_rcu() to slab_common.c rao.shoaib 2018-04-02 7:59 ` kbuild test robot 2018-04-02 7:59 ` kbuild test robot 2018-04-02 9:45 ` kbuild test robot 2018-04-02 9:45 ` kbuild test robot 2018-04-02 15:43 ` Paul E. McKenney 2018-04-03 17:22 [PATCH 0/2] Move kfree_rcu out of rcu code and use kfree_bulk rao.shoaib 2018-04-03 17:22 ` [PATCH 1/2] Move kfree_call_rcu() to slab_common.c rao.shoaib
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=be1abd24-56c8-45bc-fecc-3f0c5b978678@oracle.com \ --to=rao.shoaib@oracle.com \ --cc=boqun.feng@gmail.com \ --cc=brouer@redhat.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=paulmck@linux.vnet.ibm.com \ --cc=willy@infradead.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.