Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 2 Jul 2019 15:24:09 -0700
From:   Andrew Morton <akpm@linux-foundation.org>
To:     Henry Burns <henryburns@google.com>
Cc:     Shakeel Butt <shakeelb@google.com>,
        Vitaly Wool <vitalywool@gmail.com>,
        Vitaly Vul <vitaly.vul@sony.com>,
        Mike Rapoport <rppt@linux.vnet.ibm.com>,
        Xidong Wang <wangxidong_97@163.com>,
        Jonathan Adams <jwadams@google.com>,
        Linux MM <linux-mm@kvack.org>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] mm/z3fold.c: Lock z3fold page before
 __SetPageMovable()
Message-Id: <20190702152409.21c6c3787d125d61fb47840a@linux-foundation.org>
In-Reply-To: <CAGQXPTiONoPARFTep-kzECtggS+zo2pCivbvPEakRF+qqq9SWA@mail.gmail.com>
References: <20190702005122.41036-1-henryburns@google.com>
        <CALvZod5Fb+2mR_KjKq06AHeRYyykZatA4woNt_K5QZNETvw4nw@mail.gmail.com>
        <CAGQXPTjU0xAWCLTWej8DdZ5TbH91m8GzeiCh5pMJLQajtUGu_g@mail.gmail.com>
        <20190702141930.e31bf1c07a77514d976ef6e2@linux-foundation.org>
        <CAGQXPTiONoPARFTep-kzECtggS+zo2pCivbvPEakRF+qqq9SWA@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, 2 Jul 2019 15:17:47 -0700 Henry Burns <henryburns@google.com> wrote:

> > > > > +       if (can_sleep) {
> > > > > +               lock_page(page);
> > > > > +               __SetPageMovable(page, pool->inode->i_mapping);
> > > > > +               unlock_page(page);
> > > > > +       } else {
> > > > > +               if (!WARN_ON(!trylock_page(page))) {
> > > > > +                       __SetPageMovable(page, pool->inode->i_mapping);
> > > > > +                       unlock_page(page);
> > > > > +               } else {
> > > > > +                       pr_err("Newly allocated z3fold page is locked\n");
> > > > > +                       WARN_ON(1);

The WARN_ON will have already warned in this case.

But the whole idea of warning in this case may be undesirable.  We KNOW
that the warning will sometimes trigger (yes?).  So what's the point in
scaring users?

Also, pr_err(...)+WARN_ON(1) can basically be replaced with WARN(1, ...)?

> > > > > +               }
> > > > > +       }