Skip to content

Commit

Permalink
cifs: simplify refcounting for oplock breaks
Browse files Browse the repository at this point in the history
Currently, we take a sb->s_active reference and a cifsFileInfo reference
when an oplock break workqueue job is queued. This is unnecessary and
more complicated than it needs to be. Also as Al points out,
deactivate_super has non-trivial locking implications so it's best to
avoid that if we can.

Instead, just cancel any pending oplock breaks for this filehandle
synchronously in cifsFileInfo_put after taking it off the lists.
That should ensure that this job doesn't outlive the structures it
depends on.

Reported-by: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve French <sfrench@us.ibm.com>
  • Loading branch information
Jeff Layton authored and Steve French committed Jul 31, 2011
1 parent 5980fc9 commit ad63594
Show file tree
Hide file tree
Showing 5 changed files with 4 additions and 58 deletions.
18 changes: 0 additions & 18 deletions fs/cifs/cifsfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -86,24 +86,6 @@ extern mempool_t *cifs_sm_req_poolp;
extern mempool_t *cifs_req_poolp;
extern mempool_t *cifs_mid_poolp;

void
cifs_sb_active(struct super_block *sb)
{
struct cifs_sb_info *server = CIFS_SB(sb);

if (atomic_inc_return(&server->active) == 1)
atomic_inc(&sb->s_active);
}

void
cifs_sb_deactive(struct super_block *sb)
{
struct cifs_sb_info *server = CIFS_SB(sb);

if (atomic_dec_and_test(&server->active))
deactivate_super(sb);
}

static int
cifs_read_super(struct super_block *sb)
{
Expand Down
4 changes: 0 additions & 4 deletions fs/cifs/cifsfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@ extern struct file_system_type cifs_fs_type;
extern const struct address_space_operations cifs_addr_ops;
extern const struct address_space_operations cifs_addr_ops_smallbuf;

/* Functions related to super block operations */
extern void cifs_sb_active(struct super_block *sb);
extern void cifs_sb_deactive(struct super_block *sb);

/* Functions related to inodes */
extern const struct inode_operations cifs_dir_inode_ops;
extern struct inode *cifs_root_iget(struct super_block *);
Expand Down
2 changes: 0 additions & 2 deletions fs/cifs/cifsglob.h
Original file line number Diff line number Diff line change
Expand Up @@ -942,8 +942,6 @@ GLOBAL_EXTERN spinlock_t siduidlock;
GLOBAL_EXTERN spinlock_t sidgidlock;

void cifs_oplock_break(struct work_struct *work);
void cifs_oplock_break_get(struct cifsFileInfo *cfile);
void cifs_oplock_break_put(struct cifsFileInfo *cfile);

extern const struct slow_work_ops cifs_oplock_break_ops;

Expand Down
27 changes: 2 additions & 25 deletions fs/cifs/file.c
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,8 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
}
spin_unlock(&cifs_file_list_lock);

cancel_work_sync(&cifs_file->oplock_break);

if (!tcon->need_reconnect && !cifs_file->invalidHandle) {
int xid, rc;

Expand Down Expand Up @@ -2418,31 +2420,6 @@ void cifs_oplock_break(struct work_struct *work)
cinode->clientCanCacheRead ? 1 : 0);
cFYI(1, "Oplock release rc = %d", rc);
}

/*
* We might have kicked in before is_valid_oplock_break()
* finished grabbing reference for us. Make sure it's done by
* waiting for cifs_file_list_lock.
*/
spin_lock(&cifs_file_list_lock);
spin_unlock(&cifs_file_list_lock);

cifs_oplock_break_put(cfile);
}

/* must be called while holding cifs_file_list_lock */
void cifs_oplock_break_get(struct cifsFileInfo *cfile)
{
cifs_sb_active(cfile->dentry->d_sb);
cifsFileInfo_get(cfile);
}

void cifs_oplock_break_put(struct cifsFileInfo *cfile)
{
struct super_block *sb = cfile->dentry->d_sb;

cifsFileInfo_put(cfile);
cifs_sb_deactive(sb);
}

const struct address_space_operations cifs_addr_ops = {
Expand Down
11 changes: 2 additions & 9 deletions fs/cifs/misc.c
Original file line number Diff line number Diff line change
Expand Up @@ -585,15 +585,8 @@ is_valid_oplock_break(struct smb_hdr *buf, struct TCP_Server_Info *srv)

cifs_set_oplock_level(pCifsInode,
pSMB->OplockLevel ? OPLOCK_READ : 0);
/*
* cifs_oplock_break_put() can't be called
* from here. Get reference after queueing
* succeeded. cifs_oplock_break() will
* synchronize using cifs_file_list_lock.
*/
if (queue_work(system_nrt_wq,
&netfile->oplock_break))
cifs_oplock_break_get(netfile);
queue_work(system_nrt_wq,
&netfile->oplock_break);
netfile->oplock_break_cancelled = false;

spin_unlock(&cifs_file_list_lock);
Expand Down

0 comments on commit ad63594

Please sign in to comment.