Re: Buffered I/O broken on s390x with page faults disabled (gfs2)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 9, 2022 at 10:42 AM Andreas Gruenbacher <agruenba@xxxxxxxxxx> wrote:
>
> +       while (start != end) {
> +               if (fixup_user_fault(mm, start, fault_flags, NULL))
> +                       goto out;
> +               start += PAGE_SIZE;
> +       }
> +       mmap_read_unlock(mm);
> +
> +out:
> +       if (size > (unsigned long)uaddr - start)
> +               return size - ((unsigned long)uaddr - start);
> +       return 0;
>  }

What?

That "goto out" is completely broken. It explicitly avoids the
"mmap_read_unlock()" for some reason I can't for the life of me
understand.

You must have done that on purpose, since a simple "break" would have
been the sane and simple thing to do, but it looks *entirely* wrong to
me.

I think the whole loop should just be

        mmap_read_lock(mm);
        do {
                if (fixup_user_fault(mm, start, fault_flags, NULL))
                        break;
                start = (start + PAGE_SIZE) & PAGE_MASK;
        } while (start != end);
        mmap_read_unlock(mm);

which also doesn't need that first unlooped iteration (not that I
think that passing in the non-masked starting address for the first
case actually helps, but that's a different thing).

                 Linus



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux