new ioctl cmd added to query zone device type. This will be
used once the test_hmm adds zone device coherent type.
Signed-off-by: Alex Sierra <[email protected]>
Acked-by: Felix Kuehling <[email protected]>
Reviewed-by: Alistair Poppple <[email protected]>
Signed-off-by: Christoph Hellwig <[email protected]>
---
lib/test_hmm.c | 23 +++++++++++++++++++++--
lib/test_hmm_uapi.h | 8 ++++++++
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/lib/test_hmm.c b/lib/test_hmm.c
index cfe632047839..7a27584484ce 100644
--- a/lib/test_hmm.c
+++ b/lib/test_hmm.c
@@ -87,6 +87,7 @@ struct dmirror_chunk {
struct dmirror_device {
struct cdev cdevice;
struct hmm_devmem *devmem;
+ unsigned int zone_device_type;
unsigned int devmem_capacity;
unsigned int devmem_count;
@@ -1026,6 +1027,15 @@ static int dmirror_snapshot(struct dmirror *dmirror,
return ret;
}
+static int dmirror_get_device_type(struct dmirror *dmirror,
+ struct hmm_dmirror_cmd *cmd)
+{
+ mutex_lock(&dmirror->mutex);
+ cmd->zone_device_type = dmirror->mdevice->zone_device_type;
+ mutex_unlock(&dmirror->mutex);
+
+ return 0;
+}
static long dmirror_fops_unlocked_ioctl(struct file *filp,
unsigned int command,
unsigned long arg)
@@ -1076,6 +1086,9 @@ static long dmirror_fops_unlocked_ioctl(struct file *filp,
ret = dmirror_snapshot(dmirror, &cmd);
break;
+ case HMM_DMIRROR_GET_MEM_DEV_TYPE:
+ ret = dmirror_get_device_type(dmirror, &cmd);
+ break;
default:
return -EINVAL;
}
@@ -1260,14 +1273,20 @@ static void dmirror_device_remove(struct dmirror_device *mdevice)
static int __init hmm_dmirror_init(void)
{
int ret;
- int id;
+ int id = 0;
+ int ndevices = 0;
ret = alloc_chrdev_region(&dmirror_dev, 0, DMIRROR_NDEVICES,
"HMM_DMIRROR");
if (ret)
goto err_unreg;
- for (id = 0; id < DMIRROR_NDEVICES; id++) {
+ memset(dmirror_devices, 0, DMIRROR_NDEVICES * sizeof(dmirror_devices[0]));
+ dmirror_devices[ndevices++].zone_device_type =
+ HMM_DMIRROR_MEMORY_DEVICE_PRIVATE;
+ dmirror_devices[ndevices++].zone_device_type =
+ HMM_DMIRROR_MEMORY_DEVICE_PRIVATE;
+ for (id = 0; id < ndevices; id++) {
ret = dmirror_device_init(dmirror_devices + id, id);
if (ret)
goto err_chrdev;
diff --git a/lib/test_hmm_uapi.h b/lib/test_hmm_uapi.h
index f14dea5dcd06..17f842f1aa02 100644
--- a/lib/test_hmm_uapi.h
+++ b/lib/test_hmm_uapi.h
@@ -19,6 +19,7 @@
* @npages: (in) number of pages to read/write
* @cpages: (out) number of pages copied
* @faults: (out) number of device page faults seen
+ * @zone_device_type: (out) zone device memory type
*/
struct hmm_dmirror_cmd {
__u64 addr;
@@ -26,6 +27,7 @@ struct hmm_dmirror_cmd {
__u64 npages;
__u64 cpages;
__u64 faults;
+ __u64 zone_device_type;
};
/* Expose the address space of the calling process through hmm device file */
@@ -35,6 +37,7 @@ struct hmm_dmirror_cmd {
#define HMM_DMIRROR_SNAPSHOT _IOWR('H', 0x03, struct hmm_dmirror_cmd)
#define HMM_DMIRROR_EXCLUSIVE _IOWR('H', 0x04, struct hmm_dmirror_cmd)
#define HMM_DMIRROR_CHECK_EXCLUSIVE _IOWR('H', 0x05, struct hmm_dmirror_cmd)
+#define HMM_DMIRROR_GET_MEM_DEV_TYPE _IOWR('H', 0x06, struct hmm_dmirror_cmd)
/*
* Values returned in hmm_dmirror_cmd.ptr for HMM_DMIRROR_SNAPSHOT.
@@ -62,4 +65,9 @@ enum {
HMM_DMIRROR_PROT_DEV_PRIVATE_REMOTE = 0x30,
};
+enum {
+ /* 0 is reserved to catch uninitialized type fields */
+ HMM_DMIRROR_MEMORY_DEVICE_PRIVATE = 1,
+};
+
#endif /* _LIB_TEST_HMM_UAPI_H */
--
2.32.0
On 5/31/2022 12:31 PM, Andrew Morton wrote:
> On Tue, 31 May 2022 10:56:23 -0500 Alex Sierra <[email protected]> wrote:
>
>> new ioctl cmd added to query zone device type. This will be
>> used once the test_hmm adds zone device coherent type.
>>
>> @@ -1026,6 +1027,15 @@ static int dmirror_snapshot(struct dmirror *dmirror,
>> return ret;
>> }
>>
>> +static int dmirror_get_device_type(struct dmirror *dmirror,
>> + struct hmm_dmirror_cmd *cmd)
>> +{
>> + mutex_lock(&dmirror->mutex);
>> + cmd->zone_device_type = dmirror->mdevice->zone_device_type;
>> + mutex_unlock(&dmirror->mutex);
> What does the locking here do?
>
> Presumably cmd->zone_device_type can become out of date the instant the
> mutex is released, so what was the point in taking the mutex?
Actually this is not used at all. Thanks for finding it. Honestly, I
don't remember what we used this type request for.
I will remove all related code and send a new patch series version.
Regards,
Alex Sierra
>
> And does it make sense to return potentially out-of-date info to
> userspace? Perhaps this interface simply shouldn't exist?
On Tue, 31 May 2022 10:56:23 -0500 Alex Sierra <[email protected]> wrote:
> new ioctl cmd added to query zone device type. This will be
> used once the test_hmm adds zone device coherent type.
>
> @@ -1026,6 +1027,15 @@ static int dmirror_snapshot(struct dmirror *dmirror,
> return ret;
> }
>
> +static int dmirror_get_device_type(struct dmirror *dmirror,
> + struct hmm_dmirror_cmd *cmd)
> +{
> + mutex_lock(&dmirror->mutex);
> + cmd->zone_device_type = dmirror->mdevice->zone_device_type;
> + mutex_unlock(&dmirror->mutex);
What does the locking here do?
Presumably cmd->zone_device_type can become out of date the instant the
mutex is released, so what was the point in taking the mutex?
And does it make sense to return potentially out-of-date info to
userspace? Perhaps this interface simply shouldn't exist?