Re: [PATCH] btrfs: remove btrfs_writepage_cow_fixup

[Gerald Schaefer <gerald.schaefer@linux.ibm.com>]

On Wed, 29 Jun 2022 09:58:37 +0200
Christoph Hellwig <hch@lst.de> wrote:

> On Tue, Jun 28, 2022 at 01:53:56PM +0200, David Sterba wrote:
> > This would work only for the higher level API where eg. RDMA notifies
> > the filesystem, but there's still the s390 case that is part of the
> > hardware architecture. The fixup worker is there as a safety for all
> > other cases, I'm not fine removing or ignoring it.  
> 
> I'd really like to have a confirmation of this whole s390 theory.
> s390 does treat some dirtying different than the other architectures,
> but none of that should leak into the file system API if any way that
> bypasses ->page_mkwrite.
> 
> Because if it did most file systems would be completely broken on
> s390.

Could you please be more specific about what exactly you mean with
"the s390 case that is part of the hardware architecture"?

One thing that s390 might handle different from others, is that it
is not using a HW dirty bit in the PTE, but instead a fault-triggered
SW dirty bit.

E.g. pte_mkwrite() will mark a PTE as writable (via another SW bit),
but not clear the HW protection bit, which would then generate a
fault on first write access. In handle_pte_fault(), the PTE would
then be marked as dirty via pte_mkdirty(), which also clears the HW
protection bit, at least for pte_write() PTEs.
For the !pte_write() COW case, we would go through do_wp_page() like
everybody else, but probably still end up in some pte_mkdirty()
eventually, to avoid getting another fault.

Not being familiar with either btrfs, any other fs, or RDMA, I cannot
really follow the discussion here. Still it seems to me that you are
not talking about special s390 HW architecture regarding PTE, but
rather about some (struct) page dirtying on the COW path, which should
be completely common code and not subject to any s390 special case.

Somewhere in this thread it was also mentioned that "s390 can not do
page flags update atomically", which I can not confirm, in case this
was the question. The code in include/linux/page-flags.h seems to
use normal (arch)_test/set/clear_bit operations, which should always be
atomic on s390.