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@xxxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
