Re: [PATCH v2] mm/hmm/test: simplify hmm test code: use miscdevice instead of char dev

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/17/22 19:34, Mika Penttilä wrote:
On 17.3.2022 16.15, Jason Gunthorpe wrote:
On Thu, Mar 17, 2022 at 08:58:52AM +0200, Mika Penttilä wrote:
@@ -1225,7 +1232,11 @@ static int dmirror_device_init(struct dmirror_device
*mdevice, int id)

         cdev_init(&mdevice->cdevice, &dmirror_fops);
         mdevice->cdevice.owner = THIS_MODULE;
-       ret = cdev_add(&mdevice->cdevice, dev, 1);
+       device_initialize(&mdevice->device);
+       dev_set_name(&mdevice->device, "%s", dmirror_device_names[id]);
+       mdevice->device.devt = dev;
+
+       ret = cdev_device_add(&mdevice->cdevice, &mdevice->device);
         if (ret)
                 return ret;

Right, miscdev isn't that helpful in the end..

Jason


To wrap up, I could send a v3 formal patch with either this cdev way or the miscdev way.. Both eliminate the user space /proc/devices parsing and mknod'ing. Jason brought up concerns using miscdevice like this, although for this case it works correctly. miscdevice also provides a little more cleanup and simpler code, but the difference is not huge. So what do people prefer?


No strong preference from me either way, but I would like to bottom on
the potential problems with using misdevice. The fact that miscdevice is
misused in other drivers does not seem like a reason that it must be not
used here...Jason, after Mika's explanation, what's your latest
assessment?

In other words, given that the usage here is correct, is there some
reason that miscdevice is still a poor design fit? Is cdev_device_add()
a better choice here, for design reasons?

Also, is there any change that could or should be made to miscdevice,
that you have in mind?


thanks,
--
John Hubbard
NVIDIA





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux