public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Thorsten Blum <thorsten.blum@linux.dev>
To: "Maciej W. Rozycki" <macro@orcam.me.uk>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Oleg Nesterov <oleg@redhat.com>,
	linux-mips@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] MIPS: Fix MAX_REG_OFFSET and remove zero-length struct member
Date: Fri, 18 Apr 2025 15:38:36 +0200	[thread overview]
Message-ID: <B71034AC-B0FC-4C5F-8562-661D6AD11056@linux.dev> (raw)
In-Reply-To: <alpine.DEB.2.21.2504181337350.18253@angie.orcam.me.uk>

On 18. Apr 2025, at 14:44, Maciej W. Rozycki wrote:
> On Fri, 18 Apr 2025, Thorsten Blum wrote:
>>> Though the fix is incorrect for CPU_CAVIUM_OCTEON, because it doesn't 
>>> allow one to access the second half of the last register, and I find it 
>>> exceedingly complex anyway.  Just:
>>> 
>>> #define MAX_REG_OFFSET \
>>> (offsetof(struct pt_regs, __last) - sizeof(unsigned long))
>>> 
>>> will do (as `regs_get_register' operates on `unsigned long' quantities).
>> 
>> Does regs_get_register() even work for CPU_CAVIUM_OCTEON when accessing
>> the last two registers because they're both ULL, not UL? (independent of
>> my patch)
> 
> Or rather two arrays of registers.  With 32-bit configurations their 
> contents have to be retrieved by pieces.  I don't know if it's handled by 
> the caller(s) though as I'm not familiar with this interface.

Ah, CPU_CAVIUM_OCTEON seems to be 64-bit only, so there's no difference
between UL and ULL. Then both my patch and your suggestion:

  #define MAX_REG_OFFSET (offsetof(struct pt_regs, __last) - sizeof(unsigned long))

should be fine.

I still prefer my approach without '__last[0]' because it also silences
the following false-positive Coccinelle warning, which is how I stumbled
upon this in the first place:

  ./ptrace.h:51:15-21: WARNING use flexible-array member instead

Would it make sense to also change the register arrays 'mpl' and 'mtp'
from ULL to UL? ULL seems unnecessarily confusing to me.

Thanks,
Thorsten


  reply	other threads:[~2025-04-18 13:39 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-17 17:47 [PATCH v2] MIPS: Fix MAX_REG_OFFSET and remove zero-length struct member Thorsten Blum
2025-04-18  7:57 ` Thomas Bogendoerfer
2025-04-18 10:06   ` Thorsten Blum
2025-04-18 10:36     ` Maciej W. Rozycki
2025-04-18 11:05       ` Thorsten Blum
2025-04-18 12:44         ` Maciej W. Rozycki
2025-04-18 13:38           ` Thorsten Blum [this message]
2025-04-18 15:14             ` Maciej W. Rozycki
2025-04-18 20:18               ` Thorsten Blum
2025-04-18 20:21                 ` Thorsten Blum
2025-04-19  2:56                   ` Huacai Chen
2025-04-19 10:35                     ` Thorsten Blum
2025-04-27  7:12                   ` Thomas Bogendoerfer
2025-04-27 16:22             ` Thomas Bogendoerfer

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=B71034AC-B0FC-4C5F-8562-661D6AD11056@linux.dev \
    --to=thorsten.blum@linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=macro@orcam.me.uk \
    --cc=oleg@redhat.com \
    --cc=tsbogend@alpha.franken.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox