Messages in this thread | | | Subject | Re: [PATCH v1.2.26] wan: new driver retina | From | Krzysztof Halasa <> | Date | Fri, 30 May 2008 18:15:55 +0200 |
| |
"Matti Linnanvuori" <mattilinn@gmail.com> writes:
> I would not like to change a legacy driver much.
But this is a new driver, right? I guess the SIOCDEVPRIVATEs will go at some point so the change can't be really avoided.
>>> +static int fepci_char_open(struct inode *inode, struct file *filp) >>> +{ >>> + unsigned int minor = MINOR(inode->i_rdev); >>> + if (unlikely(minor >= find_cnt || card_privates[minor].pci_dev == NULL)) >>> + return -ENXIO; >>> + filp->f_op = &fepci_char_fops; >>> + if (unlikely(!try_module_get(THIS_MODULE))) >>> + return -EBUSY; >> >> That won't work race-free, use owner field. > > You mean the f_op assignment? I have removed that.
Actually I meant try_module_get() from within the driver, for example your module may get unloaded while in this function, before this try_module_get(), and that would be fatal. If you set the owner field the reference count will be incremented by the caller first.
Though you may be right about the assignement as well.
>>> +static int fepci_stream_open_down(struct net_device *dev, >>> + struct fepci_ch_private *fp) >>> +{ >>> + unsigned tx_pages, rx_pages, tx_order, rx_order; >>> + unsigned page_number; >>> + unsigned int i; >>> + >>> + if (unlikely(fp->in_eth_mode)) { >>> + dev_warn(&dev->dev, >>> + "Interface is in Ethernet mode, " >>> + "cannot open stream interface\n"); >>> + return -EBUSY; >>> + }
> Private net device ioctls are locked with rtnl_lock, so I see no > race there.
Yes, I thought stream mode = character device but now I see it's another flavour of network device(?), which is never up and exists only for the ability to receive netdev ioctls (though it also uses char dev ioctls).
Did I get this right?
Why not simply use a character device, with normal read, write etc? -- Krzysztof Halasa
| |