All of lore.kernel.org
 help / color / mirror / Atom feed
From: steve.capper@linaro.org (Steve Capper)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V2 0/3] PTE fixes for arm and arm64
Date: Wed, 26 Mar 2014 13:23:19 +0000	[thread overview]
Message-ID: <20140326132318.GA30750@linaro.org> (raw)
In-Reply-To: <20140326110141.GY7528@n2100.arm.linux.org.uk>

On Wed, Mar 26, 2014 at 11:01:41AM +0000, Russell King - ARM Linux wrote:
> On Wed, Mar 26, 2014 at 10:23:19AM +0000, Steve Capper wrote:
> > If there are no objections, I was going to put the following into
> > Russell's patch system:
> > 	arm: mm: Double logical invert for pte accessors
> > 	arm: mm: Switch back to L_PTE_WRITE
> 
> I'm not all that happy with double inversions - I think they just serve
> to cause confusion (and it was confusing, which is why I removed it.)
> I'll only take them if you have a really good reason why you want to
> bring it back.

Hi Russell,
The problem I'm trying to solve is for LPAE, where we have flags in the
upper 32 bits of a page table entry that are tested for with a bitwise
and, then subsequently downcast by a store to 32-bit integer:

	gather_stats(page, md, pte_dirty(*pte), 1);
and,

	static inline unsigned long huge_pte_write(pte_t pte)
	{
		return pte_write(pte);
	}

(and other cases that may arise in future).

We erroneously get false returned in situations where we shouldn't, and
it's tricky to track this down.

My reasoning was that applying double logical inverts to all the pte
accessors that don't already have one ensures that we get values that
are safe to downcast, and this is safe for cases where new accessors
are added and the convention is followed and for where bit layouts are
changed.

I agree that this is confusing (and does look rather ugly), and I would
happily try alternatives.

I had tried to create a helper macro, pte_isset, but this didn't attract
any positive comments:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-February/235380.html

I tried bool types, but it looked a little too cumbersome to me.

Would the double logical invert be more acceptable if it were better
commented?

Cheers,
-- 
Steve

  reply	other threads:[~2014-03-26 13:23 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-25 11:38 [PATCH V2 0/3] PTE fixes for arm and arm64 Steve Capper
2014-02-25 11:38 ` [PATCH V2 1/3] arm: mm: Double logical invert for pte accessors Steve Capper
2014-02-28 15:45   ` Catalin Marinas
2014-03-26 11:01   ` Catalin Marinas
2014-02-25 11:38 ` [PATCH V2 2/3] arm64: mm: Add double logical invert to " Steve Capper
2014-02-28 15:46   ` Catalin Marinas
2014-02-25 11:38 ` [PATCH V2 3/3] arm: mm: Switch back to L_PTE_WRITE Steve Capper
2014-03-26 11:00   ` Catalin Marinas
2014-02-25 15:22 ` [PATCH V2 0/3] PTE fixes for arm and arm64 Will Deacon
2014-02-25 17:08   ` Steve Capper
2014-03-26 10:23 ` Steve Capper
2014-03-26 11:01   ` Russell King - ARM Linux
2014-03-26 13:23     ` Steve Capper [this message]
2014-03-26 13:48       ` Russell King - ARM Linux
2014-03-27 10:01         ` Steve Capper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140326132318.GA30750@linaro.org \
    --to=steve.capper@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.