[LU-3491] replace printing of i_ino in llite with ll_inode2fid()

• Status: Resolved
• Project: Lustre
• Affects Version/s: Lustre 2.5.0
• Fix Version/s: Lustre 2.6.0
• Type: Improvement
• Priority: Minor
• Reporter: Andreas Dilger
• Assignee: James Nunez (Inactive)
• Resolution: Fixed

Description

The printing of i_ino/i_generation in llite messages is not nearly so useful as printing the full inode FID, since i_ino is a "compressed" version of the FID and there may be duplicate values for i_ino in some cases (especially if running on a 32-bit client).

An audit should be made of i_ino/i_generation usage in the lustre/

{llite,mdc}

/*.c files, and replace this with DFID/PFID(ll_inode2fid(inode)).

Comments

[James Nunez (Inactive)]

The patch, for llite, for this ticket is almost complete, but a couple of questions came to mind while modifying the code.

1. Do we want consistency in the way the inode/FID is printed? Sometimes we print the FID first then the inode in parenthesis. Other times we print the inode first and then the FID in parenthesis. Examples:

CDEBUG(D_DLMTRACE, "setting l_ast_data to inode %p ("DFID")\n", inode, PFID(ll_inode2fid(inode)));

and

CDEBUG(D_VFSTRACE, "VFS Op:name=%.*s,dir="DFID"(%p),intent=%s\n", dentry->d_name.len, dentry->d_name.name, PFID(ll_inode2fid(dir)), dir, LL_IT2STR(it));

2. Whenever possible, do we want to print the inode along with the FID? For example, we have times where we just print the Fid:

CERROR("inode "DFID": rc = %d\n", PFID(ll_inode2fid(inode)), rc);

Should we actually print, when possible:

CERROR("inode %p ("DFID"): rc = %d\n", inode, PFID(ll_inode2fid(inode)), rc);

3. In some files, I see the following:

CDEBUG(D_INFO, "encoding for (%lu,"DFID") maxlen=%d minlen=%d\n", inode->i_ino, PFID(ll_inode2fid(inode)), *plen, (int)sizeof(struct lustre_nfs_fid));

and

CDEBUG(D_INFO, "getting parent for (%lu,"DFID")\n", dir->i_ino, PFID(ll_inode2fid(dir)));

It seems redundant to print both i_ino and FID for the same inode. Should we just print the FID in these cases?

[Andreas Dilger]

James, thanks for your attention to the details.

I agree that printing both the FID and i_ino is redundant, since the inode number is just a compressed 32-bit or 64-bit version of the FID.

Please always print the device name or fsname at the start of any console error message. If we need to print the inode pointer, it should be in parenthesis after the FID, since it is much less important that the FID, and will rarely be of use to anyone without a crashdump.

[James Nunez (Inactive)]

Proposed patch for llite is at:

http://review.whamcloud.com/6848

[James Nunez (Inactive)]

Accidentally closed issue. Patch has not landed yet.

[James Nunez (Inactive)]

Landed to master