From: Jan Kara <jack@suse.cz>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: jack@suse.cz, linux-ext4@vger.kernel.org
Subject: Re: [bug report] ext4: do not create EA inode under buffer lock
Date: Thu, 14 Mar 2024 11:55:00 +0100 [thread overview]
Message-ID: <20240314105500.nalegmszhrs7hwsn@quack3> (raw)
In-Reply-To: <6e5f8a70-1cba-41fa-98f3-2ef3bcc29017@moroto.mountain>
Hello!
On Tue 27-02-24 12:17:11, Dan Carpenter wrote:
> The patch ea554578483b: "ext4: do not create EA inode under buffer
> lock" from Feb 9, 2024 (linux-next), leads to the following Smatch
> static checker warning:
>
> fs/ext4/xattr.c:2265 ext4_xattr_ibody_set()
> warn: duplicate check 'error' (previous on line 2255)
>
> fs/ext4/xattr.c
> 2232 int ext4_xattr_ibody_set(handle_t *handle, struct inode *inode,
> 2233 struct ext4_xattr_info *i,
> 2234 struct ext4_xattr_ibody_find *is)
> 2235 {
> 2236 struct ext4_xattr_ibody_header *header;
> 2237 struct ext4_xattr_search *s = &is->s;
> 2238 struct inode *ea_inode = NULL;
> 2239 int error;
> 2240
> 2241 if (!EXT4_INODE_HAS_XATTR_SPACE(inode))
> 2242 return -ENOSPC;
> 2243
> 2244 /* If we need EA inode, prepare it before locking the buffer */
> 2245 if (i->value && i->in_inode) {
> 2246 WARN_ON_ONCE(!i->value_len);
> 2247
> 2248 ea_inode = ext4_xattr_inode_lookup_create(handle, inode,
> 2249 i->value, i->value_len);
> 2250 if (IS_ERR(ea_inode))
> 2251 return PTR_ERR(ea_inode);
> 2252 }
> 2253 error = ext4_xattr_set_entry(i, s, handle, inode, ea_inode,
> 2254 false /* is_block */);
> 2255 if (error) {
> ^^^^^
>
> 2256 if (ea_inode) {
> 2257 int error2;
> 2258
> 2259 error2 = ext4_xattr_inode_dec_ref(handle, ea_inode);
> 2260 if (error2)
> 2261 ext4_warning_inode(ea_inode, "dec ref error=%d",
> 2262 error2);
> 2263
> 2264 /* If there was an error, revert the quota charge. */
> --> 2265 if (error)
> ^^^^^
> We know "error" is non-zero. I'm not sure whether to delete this check
> or change "error" to "error2".
Deleting the check is the right solution. The patch didn't go upstream in
the end anyway for now because it has some functional issues but I've fixed
this up locally. Thanks for report!
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
prev parent reply other threads:[~2024-03-14 10:55 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-27 9:17 [bug report] ext4: do not create EA inode under buffer lock Dan Carpenter
2024-03-14 10:55 ` Jan Kara [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=20240314105500.nalegmszhrs7hwsn@quack3 \
--to=jack@suse.cz \
--cc=dan.carpenter@linaro.org \
--cc=linux-ext4@vger.kernel.org \
/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