* Re: oops in deferred close
[not found] ` <CACdtm0b9AJ7KFLnhW5w9oqucPjNS0MEAj4+JmBO=R9rmf1CPjA@mail.gmail.com>
@ 2021-05-17 6:02 ` Rohith Surabattula
0 siblings, 0 replies; only message in thread
From: Rohith Surabattula @ 2021-05-17 6:02 UTC (permalink / raw)
To: Aurélien Aptel
Cc: Paulo Alcantara, Steve French, ronnie sahlberg, Pavel Shilovsky,
linux-cifs, Shyam Prasad N
[-- Attachment #1: Type: text/plain, Size: 1475 bytes --]
Hi All,
Have updated the commit message.
Please review and let me know if it is fine.
Regards,
Rohith
On Fri, May 14, 2021 at 3:49 PM Rohith Surabattula
<rohiths.msft@gmail.com> wrote:
>
> Hi Aurélien,
>
> Sorry for the late reply.I am on vacation this week.
>
> I have added the oplock_break_received flag as part of my first commit
> to achieve synchronization between oplock thread and open thread. But
> there is already a lock "open_file_lock" to achieve the same. So, I
> removed the same.
>
> find_readable_file takes open_file_lock and then traverse the
> openFileList. Similarly, cifs_oplock_break while closing the deferred
> handle(i.e cifsFileInfo_put) takes open_file_lock and then sends close
> to the server.
>
> Regards,
> Rohith
>
> On Tue, May 11, 2021 at 9:59 PM Aurélien Aptel <aaptel@suse.com> wrote:
> >
> > Hi,
> >
> > Rohith Surabattula <rohiths.msft@gmail.com> writes:
> > > Will fix the oops which caused the above issue.
> >
> > The patch Steve forwarded to me (I think you forgot to attach it) is not
> > so obvious to me. Could you explain more in the commit msg why you
> > removed oplock_break_received flag
> >
> > 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)
> >
[-- Attachment #2: 0001-Fix-kernel-oops-when-CONFIG_DEBUG_ATOMIC_SLEEP-is-en.patch --]
[-- Type: application/octet-stream, Size: 6803 bytes --]
From 0e0dd29ee9790fbc8e490f986640accedb259d43 Mon Sep 17 00:00:00 2001
From: Rohith Surabattula <rohiths@microsoft.com>
Date: Wed, 5 May 2021 10:56:47 +0000
Subject: [PATCH] Fix kernel oops when CONFIG_DEBUG_ATOMIC_SLEEP is
enabled.
Removed oplock_break_received flag which was added to achive
synchronization between oplock handler and open handler by earlier commit.
It is not needed because there is an existing lock open_file_lock to achieve
the same.find_readable_file takes open_file_lock and then traverse the
openFileList. Similarly, cifs_oplock_break while closing the deferred
handle(i.e cifsFileInfo_put) takes open_file_lock and then sends close
to the server.
Added comments for better readability.
Signed-off-by: Rohith Surabattula <rohiths@microsoft.com>
---
fs/cifs/cifsfs.c | 2 +-
fs/cifs/cifsglob.h | 3 +--
fs/cifs/file.c | 27 ++++++++++++---------------
fs/cifs/misc.c | 9 +++++++++
4 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index d7ea9c5fe0f8..2ffcb29d5c8f 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -133,7 +133,7 @@ struct workqueue_struct *cifsiod_wq;
struct workqueue_struct *decrypt_wq;
struct workqueue_struct *fileinfo_put_wq;
struct workqueue_struct *cifsoplockd_wq;
-struct workqueue_struct *deferredclose_wq;
+struct workqueue_struct *deferredclose_wq;
__u32 cifs_lock_secret;
/*
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index d88b4b523dcc..ea90c53386b8 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1257,8 +1257,7 @@ struct cifsFileInfo {
struct work_struct oplock_break; /* work for oplock breaks */
struct work_struct put; /* work for the final part of _put */
struct delayed_work deferred;
- bool oplock_break_received; /* Flag to indicate oplock break */
- bool deferred_scheduled;
+ bool deferred_close_scheduled; /* Flag to indicate close is scheduled */
};
struct cifs_io_parms {
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 919c82d4713d..ccdf9b62062a 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -323,8 +323,7 @@ cifs_new_fileinfo(struct cifs_fid *fid, struct file *file,
cfile->dentry = dget(dentry);
cfile->f_flags = file->f_flags;
cfile->invalidHandle = false;
- cfile->oplock_break_received = false;
- cfile->deferred_scheduled = false;
+ cfile->deferred_close_scheduled = false;
cfile->tlink = cifs_get_tlink(tlink);
INIT_WORK(&cfile->oplock_break, cifs_oplock_break);
INIT_WORK(&cfile->put, cifsFileInfo_put_work);
@@ -574,21 +573,18 @@ int cifs_open(struct inode *inode, struct file *file)
file->f_op = &cifs_file_direct_ops;
}
- spin_lock(&CIFS_I(inode)->deferred_lock);
/* Get the cached handle as SMB2 close is deferred */
rc = cifs_get_readable_path(tcon, full_path, &cfile);
if (rc == 0) {
if (file->f_flags == cfile->f_flags) {
file->private_data = cfile;
+ spin_lock(&CIFS_I(inode)->deferred_lock);
cifs_del_deferred_close(cfile);
spin_unlock(&CIFS_I(inode)->deferred_lock);
goto out;
} else {
- spin_unlock(&CIFS_I(inode)->deferred_lock);
_cifsFileInfo_put(cfile, true, false);
}
- } else {
- spin_unlock(&CIFS_I(inode)->deferred_lock);
}
if (server->oplocks)
@@ -878,12 +874,12 @@ void smb2_deferred_work_close(struct work_struct *work)
struct cifsFileInfo, deferred.work);
spin_lock(&CIFS_I(d_inode(cfile->dentry))->deferred_lock);
- if (!cfile->deferred_scheduled) {
+ if (!cfile->deferred_close_scheduled) {
spin_unlock(&CIFS_I(d_inode(cfile->dentry))->deferred_lock);
return;
}
cifs_del_deferred_close(cfile);
- cfile->deferred_scheduled = false;
+ cfile->deferred_close_scheduled = false;
spin_unlock(&CIFS_I(d_inode(cfile->dentry))->deferred_lock);
_cifsFileInfo_put(cfile, true, false);
}
@@ -905,14 +901,15 @@ int cifs_close(struct inode *inode, struct file *file)
inode->i_ctime = inode->i_mtime = current_time(inode);
spin_lock(&cinode->deferred_lock);
cifs_add_deferred_close(cfile, dclose);
- if (cfile->deferred_scheduled) {
+ if (cfile->deferred_close_scheduled &&
+ delayed_work_pending(&cfile->deferred)) {
mod_delayed_work(deferredclose_wq,
&cfile->deferred, cifs_sb->ctx->acregmax);
} else {
/* Deferred close for files */
queue_delayed_work(deferredclose_wq,
&cfile->deferred, cifs_sb->ctx->acregmax);
- cfile->deferred_scheduled = true;
+ cfile->deferred_close_scheduled = true;
spin_unlock(&cinode->deferred_lock);
return 0;
}
@@ -2020,8 +2017,7 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
if (fsuid_only && !uid_eq(open_file->uid, current_fsuid()))
continue;
if (OPEN_FMODE(open_file->f_flags) & FMODE_READ) {
- if ((!open_file->invalidHandle) &&
- (!open_file->oplock_break_received)) {
+ if ((!open_file->invalidHandle)) {
/* found a good file */
/* lock it so it will not be closed on us */
cifsFileInfo_get(open_file);
@@ -4874,13 +4870,14 @@ void cifs_oplock_break(struct work_struct *work)
}
/*
* When oplock break is received and there are no active
- * file handles but cached, then set the flag oplock_break_received.
+ * file handles but cached, then schedule deferred close immediately.
* So, new open will not use cached handle.
*/
spin_lock(&CIFS_I(inode)->deferred_lock);
is_deferred = cifs_is_deferred_close(cfile, &dclose);
- if (is_deferred && cfile->deferred_scheduled) {
- cfile->oplock_break_received = true;
+ if (is_deferred &&
+ cfile->deferred_close_scheduled &&
+ delayed_work_pending(&cfile->deferred)) {
mod_delayed_work(deferredclose_wq, &cfile->deferred, 0);
}
spin_unlock(&CIFS_I(inode)->deferred_lock);
diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
index 524dbdfb7184..cd705f8a4e31 100644
--- a/fs/cifs/misc.c
+++ b/fs/cifs/misc.c
@@ -672,6 +672,9 @@ cifs_add_pending_open(struct cifs_fid *fid, struct tcon_link *tlink,
spin_unlock(&tlink_tcon(open->tlink)->open_file_lock);
}
+/*
+ * Critical section which runs after acquiring deferred_lock.
+ */
bool
cifs_is_deferred_close(struct cifsFileInfo *cfile, struct cifs_deferred_close **pdclose)
{
@@ -688,6 +691,9 @@ cifs_is_deferred_close(struct cifsFileInfo *cfile, struct cifs_deferred_close **
return false;
}
+/*
+ * Critical section which runs after acquiring deferred_lock.
+ */
void
cifs_add_deferred_close(struct cifsFileInfo *cfile, struct cifs_deferred_close *dclose)
{
@@ -707,6 +713,9 @@ cifs_add_deferred_close(struct cifsFileInfo *cfile, struct cifs_deferred_close *
list_add_tail(&dclose->dlist, &CIFS_I(d_inode(cfile->dentry))->deferred_closes);
}
+/*
+ * Critical section which runs after acquiring deferred_lock.
+ */
void
cifs_del_deferred_close(struct cifsFileInfo *cfile)
{
--
2.30.2
^ permalink raw reply related [flat|nested] only message in thread