On Sat, Jul 22, 2023 at 04:42:33AM +0100, Matthew Wilcox wrote:
What I want to do is allow us to set/clear multiple bits in the same instruction that unlocks the folio. The obvious one is folio_mark_uptodate(); this is commonly paired with folio_unlock(), and it's a crucial part of page cache reads. There are some less obvious ones like folio_start_writeback() and folio_unlock(), which isn't included in this patch series. If we're going to set one bit and clear another bit, we have to use the xor/eor instruction, and that's what we do. On some architectures, such as MIPS, there's actually a separate function to implement all of this and so passing it a (constant) mask (instead of calculating that mask from what is now a variable bit) makes sense, and we can then use that function to implement both clear_bit_unlock_is_negative_byte() and change_and_unlock_is_negative_byte(). I'm still going around on this. I might change the API to always pass in a mask from folio_set_unlock(). And maybe we actualy get rid of clear_bit_unlock_is_negative_byte() since it's essentially a subset of change_and_unlock_is_negative_byte(). But I can change m68k to use andi.b for now if you feel strongly.
I do prefer to be consistent. We already have strange inconsistencies for bad reasons or no reason at all. However, if it's temporary, I suppose it doesn't matter that much. With the new code in the diff in your git tree, the use of eori is now consistent if you get rid of the old clear bit version as separate code. If there's a decent chance it will be a while before that next stage, I would personally recommend using the andi. However, I would defer to the others on the list if people think it's fine. Brad Boyer flar@xxxxxxxxxxxxx
