Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <fbf5bc8a-6c82-a43e-dd96-8a9d2b7d3bf4@samsung.com>
Date:   Thu, 23 Mar 2023 16:00:37 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
        Thunderbird/102.8.0
Subject: Re: [RFC v2 0/5] remove page_endio()
Content-Language: en-US
To:     Matthew Wilcox <willy@infradead.org>
CC:     <senozhatsky@chromium.org>, <viro@zeniv.linux.org.uk>,
        <axboe@kernel.dk>, <brauner@kernel.org>,
        <akpm@linux-foundation.org>, <minchan@kernel.org>,
        <hubcap@omnibond.com>, <martin@omnibond.com>, <mcgrof@kernel.org>,
        <devel@lists.orangefs.org>, <linux-mm@kvack.org>,
        <linux-kernel@vger.kernel.org>, <linux-fsdevel@vger.kernel.org>,
        <linux-block@vger.kernel.org>, <gost.dev@samsung.com>
From:   Pankaj Raghav <p.raghav@samsung.com>
In-Reply-To: <ZBtSevjWLybE6S07@casper.infradead.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
CMS-TYPE: 201P
References: <CGME20230322135015eucas1p2ff980e76159f0ceef7bf66934580bd6c@eucas1p2.samsung.com>
        <20230322135013.197076-1-p.raghav@samsung.com>
        <ZBtSevjWLybE6S07@casper.infradead.org>
Precedence: bulk

>> Open questions:
>> - Willy pointed out that the calls to folio_set_error() and
>>   folio_clear_uptodate() are not needed anymore in the read path when an
>>   error happens[2]. I still don't understand 100% why they aren't needed
>>   anymore as I see those functions are still called in iomap. It will be
>>   good to put that rationale as a part of the commit message.
> 
> page_endio() was generic.  It needed to handle a lot of cases.  When it's
> being inlined into various completion routines, we know which cases we
> need to handle and can omit all the cases which we don't.
> 
> We know the uptodate flag is clear.  If the uptodate flag is set,
> we don't call the filesystem's read path.  Since we know it's clear,
> we don't need to clear it.
> 
Got it.

> We don't need to set the error flag.  Only some filesystems still use
> the error flag, and orangefs isn't one of them.  I'd like to get rid
> of the error flag altogether, and I've sent patches in the past which
> get us a lot closer to that desired outcome.  Not sure we're there yet.
> Regardless, generic code doesn't check the error flag.

Thanks for the explanation. I think found the series you are referring here.

https://lore.kernel.org/linux-mm/20220527155036.524743-1-willy@infradead.org/#t

I see orangefs is still setting the error flag in orangefs_read_folio(), so
it should be removed at some point?

I also changed mpage to **not set** the error flag in the read path. It does beg
the question whether block_read_full_folio() and iomap_finish_folio_read() should
also follow the suit.

--
Pankaj