fs/nfs/callback_proc.c: In function ‘do_callback_layoutrecall’:
fs/nfs/callback_proc.c:115:26: warning: ‘lo’ may be used uninitialized in this function
No functional change. If no layout is found, we'll return before using
"lo".
Signed-off-by: Jim Rees <[email protected]>
---
fs/nfs/callback_proc.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index 43926ad..93633f1 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -112,7 +112,7 @@ static u32 initiate_file_draining(struct nfs_client *clp,
struct cb_layoutrecallargs *args)
{
struct nfs_server *server;
- struct pnfs_layout_hdr *lo;
+ struct pnfs_layout_hdr *lo = NULL;
struct inode *ino;
bool found = false;
u32 rv = NFS4ERR_NOMATCHING_LAYOUT;
--
1.7.4.1
Benny Halevy wrote:
On 2011-10-16 17:06, Jim Rees wrote:
> fs/nfs/callback_proc.c: In function ‘do_callback_layoutrecall’:
> fs/nfs/callback_proc.c:115:26: warning: ‘lo’ may be used uninitialized in this function
>
> No functional change. If no layout is found, we'll return before using
> "lo".
>
> Signed-off-by: Jim Rees <[email protected]>
> ---
> fs/nfs/callback_proc.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
> index 43926ad..93633f1 100644
> --- a/fs/nfs/callback_proc.c
> +++ b/fs/nfs/callback_proc.c
> @@ -112,7 +112,7 @@ static u32 initiate_file_draining(struct nfs_client *clp,
> struct cb_layoutrecallargs *args)
> {
> struct nfs_server *server;
> - struct pnfs_layout_hdr *lo;
> + struct pnfs_layout_hdr *lo = NULL;
> struct inode *ino;
> bool found = false;
> u32 rv = NFS4ERR_NOMATCHING_LAYOUT;
Hmm, the warning seems bogus since we use lo only iff found==true
and it is set iff found==true
I wonder why I don't see that warning.
What compiler/version are you using?
gcc (Ubuntu/Linaro 4.5.2-8ubuntu4) 4.5.2
I don't remember seeing this warning before either, but I can't think what I
might have changed that would make a difference. I did turn on SMP, which I
didn't have before (non-SMP kernels don't seem to work on the latest
Virtualbox). And yes, the warning is bogus, but should be fixed anyway.
I have this warning also:
fs/nfs/callback_proc.c:116: warning: ‘ino’ may be used uninitialized in this
function
17.10.2011 04:06, Jim Rees пишет:
> fs/nfs/callback_proc.c: In function ‘do_callback_layoutrecall’:
> fs/nfs/callback_proc.c:115:26: warning: ‘lo’ may be used uninitialized in this function
>
> No functional change. If no layout is found, we'll return before using
> "lo".
>
> Signed-off-by: Jim Rees<[email protected]>
> ---
> fs/nfs/callback_proc.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
> index 43926ad..93633f1 100644
> --- a/fs/nfs/callback_proc.c
> +++ b/fs/nfs/callback_proc.c
> @@ -112,7 +112,7 @@ static u32 initiate_file_draining(struct nfs_client *clp,
> struct cb_layoutrecallargs *args)
> {
> struct nfs_server *server;
> - struct pnfs_layout_hdr *lo;
> + struct pnfs_layout_hdr *lo = NULL;
> struct inode *ino;
> bool found = false;
> u32 rv = NFS4ERR_NOMATCHING_LAYOUT;
--
Best regards,
Stanislav Kinsbursky
On 2011-10-16 17:06, Jim Rees wrote:
> fs/nfs/callback_proc.c: In function ?do_callback_layoutrecall?:
> fs/nfs/callback_proc.c:115:26: warning: ?lo? may be used uninitialized in this function
>
> No functional change. If no layout is found, we'll return before using
> "lo".
>
> Signed-off-by: Jim Rees <[email protected]>
> ---
> fs/nfs/callback_proc.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
> index 43926ad..93633f1 100644
> --- a/fs/nfs/callback_proc.c
> +++ b/fs/nfs/callback_proc.c
> @@ -112,7 +112,7 @@ static u32 initiate_file_draining(struct nfs_client *clp,
> struct cb_layoutrecallargs *args)
> {
> struct nfs_server *server;
> - struct pnfs_layout_hdr *lo;
> + struct pnfs_layout_hdr *lo = NULL;
> struct inode *ino;
> bool found = false;
> u32 rv = NFS4ERR_NOMATCHING_LAYOUT;
Hmm, the warning seems bogus since we use lo only iff found==true
and it is set iff found==true
I wonder why I don't see that warning.
What compiler/version are you using?
At any rate, this made me think of a cleaner way to implement this:
git diff --stat -p -M
fs/nfs/callback_proc.c | 56 ++++++++++++++++++++++++++++++++---------------
1 files changed, 38 insertions(+), 18 deletions(-)
diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index 43926ad..e8d83a7 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -108,42 +108,62 @@ int nfs4_validate_delegation_stateid(struct nfs_delegation *delegation, const nf
#if defined(CONFIG_NFS_V4_1)
-static u32 initiate_file_draining(struct nfs_client *clp,
- struct cb_layoutrecallargs *args)
+/*
+ * Lookup a layout by filehandle.
+ *
+ * Note: gets a refcount on the layout hdr and on its respective inode.
+ * Caller must put the layout hdr and the inode.
+ *
+ * TODO: keep track of all layouts (and delegations) in a hash table
+ * hashed by filehandle.
+ */
+static struct pnfs_layout_hdr * get_layout_by_fh_locked(struct nfs_client *clp, struct nfs_fh *fh)
{
struct nfs_server *server;
- struct pnfs_layout_hdr *lo;
struct inode *ino;
- bool found = false;
- u32 rv = NFS4ERR_NOMATCHING_LAYOUT;
- LIST_HEAD(free_me_list);
+ struct pnfs_layout_hdr *lo;
- spin_lock(&clp->cl_lock);
- rcu_read_lock();
list_for_each_entry_rcu(server, &clp->cl_superblocks, client_link) {
list_for_each_entry(lo, &server->layouts, plh_layouts) {
- if (nfs_compare_fh(&args->cbl_fh,
- &NFS_I(lo->plh_inode)->fh))
+ if (nfs_compare_fh(fh, &NFS_I(lo->plh_inode)->fh))
continue;
ino = igrab(lo->plh_inode);
if (!ino)
continue;
- found = true;
- /* Without this, layout can be freed as soon
- * as we release cl_lock.
- */
get_layout_hdr(lo);
- break;
+ return lo;
}
- if (found)
- break;
}
+
+ return NULL;
+}
+
+static struct pnfs_layout_hdr * get_layout_by_fh(struct nfs_client *clp, struct nfs_fh *fh)
+{
+ struct pnfs_layout_hdr *lo;
+
+ spin_lock(&clp->cl_lock);
+ rcu_read_lock();
+ lo = get_layout_by_fh_locked(clp, fh);
rcu_read_unlock();
spin_unlock(&clp->cl_lock);
- if (!found)
+ return lo;
+}
+
+static u32 initiate_file_draining(struct nfs_client *clp,
+ struct cb_layoutrecallargs *args)
+{
+ struct inode *ino;
+ struct pnfs_layout_hdr *lo;
+ u32 rv = NFS4ERR_NOMATCHING_LAYOUT;
+ LIST_HEAD(free_me_list);
+
+ lo = get_layout_by_fh(clp, &args->cbl_fh);
+ if (!lo)
return NFS4ERR_NOMATCHING_LAYOUT;
+ ino = lo->plh_inode;
spin_lock(&ino->i_lock);
if (test_bit(NFS_LAYOUT_BULK_RECALL, &lo->plh_flags) ||
mark_matching_lsegs_invalid(lo, &free_me_list,