Messages in this thread | | | From | Linus Torvalds <> | Date | Wed, 16 Nov 2022 09:52:06 -0800 | Subject | Re: mm: mmu_gather: do not define delayed_rmap if not used |
| |
On Tue, Nov 15, 2022 at 11:51 PM Alexander Gordeev <agordeev@linux.ibm.com> wrote: > > In cases the delayed rmap removal is not used (which are > currently UP and s390) skip delayed_rmap flag and make > the related code paths no-op.
So I'm not convinced about this patch.
I particularly dislike adding even more #ifdef's around the data structure - it already is pretty nasty, and it was hard to see where things were initialized.
The only actual code impact of this is in tlb_next_batch(), which tests for "do I have delayed rmaps pending, in which case I won't add new batches". Everything else is already either optimized away, or just "one bit declared in a structure that already has bitfields and has room for several extra bits":
And that "I need to allocate new batches" case really doesn't matter anyway - it's not even build at all on s390, and on UP where it's there but technically pointless to have the test it really isn't noticeable.
So the previous patch I was "this shouldn't actually _matter_, but it does seem cleaner to do it this way".
But _this_ patch makes me go "it still doesn't matter, but now this patch is actually adding extra infrastructure for the 'not-mattering' case".
So I don't _hate_ this patch, but I think this actually makes the current mess wrt our 'struct mmu_gather' worse rather than better.
That structure is already a pain, with horrendous initialization and different bit-fields having different lifetimes. I'd rather have one unconditional simple bitfield, than have another bitfield that has conditional complications.
Linus
| |