| Message ID | 1306249644-23422-1-git-send-email-bharrosh@panasas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2011-05-24 18:07, Boaz Harrosh wrote: > Fix BUGs in the new "Use global-device-cache". > > One thing I don't understand is why the compiler did > not complain when the code was returning the wrong > type of structure > > Signed-off-by: Boaz Harrosh <bharrosh@panasas.com> > --- > fs/nfs/objlayout/objio_osd.c | 28 +++++++++++++++++++--------- > 1 files changed, 19 insertions(+), 9 deletions(-) > > diff --git a/fs/nfs/objlayout/objio_osd.c b/fs/nfs/objlayout/objio_osd.c > index 167cd1e..faacde2 100644 > --- a/fs/nfs/objlayout/objio_osd.c > +++ b/fs/nfs/objlayout/objio_osd.c > @@ -56,6 +56,7 @@ objio_free_deviceid_node(struct nfs4_deviceid_node *d) > { > struct objio_dev_ent *de = container_of(d, struct objio_dev_ent, id_node); > > + dprintk("%s: free od=%p\n", __func__, de->od); > osduld_put_device(de->od); > kfree(de); > } > @@ -64,14 +65,18 @@ static struct objio_dev_ent *_dev_list_find(const struct nfs_server *nfss, > const struct nfs4_deviceid *d_id) > { > struct nfs4_deviceid_node *d; > + struct objio_dev_ent *de; > > d = nfs4_find_get_deviceid(nfss->pnfs_curr_ld, nfss->nfs_client, d_id); > if (!d) > return NULL; > - return container_of(d, struct objio_dev_ent, id_node); > + > + de = container_of(d, struct objio_dev_ent, id_node); > + return de; That's not really required as container_of() does the type casting for you. > } > > -static int _dev_list_add(const struct nfs_server *nfss, > +static struct objio_dev_ent * > +_dev_list_add(const struct nfs_server *nfss, > const struct nfs4_deviceid *d_id, struct osd_dev *od, > gfp_t gfp_flags) > { > @@ -79,9 +84,12 @@ static int _dev_list_add(const struct nfs_server *nfss, > struct objio_dev_ent *de = kzalloc(sizeof(*de), gfp_flags); > struct objio_dev_ent *n; > > - if (!de) > - return -ENOMEM; > + if (!de) { > + dprintk("%s: -ENOMEM od=%p\n", __func__, od); > + return NULL; better return ERR_PTR(-ENOMEM) that will percolate up the stack via _device_lookup. Thanks! Benny > + } > > + dprintk("%s: Adding od=%p\n", __func__, od); > nfs4_init_deviceid_node(&de->id_node, > nfss->pnfs_curr_ld, > nfss->nfs_client, > @@ -91,11 +99,12 @@ static int _dev_list_add(const struct nfs_server *nfss, > d = nfs4_insert_deviceid_node(&de->id_node); > n = container_of(d, struct objio_dev_ent, id_node); > if (n != de) { > - BUG_ON(n->od != od); > + dprintk("%s: Race with other n->od=%p\n", __func__, n->od); > objio_free_deviceid_node(&de->id_node); > + de = n; > } > > - return 0; > + return de; > } > > struct caps_buffers { > @@ -117,7 +126,7 @@ struct objio_segment { > unsigned comps_index; > unsigned num_comps; > /* variable length */ > - struct objio_dev_ent *ods[0]; > + struct objio_dev_ent *ods[]; > }; > > static inline struct objio_segment * > @@ -176,12 +185,13 @@ static struct objio_dev_ent *_device_lookup(struct pnfs_layout_hdr *pnfslay, > goto out; > } > > - _dev_list_add(NFS_SERVER(pnfslay->plh_inode), d_id, od, gfp_flags); > + ode = _dev_list_add(NFS_SERVER(pnfslay->plh_inode), d_id, od, > + gfp_flags); > > out: > dprintk("%s: return=%d\n", __func__, err); > objlayout_put_deviceinfo(deviceaddr); > - return err ? ERR_PTR(err) : od; > + return err ? ERR_PTR(err) : ode; > } > > static int objio_devices_lookup(struct pnfs_layout_hdr *pnfslay, -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/24/2011 08:14 PM, Benny Halevy wrote: > On 2011-05-24 18:07, Boaz Harrosh wrote: >> Fix BUGs in the new "Use global-device-cache". >> >> One thing I don't understand is why the compiler did >> not complain when the code was returning the wrong >> type of structure >> >> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com> >> --- >> fs/nfs/objlayout/objio_osd.c | 28 +++++++++++++++++++--------- >> 1 files changed, 19 insertions(+), 9 deletions(-) >> >> diff --git a/fs/nfs/objlayout/objio_osd.c b/fs/nfs/objlayout/objio_osd.c >> index 167cd1e..faacde2 100644 >> --- a/fs/nfs/objlayout/objio_osd.c >> +++ b/fs/nfs/objlayout/objio_osd.c >> @@ -56,6 +56,7 @@ objio_free_deviceid_node(struct nfs4_deviceid_node *d) >> { >> struct objio_dev_ent *de = container_of(d, struct objio_dev_ent, id_node); >> >> + dprintk("%s: free od=%p\n", __func__, de->od); >> osduld_put_device(de->od); >> kfree(de); >> } >> @@ -64,14 +65,18 @@ static struct objio_dev_ent *_dev_list_find(const struct nfs_server *nfss, >> const struct nfs4_deviceid *d_id) >> { >> struct nfs4_deviceid_node *d; >> + struct objio_dev_ent *de; >> >> d = nfs4_find_get_deviceid(nfss->pnfs_curr_ld, nfss->nfs_client, d_id); >> if (!d) >> return NULL; >> - return container_of(d, struct objio_dev_ent, id_node); >> + >> + de = container_of(d, struct objio_dev_ent, id_node); >> + return de; > > That's not really required as container_of() does the type casting > for you. > Ye, I know that change is just a left over from a print between the set and the return. (Else how could I debug this) I than removed the print because these come a lot in a git clone for example. >> } >> >> -static int _dev_list_add(const struct nfs_server *nfss, >> +static struct objio_dev_ent * >> +_dev_list_add(const struct nfs_server *nfss, >> const struct nfs4_deviceid *d_id, struct osd_dev *od, >> gfp_t gfp_flags) >> { >> @@ -79,9 +84,12 @@ static int _dev_list_add(const struct nfs_server *nfss, >> struct objio_dev_ent *de = kzalloc(sizeof(*de), gfp_flags); >> struct objio_dev_ent *n; >> >> - if (!de) >> - return -ENOMEM; >> + if (!de) { >> + dprintk("%s: -ENOMEM od=%p\n", __func__, od); >> + return NULL; > > better return ERR_PTR(-ENOMEM) > that will percolate up the stack via _device_lookup. > Right missed that one > Thanks! > What thanks? beers on you next time! ;-) > Benny > Boaz >> + } >> >> + dprintk("%s: Adding od=%p\n", __func__, od); >> nfs4_init_deviceid_node(&de->id_node, >> nfss->pnfs_curr_ld, >> nfss->nfs_client, >> @@ -91,11 +99,12 @@ static int _dev_list_add(const struct nfs_server *nfss, >> d = nfs4_insert_deviceid_node(&de->id_node); >> n = container_of(d, struct objio_dev_ent, id_node); >> if (n != de) { >> - BUG_ON(n->od != od); >> + dprintk("%s: Race with other n->od=%p\n", __func__, n->od); >> objio_free_deviceid_node(&de->id_node); >> + de = n; >> } >> >> - return 0; >> + return de; >> } >> >> struct caps_buffers { >> @@ -117,7 +126,7 @@ struct objio_segment { >> unsigned comps_index; >> unsigned num_comps; >> /* variable length */ >> - struct objio_dev_ent *ods[0]; >> + struct objio_dev_ent *ods[]; >> }; >> >> static inline struct objio_segment * >> @@ -176,12 +185,13 @@ static struct objio_dev_ent *_device_lookup(struct pnfs_layout_hdr *pnfslay, >> goto out; >> } >> >> - _dev_list_add(NFS_SERVER(pnfslay->plh_inode), d_id, od, gfp_flags); >> + ode = _dev_list_add(NFS_SERVER(pnfslay->plh_inode), d_id, od, >> + gfp_flags); >> >> out: >> dprintk("%s: return=%d\n", __func__, err); >> objlayout_put_deviceinfo(deviceaddr); >> - return err ? ERR_PTR(err) : od; >> + return err ? ERR_PTR(err) : ode; >> } >> >> static int objio_devices_lookup(struct pnfs_layout_hdr *pnfslay, > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfs/objlayout/objio_osd.c b/fs/nfs/objlayout/objio_osd.c index 167cd1e..faacde2 100644 --- a/fs/nfs/objlayout/objio_osd.c +++ b/fs/nfs/objlayout/objio_osd.c @@ -56,6 +56,7 @@ objio_free_deviceid_node(struct nfs4_deviceid_node *d) { struct objio_dev_ent *de = container_of(d, struct objio_dev_ent, id_node); + dprintk("%s: free od=%p\n", __func__, de->od); osduld_put_device(de->od); kfree(de); } @@ -64,14 +65,18 @@ static struct objio_dev_ent *_dev_list_find(const struct nfs_server *nfss, const struct nfs4_deviceid *d_id) { struct nfs4_deviceid_node *d; + struct objio_dev_ent *de; d = nfs4_find_get_deviceid(nfss->pnfs_curr_ld, nfss->nfs_client, d_id); if (!d) return NULL; - return container_of(d, struct objio_dev_ent, id_node); + + de = container_of(d, struct objio_dev_ent, id_node); + return de; } -static int _dev_list_add(const struct nfs_server *nfss, +static struct objio_dev_ent * +_dev_list_add(const struct nfs_server *nfss, const struct nfs4_deviceid *d_id, struct osd_dev *od, gfp_t gfp_flags) { @@ -79,9 +84,12 @@ static int _dev_list_add(const struct nfs_server *nfss, struct objio_dev_ent *de = kzalloc(sizeof(*de), gfp_flags); struct objio_dev_ent *n; - if (!de) - return -ENOMEM; + if (!de) { + dprintk("%s: -ENOMEM od=%p\n", __func__, od); + return NULL; + } + dprintk("%s: Adding od=%p\n", __func__, od); nfs4_init_deviceid_node(&de->id_node, nfss->pnfs_curr_ld, nfss->nfs_client, @@ -91,11 +99,12 @@ static int _dev_list_add(const struct nfs_server *nfss, d = nfs4_insert_deviceid_node(&de->id_node); n = container_of(d, struct objio_dev_ent, id_node); if (n != de) { - BUG_ON(n->od != od); + dprintk("%s: Race with other n->od=%p\n", __func__, n->od); objio_free_deviceid_node(&de->id_node); + de = n; } - return 0; + return de; } struct caps_buffers { @@ -117,7 +126,7 @@ struct objio_segment { unsigned comps_index; unsigned num_comps; /* variable length */ - struct objio_dev_ent *ods[0]; + struct objio_dev_ent *ods[]; }; static inline struct objio_segment * @@ -176,12 +185,13 @@ static struct objio_dev_ent *_device_lookup(struct pnfs_layout_hdr *pnfslay, goto out; } - _dev_list_add(NFS_SERVER(pnfslay->plh_inode), d_id, od, gfp_flags); + ode = _dev_list_add(NFS_SERVER(pnfslay->plh_inode), d_id, od, + gfp_flags); out: dprintk("%s: return=%d\n", __func__, err); objlayout_put_deviceinfo(deviceaddr); - return err ? ERR_PTR(err) : od; + return err ? ERR_PTR(err) : ode; } static int objio_devices_lookup(struct pnfs_layout_hdr *pnfslay,
Fix BUGs in the new "Use global-device-cache". One thing I don't understand is why the compiler did not complain when the code was returning the wrong type of structure Signed-off-by: Boaz Harrosh <bharrosh@panasas.com> --- fs/nfs/objlayout/objio_osd.c | 28 +++++++++++++++++++--------- 1 files changed, 19 insertions(+), 9 deletions(-)