The rcu callback free_dm_hw_stat() just calls a kfree(),
so we use kfree_rcu() instead of the call_rcu(free_dm_hw_stat).
Signed-off-by: Lai Jiangshan <[email protected]>
---
net/core/drop_monitor.c | 12 ++----------
1 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 36e603c..32365e7 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -207,14 +207,6 @@ static void trace_napi_poll_hit(void *ignore, struct napi_struct *napi)
rcu_read_unlock();
}
-
-static void free_dm_hw_stat(struct rcu_head *head)
-{
- struct dm_hw_stat_delta *n;
- n = container_of(head, struct dm_hw_stat_delta, rcu);
- kfree(n);
-}
-
static int set_all_monitor_traces(int state)
{
int rc = 0;
@@ -245,7 +237,7 @@ static int set_all_monitor_traces(int state)
list_for_each_entry_safe(new_stat, temp, &hw_stats_list, list) {
if (new_stat->dev == NULL) {
list_del_rcu(&new_stat->list);
- call_rcu(&new_stat->rcu, free_dm_hw_stat);
+ kfree_rcu(new_stat, rcu);
}
}
break;
@@ -314,7 +306,7 @@ static int dropmon_net_event(struct notifier_block *ev_block,
new_stat->dev = NULL;
if (trace_state == TRACE_OFF) {
list_del_rcu(&new_stat->list);
- call_rcu(&new_stat->rcu, free_dm_hw_stat);
+ kfree_rcu(new_stat, rcu);
break;
}
}
--
1.7.4
The rcu callback fc_rport_free_rcu() just calls a kfree(),
so we use kfree_rcu() instead of the call_rcu(fc_rport_free_rcu).
Signed-off-by: Lai Jiangshan <[email protected]>
---
drivers/scsi/libfc/fc_rport.c | 14 +-------------
1 files changed, 1 insertions(+), 13 deletions(-)
diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c
index a7175ad..29abab0 100644
--- a/drivers/scsi/libfc/fc_rport.c
+++ b/drivers/scsi/libfc/fc_rport.c
@@ -151,18 +151,6 @@ static struct fc_rport_priv *fc_rport_create(struct fc_lport *lport,
}
/**
- * fc_rport_free_rcu() - Free a remote port
- * @rcu: The rcu_head structure inside the remote port
- */
-static void fc_rport_free_rcu(struct rcu_head *rcu)
-{
- struct fc_rport_priv *rdata;
-
- rdata = container_of(rcu, struct fc_rport_priv, rcu);
- kfree(rdata);
-}
-
-/**
* fc_rport_destroy() - Free a remote port after last reference is released
* @kref: The remote port's kref
*/
@@ -171,7 +159,7 @@ static void fc_rport_destroy(struct kref *kref)
struct fc_rport_priv *rdata;
rdata = container_of(kref, struct fc_rport_priv, kref);
- call_rcu(&rdata->rcu, fc_rport_free_rcu);
+ kfree_rcu(rdata, rcu);
}
/**
--
1.7.4
The rcu callback fc_rport_free_rcu() just calls a kfree(),
so we use kfree_rcu() instead of the call_rcu(fc_rport_free_rcu).
Signed-off-by: Lai Jiangshan <[email protected]>
---
net/ipv4/fib_semantics.c | 10 +---------
1 files changed, 1 insertions(+), 9 deletions(-)
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 12d3dc3..730c03e 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -145,16 +145,8 @@ static const struct
},
};
-
/* Release a nexthop info record */
-static void free_fib_info_rcu(struct rcu_head *head)
-{
- struct fib_info *fi = container_of(head, struct fib_info, rcu);
-
- kfree(fi);
-}
-
void free_fib_info(struct fib_info *fi)
{
if (fi->fib_dead == 0) {
@@ -168,7 +160,7 @@ void free_fib_info(struct fib_info *fi)
} endfor_nexthops(fi);
fib_info_cnt--;
release_net(fi->fib_net);
- call_rcu(&fi->rcu, free_fib_info_rcu);
+ kfree_rcu(fi, rcu);
}
void fib_release_info(struct fib_info *fi)
--
1.7.4
The rcu callback __leaf_info_free_rcu() just calls a kfree(),
so we use kfree_rcu() instead of the call_rcu(__leaf_info_free_rcu).
Signed-off-by: Lai Jiangshan <[email protected]>
---
net/ipv4/fib_trie.c | 7 +------
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 0f28034..6a60915 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -350,14 +350,9 @@ static inline void free_leaf(struct leaf *l)
call_rcu_bh(&l->rcu, __leaf_free_rcu);
}
-static void __leaf_info_free_rcu(struct rcu_head *head)
-{
- kfree(container_of(head, struct leaf_info, rcu));
-}
-
static inline void free_leaf_info(struct leaf_info *leaf)
{
- call_rcu(&leaf->rcu, __leaf_info_free_rcu);
+ kfree_rcu(leaf, rcu);
}
static struct tnode *tnode_alloc(size_t size)
--
1.7.4
The rcu callback disk_free_ptbl_rcu_cb() just calls a kfree(),
so we use kfree_rcu() instead of the call_rcu(disk_free_ptbl_rcu_cb).
Signed-off-by: Lai Jiangshan <[email protected]>
---
block/genhd.c | 10 +---------
1 files changed, 1 insertions(+), 9 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index cbf1112..2800cc8 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1018,14 +1018,6 @@ static const struct attribute_group *disk_attr_groups[] = {
NULL
};
-static void disk_free_ptbl_rcu_cb(struct rcu_head *head)
-{
- struct disk_part_tbl *ptbl =
- container_of(head, struct disk_part_tbl, rcu_head);
-
- kfree(ptbl);
-}
-
/**
* disk_replace_part_tbl - replace disk->part_tbl in RCU-safe way
* @disk: disk to replace part_tbl for
@@ -1046,7 +1038,7 @@ static void disk_replace_part_tbl(struct gendisk *disk,
if (old_ptbl) {
rcu_assign_pointer(old_ptbl->last_lookup, NULL);
- call_rcu(&old_ptbl->rcu_head, disk_free_ptbl_rcu_cb);
+ kfree_rcu(old_ptbl, rcu_head);
}
}
--
1.7.4
The rcu callback __gen_kill_estimator() just calls a kfree(),
so we use kfree_rcu() instead of the call_rcu(__gen_kill_estimator).
Signed-off-by: Lai Jiangshan <[email protected]>
---
net/core/gen_estimator.c | 9 +--------
1 files changed, 1 insertions(+), 8 deletions(-)
diff --git a/net/core/gen_estimator.c b/net/core/gen_estimator.c
index 7c23733..43b03dd 100644
--- a/net/core/gen_estimator.c
+++ b/net/core/gen_estimator.c
@@ -249,13 +249,6 @@ int gen_new_estimator(struct gnet_stats_basic_packed *bstats,
}
EXPORT_SYMBOL(gen_new_estimator);
-static void __gen_kill_estimator(struct rcu_head *head)
-{
- struct gen_estimator *e = container_of(head,
- struct gen_estimator, e_rcu);
- kfree(e);
-}
-
/**
* gen_kill_estimator - remove a rate estimator
* @bstats: basic statistics
@@ -279,7 +272,7 @@ void gen_kill_estimator(struct gnet_stats_basic_packed *bstats,
write_unlock(&est_lock);
list_del_rcu(&e->list);
- call_rcu(&e->e_rcu, __gen_kill_estimator);
+ kfree_rcu(e, e_rcu);
}
spin_unlock_bh(&est_tree_lock);
}
--
1.7.4
The rcu callback ip_mc_list_reclaim() just calls a kfree(),
so we use kfree_rcu() instead of the call_rcu(ip_mc_list_reclaim).
Signed-off-by: Lai Jiangshan <[email protected]>
---
net/ipv4/igmp.c | 8 +-------
1 files changed, 1 insertions(+), 7 deletions(-)
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index e0e77e2..cf187b6 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -149,17 +149,11 @@ static void ip_mc_clear_src(struct ip_mc_list *pmc);
static int ip_mc_add_src(struct in_device *in_dev, __be32 *pmca, int sfmode,
int sfcount, __be32 *psfsrc, int delta);
-
-static void ip_mc_list_reclaim(struct rcu_head *head)
-{
- kfree(container_of(head, struct ip_mc_list, rcu));
-}
-
static void ip_ma_put(struct ip_mc_list *im)
{
if (atomic_dec_and_test(&im->refcnt)) {
in_dev_put(im->interface);
- call_rcu(&im->rcu, ip_mc_list_reclaim);
+ kfree_rcu(im, rcu);
}
}
--
1.7.4
The rcu callback ip_sf_socklist_reclaim() just calls a kfree(),
so we use kfree_rcu() instead of the call_rcu(ip_sf_socklist_reclaim).
Signed-off-by: Lai Jiangshan <[email protected]>
---
net/ipv4/igmp.c | 12 +++---------
1 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index cf187b6..cf98a36 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -1830,12 +1830,6 @@ done:
}
EXPORT_SYMBOL(ip_mc_join_group);
-static void ip_sf_socklist_reclaim(struct rcu_head *rp)
-{
- kfree(container_of(rp, struct ip_sf_socklist, rcu));
- /* sk_omem_alloc should have been decreased by the caller*/
-}
-
static int ip_mc_leave_src(struct sock *sk, struct ip_mc_socklist *iml,
struct in_device *in_dev)
{
@@ -1852,7 +1846,7 @@ static int ip_mc_leave_src(struct sock *sk, struct ip_mc_socklist *iml,
rcu_assign_pointer(iml->sflist, NULL);
/* decrease mem now to avoid the memleak warning */
atomic_sub(IP_SFLSIZE(psf->sl_max), &sk->sk_omem_alloc);
- call_rcu(&psf->rcu, ip_sf_socklist_reclaim);
+ kfree_rcu(psf, rcu);
return err;
}
@@ -2020,7 +2014,7 @@ int ip_mc_source(int add, int omode, struct sock *sk, struct
newpsl->sl_addr[i] = psl->sl_addr[i];
/* decrease mem now to avoid the memleak warning */
atomic_sub(IP_SFLSIZE(psl->sl_max), &sk->sk_omem_alloc);
- call_rcu(&psl->rcu, ip_sf_socklist_reclaim);
+ kfree_rcu(psl, rcu);
}
rcu_assign_pointer(pmc->sflist, newpsl);
psl = newpsl;
@@ -2121,7 +2115,7 @@ int ip_mc_msfilter(struct sock *sk, struct ip_msfilter *msf, int ifindex)
psl->sl_count, psl->sl_addr, 0);
/* decrease mem now to avoid the memleak warning */
atomic_sub(IP_SFLSIZE(psl->sl_max), &sk->sk_omem_alloc);
- call_rcu(&psl->rcu, ip_sf_socklist_reclaim);
+ kfree_rcu(psl, rcu);
} else
(void) ip_mc_del_src(in_dev, &msf->imsf_multiaddr, pmc->sfmode,
0, NULL, 0);
--
1.7.4
The rcu callback ip_mc_socklist_reclaim() just calls a kfree(),
so we use kfree_rcu() instead of the call_rcu(ip_mc_socklist_reclaim).
Signed-off-by: Lai Jiangshan <[email protected]>
---
net/ipv4/igmp.c | 12 ++----------
1 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index cf98a36..998dff0 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -1850,14 +1850,6 @@ static int ip_mc_leave_src(struct sock *sk, struct ip_mc_socklist *iml,
return err;
}
-
-static void ip_mc_socklist_reclaim(struct rcu_head *rp)
-{
- kfree(container_of(rp, struct ip_mc_socklist, rcu));
- /* sk_omem_alloc should have been decreased by the caller*/
-}
-
-
/*
* Ask a socket to leave a group.
*/
@@ -1897,7 +1889,7 @@ int ip_mc_leave_group(struct sock *sk, struct ip_mreqn *imr)
rtnl_unlock();
/* decrease mem now to avoid the memleak warning */
atomic_sub(sizeof(*iml), &sk->sk_omem_alloc);
- call_rcu(&iml->rcu, ip_mc_socklist_reclaim);
+ kfree_rcu(iml, rcu);
return 0;
}
if (!in_dev)
@@ -2312,7 +2304,7 @@ void ip_mc_drop_socket(struct sock *sk)
ip_mc_dec_group(in_dev, iml->multi.imr_multiaddr.s_addr);
/* decrease mem now to avoid the memleak warning */
atomic_sub(sizeof(*iml), &sk->sk_omem_alloc);
- call_rcu(&iml->rcu, ip_mc_socklist_reclaim);
+ kfree_rcu(iml, rcu);
}
rtnl_unlock();
}
--
1.7.4
On 2011-03-18 04:42, Lai Jiangshan wrote:
>
>
> The rcu callback disk_free_ptbl_rcu_cb() just calls a kfree(),
> so we use kfree_rcu() instead of the call_rcu(disk_free_ptbl_rcu_cb).
Thanks, I'll add this.
--
Jens Axboe
On Fri, Mar 18, 2011 at 07:59:53AM +0100, Jens Axboe wrote:
> On 2011-03-18 04:42, Lai Jiangshan wrote:
> >
> >
> > The rcu callback disk_free_ptbl_rcu_cb() just calls a kfree(),
> > so we use kfree_rcu() instead of the call_rcu(disk_free_ptbl_rcu_cb).
>
> Thanks, I'll add this.
Don't be in too much of a hurry ... kfree_rcu() doesn't appear to have
hit Linus' tree yet.
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
On Fri, Mar 18, 2011 at 11:39:43AM +0800, Lai Jiangshan wrote:
>
>
> The rcu callback free_dm_hw_stat() just calls a kfree(),
> so we use kfree_rcu() instead of the call_rcu(free_dm_hw_stat).
>
> Signed-off-by: Lai Jiangshan <[email protected]>
Acked-by: Neil Horman <[email protected]>
> ---
> net/core/drop_monitor.c | 12 ++----------
> 1 files changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
> index 36e603c..32365e7 100644
> --- a/net/core/drop_monitor.c
> +++ b/net/core/drop_monitor.c
> @@ -207,14 +207,6 @@ static void trace_napi_poll_hit(void *ignore, struct napi_struct *napi)
> rcu_read_unlock();
> }
>
> -
> -static void free_dm_hw_stat(struct rcu_head *head)
> -{
> - struct dm_hw_stat_delta *n;
> - n = container_of(head, struct dm_hw_stat_delta, rcu);
> - kfree(n);
> -}
> -
> static int set_all_monitor_traces(int state)
> {
> int rc = 0;
> @@ -245,7 +237,7 @@ static int set_all_monitor_traces(int state)
> list_for_each_entry_safe(new_stat, temp, &hw_stats_list, list) {
> if (new_stat->dev == NULL) {
> list_del_rcu(&new_stat->list);
> - call_rcu(&new_stat->rcu, free_dm_hw_stat);
> + kfree_rcu(new_stat, rcu);
> }
> }
> break;
> @@ -314,7 +306,7 @@ static int dropmon_net_event(struct notifier_block *ev_block,
> new_stat->dev = NULL;
> if (trace_state == TRACE_OFF) {
> list_del_rcu(&new_stat->list);
> - call_rcu(&new_stat->rcu, free_dm_hw_stat);
> + kfree_rcu(new_stat, rcu);
> break;
> }
> }
> --
> 1.7.4
>
From: Lai Jiangshan <[email protected]>
Date: Fri, 18 Mar 2011 11:39:43 +0800
>
>
> The rcu callback free_dm_hw_stat() just calls a kfree(),
> so we use kfree_rcu() instead of the call_rcu(free_dm_hw_stat).
>
> Signed-off-by: Lai Jiangshan <[email protected]>
Acked-by: David S. Miller <[email protected]>
From: Lai Jiangshan <[email protected]>
Date: Fri, 18 Mar 2011 11:42:11 +0800
>
>
> The rcu callback fc_rport_free_rcu() just calls a kfree(),
> so we use kfree_rcu() instead of the call_rcu(fc_rport_free_rcu).
>
> Signed-off-by: Lai Jiangshan <[email protected]>
Acked-by: David S. Miller <[email protected]>
From: Lai Jiangshan <[email protected]>
Date: Fri, 18 Mar 2011 11:43:26 +0800
>
>
> The rcu callback __gen_kill_estimator() just calls a kfree(),
> so we use kfree_rcu() instead of the call_rcu(__gen_kill_estimator).
>
> Signed-off-by: Lai Jiangshan <[email protected]>
Acked-by: David S. Miller <[email protected]>
From: Lai Jiangshan <[email protected]>
Date: Fri, 18 Mar 2011 11:44:46 +0800
>
>
> The rcu callback ip_sf_socklist_reclaim() just calls a kfree(),
> so we use kfree_rcu() instead of the call_rcu(ip_sf_socklist_reclaim).
>
> Signed-off-by: Lai Jiangshan <[email protected]>
Acked-by: David S. Miller <[email protected]>
From: Lai Jiangshan <[email protected]>
Date: Fri, 18 Mar 2011 11:45:08 +0800
>
>
> The rcu callback ip_mc_socklist_reclaim() just calls a kfree(),
> so we use kfree_rcu() instead of the call_rcu(ip_mc_socklist_reclaim).
>
> Signed-off-by: Lai Jiangshan <[email protected]>
Acked-by: David S. Miller <[email protected]>
From: Lai Jiangshan <[email protected]>
Date: Fri, 18 Mar 2011 11:44:08 +0800
>
>
> The rcu callback ip_mc_list_reclaim() just calls a kfree(),
> so we use kfree_rcu() instead of the call_rcu(ip_mc_list_reclaim).
>
> Signed-off-by: Lai Jiangshan <[email protected]>
Acked-by: David S. Miller <[email protected]>
From: Lai Jiangshan <[email protected]>
Date: Fri, 18 Mar 2011 11:42:34 +0800
>
>
> The rcu callback __leaf_info_free_rcu() just calls a kfree(),
> so we use kfree_rcu() instead of the call_rcu(__leaf_info_free_rcu).
>
> Signed-off-by: Lai Jiangshan <[email protected]>
Acked-by: David S. Miller <[email protected]>
On Thu, 2011-03-17 at 20:41 -0700, Lai Jiangshan wrote:
>
> The rcu callback fc_rport_free_rcu() just calls a kfree(),
> so we use kfree_rcu() instead of the call_rcu(fc_rport_free_rcu).
>
> Signed-off-by: Lai Jiangshan <[email protected]>
> ---
> drivers/scsi/libfc/fc_rport.c | 14 +-------------
> 1 files changed, 1 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c
> index a7175ad..29abab0 100644
> --- a/drivers/scsi/libfc/fc_rport.c
> +++ b/drivers/scsi/libfc/fc_rport.c
> @@ -151,18 +151,6 @@ static struct fc_rport_priv *fc_rport_create(struct fc_lport *lport,
> }
>
> /**
> - * fc_rport_free_rcu() - Free a remote port
> - * @rcu: The rcu_head structure inside the remote port
> - */
> -static void fc_rport_free_rcu(struct rcu_head *rcu)
> -{
> - struct fc_rport_priv *rdata;
> -
> - rdata = container_of(rcu, struct fc_rport_priv, rcu);
> - kfree(rdata);
> -}
> -
> -/**
> * fc_rport_destroy() - Free a remote port after last reference is released
> * @kref: The remote port's kref
> */
> @@ -171,7 +159,7 @@ static void fc_rport_destroy(struct kref *kref)
> struct fc_rport_priv *rdata;
>
> rdata = container_of(kref, struct fc_rport_priv, kref);
> - call_rcu(&rdata->rcu, fc_rport_free_rcu);
> + kfree_rcu(rdata, rcu);
I think this last line should be:
kfree_rcu(rdata, &rdata->rcu);
Thanks, //Rob
On Tue, Mar 22, 2011 at 10:28:33AM -0700, Robert Love wrote:
> On Thu, 2011-03-17 at 20:41 -0700, Lai Jiangshan wrote:
> >
> > The rcu callback fc_rport_free_rcu() just calls a kfree(),
> > so we use kfree_rcu() instead of the call_rcu(fc_rport_free_rcu).
> >
> > Signed-off-by: Lai Jiangshan <[email protected]>
> > ---
> > drivers/scsi/libfc/fc_rport.c | 14 +-------------
> > 1 files changed, 1 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c
> > index a7175ad..29abab0 100644
> > --- a/drivers/scsi/libfc/fc_rport.c
> > +++ b/drivers/scsi/libfc/fc_rport.c
> > @@ -151,18 +151,6 @@ static struct fc_rport_priv *fc_rport_create(struct fc_lport *lport,
> > }
> >
> > /**
> > - * fc_rport_free_rcu() - Free a remote port
> > - * @rcu: The rcu_head structure inside the remote port
> > - */
> > -static void fc_rport_free_rcu(struct rcu_head *rcu)
> > -{
> > - struct fc_rport_priv *rdata;
> > -
> > - rdata = container_of(rcu, struct fc_rport_priv, rcu);
> > - kfree(rdata);
> > -}
> > -
> > -/**
> > * fc_rport_destroy() - Free a remote port after last reference is released
> > * @kref: The remote port's kref
> > */
> > @@ -171,7 +159,7 @@ static void fc_rport_destroy(struct kref *kref)
> > struct fc_rport_priv *rdata;
> >
> > rdata = container_of(kref, struct fc_rport_priv, kref);
> > - call_rcu(&rdata->rcu, fc_rport_free_rcu);
> > + kfree_rcu(rdata, rcu);
>
> I think this last line should be:
>
> kfree_rcu(rdata, &rdata->rcu);
Hello, Robert,
I believe that it is correct as is. The kfree_rcu() definition is as
follows:
#define kfree_rcu(ptr, rcu_head) \
__kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
Then __kfree_rcu() encodes the offset into the rcu_head so that it can
be handled properly at the end of the grace period.
Thanx, Paul
On Tue, Mar 22, 2011 at 11:50:14PM -0700, Paul E. McKenney wrote:
> On Tue, Mar 22, 2011 at 10:28:33AM -0700, Robert Love wrote:
> > On Thu, 2011-03-17 at 20:41 -0700, Lai Jiangshan wrote:
> > > - call_rcu(&rdata->rcu, fc_rport_free_rcu);
> > > + kfree_rcu(rdata, rcu);
> >
> > I think this last line should be:
> >
> > kfree_rcu(rdata, &rdata->rcu);
>
> Hello, Robert,
>
> I believe that it is correct as is. The kfree_rcu() definition is as
> follows:
>
> #define kfree_rcu(ptr, rcu_head) \
> __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
>
> Then __kfree_rcu() encodes the offset into the rcu_head so that it can
> be handled properly at the end of the grace period.
To be fair to Robert, I had the same thought, but decided to check the
definition of kfree_rcu before I made the same comment. Unfortunately,
the definition of kfree_rcu is still not in Linus' tree (and there's no
indication in the patch series about where to find the definition). It
would have been better if kfree_rcu were patch 1/n in the series, then
we'd've been able to review it more effectively.
Any idea when kfree_rcu will be submitted to Linus?
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
On Wed, Mar 23, 2011 at 05:22:57AM -0600, Matthew Wilcox wrote:
> On Tue, Mar 22, 2011 at 11:50:14PM -0700, Paul E. McKenney wrote:
> > On Tue, Mar 22, 2011 at 10:28:33AM -0700, Robert Love wrote:
> > > On Thu, 2011-03-17 at 20:41 -0700, Lai Jiangshan wrote:
> > > > - call_rcu(&rdata->rcu, fc_rport_free_rcu);
> > > > + kfree_rcu(rdata, rcu);
> > >
> > > I think this last line should be:
> > >
> > > kfree_rcu(rdata, &rdata->rcu);
> >
> > Hello, Robert,
> >
> > I believe that it is correct as is. The kfree_rcu() definition is as
> > follows:
> >
> > #define kfree_rcu(ptr, rcu_head) \
> > __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
> >
> > Then __kfree_rcu() encodes the offset into the rcu_head so that it can
> > be handled properly at the end of the grace period.
>
> To be fair to Robert, I had the same thought, but decided to check the
> definition of kfree_rcu before I made the same comment. Unfortunately,
> the definition of kfree_rcu is still not in Linus' tree (and there's no
> indication in the patch series about where to find the definition). It
> would have been better if kfree_rcu were patch 1/n in the series, then
> we'd've been able to review it more effectively.
Good point! That said, it is a bit ugly no matter how we handle it
because any of the patches that have not received acked-bys will need
to go up through their respective maintainer trees. Last time I had a
situation like this, it ended up stalling my commit stream for several
months, so was trying to separate them in order to avoid the stall.
But perhaps this is one of those situations where there is simply no
good way to handle the full series. :-(
> Any idea when kfree_rcu will be submitted to Linus?
I have it queued, and it passes mild testing. I therefore expect to
push it for the next merge window.
So, do you guys want to carry the patches for your subsystems, or are
you willing to ack this one so that I can push it via -tip?
Thanx, Paul
On Tue, 2011-03-22 at 23:50 -0700, Paul E. McKenney wrote:
> The kfree_rcu() definition is as
> follows:
>
> #define kfree_rcu(ptr, rcu_head) \
> __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
Isn't this one of those cases where the obvious use of the interface is
definitely wrong?
It's also another nasty pseudo C prototype. I know we do this sort of
thing for container_of et al, but I don't really think we want to extend
it.
Why not make the interface take a pointer to the embedding structure and
one to the rcu_head ... that way all pointer mathematics can be
contained inside the RCU routines.
James
On Wed, Mar 23, 2011 at 09:05:51AM -0500, James Bottomley wrote:
> On Tue, 2011-03-22 at 23:50 -0700, Paul E. McKenney wrote:
> > The kfree_rcu() definition is as
> > follows:
> >
> > #define kfree_rcu(ptr, rcu_head) \
> > __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
>
> Isn't this one of those cases where the obvious use of the interface is
> definitely wrong?
>
> It's also another nasty pseudo C prototype. I know we do this sort of
> thing for container_of et al, but I don't really think we want to extend
> it.
>
> Why not make the interface take a pointer to the embedding structure and
> one to the rcu_head ... that way all pointer mathematics can be
> contained inside the RCU routines.
Hello, James,
If you pass in a pair of pointers, then it is difficult for RCU to detect
bugs where the two pointers are unrelated. Yes, you can do some sanity
checks, but these get cumbersome and have corner cases where they can
be fooled. In contrast, Lai's interface allows the compiler to do the
needed type checking -- unless the second argument is a field of type
struct rcu_head in the structure pointed to by the first argument, the
compiler will complain.
Either way, the pointer mathematics are buried in the RCU API.
Or am I missing something here?
Thanx, Paul
On Wed, 2011-03-23 at 15:24 -0700, Paul E. McKenney wrote:
> On Wed, Mar 23, 2011 at 09:05:51AM -0500, James Bottomley wrote:
> > On Tue, 2011-03-22 at 23:50 -0700, Paul E. McKenney wrote:
> > > The kfree_rcu() definition is as
> > > follows:
> > >
> > > #define kfree_rcu(ptr, rcu_head) \
> > > __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
> >
> > Isn't this one of those cases where the obvious use of the interface is
> > definitely wrong?
> >
> > It's also another nasty pseudo C prototype. I know we do this sort of
> > thing for container_of et al, but I don't really think we want to extend
> > it.
> >
> > Why not make the interface take a pointer to the embedding structure and
> > one to the rcu_head ... that way all pointer mathematics can be
> > contained inside the RCU routines.
>
> Hello, James,
>
> If you pass in a pair of pointers, then it is difficult for RCU to detect
> bugs where the two pointers are unrelated. Yes, you can do some sanity
> checks, but these get cumbersome and have corner cases where they can
> be fooled. In contrast, Lai's interface allows the compiler to do the
> needed type checking -- unless the second argument is a field of type
> struct rcu_head in the structure pointed to by the first argument, the
> compiler will complain.
>
> Either way, the pointer mathematics are buried in the RCU API.
>
> Or am I missing something here?
No ... I like the utility ... I just dislike the inelegance of having to
name a structure element in what looks like a C prototype.
I can see this proliferating everywhere since most of our reference
counting release callbacks basically free the enclosing object ...
James
On Wed, Mar 23, 2011 at 09:05:51AM -0500, James Bottomley wrote:
> > #define kfree_rcu(ptr, rcu_head) \
> > __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
>
> Isn't this one of those cases where the obvious use of the interface is
> definitely wrong?
But it's a compile time breakage if you use it wrong, not runtime.
> It's also another nasty pseudo C prototype. I know we do this sort of
> thing for container_of et al, but I don't really think we want to extend
> it.
We do it for list_entry, list_for_each_entry, etc. And those are very
widespread within the kernel.
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
On Wed, Mar 23, 2011 at 05:45:32PM -0500, James Bottomley wrote:
> On Wed, 2011-03-23 at 15:24 -0700, Paul E. McKenney wrote:
> > On Wed, Mar 23, 2011 at 09:05:51AM -0500, James Bottomley wrote:
> > > On Tue, 2011-03-22 at 23:50 -0700, Paul E. McKenney wrote:
> > > > The kfree_rcu() definition is as
> > > > follows:
> > > >
> > > > #define kfree_rcu(ptr, rcu_head) \
> > > > __kfree_rcu(&((ptr)->rcu_head), offsetof(typeof(*(ptr)), rcu_head))
> > >
> > > Isn't this one of those cases where the obvious use of the interface is
> > > definitely wrong?
> > >
> > > It's also another nasty pseudo C prototype. I know we do this sort of
> > > thing for container_of et al, but I don't really think we want to extend
> > > it.
> > >
> > > Why not make the interface take a pointer to the embedding structure and
> > > one to the rcu_head ... that way all pointer mathematics can be
> > > contained inside the RCU routines.
> >
> > Hello, James,
> >
> > If you pass in a pair of pointers, then it is difficult for RCU to detect
> > bugs where the two pointers are unrelated. Yes, you can do some sanity
> > checks, but these get cumbersome and have corner cases where they can
> > be fooled. In contrast, Lai's interface allows the compiler to do the
> > needed type checking -- unless the second argument is a field of type
> > struct rcu_head in the structure pointed to by the first argument, the
> > compiler will complain.
> >
> > Either way, the pointer mathematics are buried in the RCU API.
> >
> > Or am I missing something here?
>
> No ... I like the utility ... I just dislike the inelegance of having to
> name a structure element in what looks like a C prototype.
>
> I can see this proliferating everywhere since most of our reference
> counting release callbacks basically free the enclosing object ...
Indeed! Improvements are welcome -- it is just that I am not convinced
that the dual-pointer approach is really an improvement.
The C preprocessor... It is ugly, inelegant, painful, annoying, and
should have been strangled at birth -- but it is always there when you
need it!
Thanx, Paul