Re: [PATCH v2 0/5] IB/hfi1: Remove write() and use ioctl() for user access

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

 



On Thu, May 12, 2016 at 01:25:08PM -0600, Jason Gunthorpe wrote:
On Thu, May 12, 2016 at 03:07:38PM -0400, Dennis Dalessandro wrote:
>>There is also a driver software version being exported via a sysfs
>>file. This is needed so that user space applications (psm) can
>>determine if it needs to do ioctl() or write().
>
>Why? Don't do this, just call ioctl() and if it fails then use write().

Is it really that big of a deal to export a version number?

If it isn't needed, don't add it..

For the reason I gave, I think it is needed so unless you are vehemently opposed to it I would prefer to leave it.

>another reference counted structure. You need to consider how all this
>works when the driver is removed while the cdev is still open (or
>driver remove is racing with the cdev release).

The driver can't be removed while the cdev is still open. I tested with a
test code that opens /dev/hfi1_0 and spins. The use count as reported by
lsmod ticks up and the driver can not be unloaded until I ctrl+c the test
program.

Drivers can be removed in other ways, eg pci hot unplug. Do not assume
module_exit is the only way and rely on module ref counting for
correctness.

Point taken. I'll look into this. So are you perhaps suggesting we do something like is done for uverbs_dev in ib_verbs_add_one() where there is a kobj for uverbs_dev and the parent of uverbs_dev->cdeb is set to that? In our case it would probably be something like hfi1_devdata.

-Denny
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux