public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* kmallocs with swapped parameters in fs/dlm/lock.c
@ 2011-07-10  0:30 Dr. David Alan Gilbert
  2011-07-10 20:56 ` Jesper Juhl
  2011-07-11 13:23 ` David Teigland
  0 siblings, 2 replies; 6+ messages in thread
From: Dr. David Alan Gilbert @ 2011-07-10  0:30 UTC (permalink / raw)
  To: teigland; +Cc: jj, linux-kernel

Hi David,
  Sparse spotted a couple of places in fs/dlm/lock.c where the size/flags
parameters to kmalloc got swapped:

The sparse output is:
fs/dlm/lock.c:852:42: warning: incorrect type in argument 1 (different base types)
fs/dlm/lock.c:852:42:    expected unsigned long [unsigned] [usertype] size
fs/dlm/lock.c:852:42:    got restricted gfp_t
fs/dlm/lock.c:852:64: warning: incorrect type in argument 2 (different base types)
fs/dlm/lock.c:852:64:    expected restricted gfp_t [usertype] flags
fs/dlm/lock.c:852:64:    got unsigned long
fs/dlm/lock.c:4136:27: warning: incorrect type in argument 1 (different base types)
fs/dlm/lock.c:4136:27:    expected unsigned long [unsigned] [usertype] size
fs/dlm/lock.c:4136:27:    got restricted gfp_t
fs/dlm/lock.c:4136:39: warning: incorrect type in argument 2 (different base types)
fs/dlm/lock.c:4136:39:    expected restricted gfp_t [usertype] flags
fs/dlm/lock.c:4136:39:    got unsigned long

Line 852 is:
                        warned = kmalloc(GFP_KERNEL, num_nodes * sizeof(int));
Line 4136 is:
        ms_stub = kmalloc(GFP_KERNEL, sizeof(struct dlm_message));

and at least one of the defs of kmalloc is:
slab_def.h:static __always_inline void *kmalloc(size_t size, gfp_t flags)

Given that I haven't got a clue how to test a dlm, it's probably
best that you patch it.

Jesper:
  I noticed in your patch:
https://lkml.org/lkml/2011/7/2/90

where you had done a kmalloc->kzalloc transform you kept the swap.

This looks like new code in 3.0rc so I don't think you could quite
call it a regression, but it should probably be fixed fairly quickly
I guess.

Dave
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\ gro.gilbert @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: kmallocs with swapped parameters in fs/dlm/lock.c
  2011-07-10  0:30 kmallocs with swapped parameters in fs/dlm/lock.c Dr. David Alan Gilbert
@ 2011-07-10 20:56 ` Jesper Juhl
  2011-07-11  8:25   ` Dr. David Alan Gilbert
  2011-07-11 13:23 ` David Teigland
  1 sibling, 1 reply; 6+ messages in thread
From: Jesper Juhl @ 2011-07-10 20:56 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: teigland, linux-kernel

On Sun, 10 Jul 2011, Dr. David Alan Gilbert wrote:

> Hi David,
>   Sparse spotted a couple of places in fs/dlm/lock.c where the size/flags
> parameters to kmalloc got swapped:
> 
[...]
> Jesper:
>   I noticed in your patch:
> https://lkml.org/lkml/2011/7/2/90
> 
> where you had done a kmalloc->kzalloc transform you kept the swap.
> 
> This looks like new code in 3.0rc so I don't think you could quite
> call it a regression, but it should probably be fixed fairly quickly
> I guess.
> 
D'oh, I completely overlooked that.
Thank you for noticing and letting me know. I've just submitted an updated 
patch.

-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: kmallocs with swapped parameters in fs/dlm/lock.c
  2011-07-10 20:56 ` Jesper Juhl
@ 2011-07-11  8:25   ` Dr. David Alan Gilbert
  2011-07-11  8:33     ` Jesper Juhl
  0 siblings, 1 reply; 6+ messages in thread
From: Dr. David Alan Gilbert @ 2011-07-11  8:25 UTC (permalink / raw)
  To: Jesper Juhl; +Cc: teigland, linux-kernel

* Jesper Juhl (jj@chaosbits.net) wrote:
> On Sun, 10 Jul 2011, Dr. David Alan Gilbert wrote:
> 
> > Hi David,
> >   Sparse spotted a couple of places in fs/dlm/lock.c where the size/flags
> > parameters to kmalloc got swapped:
> > 
> [...]
> > Jesper:
> >   I noticed in your patch:
> > https://lkml.org/lkml/2011/7/2/90
> > 
> > where you had done a kmalloc->kzalloc transform you kept the swap.
> > 
> > This looks like new code in 3.0rc so I don't think you could quite
> > call it a regression, but it should probably be fixed fairly quickly
> > I guess.
> > 
> D'oh, I completely overlooked that.

No problem.

> Thank you for noticing and letting me know. I've just submitted an updated 
> patch.

Great - it's probably worth you fixing the 2nd case in the same file
in the same patch, rather than just the one that you were changing
anyway?

Dave
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\ gro.gilbert @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: kmallocs with swapped parameters in fs/dlm/lock.c
  2011-07-11  8:25   ` Dr. David Alan Gilbert
@ 2011-07-11  8:33     ` Jesper Juhl
  2011-07-11 13:25       ` David Teigland
  0 siblings, 1 reply; 6+ messages in thread
From: Jesper Juhl @ 2011-07-11  8:33 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: teigland, linux-kernel

On Mon, 11 Jul 2011, Dr. David Alan Gilbert wrote:

> * Jesper Juhl (jj@chaosbits.net) wrote:
> > On Sun, 10 Jul 2011, Dr. David Alan Gilbert wrote:
> > 
> > > Hi David,
> > >   Sparse spotted a couple of places in fs/dlm/lock.c where the size/flags
> > > parameters to kmalloc got swapped:
> > > 
> > [...]
> > > Jesper:
> > >   I noticed in your patch:
> > > https://lkml.org/lkml/2011/7/2/90
> > > 
> > > where you had done a kmalloc->kzalloc transform you kept the swap.
> > > 
> > > This looks like new code in 3.0rc so I don't think you could quite
> > > call it a regression, but it should probably be fixed fairly quickly
> > > I guess.
> > > 
> > D'oh, I completely overlooked that.
> 
> No problem.
> 
> > Thank you for noticing and letting me know. I've just submitted an updated 
> > patch.
> 
> Great - it's probably worth you fixing the 2nd case in the same file
> in the same patch, rather than just the one that you were changing
> anyway?
> 
I guess so. I'll update my patch later this evening when I get home from 
work.

-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: kmallocs with swapped parameters in fs/dlm/lock.c
  2011-07-10  0:30 kmallocs with swapped parameters in fs/dlm/lock.c Dr. David Alan Gilbert
  2011-07-10 20:56 ` Jesper Juhl
@ 2011-07-11 13:23 ` David Teigland
  1 sibling, 0 replies; 6+ messages in thread
From: David Teigland @ 2011-07-11 13:23 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: jj, linux-kernel

On Sun, Jul 10, 2011 at 01:30:32AM +0100, Dr. David Alan Gilbert wrote:
> Hi David,
>   Sparse spotted a couple of places in fs/dlm/lock.c where the size/flags
> parameters to kmalloc got swapped:
> 
> The sparse output is:
> fs/dlm/lock.c:852:42: warning: incorrect type in argument 1 (different base types)
> fs/dlm/lock.c:852:42:    expected unsigned long [unsigned] [usertype] size
> fs/dlm/lock.c:852:42:    got restricted gfp_t
> fs/dlm/lock.c:852:64: warning: incorrect type in argument 2 (different base types)
> fs/dlm/lock.c:852:64:    expected restricted gfp_t [usertype] flags
> fs/dlm/lock.c:852:64:    got unsigned long
> fs/dlm/lock.c:4136:27: warning: incorrect type in argument 1 (different base types)
> fs/dlm/lock.c:4136:27:    expected unsigned long [unsigned] [usertype] size
> fs/dlm/lock.c:4136:27:    got restricted gfp_t
> fs/dlm/lock.c:4136:39: warning: incorrect type in argument 2 (different base types)
> fs/dlm/lock.c:4136:39:    expected restricted gfp_t [usertype] flags
> fs/dlm/lock.c:4136:39:    got unsigned long
> 
> Line 852 is:
>                         warned = kmalloc(GFP_KERNEL, num_nodes * sizeof(int));
> Line 4136 is:
>         ms_stub = kmalloc(GFP_KERNEL, sizeof(struct dlm_message));
> 
> and at least one of the defs of kmalloc is:
> slab_def.h:static __always_inline void *kmalloc(size_t size, gfp_t flags)
> 
> Given that I haven't got a clue how to test a dlm, it's probably
> best that you patch it.
> 
> Jesper:
>   I noticed in your patch:
> https://lkml.org/lkml/2011/7/2/90
> 
> where you had done a kmalloc->kzalloc transform you kept the swap.
> 
> This looks like new code in 3.0rc so I don't think you could quite
> call it a regression, but it should probably be fixed fairly quickly
> I guess.

Thanks, I'll get that fixed.
Dave


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: kmallocs with swapped parameters in fs/dlm/lock.c
  2011-07-11  8:33     ` Jesper Juhl
@ 2011-07-11 13:25       ` David Teigland
  0 siblings, 0 replies; 6+ messages in thread
From: David Teigland @ 2011-07-11 13:25 UTC (permalink / raw)
  To: Jesper Juhl; +Cc: Dr. David Alan Gilbert, linux-kernel

On Mon, Jul 11, 2011 at 10:33:20AM +0200, Jesper Juhl wrote:
> On Mon, 11 Jul 2011, Dr. David Alan Gilbert wrote:
> 
> > * Jesper Juhl (jj@chaosbits.net) wrote:
> > > On Sun, 10 Jul 2011, Dr. David Alan Gilbert wrote:
> > > 
> > > > Hi David,
> > > >   Sparse spotted a couple of places in fs/dlm/lock.c where the size/flags
> > > > parameters to kmalloc got swapped:
> > > > 
> > > [...]
> > > > Jesper:
> > > >   I noticed in your patch:
> > > > https://lkml.org/lkml/2011/7/2/90
> > > > 
> > > > where you had done a kmalloc->kzalloc transform you kept the swap.
> > > > 
> > > > This looks like new code in 3.0rc so I don't think you could quite
> > > > call it a regression, but it should probably be fixed fairly quickly
> > > > I guess.
> > > > 
> > > D'oh, I completely overlooked that.
> > 
> > No problem.
> > 
> > > Thank you for noticing and letting me know. I've just submitted an updated 
> > > patch.
> > 
> > Great - it's probably worth you fixing the 2nd case in the same file
> > in the same patch, rather than just the one that you were changing
> > anyway?
> > 
> I guess so. I'll update my patch later this evening when I get home from 
> work.

I'll wait for your patch(es) before fixing any of these myself.
Thanks,
Dave

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-07-11 13:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-10  0:30 kmallocs with swapped parameters in fs/dlm/lock.c Dr. David Alan Gilbert
2011-07-10 20:56 ` Jesper Juhl
2011-07-11  8:25   ` Dr. David Alan Gilbert
2011-07-11  8:33     ` Jesper Juhl
2011-07-11 13:25       ` David Teigland
2011-07-11 13:23 ` David Teigland

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox