Yes. Whether you're wrong not to be doing that already, I've not checked;
but I think doing so will make unionfs safer against any future changes
in the relationship between set_page_dirty and clear_page_dirty_for_io.
For example, look at clear_page_dirty_for_io: it's decrementing some
statistics which __set_page_dirty_nobuffers increments. Does use of
unionfs (over some filesystems) make those numbers wrap increasingly
negative? Does adding this set_page_dirty(lower_page) correct that?
I suspect so, but may be wrong.
I'm not going to try and guess what assorted filesystems are up to,
and not all of them will be bugfree. The crucial point of PageUptodate
is that we insert a filesystem page into the page cache before it's had
any data read in: it needs to be !PageUptodate until the data is there,
and then marked PageUptodate to say the data is good and others can
start using it. See mm/filemap.c.
That was my point, and I don't really have more to add. It's unusual
to do anything with PageUptodate when writing. By clearing it when the
lower level has an error, you're throwing away the changes already made
at the upper level. You might have some good reason for that, but it's
worth questioning. If you don't know why you're Set/Clear'ing it there,
better to just take that out.