linux-kernel - Re: [RFC v2 0/5] remove page

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: <fbf5bc8a-6c82-a43e-dd96-8a9d2b7d3bf4@samsung.com>
Date:   Thu, 23 Mar 2023 16:00:37 +0100
From:   Pankaj Raghav <p.raghav@...sung.com>
To:     Matthew Wilcox <willy@...radead.org>
CC:     <senozhatsky@...omium.org>, <viro@...iv.linux.org.uk>,
        <axboe@...nel.dk>, <brauner@...nel.org>,
        <akpm@...ux-foundation.org>, <minchan@...nel.org>,
        <hubcap@...ibond.com>, <martin@...ibond.com>, <mcgrof@...nel.org>,
        <devel@...ts.orangefs.org>, <linux-mm@...ck.org>,
        <linux-kernel@...r.kernel.org>, <linux-fsdevel@...r.kernel.org>,
        <linux-block@...r.kernel.org>, <gost.dev@...sung.com>
Subject: Re: [RFC v2 0/5] remove page_endio()

>> 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