[PATCH] dm: remove pointless comparison

[PATCH] dm: remove pointless comparison

[Mikulas Patocka]

Remove pointless comparison in dm_get_from_kobject. The comparison is
always true and the compiler optimizes it out anyway.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 drivers/md/dm.c |    2 --
 1 file changed, 2 deletions(-)

Index: linux-3.13-rc7/drivers/md/dm.c
===================================================================
--- linux-3.13-rc7.orig/drivers/md/dm.c	2014-01-07 01:28:27.000000000 +0100
+++ linux-3.13-rc7/drivers/md/dm.c	2014-01-07 01:28:39.000000000 +0100
@@ -2922,8 +2922,6 @@ struct mapped_device *dm_get_from_kobjec
 	struct mapped_device *md;
 
 	md = container_of(kobj, struct mapped_device, kobj);
-	if (&md->kobj != kobj)
-		return NULL;
 
 	if (test_bit(DMF_FREEING, &md->flags) ||
 	    dm_deleting_md(md))

Re: [PATCH] dm: remove pointless comparison

[Joe Thornber]

On Mon, Jan 06, 2014 at 10:53:28PM -0500, Mikulas Patocka wrote:
> Remove pointless comparison in dm_get_from_kobject. The comparison is
> always true and the compiler optimizes it out anyway.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> 
> ---
>  drivers/md/dm.c |    2 --
>  1 file changed, 2 deletions(-)
> 
> Index: linux-3.13-rc7/drivers/md/dm.c
> ===================================================================
> --- linux-3.13-rc7.orig/drivers/md/dm.c	2014-01-07 01:28:27.000000000 +0100
> +++ linux-3.13-rc7/drivers/md/dm.c	2014-01-07 01:28:39.000000000 +0100
> @@ -2922,8 +2922,6 @@ struct mapped_device *dm_get_from_kobjec
>  	struct mapped_device *md;
>  
>  	md = container_of(kobj, struct mapped_device, kobj);
> -	if (&md->kobj != kobj)
> -		return NULL;

What were they trying to check?  The compiler?

Re: dm: remove pointless comparison

[Mike Snitzer]

On Tue, Jan 07 2014 at  5:39am -0500,
Joe Thornber <thornber@redhat.com> wrote:

> On Mon, Jan 06, 2014 at 10:53:28PM -0500, Mikulas Patocka wrote:
> > Remove pointless comparison in dm_get_from_kobject. The comparison is
> > always true and the compiler optimizes it out anyway.
> > 
> > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> > 
> > ---
> >  drivers/md/dm.c |    2 --
> >  1 file changed, 2 deletions(-)
> > 
> > Index: linux-3.13-rc7/drivers/md/dm.c
> > ===================================================================
> > --- linux-3.13-rc7.orig/drivers/md/dm.c	2014-01-07 01:28:27.000000000 +0100
> > +++ linux-3.13-rc7/drivers/md/dm.c	2014-01-07 01:28:39.000000000 +0100
> > @@ -2922,8 +2922,6 @@ struct mapped_device *dm_get_from_kobjec
> >  	struct mapped_device *md;
> >  
> >  	md = container_of(kobj, struct mapped_device, kobj);
> > -	if (&md->kobj != kobj)
> > -		return NULL;
> 
> What were they trying to check?  The compiler?
> 

Milan's commit 784aae735d, and the current dm.c code, offers this
comment above dm_get_from_kobject:

/*
 * struct mapped_device should not be exported outside of dm.c
 * so use this check to verify that kobj is part of md structure
 */

I'm lacking the vision to _know_ why the kobject would suddenly be
outside of the mapped_device without us knowing via code change..

The check does seem overly cautious/useless (if we remove it the above
comment should be removed too).

Re: dm: remove pointless comparison

[Milan Broz]

On 01/07/2014 04:52 PM, Mike Snitzer wrote:
> On Tue, Jan 07 2014 at  5:39am -0500,
> Joe Thornber <thornber@redhat.com> wrote:
> 
>> On Mon, Jan 06, 2014 at 10:53:28PM -0500, Mikulas Patocka wrote:
>>> Remove pointless comparison in dm_get_from_kobject. The comparison is
>>> always true and the compiler optimizes it out anyway.
>>>gmane.linux.kernel.device-mapper.devel
>>> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
>>>
>>> ---
>>>  drivers/md/dm.c |    2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>> Index: linux-3.13-rc7/drivers/md/dm.c
>>> ===================================================================
>>> --- linux-3.13-rc7.orig/drivers/md/dm.c	2014-01-07 01:28:27.000000000 +0100
>>> +++ linux-3.13-rc7/drivers/md/dm.c	2014-01-07 01:28:39.000000000 +0100
>>> @@ -2922,8 +2922,6 @@ struct mapped_device *dm_get_from_kobjec
>>>  	struct mapped_device *md;
>>>  
>>>  	md = container_of(kobj, struct mapped_device, kobj);
>>> -	if (&md->kobj != kobj)
>>> -		return NULL;
>>
>> What were they trying to check?  The compiler?
>>
> 
> Milan's commit 784aae735d, and the current dm.c code, offers this
> comment above dm_get_from_kobject:
> 
> /*
>  * struct mapped_device should not be exported outside of dm.c
>  * so use this check to verify that kobj is part of md structure
>  */
> 
> I'm lacking the vision to _know_ why the kobject would suddenly be
> outside of the mapped_device without us knowing via code change..
> 
> The check does seem overly cautious/useless (if we remove it the above
> comment should be removed too).

Yes, please remove both :) The check doesn't make sense.

I think it is just relict of some experiments before I committed this simple
embedded sysfs kobj handling (there were always problems like the missing .release
function you are just solving in other thread).

Milan

 

Re: dm: remove pointless comparison

[Mike Snitzer]

On Tue, Jan 07 2014 at 12:28pm -0500,
Milan Broz <gmazyland@gmail.com> wrote:

> On 01/07/2014 04:52 PM, Mike Snitzer wrote:
> > On Tue, Jan 07 2014 at  5:39am -0500,
> > Joe Thornber <thornber@redhat.com> wrote:
> > 
> >> On Mon, Jan 06, 2014 at 10:53:28PM -0500, Mikulas Patocka wrote:
> >>> Remove pointless comparison in dm_get_from_kobject. The comparison is
> >>> always true and the compiler optimizes it out anyway.
> >>>gmane.linux.kernel.device-mapper.devel
> >>> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> >>>
> >>> ---
> >>>  drivers/md/dm.c |    2 --
> >>>  1 file changed, 2 deletions(-)
> >>>
> >>> Index: linux-3.13-rc7/drivers/md/dm.c
> >>> ===================================================================
> >>> --- linux-3.13-rc7.orig/drivers/md/dm.c	2014-01-07 01:28:27.000000000 +0100
> >>> +++ linux-3.13-rc7/drivers/md/dm.c	2014-01-07 01:28:39.000000000 +0100
> >>> @@ -2922,8 +2922,6 @@ struct mapped_device *dm_get_from_kobjec
> >>>  	struct mapped_device *md;
> >>>  
> >>>  	md = container_of(kobj, struct mapped_device, kobj);
> >>> -	if (&md->kobj != kobj)
> >>> -		return NULL;
> >>
> >> What were they trying to check?  The compiler?
> >>
> > 
> > Milan's commit 784aae735d, and the current dm.c code, offers this
> > comment above dm_get_from_kobject:
> > 
> > /*
> >  * struct mapped_device should not be exported outside of dm.c
> >  * so use this check to verify that kobj is part of md structure
> >  */
> > 
> > I'm lacking the vision to _know_ why the kobject would suddenly be
> > outside of the mapped_device without us knowing via code change..
> > 
> > The check does seem overly cautious/useless (if we remove it the above
> > comment should be removed too).
> 
> Yes, please remove both :) The check doesn't make sense.

Ok, I'll take that as your Acked-by, thanks.