On 2020-07-06 10:30:27 +0200, Aurélien Aptel wrote:
Paul Aurich <paul@xxxxxxxxxxxxxx> writes:
Changes since v2:
- address sparse lock warnings by inlining smb2_tcon_has_lease and
hardcoding some return values (seems to help sparse's analysis)
Ah, I think the issue is not the inlining but rather you need to
instruct sparse that smb2_tcon_hash_lease is expected to release the
lock.
https://www.kernel.org/doc/html/v4.12/dev-tools/sparse.html#using-sparse-for-lock-checking
Probably with __releases somewhere in the func prototype.
I tried various iterations of that without finding one that seems to work.
I suspect it's because the unlocking is _conditional_.
w/o the inline and with __releases(&cifs_tcp_ses_lock):
fs/cifs/smb2misc.c:508:1: warning: context imbalance in 'smb2_tcon_has_lease'
- different lock contexts for basic block
fs/cifs/smb2misc.c:612:25: warning: context imbalance in
'smb2_is_valid_lease_break'- different lock contexts for basic block
Digging further, I found __acquire and __release (not plural), which can be
used in individual blocks. The following seems to silence the sparse warnings
- does this seem valid?
@@ -504,7 +504,7 @@ cifs_ses_oplock_break(struct work_struct *work)
kfree(lw);
}
-static inline bool
+static bool
smb2_tcon_has_lease(struct cifs_tcon *tcon, struct smb2_lease_break *rsp)
{
bool found;
@@ -521,6 +521,7 @@ smb2_tcon_has_lease(struct cifs_tcon *tcon, struct smb2_lease_break *rsp)
lease_state = le32_to_cpu(rsp->NewLeaseState);
+ __acquire(cifs_tcp_ses_lock);
spin_lock(&tcon->open_file_lock);
list_for_each(tmp, &tcon->openFileList) {
cfile = list_entry(tmp, struct cifsFileInfo, tlist);
@@ -571,8 +572,7 @@ smb2_tcon_has_lease(struct cifs_tcon *tcon, struct smb2_lease_break *rsp)
}
spin_unlock(&tcon->open_file_lock);
- if (!found)
- return false;
+ if (found) {
spin_unlock(&cifs_tcp_ses_lock);
lw = kmalloc(sizeof(struct smb2_lease_break_work), GFP_KERNEL);
@@ -586,7 +586,12 @@ smb2_tcon_has_lease(struct cifs_tcon *tcon, struct smb2_lease_break *rsp)
lw->lease_state = rsp->NewLeaseState;
memcpy(lw->lease_key, lease_key, SMB2_LEASE_KEY_SIZE);
queue_work(cifsiod_wq, &lw->lease_break);
- return true;
+ } else {
+ /* for sparse */
+ __release(&cifs_tcp_ses_lock);
+ }
+
+ return found;
}
static bool
@@ -614,6 +619,8 @@ smb2_is_valid_lease_break(char *buffer)
cifs_stats_inc(
&tcon->stats.cifs_stats.num_oplock_brks);
if (smb2_tcon_has_lease(tcon, rsp)) {
+ /* Unlocked in smb2_tcon_has_lease */
+ __release(&cifs_tcp_ses_lock);
return true;
}
~Paul
