[Cluster-devel] PROBLEM: there exists a wrong return value of function gfs2_set_dqblk()

[Bob Peterson <rpeterso@redhat.com>]

----- Original Message -----
> Summary:
>     there exists a wrong return value of function gfs2_set_dqblk().
> 
> Bug Description:
> 
>    In function gfs2_set_dqblk() at fs/gfs2/quota.c?1524, the call to
>    gfs2_alloc_get() in line 1599 may return a NULL pointer, and thus
>    function gfs2_set_dqblk() will return the value of variable error. And,
>    the function gfs2_set_dqblk() will return 0 at last when it runs well.
>    However, when the call to gfs2_alloc_get() in line 1599 return a NULL
>    pointer, the value of error is 0. So the function gfs2_set_dqblk() will
>    return 0 to its caller functions when it runs error because of the
>    failing call to gfs2_alloc_get(), leading to a wrong return value of
>    function gfs2_set_dqblk().
> The related code snippets in gfs2_set_dqblk() is as following.
> gfs2_set_dqblk @@fs/gfs2/quota.c?1524
> 1524 static int gfs2_set_dqblk(struct super_block *sb, int type, qid_t id,
> 1525                           struct fs_disk_quota *fdq)
> 1526 {
> 	     ...
> 1573         /* Check for existing entry, if none then alloc new blocks */
> 1574         error = update_qd(sdp, qd);
> 1575         if (error)
> 1576                 goto out_i;
> 	     ...
> 1598         if (alloc_required) {
> 1599                 al = gfs2_alloc_get(ip);
> 1600                 if (al == NULL)
> 1601                         goto out_i;
> 1602                 gfs2_write_calc_reserv(ip, sizeof(struct gfs2_quota),
> 1603                                        &data_blocks, &ind_blocks);
> 1604                 blocks = al->al_requested = 1 + data_blocks +
> ind_blocks;
> 1605                 error = gfs2_inplace_reserve(ip);
> 1606                 if (error)
> 1607                         goto out_alloc;
> 1608                 blocks += gfs2_rg_blocks(al);
> 1609         }
> 1610
> 1611         /* Some quotas span block boundaries and can update two blocks,
> 1612            adding an extra block to the transaction to handle such
> quotas */
> 1613         error = gfs2_trans_begin(sdp, blocks + RES_DINODE + 2, 0);
> 1614         if (error)
> 1615                 goto out_release;
> 	     ...
> 1627 out_i:
> 1628         gfs2_glock_dq_uninit(&i_gh);
> 1629 out_q:
> 1630         gfs2_glock_dq_uninit(&q_gh);
> 1631 out_put:
> 1632         mutex_unlock(&ip->i_inode.i_mutex);
> 1633         qd_put(qd);
> 1634         return error;
> 1635 }
> 
> Generally, the return value of caller functions which call function
> gfs2_alloc_get() shall be set to -ENOMEM when the call to gfs2_alloc_get()
> returns a NULL pointer, like the following codes in another file.
> leaf_dealloc @@fs/gfs2/dir.c: 1857
> 1857 static int leaf_dealloc(struct gfs2_inode *dip, u32 index, u32 len,
> 1858                         u64 leaf_no, void *data)
> 1859 {
>              ...
> 1876         if (!gfs2_alloc_get(dip)) {
> 1877                 error = -ENOMEM;
> 1878                 goto out;
> 1879         }
>              ...
> 1959 out:
> 1960         kfree(ht);
> 1961         return error;
> 1962 }
> 
> Kernel version:
>     2.6.39

Hi,

2.6.39 is a very old kernel and the newer code has changed a lot since then.
If this is a problem in RHEL6, you should contact Red Hat's GSS support to
get help. If this is a problem on Centos or other, I recommend moving to a
newer kernel to see if the problem has already been fixed.

Regards,

Bob Peterson
Red Hat File Systems