See Documentation/timers/timers-howto.rst, msleep() is not
for (1ms - 20ms), There is a more advanced API is used.
Signed-off-by: Yejune Deng <[email protected]>
---
fs/cifs/cifsfs.c | 4 ++--
fs/cifs/connect.c | 14 +++++++-------
fs/cifs/file.c | 6 +++---
fs/cifs/smbdirect.c | 2 +-
4 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 472cb77..d35ce52 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -664,10 +664,10 @@ static void cifs_umount_begin(struct super_block *sb)
cifs_dbg(FYI, "wake up tasks now - umount begin not complete\n");
wake_up_all(&tcon->ses->server->request_q);
wake_up_all(&tcon->ses->server->response_q);
- msleep(1); /* yield */
+ fsleep(1000); /* yield */
/* we have to kick the requests once more */
wake_up_all(&tcon->ses->server->response_q);
- msleep(1);
+ fsleep(1000);
}
return;
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 44f9cce..62a9c64 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -538,7 +538,7 @@ static inline int reconn_setup_dfs_targets(struct cifs_sb_info *cifs_sb,
if (rc) {
cifs_dbg(FYI, "reconnect error %d\n", rc);
mutex_unlock(&server->srv_mutex);
- msleep(3000);
+ ssleep(3);
} else {
atomic_inc(&tcpSesReconnectCount);
set_credits(server, 1);
@@ -621,7 +621,7 @@ static inline int reconn_setup_dfs_targets(struct cifs_sb_info *cifs_sb,
server->bigbuf = (char *)cifs_buf_get();
if (!server->bigbuf) {
cifs_server_dbg(VFS, "No memory for large SMB response\n");
- msleep(3000);
+ ssleep(3);
/* retry will check if exiting */
return false;
}
@@ -634,7 +634,7 @@ static inline int reconn_setup_dfs_targets(struct cifs_sb_info *cifs_sb,
server->smallbuf = (char *)cifs_small_buf_get();
if (!server->smallbuf) {
cifs_server_dbg(VFS, "No memory for SMB response\n");
- msleep(1000);
+ ssleep(1);
/* retry will check if exiting */
return false;
}
@@ -729,7 +729,7 @@ static inline int reconn_setup_dfs_targets(struct cifs_sb_info *cifs_sb,
* to clear and app threads to set tcpStatus
* CifsNeedReconnect if server hung.
*/
- usleep_range(1000, 2000);
+ fsleep(1000);
length = 0;
continue;
}
@@ -790,7 +790,7 @@ static inline int reconn_setup_dfs_targets(struct cifs_sb_info *cifs_sb,
*/
cifs_dbg(FYI, "RFC 1002 negative session response\n");
/* give server a second to clean up */
- msleep(1000);
+ ssleep(1);
/*
* Always try 445 first on reconnect since we get NACK
* on some if we ever connected to port 139 (the NACK
@@ -944,7 +944,7 @@ static void clean_demultiplex_info(struct TCP_Server_Info *server)
* response and going ahead and killing cifsd.
*/
cifs_dbg(FYI, "Wait for exit from demultiplex thread\n");
- msleep(46000);
+ ssleep(46);
/*
* If threads still have not exited they are probably never
* coming home not much else we can do but free the memory.
@@ -3655,7 +3655,7 @@ static void rfc1002mangle(char *target, char *source, unsigned int length)
* significant slowing down on mount
* for everyone else
*/
- usleep_range(1000, 2000);
+ fsleep(1000);
}
/*
* else the negprot may still work without this
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index be46fab..75538a8 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -283,7 +283,7 @@ int cifs_posix_open(char *full_path, struct inode **pinode,
cifs_down_write(struct rw_semaphore *sem)
{
while (!down_write_trylock(sem))
- msleep(10);
+ fsleep(10000);
}
static void cifsFileInfo_put_work(struct work_struct *work);
@@ -2828,7 +2828,7 @@ size_t get_numpages(const size_t wsize, const size_t len, size_t *cur_len)
if (wsize < wdata->bytes) {
add_credits_and_wake_if(server, &credits, 0);
- msleep(1000);
+ ssleep(1);
}
} while (wsize < wdata->bytes);
wdata->credits = credits;
@@ -3563,7 +3563,7 @@ static int cifs_resend_rdata(struct cifs_readdata *rdata,
if (rsize < rdata->bytes) {
add_credits_and_wake_if(server, &credits, 0);
- msleep(1000);
+ ssleep(1);
}
} while (rsize < rdata->bytes);
rdata->credits = credits;
diff --git a/fs/cifs/smbdirect.c b/fs/cifs/smbdirect.c
index b029ed3..84f97f8 100644
--- a/fs/cifs/smbdirect.c
+++ b/fs/cifs/smbdirect.c
@@ -1372,7 +1372,7 @@ void smbd_destroy(struct TCP_Server_Info *server)
wake_up_interruptible_all(&info->wait_mr);
while (atomic_read(&info->mr_used_count)) {
mutex_unlock(&server->srv_mutex);
- msleep(1000);
+ ssleep(1);
mutex_lock(&server->srv_mutex);
}
destroy_mr_list(info);
--
1.9.1
This patch seems reasonable at first glance, but I was a little
concerned that we don't see many users yet of fsleep. Has there been
pushback on converting "yield" situations from using msleep to fsleep?
On Thu, Dec 10, 2020 at 3:09 AM Yejune Deng <[email protected]> wrote:
>
> See Documentation/timers/timers-howto.rst, msleep() is not
> for (1ms - 20ms), There is a more advanced API is used.
>
> Signed-off-by: Yejune Deng <[email protected]>
> ---
> fs/cifs/cifsfs.c | 4 ++--
> fs/cifs/connect.c | 14 +++++++-------
> fs/cifs/file.c | 6 +++---
> fs/cifs/smbdirect.c | 2 +-
> 4 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 472cb77..d35ce52 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -664,10 +664,10 @@ static void cifs_umount_begin(struct super_block *sb)
> cifs_dbg(FYI, "wake up tasks now - umount begin not complete\n");
> wake_up_all(&tcon->ses->server->request_q);
> wake_up_all(&tcon->ses->server->response_q);
> - msleep(1); /* yield */
> + fsleep(1000); /* yield */
> /* we have to kick the requests once more */
> wake_up_all(&tcon->ses->server->response_q);
> - msleep(1);
> + fsleep(1000);
> }
>
> return;
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 44f9cce..62a9c64 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -538,7 +538,7 @@ static inline int reconn_setup_dfs_targets(struct cifs_sb_info *cifs_sb,
> if (rc) {
> cifs_dbg(FYI, "reconnect error %d\n", rc);
> mutex_unlock(&server->srv_mutex);
> - msleep(3000);
> + ssleep(3);
> } else {
> atomic_inc(&tcpSesReconnectCount);
> set_credits(server, 1);
> @@ -621,7 +621,7 @@ static inline int reconn_setup_dfs_targets(struct cifs_sb_info *cifs_sb,
> server->bigbuf = (char *)cifs_buf_get();
> if (!server->bigbuf) {
> cifs_server_dbg(VFS, "No memory for large SMB response\n");
> - msleep(3000);
> + ssleep(3);
> /* retry will check if exiting */
> return false;
> }
> @@ -634,7 +634,7 @@ static inline int reconn_setup_dfs_targets(struct cifs_sb_info *cifs_sb,
> server->smallbuf = (char *)cifs_small_buf_get();
> if (!server->smallbuf) {
> cifs_server_dbg(VFS, "No memory for SMB response\n");
> - msleep(1000);
> + ssleep(1);
> /* retry will check if exiting */
> return false;
> }
> @@ -729,7 +729,7 @@ static inline int reconn_setup_dfs_targets(struct cifs_sb_info *cifs_sb,
> * to clear and app threads to set tcpStatus
> * CifsNeedReconnect if server hung.
> */
> - usleep_range(1000, 2000);
> + fsleep(1000);
> length = 0;
> continue;
> }
> @@ -790,7 +790,7 @@ static inline int reconn_setup_dfs_targets(struct cifs_sb_info *cifs_sb,
> */
> cifs_dbg(FYI, "RFC 1002 negative session response\n");
> /* give server a second to clean up */
> - msleep(1000);
> + ssleep(1);
> /*
> * Always try 445 first on reconnect since we get NACK
> * on some if we ever connected to port 139 (the NACK
> @@ -944,7 +944,7 @@ static void clean_demultiplex_info(struct TCP_Server_Info *server)
> * response and going ahead and killing cifsd.
> */
> cifs_dbg(FYI, "Wait for exit from demultiplex thread\n");
> - msleep(46000);
> + ssleep(46);
> /*
> * If threads still have not exited they are probably never
> * coming home not much else we can do but free the memory.
> @@ -3655,7 +3655,7 @@ static void rfc1002mangle(char *target, char *source, unsigned int length)
> * significant slowing down on mount
> * for everyone else
> */
> - usleep_range(1000, 2000);
> + fsleep(1000);
> }
> /*
> * else the negprot may still work without this
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index be46fab..75538a8 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -283,7 +283,7 @@ int cifs_posix_open(char *full_path, struct inode **pinode,
> cifs_down_write(struct rw_semaphore *sem)
> {
> while (!down_write_trylock(sem))
> - msleep(10);
> + fsleep(10000);
> }
>
> static void cifsFileInfo_put_work(struct work_struct *work);
> @@ -2828,7 +2828,7 @@ size_t get_numpages(const size_t wsize, const size_t len, size_t *cur_len)
>
> if (wsize < wdata->bytes) {
> add_credits_and_wake_if(server, &credits, 0);
> - msleep(1000);
> + ssleep(1);
> }
> } while (wsize < wdata->bytes);
> wdata->credits = credits;
> @@ -3563,7 +3563,7 @@ static int cifs_resend_rdata(struct cifs_readdata *rdata,
>
> if (rsize < rdata->bytes) {
> add_credits_and_wake_if(server, &credits, 0);
> - msleep(1000);
> + ssleep(1);
> }
> } while (rsize < rdata->bytes);
> rdata->credits = credits;
> diff --git a/fs/cifs/smbdirect.c b/fs/cifs/smbdirect.c
> index b029ed3..84f97f8 100644
> --- a/fs/cifs/smbdirect.c
> +++ b/fs/cifs/smbdirect.c
> @@ -1372,7 +1372,7 @@ void smbd_destroy(struct TCP_Server_Info *server)
> wake_up_interruptible_all(&info->wait_mr);
> while (atomic_read(&info->mr_used_count)) {
> mutex_unlock(&server->srv_mutex);
> - msleep(1000);
> + ssleep(1);
> mutex_lock(&server->srv_mutex);
> }
> destroy_mr_list(info);
> --
> 1.9.1
>
--
Thanks,
Steve
No。I just see Documentation/timers/timers-howto.rst and don't
recommend using msleep() for (1ms - 20ms). It recommends using
usleep_range(). And fsleep() is flexible sleeping.
On Wed, Jan 6, 2021 at 12:27 PM Steve French <[email protected]> wrote:
>
> This patch seems reasonable at first glance, but I was a little
> concerned that we don't see many users yet of fsleep. Has there been
> pushback on converting "yield" situations from using msleep to fsleep?
>
> On Thu, Dec 10, 2020 at 3:09 AM Yejune Deng <[email protected]> wrote:
> >
> > See Documentation/timers/timers-howto.rst, msleep() is not
> > for (1ms - 20ms), There is a more advanced API is used.
> >
> > Signed-off-by: Yejune Deng <[email protected]>
> > ---
> > fs/cifs/cifsfs.c | 4 ++--
> > fs/cifs/connect.c | 14 +++++++-------
> > fs/cifs/file.c | 6 +++---
> > fs/cifs/smbdirect.c | 2 +-
> > 4 files changed, 13 insertions(+), 13 deletions(-)
> >
> > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> > index 472cb77..d35ce52 100644
> > --- a/fs/cifs/cifsfs.c
> > +++ b/fs/cifs/cifsfs.c
> > @@ -664,10 +664,10 @@ static void cifs_umount_begin(struct super_block *sb)
> > cifs_dbg(FYI, "wake up tasks now - umount begin not complete\n");
> > wake_up_all(&tcon->ses->server->request_q);
> > wake_up_all(&tcon->ses->server->response_q);
> > - msleep(1); /* yield */
> > + fsleep(1000); /* yield */
> > /* we have to kick the requests once more */
> > wake_up_all(&tcon->ses->server->response_q);
> > - msleep(1);
> > + fsleep(1000);
> > }
> >
> > return;
> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> > index 44f9cce..62a9c64 100644
> > --- a/fs/cifs/connect.c
> > +++ b/fs/cifs/connect.c
> > @@ -538,7 +538,7 @@ static inline int reconn_setup_dfs_targets(struct cifs_sb_info *cifs_sb,
> > if (rc) {
> > cifs_dbg(FYI, "reconnect error %d\n", rc);
> > mutex_unlock(&server->srv_mutex);
> > - msleep(3000);
> > + ssleep(3);
> > } else {
> > atomic_inc(&tcpSesReconnectCount);
> > set_credits(server, 1);
> > @@ -621,7 +621,7 @@ static inline int reconn_setup_dfs_targets(struct cifs_sb_info *cifs_sb,
> > server->bigbuf = (char *)cifs_buf_get();
> > if (!server->bigbuf) {
> > cifs_server_dbg(VFS, "No memory for large SMB response\n");
> > - msleep(3000);
> > + ssleep(3);
> > /* retry will check if exiting */
> > return false;
> > }
> > @@ -634,7 +634,7 @@ static inline int reconn_setup_dfs_targets(struct cifs_sb_info *cifs_sb,
> > server->smallbuf = (char *)cifs_small_buf_get();
> > if (!server->smallbuf) {
> > cifs_server_dbg(VFS, "No memory for SMB response\n");
> > - msleep(1000);
> > + ssleep(1);
> > /* retry will check if exiting */
> > return false;
> > }
> > @@ -729,7 +729,7 @@ static inline int reconn_setup_dfs_targets(struct cifs_sb_info *cifs_sb,
> > * to clear and app threads to set tcpStatus
> > * CifsNeedReconnect if server hung.
> > */
> > - usleep_range(1000, 2000);
> > + fsleep(1000);
> > length = 0;
> > continue;
> > }
> > @@ -790,7 +790,7 @@ static inline int reconn_setup_dfs_targets(struct cifs_sb_info *cifs_sb,
> > */
> > cifs_dbg(FYI, "RFC 1002 negative session response\n");
> > /* give server a second to clean up */
> > - msleep(1000);
> > + ssleep(1);
> > /*
> > * Always try 445 first on reconnect since we get NACK
> > * on some if we ever connected to port 139 (the NACK
> > @@ -944,7 +944,7 @@ static void clean_demultiplex_info(struct TCP_Server_Info *server)
> > * response and going ahead and killing cifsd.
> > */
> > cifs_dbg(FYI, "Wait for exit from demultiplex thread\n");
> > - msleep(46000);
> > + ssleep(46);
> > /*
> > * If threads still have not exited they are probably never
> > * coming home not much else we can do but free the memory.
> > @@ -3655,7 +3655,7 @@ static void rfc1002mangle(char *target, char *source, unsigned int length)
> > * significant slowing down on mount
> > * for everyone else
> > */
> > - usleep_range(1000, 2000);
> > + fsleep(1000);
> > }
> > /*
> > * else the negprot may still work without this
> > diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> > index be46fab..75538a8 100644
> > --- a/fs/cifs/file.c
> > +++ b/fs/cifs/file.c
> > @@ -283,7 +283,7 @@ int cifs_posix_open(char *full_path, struct inode **pinode,
> > cifs_down_write(struct rw_semaphore *sem)
> > {
> > while (!down_write_trylock(sem))
> > - msleep(10);
> > + fsleep(10000);
> > }
> >
> > static void cifsFileInfo_put_work(struct work_struct *work);
> > @@ -2828,7 +2828,7 @@ size_t get_numpages(const size_t wsize, const size_t len, size_t *cur_len)
> >
> > if (wsize < wdata->bytes) {
> > add_credits_and_wake_if(server, &credits, 0);
> > - msleep(1000);
> > + ssleep(1);
> > }
> > } while (wsize < wdata->bytes);
> > wdata->credits = credits;
> > @@ -3563,7 +3563,7 @@ static int cifs_resend_rdata(struct cifs_readdata *rdata,
> >
> > if (rsize < rdata->bytes) {
> > add_credits_and_wake_if(server, &credits, 0);
> > - msleep(1000);
> > + ssleep(1);
> > }
> > } while (rsize < rdata->bytes);
> > rdata->credits = credits;
> > diff --git a/fs/cifs/smbdirect.c b/fs/cifs/smbdirect.c
> > index b029ed3..84f97f8 100644
> > --- a/fs/cifs/smbdirect.c
> > +++ b/fs/cifs/smbdirect.c
> > @@ -1372,7 +1372,7 @@ void smbd_destroy(struct TCP_Server_Info *server)
> > wake_up_interruptible_all(&info->wait_mr);
> > while (atomic_read(&info->mr_used_count)) {
> > mutex_unlock(&server->srv_mutex);
> > - msleep(1000);
> > + ssleep(1);
> > mutex_lock(&server->srv_mutex);
> > }
> > destroy_mr_list(info);
> > --
> > 1.9.1
> >
>
>
> --
> Thanks,
>
> Steve
Yejune Deng <[email protected]> writes:
> No。I just see Documentation/timers/timers-howto.rst and don't
> recommend using msleep() for (1ms - 20ms). It recommends using
> usleep_range(). And fsleep() is flexible sleeping.
I think what Steve is asking is does using fsleep() changes anything
regarding yielding i.e. does it affect how tasks/processes get
scheduled. AFAIK, this msleep(1) in the code is to give a hint and let
the kernel run other tasks, in particular the ones waiting on the
request and response queue.
Cheers,
--
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)