From: Heming Zhao <heming.zhao@suse.com>
To: Joseph Qi <joseph.qi@linux.alibaba.com>
Cc: ocfs2-devel@lists.linux.dev, ailiop@suse.com
Subject: Re: [PATCH v5 4/4] ocfs2: fix sparse warnings
Date: Thu, 28 Mar 2024 18:50:24 +0800 [thread overview]
Message-ID: <bb3fdfc2-ad16-4907-9d20-d3696c6f1a86@suse.com> (raw)
In-Reply-To: <b1982b17-0674-4080-8ed4-de2c0107c7f3@linux.alibaba.com>
On 3/28/24 17:01, Joseph Qi wrote:
>
>
> On 3/28/24 4:29 PM, Heming Zhao wrote:
>> 1.
>> fs/ocfs2/localalloc.c:1224:41: warning: incorrect type in argument 1 (different base types)
>> fs/ocfs2/localalloc.c:1224:41: expected unsigned long long val1
>> fs/ocfs2/localalloc.c:1224:41: got restricted __le32 [usertype] la_bm_off
>>
>> 2.
>> fs/ocfs2/export.c:258:32: warning: cast to restricted __le32
>> fs/ocfs2/export.c:259:33: warning: cast to restricted __le32
>> fs/ocfs2/export.c:260:32: warning: cast to restricted __le32
>> fs/ocfs2/export.c:272:32: warning: cast to restricted __le32
>> fs/ocfs2/export.c:273:33: warning: cast to restricted __le32
>> fs/ocfs2/export.c:274:32: warning: cast to restricted __le32
>>
>> 3.
>> fs/ocfs2/inode.c:1623:13: warning: context imbalance in 'ocfs2_inode_cache_lock' - wrong count at exit
>> fs/ocfs2/inode.c:1630:13: warning: context imbalance in 'ocfs2_inode_cache_unlock' - unexpected unlock
>>
>> 4.
>> fs/ocfs2/refcounttree.c:633:27: warning: incorrect type in assignment (different base types)
>> fs/ocfs2/refcounttree.c:633:27: expected restricted __le32 [usertype] rf_generation
>> fs/ocfs2/refcounttree.c:633:27: got unsigned int
>>
>> 5.
>> fs/ocfs2/dlm/dlmdomain.c:1316:20: warning: context imbalance in 'dlm_query_nodeinfo_handler' - different lock contexts for basic block
>>
>> 6.
>> fs/ocfs2/dlm/dlmrecovery.c:2950:9: warning: context imbalance in 'dlm_finalize_reco_handler' - different lock contexts for basic block
>>
>> Signed-off-by: Heming Zhao <heming.zhao@suse.com>
>> ---
>> v5: (first version)
>> - localalloc.c fix (No: 1) is split from v4 patch 3/3.
>> - new sparse warnings fixes (No: 2~6) for other files.
>>
>> ---
>> fs/ocfs2/dlm/dlmdomain.c | 11 +++++------
>> fs/ocfs2/dlm/dlmrecovery.c | 4 ++++
>> fs/ocfs2/export.c | 12 ++++++------
>> fs/ocfs2/inode.c | 2 ++
>> fs/ocfs2/localalloc.c | 4 ++--
>> fs/ocfs2/refcounttree.c | 2 +-
>> 6 files changed, 20 insertions(+), 15 deletions(-)
>>
>> diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c
>> index 5c04dde99981..2e0a2f338282 100644
>> --- a/fs/ocfs2/dlm/dlmdomain.c
>> +++ b/fs/ocfs2/dlm/dlmdomain.c
>> @@ -1274,7 +1274,7 @@ static int dlm_query_nodeinfo_handler(struct o2net_msg *msg, u32 len,
>> {
>> struct dlm_query_nodeinfo *qn;
>> struct dlm_ctxt *dlm = NULL;
>> - int locked = 0, status = -EINVAL;
>> + int status = -EINVAL;
>>
>> qn = (struct dlm_query_nodeinfo *) msg->buf;
>>
>> @@ -1290,12 +1290,11 @@ static int dlm_query_nodeinfo_handler(struct o2net_msg *msg, u32 len,
>> }
>>
>> spin_lock(&dlm->spinlock);
>> - locked = 1;
>> if (dlm->joining_node != qn->qn_nodenum) {
>> mlog(ML_ERROR, "Node %d queried nodes on domain %s but "
>> "joining node is %d\n", qn->qn_nodenum, qn->qn_domain,
>> dlm->joining_node);
>> - goto bail;
>> + goto unlock;
>> }
>>
>> /* Support for node query was added in 1.1 */
>> @@ -1305,14 +1304,14 @@ static int dlm_query_nodeinfo_handler(struct o2net_msg *msg, u32 len,
>> "but active dlm protocol is %d.%d\n", qn->qn_nodenum,
>> qn->qn_domain, dlm->dlm_locking_proto.pv_major,
>> dlm->dlm_locking_proto.pv_minor);
>> - goto bail;
>> + goto unlock;
>> }
>>
>> status = dlm_match_nodes(dlm, qn);
>>
>> +unlock:
>> + spin_unlock(&dlm->spinlock);
>> bail:
>> - if (locked)
>> - spin_unlock(&dlm->spinlock);
>> spin_unlock(&dlm_domain_lock);
>>
>> return status;
>> diff --git a/fs/ocfs2/dlm/dlmrecovery.c b/fs/ocfs2/dlm/dlmrecovery.c
>> index 50da8af988c1..40a0e5df3de0 100644
>> --- a/fs/ocfs2/dlm/dlmrecovery.c
>> +++ b/fs/ocfs2/dlm/dlmrecovery.c
>> @@ -2917,8 +2917,11 @@ int dlm_finalize_reco_handler(struct o2net_msg *msg, u32 len, void *data,
>> BUG();
>> }
>>
>> + spin_unlock(&dlm->spinlock);
>> +
>
> I'm not sure unlock first and then lock is a right way.
> I'd rather leave it as of now.
>
> Joseph
Me too, I will restore this function code in v6.
Thanks for reviewing.
-Heming
prev parent reply other threads:[~2024-03-28 10:50 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-28 8:29 [PATCH v5 0/4] improve write IO performance when fragmentation is high Heming Zhao
2024-03-28 8:29 ` [PATCH v5 1/4] ocfs2: " Heming Zhao
2024-03-28 8:47 ` Joseph Qi
2024-03-28 8:57 ` Joseph Qi
2024-03-28 8:29 ` [PATCH v5 2/4] ocfs2: adjust enabling place for la window Heming Zhao
2024-03-28 8:58 ` Joseph Qi
2024-03-28 8:29 ` [PATCH v5 3/4] ocfs2: speed up chain-list searching Heming Zhao
2024-03-28 8:58 ` Joseph Qi
2024-03-28 8:29 ` [PATCH v5 4/4] ocfs2: fix sparse warnings Heming Zhao
2024-03-28 9:01 ` Joseph Qi
2024-03-28 10:50 ` Heming Zhao [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bb3fdfc2-ad16-4907-9d20-d3696c6f1a86@suse.com \
--to=heming.zhao@suse.com \
--cc=ailiop@suse.com \
--cc=joseph.qi@linux.alibaba.com \
--cc=ocfs2-devel@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).