On Fri, 09 Feb 2007 20:20:27 +1100 Rusty Russell <[email protected]> wrote:
> +#define log(...) \
> + do { \
> + mm_segment_t oldfs = get_fs(); \
> + char buf[100]; \
> + sprintf(buf, "lguest:" __VA_ARGS__); \
> + set_fs(KERNEL_DS); \
> + sys_write(1, buf, strlen(buf)); \
> + set_fs(oldfs); \
> + } while(0)
Due to gcc shortcomings, each instance of this will chew an additional 100
bytes of stack. Unless they fixed it recently. Is a bit of a timebomb. I
guess ksaprintf() could be used.
It also looks a bit, umm, innovative.
On Fri, 2007-02-09 at 01:35 -0800, Andrew Morton wrote:
> On Fri, 09 Feb 2007 20:20:27 +1100 Rusty Russell <[email protected]> wrote:
>
> > +#define log(...) \
> > + do { \
> > + mm_segment_t oldfs = get_fs(); \
> > + char buf[100]; \
> > + sprintf(buf, "lguest:" __VA_ARGS__); \
> > + set_fs(KERNEL_DS); \
> > + sys_write(1, buf, strlen(buf)); \
> > + set_fs(oldfs); \
> > + } while(0)
>
> Due to gcc shortcomings, each instance of this will chew an additional 100
> bytes of stack. Unless they fixed it recently. Is a bit of a timebomb. I
> guess ksaprintf() could be used.
>
> It also looks a bit, umm, innovative.
It's also unused 8)
It's an extremely useful macro for doing grossly invasive logging of the
guest. I'll drop it if you prefer.
Cheers,
Rusty.
Rusty Russell wrote:
> On Fri, 2007-02-09 at 01:35 -0800, Andrew Morton wrote:
>
>> On Fri, 09 Feb 2007 20:20:27 +1100 Rusty Russell <[email protected]> wrote:
>>
>>
>>> +#define log(...) \
>>> + do { \
>>> + mm_segment_t oldfs = get_fs(); \
>>> + char buf[100]; \
>>> + sprintf(buf, "lguest:" __VA_ARGS__); \
>>> + set_fs(KERNEL_DS); \
>>> + sys_write(1, buf, strlen(buf)); \
>>> + set_fs(oldfs); \
>>> + } while(0)
>>>
>> Due to gcc shortcomings, each instance of this will chew an additional 100
>> bytes of stack. Unless they fixed it recently. Is a bit of a timebomb. I
>> guess ksaprintf() could be used.
>>
>> It also looks a bit, umm, innovative.
>>
>
> It's also unused 8)
>
> It's an extremely useful macro for doing grossly invasive logging of the
> guest. I'll drop it if you prefer.
>
Yes, it is a bit, umm, innovative. If it is going to be kept, even if
just for devel logging, you should disable interrupts around it.
Changing segments is not a normal thing to do.
Zach
> Yes, it is a bit, umm, innovative. If it is going to be kept, even if
> just for devel logging, you should disable interrupts around it.
> Changing segments is not a normal thing to do.
Actually that wouldn't be needed because interrupts are not allowed to do any
user accesses. And contrary to the name it doesn't actually change
the segment registers, only state used by *_user.
-Andi
Andi Kleen wrote:
>> Yes, it is a bit, umm, innovative. If it is going to be kept, even if
>> just for devel logging, you should disable interrupts around it.
>> Changing segments is not a normal thing to do.
>>
>
> Actually that wouldn't be needed because interrupts are not allowed to do any
> user accesses. And contrary to the name it doesn't actually change
> the segment registers, only state used by *_user.
>
My bad, I fell for the same mistake as everyone. Set_fs is just way too
confusing of a name now. But good to know interrupts must be disable in
such a circumstance.
Zach
On Fri, Feb 09, 2007 at 03:54:37AM -0800, Zachary Amsden wrote:
> Andi Kleen wrote:
> >>Yes, it is a bit, umm, innovative. If it is going to be kept, even if
> >>just for devel logging, you should disable interrupts around it.
> >>Changing segments is not a normal thing to do.
> >>
> >
> >Actually that wouldn't be needed because interrupts are not allowed to do
> >any user accesses. And contrary to the name it doesn't actually change
> >the segment registers, only state used by *_user.
> >
>
> My bad, I fell for the same mistake as everyone. Set_fs is just way too
You could change the name. Only 654 occurrences all over the tree @)
> confusing of a name now. But good to know interrupts must be disable in
> such a circumstance.
+not
-Andi
Andi Kleen wrote:
>>>
>>>
>> My bad, I fell for the same mistake as everyone. Set_fs is just way too
>>
>
> You could change the name. Only 654 occurrences all over the tree @)
>
I'm preparing a patch. +not. This legacy thing is certainly taking a
long time to disappear. Long live x86!
>
>> confusing of a name now. But good to know interrupts must be disable in
>> such a circumstance.
>>
>
> +not
>
+ack
Zach
From: Andi Kleen <[email protected]>
Date: Fri, 9 Feb 2007 12:50:06 +0100
>
> > Yes, it is a bit, umm, innovative. If it is going to be kept, even if
> > just for devel logging, you should disable interrupts around it.
> > Changing segments is not a normal thing to do.
>
> Actually that wouldn't be needed because interrupts are not allowed to do any
> user accesses. And contrary to the name it doesn't actually change
> the segment registers, only state used by *_user.
That's right and we use this construct all throughout the
syscall compatibility layer for 64-bit platforms.