linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [bug report] cifsd: add file operations
@ 2021-08-31 11:43 Dan Carpenter
  2021-08-31 13:17 ` Namjae Jeon
  0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2021-08-31 11:43 UTC (permalink / raw)
  To: namjae.jeon; +Cc: linux-fsdevel

Hello Namjae Jeon,

The patch f44158485826: "cifsd: add file operations" from Mar 16,
2021, leads to the following
Smatch static checker warning:

	fs/xattr.c:524 vfs_removexattr()
	warn: sleeping in atomic context

fs/xattr.c
    514 
    515 int
    516 vfs_removexattr(struct user_namespace *mnt_userns, struct dentry *dentry,
    517                 const char *name)
    518 {
    519         struct inode *inode = dentry->d_inode;
    520         struct inode *delegated_inode = NULL;
    521         int error;
    522 
    523 retry_deleg:
--> 524         inode_lock(inode);
    525         error = __vfs_removexattr_locked(mnt_userns, dentry,
    526                                          name, &delegated_inode);
    527         inode_unlock(inode);
    528 
    529         if (delegated_inode) {
    530                 error = break_deleg_wait(&delegated_inode);
    531                 if (!error)
    532                         goto retry_deleg;
    533         }
    534 
    535         return error;
    536 }

The call tree is (slight edited).

ksmbd_file_table_flush() <- disables preempt
-> ksmbd_vfs_fsync()
   -> ksmbd_fd_put()
      -> __put_fd_final()
         -> __ksmbd_close_fd()
            -> __ksmbd_inode_close()
               -> ksmbd_vfs_remove_xattr()
                  -> vfs_removexattr()

fs/ksmbd/vfs_cache.c
   669  int ksmbd_file_table_flush(struct ksmbd_work *work)
   670  {
   671          struct ksmbd_file       *fp = NULL;
   672          unsigned int            id;
   673          int                     ret;
   674  
   675          read_lock(&work->sess->file_table.lock);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Disables preemption.

   676          idr_for_each_entry(work->sess->file_table.idr, fp, id) {
   677                  ret = ksmbd_vfs_fsync(work, fp->volatile_id, KSMBD_NO_FID);
   678                  if (ret)
   679                          break;
   680          }
   681          read_unlock(&work->sess->file_table.lock);
   682          return ret;
   683  }

Hopefully this bug report is clear why Smatch is complaining.  Let me
know if you have any questions.

regards,
dan carpenter

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

* Re: [bug report] cifsd: add file operations
  2021-08-31 11:43 [bug report] cifsd: add file operations Dan Carpenter
@ 2021-08-31 13:17 ` Namjae Jeon
  0 siblings, 0 replies; 2+ messages in thread
From: Namjae Jeon @ 2021-08-31 13:17 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: namjae.jeon, linux-fsdevel

2021-08-31 20:43 GMT+09:00, Dan Carpenter <dan.carpenter@oracle.com>:
> Hello Namjae Jeon,
Hi Dan,

I will fix it.
Thanks for your report!
>
> The patch f44158485826: "cifsd: add file operations" from Mar 16,
> 2021, leads to the following
> Smatch static checker warning:
>
> 	fs/xattr.c:524 vfs_removexattr()
> 	warn: sleeping in atomic context
>
> fs/xattr.c
>     514
>     515 int
>     516 vfs_removexattr(struct user_namespace *mnt_userns, struct dentry
> *dentry,
>     517                 const char *name)
>     518 {
>     519         struct inode *inode = dentry->d_inode;
>     520         struct inode *delegated_inode = NULL;
>     521         int error;
>     522
>     523 retry_deleg:
> --> 524         inode_lock(inode);
>     525         error = __vfs_removexattr_locked(mnt_userns, dentry,
>     526                                          name, &delegated_inode);
>     527         inode_unlock(inode);
>     528
>     529         if (delegated_inode) {
>     530                 error = break_deleg_wait(&delegated_inode);
>     531                 if (!error)
>     532                         goto retry_deleg;
>     533         }
>     534
>     535         return error;
>     536 }
>
> The call tree is (slight edited).
>
> ksmbd_file_table_flush() <- disables preempt
> -> ksmbd_vfs_fsync()
>    -> ksmbd_fd_put()
>       -> __put_fd_final()
>          -> __ksmbd_close_fd()
>             -> __ksmbd_inode_close()
>                -> ksmbd_vfs_remove_xattr()
>                   -> vfs_removexattr()
>
> fs/ksmbd/vfs_cache.c
>    669  int ksmbd_file_table_flush(struct ksmbd_work *work)
>    670  {
>    671          struct ksmbd_file       *fp = NULL;
>    672          unsigned int            id;
>    673          int                     ret;
>    674
>    675          read_lock(&work->sess->file_table.lock);
>                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Disables preemption.
>
>    676          idr_for_each_entry(work->sess->file_table.idr, fp, id) {
>    677                  ret = ksmbd_vfs_fsync(work, fp->volatile_id,
> KSMBD_NO_FID);
>    678                  if (ret)
>    679                          break;
>    680          }
>    681          read_unlock(&work->sess->file_table.lock);
>    682          return ret;
>    683  }
>
> Hopefully this bug report is clear why Smatch is complaining.  Let me
> know if you have any questions.
>
> regards,
> dan carpenter
>

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

end of thread, other threads:[~2021-08-31 13:17 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-31 11:43 [bug report] cifsd: add file operations Dan Carpenter
2021-08-31 13:17 ` Namjae Jeon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).