On Wed, 16 Oct 2024 14:07:31 +0200
Sven Schnelle <[email protected]> wrote:
> > +/* Return reserved data size in words */
> > +static inline int decode_fprobe_header(unsigned long val, struct fprobe
> > **fp)
> > +{
> > + unsigned long ptr;
> > +
> > + ptr = (val & FPROBE_HEADER_PTR_MASK) | ~FPROBE_HEADER_PTR_MASK;
> > + if (fp)
> > + *fp = (struct fprobe *)ptr;
> > + return val >> FPROBE_HEADER_PTR_BITS;
> > +}
>
> I think that still has the issue that the size is encoded in the
> leftmost fields of the pointer, which doesn't work on all
> architectures. I reported this already in v15
> (https://lore.kernel.org/all/[email protected]/)
From what you said in v15:
> I haven't yet fully understood why this logic is needed, but the
> WARN_ON_ONCE triggers on s390. I'm assuming this fails because fp always
> has the upper bits of the address set on x86 (and likely others). As an
> example, in my test setup, fp is 0x8feec218 on s390, while it is
> 0xffff888100add118 in x86-kvm.
Since we only need to save 4 bits for size, we could have what it is
replacing always be zero or always be f, depending on the arch. The
question then is, is s390's 4 MSBs always zero?
Thus we could make it be:
static inline int decode_fprobe_header(unsigned long val, struct fprobe **fp)
{
unsigned long ptr;
ptr = (val & FPROBE_HEADER_PTR_MASK) | FPROBE_HEADER_MSB_MASK;
if (fp)
*fp = (struct fprobe *)ptr;
return val >> FPROBE_HEADER_PTR_BITS;
}
And define FPROBE_HEADER_MSB_MASK to be either:
For most archs:
#define FPROBE_HEADER_MSB_MASK (0xf << FPROBE_HEADER_PTR_BITS)
or on s390:
#define FPROBE_HEADER_MSB_MASK (0x0)
Would this work?
-- Steve
