On Mon, Sep 13, 2010 at 8:07 AM, Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
> On Thu, Sep 02, 2010 at 02:00:13PM -0400, Fred Isaman wrote:
>> static struct pnfs_layoutdriver_type *
>> find_pnfs_driver(u32 id) {
>> - return NULL;
>> + struct pnfs_layoutdriver_type *local;
>> +
>> + spin_lock(&pnfs_spinlock);
>> + local = find_pnfs_driver_locked(id);
>> + spin_unlock(&pnfs_spinlock);
>> + return local;
>
> What about refcounting the structure?
>
The structure is pretty tightly tied to the lifetime of the driver
module. It seems like grabbing a reference on the module (which as
Trond pointed out needs to be done) is enough.
>> + spin_lock(&pnfs_spinlock);
>> + if (!find_pnfs_driver_locked(ld_type->id)) {
>> + list_add(&ld_type->pnfs_tblid, &pnfs_modules_tbl);
>> + status = 0;
>> + dprintk("%s Registering id:%u name:%s\n", __func__, ld_type->id,
>> + ld_type->name);
>> + } else
>> + printk(KERN_ERR "%s Module with id %d already loaded!\n",
>> + __func__, ld_type->id);
>> + spin_unlock(&pnfs_spinlock);
>
> In other places we generally don't bother doing these checks, so for
> pnfs with just three clients it's even more poinless.
>
But they certainly don't hurt. If it would prevent inclusion in
mainline, I'll take out the check, but intentionally introducing
potential bugs just because its done in other places seems like a poor
argument.
Fred
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
