Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1739019 > unrolled thread

[PATCH 2/4] idr: Add a function idr_get()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/4] idr: Add a function idr_get() Gargi Sharma <gs051095@gmail.com> - 2017-09-25 15:00 +0200
    Re: [PATCH 2/4] idr: Add a function idr_get() Rik van Riel <riel@surriel.com> - 2017-09-25 15:30 +0200
      Re: [PATCH 2/4] idr: Add a function idr_get() Christoph Hellwig <hch@infradead.org> - 2017-09-25 15:50 +0200
    Re: [PATCH 2/4] idr: Add a function idr_get() Oleg Nesterov <oleg@redhat.com> - 2017-09-25 16:20 +0200
      Re: [PATCH 2/4] idr: Add a function idr_get() Gargi Sharma <gs051095@gmail.com> - 2017-09-25 19:50 +0200

#1739019 — [PATCH 2/4] idr: Add a function idr_get()

idr_get(namespace, id) returns a NULL if id is not present
in the idr tree or returns the pointer to the struct if id is
present in the idr tree. With this function in the idr library,
code for pid allocation can be simplified by calling this function
instead of looking through the pidhash.

Signed-off-by: Gargi Sharma <gs051095@gmail.com>
---
 include/linux/idr.h |  1 +
 lib/idr.c           | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/include/linux/idr.h b/include/linux/idr.h
index 7c3a365..e12b174 100644
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -135,6 +135,7 @@ int idr_for_each(const struct idr *,
 		 int (*fn)(int id, void *p, void *data), void *data);
 void *idr_get_next(struct idr *, int *nextid);
 void *idr_get_next_ext(struct idr *idr, unsigned long *nextid);
+void *idr_get(struct idr *idr, int *id);
 void *idr_replace(struct idr *, void *, int id);
 void *idr_replace_ext(struct idr *idr, void *ptr, unsigned long id);
 void idr_destroy(struct idr *);
diff --git a/lib/idr.c b/lib/idr.c
index f9adf48..bb76400 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -135,6 +135,17 @@ void *idr_get_next_ext(struct idr *idr, unsigned long *nextid)
 }
 EXPORT_SYMBOL(idr_get_next_ext);
 
+void * idr_get(struct idr *idr, int *id)
+{
+	struct radix_tree_node *node;
+	void __rcu **slot = NULL;
+
+	__radix_tree_lookup(&idr->idr_rt, *id, &node, &slot);
+	if (!slot)
+		return NULL;
+	return node;
+}
+
 /**
  * idr_replace - replace pointer for given id
  * @idr: idr handle
-- 
2.7.4

[toc] | [next] | [standalone]

#1739051

[Multipart message — attachments visible in raw view] — view raw

On Mon, 2017-09-25 at 08:56 -0400, Gargi Sharma wrote:
> idr_get(namespace, id) returns a NULL if id is not present
> in the idr tree or returns the pointer to the struct if id is
> present in the idr tree. With this function in the idr library,
> code for pid allocation can be simplified by calling this function
> instead of looking through the pidhash.

> +++ b/lib/idr.c
> @@ -135,6 +135,17 @@ void *idr_get_next_ext(struct idr *idr, unsigned
> long *nextid)
>  }
>  EXPORT_SYMBOL(idr_get_next_ext);
>  
> +void * idr_get(struct idr *idr, int *id)
> +{
> +	struct radix_tree_node *node;
> +	void __rcu **slot = NULL;
> +
> +	__radix_tree_lookup(&idr->idr_rt, *id, &node, &slot);
> +	if (!slot)
> +		return NULL;
> +	return node;
> +}

I should have noticed this (much) earlier, but doesn't idr_get do
essentially the same thing as idr_find?

Also, wouldn't you want to return the pid pointer from slot,
rather than a pointer to the entire radix tree node?

-- 
All Rights Reversed.

[toc] | [prev] | [next] | [standalone]

#1739071

On Mon, Sep 25, 2017 at 09:20:07AM -0400, Rik van Riel wrote:
> > +++ b/lib/idr.c
> > @@ -135,6 +135,17 @@ void *idr_get_next_ext(struct idr *idr, unsigned
> > long *nextid)
> >  }
> >  EXPORT_SYMBOL(idr_get_next_ext);
> >  
> > +void * idr_get(struct idr *idr, int *id)
> > +{
> > +	struct radix_tree_node *node;
> > +	void __rcu **slot = NULL;
> > +
> > +	__radix_tree_lookup(&idr->idr_rt, *id, &node, &slot);
> > +	if (!slot)
> > +		return NULL;
> > +	return node;
> > +}
> 
> I should have noticed this (much) earlier, but doesn't idr_get do
> essentially the same thing as idr_find?
> 
> Also, wouldn't you want to return the pid pointer from slot,
> rather than a pointer to the entire radix tree node?

It also seems rather odd to pass id by reference here just to
dereference it a little later.

[toc] | [prev] | [next] | [standalone]

#1739082

On 09/25, Gargi Sharma wrote:
>
> idr_get(namespace, id) returns a NULL if id is not present
> in the idr tree or returns the pointer to the struct if id is
> present in the idr tree. With this function in the idr library,
> code for pid allocation can be simplified by calling this function
> instead of looking through the pidhash.

Could you explain why find_pid_ns() can't use idr_find() ?

> +void * idr_get(struct idr *idr, int *id)
> +{
> +	struct radix_tree_node *node;
> +	void __rcu **slot = NULL;
> +
> +	__radix_tree_lookup(&idr->idr_rt, *id, &node, &slot);

so why it takes "int *", not just "int" ?

Oleg.

[toc] | [prev] | [next] | [standalone]

#1739196

On Mon, Sep 25, 2017 at 7:42 PM, Oleg Nesterov <oleg@redhat.com> wrote:
> On 09/25, Gargi Sharma wrote:
>>
>> idr_get(namespace, id) returns a NULL if id is not present
>> in the idr tree or returns the pointer to the struct if id is
>> present in the idr tree. With this function in the idr library,
>> code for pid allocation can be simplified by calling this function
>> instead of looking through the pidhash.
>
> Could you explain why find_pid_ns() can't use idr_find() ?

It can. I missed this macro from the IDR library. Will change this
and drop this patch in the next version.

Thanks!
Gargi
>
>> +void * idr_get(struct idr *idr, int *id)
>> +{
>> +     struct radix_tree_node *node;
>> +     void __rcu **slot = NULL;
>> +
>> +     __radix_tree_lookup(&idr->idr_rt, *id, &node, &slot);
>
> so why it takes "int *", not just "int" ?
>
> Oleg.
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web