On 22/11/18 04:34AM, Andreas Dilger wrote:
> On Nov 7, 2022, at 06:22, Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx> wrote:
> >
> > f_crashdisk test failed with UNIX_IO_FORCE_BOUNCE=yes due to unbalanced
> > mutex unlock in below path.
> >
> > This patch fixes it.
> >
> > Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx>
> > ---
> > lib/ext2fs/unix_io.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/lib/ext2fs/unix_io.c b/lib/ext2fs/unix_io.c
> > index e53db333..5b894826 100644
> > --- a/lib/ext2fs/unix_io.c
> > +++ b/lib/ext2fs/unix_io.c
> > @@ -305,7 +305,6 @@ bounce_read:
> > while (size > 0) {
> > actual = read(data->dev, data->bounce, align_size);
> > if (actual != align_size) {
> > - mutex_unlock(data, BOUNCE_MTX);
>
> This patch doesn't show enough context, but AFAIK this is jumping before mutex_down()
> is called, so this *should* be correct as is?
Thanks for the review, Andreas.
Yeah, the patch diff above is not sufficient since it doesn't share enuf
context.
But essentially when "actual" is not equal to "align_size", then in this if
condition it goes to label "short_read:", which always goto error_unlock,
where we anyways call mutex_unlock()
Looking at a lot of labels in this function, this definitely looks like
something which can be cleaned up ("raw_read_blk()").
I will add that to my list of todos.
I have also shared the threadsan warning which detects the unbalanced mutex
unlock here [1]
[1]: https://lore.kernel.org/linux-ext4/cover.1667822611.git.ritesh.list@xxxxxxxxx/T/#md75b3ccb146e4433653bc2d7dd01329a9757ea26
-ritesh
