linux-kernel - Re: [PATCH/RESEND 2/5] mm/z3fold.c: extend compaction function

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMJBoFMDp2gbJ+CcX4T8nYcTZ-s4E4jAGsE46qfqUB1J9N2NtQ@mail.gmail.com>
Date:   Tue, 10 Jan 2017 21:49:26 +0100
From:   Vitaly Wool <vitalywool@...il.com>
To:     Dan Streetman <ddstreet@...e.org>
Cc:     Linux-MM <linux-mm@...ck.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH/RESEND 2/5] mm/z3fold.c: extend compaction function

On Wed, Jan 4, 2017 at 4:43 PM, Dan Streetman <ddstreet@...e.org> wrote:

<snip>
>>  static int z3fold_compact_page(struct z3fold_header *zhdr)
>>  {
>>         struct page *page = virt_to_page(zhdr);
>> -       void *beg = zhdr;
>> +       int ret = 0;
>
> I still don't understand why you're adding ret and using goto.  Just
> use return for each failure case.

I guess it's a matter of taste, I prefer having single function exit
elsewhere so I do it here too.

>> +
>> +       if (test_bit(MIDDLE_CHUNK_MAPPED, &page->private))
>> +               goto out;
>>
>> +       if (zhdr->middle_chunks != 0) {
>
> you appear to have just re-sent all your patches without addressing
> comments; in patch 4 you invert the check and return, which is what
> you should have done here in the first place, as that change is
> unrelated to that patch.

Not quite, I just thought we'd agreed on the patch 4 being separate. I
folded the locking fixes but not header size fixes.

~vitaly