From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 966BFC433EF for ; Sat, 13 Nov 2021 22:58:34 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id F2A256112F for ; Sat, 13 Nov 2021 22:58:33 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org F2A256112F Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=linux-foundation.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 641A56B0073; Sat, 13 Nov 2021 17:58:33 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 5EF6A6B0078; Sat, 13 Nov 2021 17:58:33 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4B7766B007B; Sat, 13 Nov 2021 17:58:33 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0182.hostedemail.com [216.40.44.182]) by kanga.kvack.org (Postfix) with ESMTP id 3CEFC6B0073 for ; Sat, 13 Nov 2021 17:58:33 -0500 (EST) Received: from smtpin30.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id E9C9D18041CE8 for ; Sat, 13 Nov 2021 22:58:32 +0000 (UTC) X-FDA: 78805422864.30.A270161 Received: from mail-ed1-f43.google.com (mail-ed1-f43.google.com [209.85.208.43]) by imf12.hostedemail.com (Postfix) with ESMTP id 7CAB010000AF for ; Sat, 13 Nov 2021 22:58:32 +0000 (UTC) Received: by mail-ed1-f43.google.com with SMTP id m20so7648640edc.5 for ; Sat, 13 Nov 2021 14:58:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=LEuGHgv8xqOKZW64ode+/vIb5U7iTdOdXwRrwQBNLu4=; b=XkNBzfN4fLGPsauuvfdIlysef4k2lXPjy/brVO3fQbIUkP4un5NBNuJbYRg+jTD39F Fa536miQLexXz4qYGv/+wUPcCm6GoksWxHM9XB4iT85385bzP6gvIXODjl0BQAhW/B+L TKzWHwI0YFNCHzXLzbJMul6gXWx7lWoYG9tMo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=LEuGHgv8xqOKZW64ode+/vIb5U7iTdOdXwRrwQBNLu4=; b=o3w/Fpvvwu5Oda7w0vlOSAio/GJU9Jy9BlS0jAn0mC6H6/1bpZEd6XCQGRfMY7lUrD ackm682KEh5osGvw1ca4xNaF9lxYoCwXGdqFBtOCIL4OLQ1jHEjDUfuDoIjZ381b0Opj gdK2WsmBAFyZkGmVWcPZNSBZDTzM1P+Tbq4cx0Br+vmlOwqoW4x/fOfyxYj4dm4GRewO 7Qw4BtL4R1bbL9M4RYA5CRBuWvy88PMm/zxAD43zIV0Qd3vWGOiohBjz5/R1DBkgrda1 ooZcVuqdNwVwDMlHw6G7nkSc5CTDQ2ixJYXrfFhjIFWAITcdJQnw6LYznlPN798kVYbV k3QA== X-Gm-Message-State: AOAM532XeeWgoeE1I1QyJgnIjwA84OXldLyOKdcS3YJboGedzQFrJnjl QiUvFaUb/rW03l13zyuyZF3cMENOuE83++A19Oc= X-Google-Smtp-Source: ABdhPJzele3w+XsYwD2N01Xo1FE3yk+hLQmAmAEfH6aJ4QUxc6TtcxgoDdHYfsUYzIawdS0G/6fIaA== X-Received: by 2002:aa7:c78f:: with SMTP id n15mr37418725eds.344.1636844310968; Sat, 13 Nov 2021 14:58:30 -0800 (PST) Received: from mail-wr1-f50.google.com (mail-wr1-f50.google.com. [209.85.221.50]) by smtp.gmail.com with ESMTPSA id my2sm1167345ejc.109.2021.11.13.14.58.30 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 13 Nov 2021 14:58:30 -0800 (PST) Received: by mail-wr1-f50.google.com with SMTP id r8so22323867wra.7 for ; Sat, 13 Nov 2021 14:58:30 -0800 (PST) X-Received: by 2002:adf:dcd0:: with SMTP id x16mr32224185wrm.229.1636844310120; Sat, 13 Nov 2021 14:58:30 -0800 (PST) MIME-Version: 1.0 References: <20211111084617.6746-1-ajaygargnsit@gmail.com> <628a49dc-f6f7-5aa4-8a4d-4f2ed19b7f3f@kernel.dk> In-Reply-To: From: Linus Torvalds Date: Sat, 13 Nov 2021 14:58:13 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] mm: shmem: do not call PageHWPoison on a ERR-page To: Yang Shi Cc: Jens Axboe , Arnd Bergmann , Ajay Garg , Hugh Dickins , Andrew Morton , Linux-MM , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 7CAB010000AF X-Stat-Signature: kjn8snax7ct8ttk8kc6p4puic9auhyc5 Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=google header.b=XkNBzfN4; dmarc=none; spf=pass (imf12.hostedemail.com: domain of torvalds@linuxfoundation.org designates 209.85.208.43 as permitted sender) smtp.mailfrom=torvalds@linuxfoundation.org X-HE-Tag: 1636844312-333863 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Sat, Nov 13, 2021 at 2:30 PM Yang Shi wrote: > > The above snippet is actually ok since if *pagep returned via > shmem_getpage()'s parameter is not NULL, then ret is 0. That's a random implementation detail, and is not ok to rely on. It may or may not be true, and is not part of the rules of error handling. If a function returns an error, you shouldn't be looking at the other stuff it returned. Here's a very recent example of the same kind of problem: https://lore.kernel.org/lkml/163663333331.414.639840290224641315.tip-bot2@tip-bot2/ where people didn't actually look properly at the return value of the function, and instead looked at the page pointers that the function filled in. See? EXACT same logic. And completely buggy. > When shmem_getpage() returns error code, *pagep is NULL IIUC. No. When a function returns an error code, you check for the error code, and don't rely on weather the function then filled in other data (or left it alone, or whatever). So the code should (a) check and handle error returns properly (b) be legible That (b) basically means that if it's not entirely trivial (and none of this was entirely trivial), then when you get an error, you just deal with it right away. You return early, and undo anything you need to undo. You don't do "oh, let's keep that error, and then do something else that maybe also generates an error". That "don't handle the error directly" was why shmem_read_mapping_page_gfp() was buggy and would cause an oops. And while the shmem_write_begin() code migth not cause an oops, it had the same fundamental bad pattern. Error handling is where 99% of all problems occur. But that also means that you should do the obvious thing wrt error handling, and not have some crazy "if function X returned an error, it will have left the return array untouched" which may or may not be true. When a function returns an error code, you do error handling based on that code. Not on some random other state. Linus