linux-kernel - Re: [Lsf-pc] [LSF/MM TOPIC] do we really need PG

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] [day] [month] [year] [list]

Message-ID: <87h93eoxhg.fsf@notabene.neil.brown.name>
Date:   Wed, 01 Mar 2017 07:45:47 +1100
From:   NeilBrown <neilb@...e.com>
To:     Jeff Layton <jlayton@...hat.com>,
        Andreas Dilger <adilger@...ger.ca>
Cc:     linux-block@...r.kernel.org,
        linux-scsi <linux-scsi@...r.kernel.org>,
        lsf-pc <lsf-pc@...ts.linuxfoundation.org>,
        Neil Brown <neilb@...e.de>,
        LKML <linux-kernel@...r.kernel.org>,
        James Bottomley <James.Bottomley@...senPartnership.com>,
        linux-mm <linux-mm@...ck.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [Lsf-pc] [LSF/MM TOPIC] do we really need PG_error at all?

On Mon, Feb 27 2017, Jeff Layton wrote:

> On Tue, 2017-02-28 at 10:32 +1100, NeilBrown wrote:
>> On Mon, Feb 27 2017, Andreas Dilger wrote:
>> 
>> > 
>> > My thought is that PG_error is definitely useful for applications to get
>> > correct errors back when doing write()/sync_file_range() so that they know
>> > there is an error in the data that _they_ wrote, rather than receiving an
>> > error for data that may have been written by another thread, and in turn
>> > clearing the error from another thread so it *doesn't* know it had a write
>> > error.
>> 
>> It might be useful in that way, but it is not currently used that way.
>> Such usage would be a change in visible behaviour.
>> 
>> sync_file_range() calls filemap_fdatawait_range(), which calls
>> filemap_check_errors().
>> If there have been any errors in the file recently, inside or outside
>> the range, the latter will return an error which will propagate up.
>> 
>> > 
>> > As for stray sync() clearing PG_error from underneath an application, that
>> > shouldn't happen since filemap_fdatawait_keep_errors() doesn't clear errors
>> > and is used by device flushing code (fdatawait_one_bdev(), wait_sb_inodes()).
>> 
>> filemap_fdatawait_keep_errors() calls __filemap_fdatawait_range() which
>> clears PG_error on every page.
>> What it doesn't do is call filemap_check_errors(), and so doesn't clear
>> AS_ENOSPC or AS_EIO.
>> 
>> 
>
> I think it's helpful to get a clear idea of what happens now in the face
> of errors and what we expect to happen, and I don't quite have that yet:
>
> --------------------------8<-----------------------------
> void page_endio(struct page *page, bool is_write, int err)
> {
>         if (!is_write) {
>                 if (!err) {
>                         SetPageUptodate(page);
>                 } else {
>                         ClearPageUptodate(page);
>                         SetPageError(page);
>                 }
>                 unlock_page(page);
>         } else {
>                 if (err) {
>                         SetPageError(page);
>                         if (page->mapping)
>                                 mapping_set_error(page->mapping, err);
>                 }
>                 end_page_writeback(page);
>         }
> }
> --------------------------8<-----------------------------
>
> ...not everything uses page_endio, but it's a good place to look since
> we have both flavors of error handling in one place.
>
> On a write error, we SetPageError and set the error in the mapping.
>
> What I'm not clear on is:
>
> 1) what happens to the page at that point when we get a writeback error?
> Does it just remain in-core and is allowed to service reads (assuming
> that it was uptodate before)?

Yes, it remains in core and can service reads.  It is no different from
a page on which a write recent succeeded, except that the write didn't
succeed so the contents of backing store might be different from the
contents of the page.

>
> Can I redirty it and have it retry the write? Is there standard behavior
> for this or is it just up to the whim of the filesystem?

Everything is at the whim of the filesystem, but I doubt if many differ
from the above.

NeilBrown

>
> I'll probably have questions about the read side as well, but for now it
> looks like it's mostly used in an ad-hoc way to communicate errors
> across subsystems (block to fs layer, for instance).
> --
> Jeff Layton <jlayton@...hat.com>

Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)