[PATCH] x86/pv: Rename pv/ro-page-fault.c to pv/emul-ro-page-fault.c

[PATCH] x86/pv: Rename pv/ro-page-fault.c to pv/emul-ro-page-fault.c

[Andrew Cooper]

To match all our other emulation handling.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/pv/Makefile                                  | 2 +-
 xen/arch/x86/pv/{ro-page-fault.c => emul-ro-page-fault.c} | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
 rename xen/arch/x86/pv/{ro-page-fault.c => emul-ro-page-fault.c} (99%)

diff --git a/xen/arch/x86/pv/Makefile b/xen/arch/x86/pv/Makefile
index 65bca04..bc777e9 100644
--- a/xen/arch/x86/pv/Makefile
+++ b/xen/arch/x86/pv/Makefile
@@ -5,12 +5,12 @@ obj-y += emulate.o
 obj-y += emul-gate-op.o
 obj-y += emul-inv-op.o
 obj-y += emul-priv-op.o
+obj-y += emul-ro-page-fault.o
 obj-y += grant_table.o
 obj-y += hypercall.o
 obj-y += iret.o
 obj-y += misc-hypercalls.o
 obj-y += mm.o
-obj-y += ro-page-fault.o
 obj-$(CONFIG_PV_SHIM) += shim.o
 obj-y += traps.o
 
diff --git a/xen/arch/x86/pv/ro-page-fault.c b/xen/arch/x86/pv/emul-ro-page-fault.c
similarity index 99%
rename from xen/arch/x86/pv/ro-page-fault.c
rename to xen/arch/x86/pv/emul-ro-page-fault.c
index 7e0e7e8..aad5332 100644
--- a/xen/arch/x86/pv/ro-page-fault.c
+++ b/xen/arch/x86/pv/emul-ro-page-fault.c
@@ -1,5 +1,5 @@
 /******************************************************************************
- * arch/x86/pv/ro-page-fault.c
+ * arch/x86/pv/emul-ro-page-fault.c
  *
  * Read-only page fault emulation for PV guests
  *
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] x86/pv: Rename pv/ro-page-fault.c to pv/emul-ro-page-fault.c

[Jan Beulich]

>>> On 02.02.18 at 17:58, <andrew.cooper3@citrix.com> wrote:
> To match all our other emulation handling.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> ---
>  xen/arch/x86/pv/Makefile                                  | 2 +-
>  xen/arch/x86/pv/{ro-page-fault.c => emul-ro-page-fault.c} | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>  rename xen/arch/x86/pv/{ro-page-fault.c => emul-ro-page-fault.c} (99%)

When this file was introduced, iirc I had specifically asked to drop
the pointless emul- prefix. If you want to make things consistent
again, please instead drop the emul- prefixes of the other files.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] x86/pv: Rename pv/ro-page-fault.c to pv/emul-ro-page-fault.c

[Andrew Cooper]

On 05/02/18 08:57, Jan Beulich wrote:
>>>> On 02.02.18 at 17:58, <andrew.cooper3@citrix.com> wrote:
>> To match all our other emulation handling.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> ---
>>  xen/arch/x86/pv/Makefile                                  | 2 +-
>>  xen/arch/x86/pv/{ro-page-fault.c => emul-ro-page-fault.c} | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>  rename xen/arch/x86/pv/{ro-page-fault.c => emul-ro-page-fault.c} (99%)
> When this file was introduced, iirc I had specifically asked to drop
> the pointless emul- prefix. If you want to make things consistent
> again, please instead drop the emul- prefixes of the other files.

No.

First of all, this file is the most recent to come into existence,
around 3 months after the others.

The point of naming things in a consistent fashion is for the benefit of
humans, and having the emulation related functionality logically grouped
is a benefit, not a detriment.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] x86/pv: Rename pv/ro-page-fault.c to pv/emul-ro-page-fault.c

[Jan Beulich]

>>> On 05.02.18 at 13:22, <andrew.cooper3@citrix.com> wrote:
> On 05/02/18 08:57, Jan Beulich wrote:
>>>>> On 02.02.18 at 17:58, <andrew.cooper3@citrix.com> wrote:
>>> To match all our other emulation handling.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> ---
>>> CC: Jan Beulich <JBeulich@suse.com>
>>> ---
>>>  xen/arch/x86/pv/Makefile                                  | 2 +-
>>>  xen/arch/x86/pv/{ro-page-fault.c => emul-ro-page-fault.c} | 2 +-
>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>  rename xen/arch/x86/pv/{ro-page-fault.c => emul-ro-page-fault.c} (99%)
>> When this file was introduced, iirc I had specifically asked to drop
>> the pointless emul- prefix. If you want to make things consistent
>> again, please instead drop the emul- prefixes of the other files.
> 
> No.
> 
> First of all, this file is the most recent to come into existence,
> around 3 months after the others.

Right - it was too late for me to realize the needlessly long names
in those earlier code movement patches.

> The point of naming things in a consistent fashion is for the benefit of
> humans, and having the emulation related functionality logically grouped
> is a benefit, not a detriment.

They're all quite well grouped now already by being in pv/. Otherwise
do you mean to also change e.g. gpr_switch.S to emul-gpr_switch.S?

Besides the plain cosmetic aspect, shell command completion is of
more use when there aren't many files with similar prefixes in a single
directory.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] x86/pv: Rename pv/ro-page-fault.c to pv/emul-ro-page-fault.c

[Andrew Cooper]

On 05/02/18 13:01, Jan Beulich wrote:
>>>> On 05.02.18 at 13:22, <andrew.cooper3@citrix.com> wrote:
>> On 05/02/18 08:57, Jan Beulich wrote:
>>>>>> On 02.02.18 at 17:58, <andrew.cooper3@citrix.com> wrote:
>>>> To match all our other emulation handling.
>>>>
>>>> No functional change.
>>>>
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> ---
>>>> CC: Jan Beulich <JBeulich@suse.com>
>>>> ---
>>>>  xen/arch/x86/pv/Makefile                                  | 2 +-
>>>>  xen/arch/x86/pv/{ro-page-fault.c => emul-ro-page-fault.c} | 2 +-
>>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>>  rename xen/arch/x86/pv/{ro-page-fault.c => emul-ro-page-fault.c} (99%)
>>> When this file was introduced, iirc I had specifically asked to drop
>>> the pointless emul- prefix. If you want to make things consistent
>>> again, please instead drop the emul- prefixes of the other files.
>> No.
>>
>> First of all, this file is the most recent to come into existence,
>> around 3 months after the others.
> Right - it was too late for me to realize the needlessly long names
> in those earlier code movement patches.

That is a very subjective point of view which I don't agree with.

Naming is all to do with conveying meaning, and shorter isn't
necessarily better.

>
>> The point of naming things in a consistent fashion is for the benefit of
>> humans, and having the emulation related functionality logically grouped
>> is a benefit, not a detriment.
> They're all quite well grouped now already by being in pv/.

That is not the relevant grouping.  Most of our emulation based logic
has an emul- prefix and this file is an odd one out.

Naming the files without their emul- prefix leaves them with no context
as to what they are doing.  "gate-op.c" or "invl-op.c" are far less
obvious to their purpose than "emul-gate-op.c" and "emul-invl-op.c".

> Otherwise do you mean to also change e.g. gpr_switch.S to emul-gpr_switch.S?

This is extra special, and as soon as I can figure out how it actually
works, I plan to replace it with something comprehensive.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] x86/pv: Rename pv/ro-page-fault.c to pv/emul-ro-page-fault.c

[Jan Beulich]

>>> On 26.02.18 at 19:15, <andrew.cooper3@citrix.com> wrote:
> On 05/02/18 13:01, Jan Beulich wrote:
>>>>> On 05.02.18 at 13:22, <andrew.cooper3@citrix.com> wrote:
>>> On 05/02/18 08:57, Jan Beulich wrote:
>>>>>>> On 02.02.18 at 17:58, <andrew.cooper3@citrix.com> wrote:
>>>>> To match all our other emulation handling.
>>>>>
>>>>> No functional change.
>>>>>
>>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>> ---
>>>>> CC: Jan Beulich <JBeulich@suse.com>
>>>>> ---
>>>>>  xen/arch/x86/pv/Makefile                                  | 2 +-
>>>>>  xen/arch/x86/pv/{ro-page-fault.c => emul-ro-page-fault.c} | 2 +-
>>>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>>>  rename xen/arch/x86/pv/{ro-page-fault.c => emul-ro-page-fault.c} (99%)
>>>> When this file was introduced, iirc I had specifically asked to drop
>>>> the pointless emul- prefix. If you want to make things consistent
>>>> again, please instead drop the emul- prefixes of the other files.
>>> No.
>>>
>>> First of all, this file is the most recent to come into existence,
>>> around 3 months after the others.
>> Right - it was too late for me to realize the needlessly long names
>> in those earlier code movement patches.
> 
> That is a very subjective point of view which I don't agree with.
> 
> Naming is all to do with conveying meaning, and shorter isn't
> necessarily better.
> 
>>
>>> The point of naming things in a consistent fashion is for the benefit of
>>> humans, and having the emulation related functionality logically grouped
>>> is a benefit, not a detriment.
>> They're all quite well grouped now already by being in pv/.
> 
> That is not the relevant grouping.  Most of our emulation based logic
> has an emul- prefix and this file is an odd one out.
> 
> Naming the files without their emul- prefix leaves them with no context
> as to what they are doing.  "gate-op.c" or "invl-op.c" are far less
> obvious to their purpose than "emul-gate-op.c" and "emul-invl-op.c".

A little less obvious, yes, but far? Furthermore, taking the file you
want to rename into account, "emul-gate-op" indeed stands for
"emulate gate operations" (likewise for the inv and priv infixes),
while "emul-ro-page-fault" does _not_ mean "emulate r/o page
faults", as that would mean we emulate something to _produce_
r/o page faults. Instead we _handle_ r/o page faults here in
order to emulate certain write operations the guest does. In that
sense, the name ought to be e.g. "emul-write-op". That, however,
would break the connection to e.g. the "ptwr" abbreviation we
continue to use, so please don't take this as a suggestion.

IOW - I can live with your objection to rename the existing
emul-*.c files (despite thinking that for name completion they're
badly named; the emul- prefix would better have been an -emul
suffix, and leaving aside mere name length), but I continue to
dislike the new name of the file here.

>> Otherwise do you mean to also change e.g. gpr_switch.S to emul-gpr_switch.S?
> 
> This is extra special, and as soon as I can figure out how it actually
> works, I plan to replace it with something comprehensive.

;-)

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel