Return -ENOMEM if alloc_workqueue() fails. Don't return success.
Fixes: d8a650adf429 ("erofs: add per-cpu threads for decompression as an
option")
Signed-off-by: Dan Carpenter
---
fs/erofs/zdata.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/erofs/zd
On Mon, Jul 18, 2022 at 07:36:14PM +0800, Gao Xiang wrote:
> Hi Dan,
>
> On Mon, Jul 18, 2022 at 02:20:08PM +0300, Dan Carpenter wrote:
> > It's easier to see what this loop is doing when the decrement is in
> > the normal place.
> >
> > Signed-off-by: Dan
It's easier to see what this loop is doing when the decrement is in
the normal place.
Signed-off-by: Dan Carpenter
---
fs/erofs/zdata.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
index 601cfcb07c50..2691100eb231 100644
--- a/fs
ixes: b858a4844cfb ("erofs: support superblock checksum")
regards,
dan carpenter
gt;owned_head == Z_EROFS_PCLUSTER_TAIL)
449 clt->tailpcl = pcl;
450 clt->owned_head = >next;
451 clt->pcl = pcl;
452 clt->cl = cl;
453 return 0;
regards,
dan carpenter
On Fri, Aug 30, 2019 at 11:36:42AM +0800, Gao Xiang wrote:
> As Dan Carpenter suggested [1], I have to remove
> all erofs likely/unlikely annotations.
>
> [1] https://lore.kernel.org/linux-fsdevel/20190829154346.GK23584@kadam/
> Reported-by: Dan Carpenter
> Signed
tive" mean here? Now that it's out of staging we
aren't applying clean up patches?
regards,
dan carpenter
ng".
> and I will review it then.
>
> p.s. if someone argues here or there, there will be endless
> issues since there is no standard at all.
More complaining... It doesn't matter if you can find ext4 that looks
like dog poop, that's irrelevant.
regards,
dan carpenter
ave
CONFIG_EROFS_FS_DEBUG disabled. Plus it has "DBG" in the name so it
felt like debug code. But I ended up focussing on it instead of seeing
the "(nofail ? __GFP_NOFAIL : 0)" bit. The DBG_BUGON() is unreachable
and misleading nonsense fluff. :(
regards,
dan carpenter
On Wed, Aug 28, 2019 at 07:02:49PM +0800, Gao Xiang wrote:
> Hi Dan,
>
> On Wed, Aug 28, 2019 at 01:55:41PM +0300, Dan Carpenter wrote:
> > Hello Gao Xiang,
> >
> > The patch 8be31270362b: "staging: erofs: introduce erofs_grab_bio"
> > from Aug 21,
last_index = first_index + i;
1278 skippage:
1279 if (++i < clusterpages)
1280 goto repeat;
regards,
dan carpenter
It turns out that my Smatch cross function DB was slightly out of date
after the move from staging/ to fs/. Once I rebuilt the DB then the
warning went away.
Anyway, thanks for taking the time to look at this.
regards,
dan carpenter
On Tue, Aug 27, 2019 at 05:36:29PM +0800, Gao Xiang wrote:
> Hi Dan,
>
> Thanks for your report.
>
> On Tue, Aug 27, 2019 at 12:03:55PM +0300, Dan Carpenter wrote:
> > Hello Gao Xiang,
> >
> > This is a semi-automatic email about new static checker warnings
of a page */
668 ++spiltted;
669 /* also update nr_pages */
670 clt->cl->nr_pages = max_t(pgoff_t, clt->cl->nr_pages, index +
1);
^ ^
Unchecked dereferences.
671 next_part:
672 /* can be used for verification */
regards,
dan carpenter
le32_to_cpu(v2->i_ctime_nsec);
> >
> > inode->i_size = le64_to_cpu(v2->i_size);
> > +
> > + /* total blocks for compressed files */
> > + if (vi->data_mapping_mode == EROFS_INODE_LAYOUT_COMPRESSION)
> > + nblks = v2->i_u.compressed_blocks;
>
> Xiang,
>
> It needs to use le32_to_cpu(). ;)
>
I wonder it the kbuild bot is going to send an email about that...
Hopefully these sorts of bugs get detected with Sparse CF=-D__CHECK_ENDIAN__
regards,
dan carpenter
out in the previous email [1].
>
> [1] https://lore.kernel.org/lkml/20190411080953.GE421@ming.t460p/
>
Please also use the Fixes tag.
Fixes: 07173c3ec276 ("block: enable multipage bvecs")
> Suggested-by: Ming Lei
> Reviewed-by: Chao Yu
> Signed-off-by: Gao Xiang
regards,
dan carpenter
On Fri, Feb 15, 2019 at 05:32:33PM +0800, Chao Yu wrote:
> On 2019/2/15 15:57, Dan Carpenter wrote:
> > On Fri, Feb 15, 2019 at 03:02:25PM +0800, Chao Yu wrote:
> >> On 2019/2/1 20:16, Gao Xiang wrote:
> >>> + /*
> >>> + * on-disk erro
DBG_BUGON(1);
> return 1;
> }
Please don't add likely/unlikely() annotations unless you have
benchmarked it and it makes a difference.
regards,
dan carpenter
.
Over time it would be good to improve and expand the standard kernel
error injection frameworks to cover more types.
regards,
dan carpenter
any need to panic or rush. It was just the kunmap_atomic() which only
affects corrupted filesystem on linux-next so it's not a big deal. The
rest was just my style grumblings and could have been addressed later or
even ignored.
regards,
dan carpenter
nd_target_block_classic(
> if (likely(!IS_ERR(candidate)))
^^
Not related to the this patch, but I wonder how this works. IS_ERR()
already has an opposite unlikely() inside so I wonder which trumps the
other?
> put_page(candidate);
> candidate = page;
> + *_ndirents = ndirents;
regards,
dan carpenter
there is an explanation.
regards,
dan carpenter
t; > +{
> > + struct posix_acl *acl;
> > + int ea_prefix, rc;
> > + char *value = NULL;
> > +
> > + switch(type) {
> > + case ACL_TYPE_ACCESS:
> > + ea_prefix = EROFS_XATTR_INDEX_POSIX_ACL_ACCESS;
> > + break;
> > + case ACL_TYPE_DEFAULT:
> > + ea_prefix = EROFS_XATTR_INDEX_POSIX_ACL_DEFAULT;
> > + break;
> > + default:
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + rc = erofs_getxattr(inode, ea_prefix, "", NULL, 0);
> > + if (rc > 0) {
> > + value = kvmalloc(rc, GFP_KERNEL);
>
> erofs_kmalloc() is enough?
>
> Thanks,
>
Hopefully, regular kmalloc() is enough.
Do really need the erofs_kmalloc() function? Regular kmalloc() has
fault injection already. Have you tried to use it?
regards,
dan carpenter
I will test it asap...
This is an awkward thing.
Just don't send it until after you test it. We also ignore RFC patches
so if you mark it as RFC that will be ignored if you want.
regards,
dan carpenter
> /* Note that at least one of 'prefix' and 'name' should be non-NULL */
The comment is sort of confusing now.
> - prefix = h->prefix != NULL ? h->prefix : h->name;
> + prefix = xattr_prefix(h);
> prefix_len = strlen(prefix);
regards,
dan carpenter
On Wed, Jan 16, 2019 at 04:59:54PM +0800, Gao Xiang wrote:
> It is more suitable to update in erofs_workgroup_get since
> it's actually the one matched with erofs_workgroup_put.
>
This patch is fine. No need to resend.
Reviewed-by: Dan Carpenter
But for future reference, I found t
LocalWords: Alighnment
^^^
Remove this.
Please wait at least a day before resending a patch.
regards,
dan carpenter
On Thu, Oct 11, 2018 at 06:49:57PM +0800, Gao Xiang wrote:
> Hi Dan,
>
> On 2018/10/11 18:18, Dan Carpenter wrote:
> > On Thu, Oct 11, 2018 at 05:46:26PM +0800, Gao Xiang wrote:
> >>
> >>
> >> On 2018/10/11 16:44, Dan Carpenter wrote:
> >>&g
he cross function DB
twice to make the warning go away. It could be because it starts out
with the old vle_get_logical_extent_head() information so it thinks it
knows how that function works.
regards,
dan carpenter
> But it is better to fix them in an independent patch. :)
Yah. Of course. This was completely unrelated.
regards,
dan carpenter
Yeah. You'd have to remove the const.
Anyway, on looking at it more, I guess this patch is fine for now. We
will probably end up changing z_erofs_vle_work_lookup() and
z_erofs_vle_work_register() some more in the future.
regards,
dan carpenter
turn values other than 0 and ENOATTR(ENODATA) rarely happens,
> it should be in the slow path...
>
What I'm trying to say is please stop adding so many likely/unlikely
annotations. You should only add them if you have the benchmark data to
show the it really is required.
regards,
dan carpenter
e callers all treate zero and one the same so there isn't any reason
to propogate the 1 back.
regards,
dan carpenter
inline. Leave it to
the compiler to decide.
regards,
dan carpenter
ork;
>
> - egrp = erofs_find_workgroup(sb, idx, );
> + egrp = erofs_find_workgroup(f->sb, f->idx, );
> if (egrp == NULL) {
> - *grp_ret = NULL;
> + *f->grp_ret = NULL;
All these pointers to pointer seem a bit messy. Just do this:
struct z_erofs_vle_workgroup *grp;
Then replace "grp" in z_erofs_vle_work_iter_begin() with finder.grp;
regards,
dan carpenter
unlock_page(page);
> - put_page(page);
> -
> - page = ERR_PTR(-EIO);
> + if (io_retries) {
> + --io_retries;
> + goto unlock_repeat;
> + }
> + err = -EIO;
> + goto err_out;
regards,
dan carpenter
On Wed, Aug 01, 2018 at 05:36:54PM +0800, Gao Xiang wrote:
> This patch adds a missing break after adding the default case.
>
> Reviewed-by: Chao Yu
> Signed-off-by: Gao Xiang
> ---
> As pointed out by Dan Carpenter:
> - fix the wrong place of fallthrough comments
Thank
37 matches
Mail list logo
