[bug report] ceph: perform asynchronous unlink if we have sufficient caps

All of lore.kernel.org
 help / color / mirror / Atom feed

* [bug report] ceph: perform asynchronous unlink if we have sufficient caps
@ 2020-01-23 15:30 Dan Carpenter
  2020-01-23 15:45 ` Jeff Layton
  0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2020-01-23 15:30 UTC (permalink / raw)
  To: jlayton; +Cc: ceph-devel

Hello Jeff Layton,

The patch d6566c62c529: "ceph: perform asynchronous unlink if we have
sufficient caps" from Apr 2, 2019, leads to the following static
checker warning:

	fs/ceph/dir.c:1059 get_caps_for_async_unlink()
	error: uninitialized symbol 'got'.

fs/ceph/dir.c
  1051  static bool get_caps_for_async_unlink(struct inode *dir, struct dentry *dentry)
  1052  {
  1053          struct ceph_inode_info *ci = ceph_inode(dir);
  1054          struct ceph_dentry_info *di;
  1055          int ret, want, got;
  1056  
  1057          want = CEPH_CAP_FILE_EXCL | CEPH_CAP_DIR_UNLINK;
  1058          ret = ceph_try_get_caps(dir, 0, want, true, &got);
  1059          dout("Fx on %p ret=%d got=%d\n", dir, ret, got);
                                                           ^^^
Uninitialized on error.

  1060          if (ret != 1 || got != want)
  1061                  return false;
  1062  
  1063          spin_lock(&dentry->d_lock);
  1064          di = ceph_dentry(dentry);

regards,
dan carpenter

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

* Re: [bug report] ceph: perform asynchronous unlink if we have sufficient caps
  2020-01-23 15:30 [bug report] ceph: perform asynchronous unlink if we have sufficient caps Dan Carpenter
@ 2020-01-23 15:45 ` Jeff Layton
  2020-01-23 17:29   ` Dan Carpenter
  0 siblings, 1 reply; 5+ messages in thread
From: Jeff Layton @ 2020-01-23 15:45 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: ceph-devel

On Thu, 2020-01-23 at 18:30 +0300, Dan Carpenter wrote:
> Hello Jeff Layton,
> 
> The patch d6566c62c529: "ceph: perform asynchronous unlink if we have
> sufficient caps" from Apr 2, 2019, leads to the following static
> checker warning:
> 
> 	fs/ceph/dir.c:1059 get_caps_for_async_unlink()
> 	error: uninitialized symbol 'got'.
> 
> fs/ceph/dir.c
>   1051  static bool get_caps_for_async_unlink(struct inode *dir, struct dentry *dentry)
>   1052  {
>   1053          struct ceph_inode_info *ci = ceph_inode(dir);
>   1054          struct ceph_dentry_info *di;
>   1055          int ret, want, got;
>   1056  
>   1057          want = CEPH_CAP_FILE_EXCL | CEPH_CAP_DIR_UNLINK;
>   1058          ret = ceph_try_get_caps(dir, 0, want, true, &got);
>   1059          dout("Fx on %p ret=%d got=%d\n", dir, ret, got);
>                                                            ^^^
> Uninitialized on error.
> 
>   1060          if (ret != 1 || got != want)
>   1061                  return false;
>   1062  
>   1063          spin_lock(&dentry->d_lock);
>   1064          di = ceph_dentry(dentry);
> 

Hi Dan,

This looks like a false positive to me?

On error, ret != 1, and got shouldn't matter in that case. If
ceph_try_get_caps does return 1 then "got" will be filled out.

Thanks,
-- 
Jeff Layton <jlayton@kernel.org>

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

* Re: [bug report] ceph: perform asynchronous unlink if we have sufficient caps
  2020-01-23 15:45 ` Jeff Layton
@ 2020-01-23 17:29   ` Dan Carpenter
  2020-01-24 10:44     ` Jeff Layton
  2020-01-24 13:01     ` Jeff Layton
  0 siblings, 2 replies; 5+ messages in thread
From: Dan Carpenter @ 2020-01-23 17:29 UTC (permalink / raw)
  To: Jeff Layton; +Cc: ceph-devel

On Thu, Jan 23, 2020 at 10:45:31AM -0500, Jeff Layton wrote:
> On Thu, 2020-01-23 at 18:30 +0300, Dan Carpenter wrote:
> > Hello Jeff Layton,
> > 
> > The patch d6566c62c529: "ceph: perform asynchronous unlink if we have
> > sufficient caps" from Apr 2, 2019, leads to the following static
> > checker warning:
> > 
> > 	fs/ceph/dir.c:1059 get_caps_for_async_unlink()
> > 	error: uninitialized symbol 'got'.
> > 
> > fs/ceph/dir.c
> >   1051  static bool get_caps_for_async_unlink(struct inode *dir, struct dentry *dentry)
> >   1052  {
> >   1053          struct ceph_inode_info *ci = ceph_inode(dir);
> >   1054          struct ceph_dentry_info *di;
> >   1055          int ret, want, got;
> >   1056  
> >   1057          want = CEPH_CAP_FILE_EXCL | CEPH_CAP_DIR_UNLINK;
> >   1058          ret = ceph_try_get_caps(dir, 0, want, true, &got);
> >   1059          dout("Fx on %p ret=%d got=%d\n", dir, ret, got);
> >                                                            ^^^
> > Uninitialized on error.
> > 
> >   1060          if (ret != 1 || got != want)
> >   1061                  return false;
> >   1062  
> >   1063          spin_lock(&dentry->d_lock);
> >   1064          di = ceph_dentry(dentry);
> > 
> 
> Hi Dan,
> 
> This looks like a false positive to me?
> 
> On error, ret != 1, and got shouldn't matter in that case. If
> ceph_try_get_caps does return 1 then "got" will be filled out.


It's complaining about the printk before the error handling.

regards,
dan carpenter

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

* Re: [bug report] ceph: perform asynchronous unlink if we have sufficient caps
  2020-01-23 17:29   ` Dan Carpenter
@ 2020-01-24 10:44     ` Jeff Layton
  2020-01-24 13:01     ` Jeff Layton
  1 sibling, 0 replies; 5+ messages in thread
From: Jeff Layton @ 2020-01-24 10:44 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: ceph-devel

On Thu, 2020-01-23 at 20:29 +0300, Dan Carpenter wrote:
> On Thu, Jan 23, 2020 at 10:45:31AM -0500, Jeff Layton wrote:
> > On Thu, 2020-01-23 at 18:30 +0300, Dan Carpenter wrote:
> > > Hello Jeff Layton,
> > > 
> > > The patch d6566c62c529: "ceph: perform asynchronous unlink if we have
> > > sufficient caps" from Apr 2, 2019, leads to the following static
> > > checker warning:
> > > 
> > > 	fs/ceph/dir.c:1059 get_caps_for_async_unlink()
> > > 	error: uninitialized symbol 'got'.
> > > 
> > > fs/ceph/dir.c
> > >   1051  static bool get_caps_for_async_unlink(struct inode *dir, struct dentry *dentry)
> > >   1052  {
> > >   1053          struct ceph_inode_info *ci = ceph_inode(dir);
> > >   1054          struct ceph_dentry_info *di;
> > >   1055          int ret, want, got;
> > >   1056  
> > >   1057          want = CEPH_CAP_FILE_EXCL | CEPH_CAP_DIR_UNLINK;
> > >   1058          ret = ceph_try_get_caps(dir, 0, want, true, &got);
> > >   1059          dout("Fx on %p ret=%d got=%d\n", dir, ret, got);
> > >                                                            ^^^
> > > Uninitialized on error.
> > > 
> > >   1060          if (ret != 1 || got != want)
> > >   1061                  return false;
> > >   1062  
> > >   1063          spin_lock(&dentry->d_lock);
> > >   1064          di = ceph_dentry(dentry);
> > > 
> > 
> > Hi Dan,
> > 
> > This looks like a false positive to me?
> > 
> > On error, ret != 1, and got shouldn't matter in that case. If
> > ceph_try_get_caps does return 1 then "got" will be filled out.
> 
> It's complaining about the printk before the error handling.
> 
> regards,
> dan carpenter
> 

Ahh thanks, I'll fix that up!

Cheers,
-- 
Jeff Layton <jlayton@kernel.org>

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

* Re: [bug report] ceph: perform asynchronous unlink if we have sufficient caps
  2020-01-23 17:29   ` Dan Carpenter
  2020-01-24 10:44     ` Jeff Layton
@ 2020-01-24 13:01     ` Jeff Layton
  1 sibling, 0 replies; 5+ messages in thread
From: Jeff Layton @ 2020-01-24 13:01 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: ceph-devel, Ilya Dryomov

On Thu, 2020-01-23 at 20:29 +0300, Dan Carpenter wrote:
> On Thu, Jan 23, 2020 at 10:45:31AM -0500, Jeff Layton wrote:
> > On Thu, 2020-01-23 at 18:30 +0300, Dan Carpenter wrote:
> > > Hello Jeff Layton,
> > > 
> > > The patch d6566c62c529: "ceph: perform asynchronous unlink if we have
> > > sufficient caps" from Apr 2, 2019, leads to the following static
> > > checker warning:
> > > 
> > > 	fs/ceph/dir.c:1059 get_caps_for_async_unlink()
> > > 	error: uninitialized symbol 'got'.
> > > 
> > > fs/ceph/dir.c
> > >   1051  static bool get_caps_for_async_unlink(struct inode *dir, struct dentry *dentry)
> > >   1052  {
> > >   1053          struct ceph_inode_info *ci = ceph_inode(dir);
> > >   1054          struct ceph_dentry_info *di;
> > >   1055          int ret, want, got;
> > >   1056  
> > >   1057          want = CEPH_CAP_FILE_EXCL | CEPH_CAP_DIR_UNLINK;
> > >   1058          ret = ceph_try_get_caps(dir, 0, want, true, &got);
> > >   1059          dout("Fx on %p ret=%d got=%d\n", dir, ret, got);
> > >                                                            ^^^
> > > Uninitialized on error.
> > > 
> > >   1060          if (ret != 1 || got != want)
> > >   1061                  return false;
> > >   1062  
> > >   1063          spin_lock(&dentry->d_lock);
> > >   1064          di = ceph_dentry(dentry);
> > > 
> > 
> > Hi Dan,
> > 
> > This looks like a false positive to me?
> > 
> > On error, ret != 1, and got shouldn't matter in that case. If
> > ceph_try_get_caps does return 1 then "got" will be filled out.
> 
> It's complaining about the printk before the error handling.
> 
> 

Duh, of course. Thanks, Dan. I fixed up that up in tree and changed it
to print "got" with ceph_cap_string as well.

Ilya, be advised that I force-pushed ceph-client/testing today with the
corrected patch.

Cheers,
-- 
Jeff Layton <jlayton@kernel.org>

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

end of thread, other threads:[~2020-01-24 13:01 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-01-23 15:30 [bug report] ceph: perform asynchronous unlink if we have sufficient caps Dan Carpenter
2020-01-23 15:45 ` Jeff Layton
2020-01-23 17:29   ` Dan Carpenter
2020-01-24 10:44     ` Jeff Layton
2020-01-24 13:01     ` Jeff Layton

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.