Re: clear_bit_unlock_is_negative_byte

[Brad Boyer <flar@allandria.com>]

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@allandria.com